fix(auth): bind internal JWTs to principal secret generation (#5053)

Author: vigneshio

CHANGELOG.md

@@ -71,6 +71,7 @@ request adding CHANGELOG notes for breaking (!) changes and possibly other secti
   and a subsequent `bootstrap` would create a second, empty set of tables in the other schema.
   Either remove the setting from the URL, or point it at the schema that already holds your
   Polaris tables.
+- Internal JWTs minted before credentials-generation binding (tokens without the `polaris-cv` claim) can no longer be used as subject tokens in token exchange; they remain valid as bearer tokens until expiry. During a rolling upgrade, an old node may still mint claim-less tokens: exchanging such a token on any already-upgraded node fails with `invalid_grant`, so clients can see intermittent exchange failures until the last old node is gone; after that, rejection is consistent.
 
 ### New Features
 
@@ -148,6 +149,9 @@ request adding CHANGELOG notes for breaking (!) changes and possibly other secti
   directly under an allowed location, at `s3://b1/ns`, and rejected it as a custom location even
   though the request asked for none. The namespace location is now compared against the
   catalog's `default-base-location`, which is what it is derived from.
+- Internal JWTs are bound to principal secret generation via `polaris-cv` (no secret material in the
+  token). Credential-generation is enforced on token exchange; bearer verify is signature and claims
+  only. Secrets-load failures during exchange return service unavailable.
 
 ### Commits
 

polaris-core/src/main/java/org/apache/polaris/core/DigestUtils.java

@@ -20,6 +20,8 @@
 
 import com.google.common.hash.Hashing;
 import java.nio.charset.StandardCharsets;
+import java.security.MessageDigest;
+import java.util.Objects;
 
 public final class DigestUtils {
   private DigestUtils() {
@@ -29,4 +31,14 @@ private DigestUtils() {
   public static String sha256Hex(String input) {
     return Hashing.sha256().hashString(input, StandardCharsets.UTF_8).toString();
   }
+
+  /**
+   * Constant-time equality check for secret material. Both arguments must be non-null; callers are
+   * expected to guard nullable inputs before comparing.
+   */
+  public static boolean constantTimeEquals(String a, String b) {
+    return MessageDigest.isEqual(
+        Objects.requireNonNull(a).getBytes(StandardCharsets.UTF_8),
+        Objects.requireNonNull(b).getBytes(StandardCharsets.UTF_8));
+  }
 }

polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisPrincipalSecrets.java

@@ -21,6 +21,7 @@
 import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonProperty;
 import java.security.SecureRandom;
+import java.util.Optional;
 import org.apache.polaris.core.DigestUtils;
 import org.jspecify.annotations.Nullable;
 
@@ -167,10 +168,45 @@ public String getPrincipalClientId() {
     return principalClientId;
   }
 
+  /**
+   * @deprecated no longer used for authentication; kept for compatibility and will be removed in a
+   *     future release. Use {@link #getCredentialsVersionForSecret(String)} instead.
+   */
+  @Deprecated
   public boolean matchesSecret(String potentialSecret) {
+    return getCredentialsVersionForSecret(potentialSecret).isPresent();
+  }
+
+  /**
+   * Credentials-generation fingerprint corresponding to the secret that matches {@code
+   * potentialSecret}: the main generation when it matches the main secret hash, the secondary
+   * generation when it matches the secondary secret hash, or empty when it matches neither. Newly
+   * minted tokens carry this fingerprint in the {@code polaris-cv} claim. That claim gates
+   * <em>token-exchange</em> eligibility against the current (and secondary) generation; bearer
+   * verify checks only the JWT signature and claims, so a token remains usable as a bearer until
+   * JWT expiry even after its generation is no longer current.
+   *
+   * <p>Comparisons are constant-time, as in {@link #matchesCredentialsVersion(String)}.
+   */
+  public Optional<String> getCredentialsVersionForSecret(String potentialSecret) {
     String potentialSecretHash = hashSecret(potentialSecret);
-    return potentialSecretHash.equals(this.mainSecretHash)
-        || potentialSecretHash.equals(this.secondarySecretHash);
+    Optional<String> mainVersion = credentialsVersionForHash(mainSecretHash);
+    Optional<String> secondaryVersion = credentialsVersionForHash(secondarySecretHash);
+    // Always compare against both hashes and combine the flags, so the amount of comparison work
+    // does not reveal which generation matched.
+    boolean matchesMain =
+        mainSecretHash != null
+            && DigestUtils.constantTimeEquals(potentialSecretHash, mainSecretHash);
+    boolean matchesSecondary =
+        secondarySecretHash != null
+            && DigestUtils.constantTimeEquals(potentialSecretHash, secondarySecretHash);
+    if (matchesMain) {
+      return mainVersion;
+    }
+    if (matchesSecondary) {
+      return secondaryVersion;
+    }
+    return Optional.empty();
   }
 
   public String getMainSecret() {
@@ -185,6 +221,54 @@ public String getMainSecretHash() {
     return mainSecretHash;
   }
 
+  /**
+   * Credentials-generation fingerprint for tokens minted against the <em>current</em> main secret.
+   * Derived from existing fields with the principal's secret salt, so no schema change is required.
+   * The value is not itself a credential and is safe to embed in signed (but not encrypted) JWTs.
+   *
+   * @return the main credentials version; never {@code null} since the main secret hash is always
+   *     set by the constructors
+   */
+  public String getCredentialsVersion() {
+    return credentialsVersionForHash(mainSecretHash).orElseThrow();
+  }
+
+  /**
+   * Returns true when {@code credentialsVersion} matches the salted fingerprint of the current main
+   * secret hash <em>or</em> the secondary secret hash. After a single rotate, the previous main
+   * hash is kept as secondary so client secrets and bound JWTs both remain valid for that
+   * generation; a further rotate/reset advances secondary and invalidates the older fingerprint.
+   *
+   * <p>Comparisons are constant-time against each candidate fingerprint.
+   */
+  public boolean matchesCredentialsVersion(@Nullable String credentialsVersion) {
+    if (credentialsVersion == null || credentialsVersion.isEmpty()) {
+      return false;
+    }
+    boolean matches = false;
+    Optional<String> mainVersion = credentialsVersionForHash(mainSecretHash);
+    if (mainVersion.isPresent()) {
+      matches |= DigestUtils.constantTimeEquals(credentialsVersion, mainVersion.get());
+    }
+    Optional<String> secondaryVersion = credentialsVersionForHash(secondarySecretHash);
+    if (secondaryVersion.isPresent()) {
+      matches |= DigestUtils.constantTimeEquals(credentialsVersion, secondaryVersion.get());
+    }
+    return matches;
+  }
+
+  /**
+   * Salted, non-reversible fingerprint of a secret-verification hash for embedding in JWTs. Uses
+   * the same per-principal {@link #secretSalt} already stored for secret hashing, which is always
+   * set by the constructors.
+   */
+  private Optional<String> credentialsVersionForHash(@Nullable String secretHash) {
+    if (secretHash == null || secretHash.isEmpty()) {
+      return Optional.empty();
+    }
+    return Optional.of(DigestUtils.sha256Hex(secretHash + ":" + secretSalt));
+  }
+
   public String getSecondarySecretHash() {
     return secondarySecretHash;
   }

polaris-core/src/test/java/org/apache/polaris/core/DigestUtilsTest.java

@@ -0,0 +1,45 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.core;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import org.junit.jupiter.api.Test;
+
+public class DigestUtilsTest {
+
+  @Test
+  void constantTimeEquals() {
+    assertThat(DigestUtils.constantTimeEquals("abc", "abc")).isTrue();
+    assertThat(DigestUtils.constantTimeEquals("abc", "abd")).isFalse();
+    assertThat(DigestUtils.constantTimeEquals("abc", "abcd")).isFalse();
+    assertThat(DigestUtils.constantTimeEquals("", "")).isTrue();
+  }
+
+  @Test
+  void constantTimeEqualsRejectsNull() {
+    assertThatThrownBy(() -> DigestUtils.constantTimeEquals(null, "a"))
+        .isInstanceOf(NullPointerException.class);
+    assertThatThrownBy(() -> DigestUtils.constantTimeEquals("a", null))
+        .isInstanceOf(NullPointerException.class);
+    assertThatThrownBy(() -> DigestUtils.constantTimeEquals(null, null))
+        .isInstanceOf(NullPointerException.class);
+  }
+}