fix(crypto): Avoid using legacy RSA algorithms

Fixes #234.

This change moves the default RSA algorithm from RS512 to PS512, thus using RSA-PSS by default rather than "legacy" RSA.

It also fixes a bug where the default HMAC algorithm was actually HS256 instead of HS512.

Author: adutra

docs/configuration.md

@@ -440,9 +440,11 @@ The expiration time of the client assertion JWT. Optional. The default is 5 minu
 
 ### `rest.auth.oauth2.client-auth.jwt.algorithm`
 
-The signing algorithm to use for the client assertion JWT. Optional. The default is `HS512` if the authentication method is `client_secret_jwt`, or `RS512` if the authentication method is `private_key_jwt`.
+The signing algorithm to use for the client assertion JWT. Optional. The default is `HS512` if the authentication method is `client_secret_jwt`, or `PS512` if the authentication method is `private_key_jwt`.
 
-Supported algorithms are: HMAC-SHA for `client_secret_jwt`, and RSA or EC for `private_key_jwt`.
+Supported algorithms are: HMAC-SHA for `client_secret_jwt`, and RSA, RSA-PSS, or EC for `private_key_jwt`.
+
+Note: legacy PKCS#1 v1.5 RSA algorithms (`RS256`, `RS384`, `RS512`) are supported but deprecated; prefer the equivalent RSASSA-PSS algorithms (`PS256`, `PS384`, `PS512`).
 
 Algorithm names must match the "alg" Param Value as described in [RFC 7518 Section 3.1](https://datatracker.ietf.org/doc/html/rfc7518#section-3.1).
 

oauth2/core/src/docs/java/com/dremio/iceberg/authmgr/oauth2/docs/DocumentationGenerator.java

@@ -65,7 +65,7 @@ public class DocumentationGenerator {
     refs.put("GrantType#JWT_BEARER", "urn:ietf:params:oauth:grant-type:jwt-bearer");
     refs.put("GrantType#TOKEN_EXCHANGE", "urn:ietf:params:oauth:grant-type:token-exchange");
     refs.put("JWSAlgorithm#HS512", "HS512");
-    refs.put("JWSAlgorithm#RS512", "RS512");
+    refs.put("JWSAlgorithm#PS512", "PS512");
     refs.put("ClientAuthenticationMethod#NONE", "none");
     refs.put("ClientAuthenticationMethod#CLIENT_SECRET_BASIC", "client_secret_basic");
     refs.put("ClientAuthenticationMethod#CLIENT_SECRET_POST", "client_secret_post");

oauth2/core/src/main/java/com/dremio/iceberg/authmgr/oauth2/config/JwtClientAuthConfig.java

@@ -80,11 +80,15 @@ public interface JwtClientAuthConfig {
   /**
    * The signing algorithm to use for the client assertion JWT. Optional. The default is {@link
    * JWSAlgorithm#HS512} if the authentication method is {@link
-   * ClientAuthenticationMethod#CLIENT_SECRET_JWT}, or {@link JWSAlgorithm#RS512} if the
+   * ClientAuthenticationMethod#CLIENT_SECRET_JWT}, or {@link JWSAlgorithm#PS512} if the
    * authentication method is {@link ClientAuthenticationMethod#PRIVATE_KEY_JWT}.
    *
-   * <p>Supported algorithms are: HMAC-SHA for {@code client_secret_jwt}, and RSA or EC for {@code
-   * private_key_jwt}.
+   * <p>Supported algorithms are: HMAC-SHA for {@code client_secret_jwt}, and RSA, RSA-PSS, or EC
+   * for {@code private_key_jwt}.
+   *
+   * <p>Note: legacy PKCS#1 v1.5 RSA algorithms ({@code RS256}, {@code RS384}, {@code RS512}) are
+   * supported but deprecated; prefer the equivalent RSASSA-PSS algorithms ({@code PS256}, {@code
+   * PS384}, {@code PS512}).
    *
    * <p>Algorithm names must match the "alg" Param Value as described in <a
    * href="https://datatracker.ietf.org/doc/html/rfc7518#section-3.1">RFC 7518 Section 3.1</a>.
@@ -136,25 +140,26 @@ public interface JwtClientAuthConfig {
   default void validate() {
     ConfigValidator validator = new ConfigValidator();
     if (getAlgorithm().isPresent()) {
-      if (JWSAlgorithm.Family.RSA.contains(getAlgorithm().get())
-          || JWSAlgorithm.Family.EC.contains(getAlgorithm().get())) {
+      JWSAlgorithm algorithm = getAlgorithm().get();
+      if (JWSAlgorithm.Family.RSA.contains(algorithm)
+          || JWSAlgorithm.Family.EC.contains(algorithm)) {
         validator.check(
             getPrivateKey().isPresent(),
             List.of(PREFIX + '.' + ALGORITHM, PREFIX + '.' + PRIVATE_KEY),
             "client assertion: JWS signing algorithm '%s' requires a private key",
-            getAlgorithm().get().getName());
-      } else if (JWSAlgorithm.Family.HMAC_SHA.contains(getAlgorithm().get())) {
+            algorithm.getName());
+      } else if (JWSAlgorithm.Family.HMAC_SHA.contains(algorithm)) {
         validator.check(
             getPrivateKey().isEmpty(),
             List.of(PREFIX + '.' + ALGORITHM, PREFIX + '.' + PRIVATE_KEY),
             "client assertion: private key must not be set for JWS algorithm '%s'",
-            getAlgorithm().get().getName());
+            algorithm.getName());
       } else {
         validator.check(
             false,
             PREFIX + '.' + ALGORITHM,
             "client assertion: unsupported JWS algorithm '%s', must be one of: %s",
-            getAlgorithm().get().getName(),
+            algorithm.getName(),
             Stream.of(
                     JWSAlgorithm.Family.HMAC_SHA.stream(),
                     JWSAlgorithm.Family.RSA.stream(),
@@ -163,6 +168,7 @@ default void validate() {
                 .map(JWSAlgorithm::getName)
                 .collect(Collectors.joining("', '", "'", "'")));
       }
+      validator.checkAlgorithm(algorithm);
     }
     if (getPrivateKey().isPresent()) {
       validator.check(

oauth2/core/src/main/java/com/dremio/iceberg/authmgr/oauth2/config/validator/ConfigValidator.java

@@ -17,14 +17,19 @@
 
 import com.google.errorprone.annotations.FormatMethod;
 import com.google.errorprone.annotations.FormatString;
+import com.nimbusds.jose.JWSAlgorithm;
 import java.net.URI;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public final class ConfigValidator {
 
+  private static final Logger LOGGER = LoggerFactory.getLogger(ConfigValidator.class);
+
   private final List<ConfigViolation> violations = new ArrayList<>();
 
   public void check(boolean cond, String offendingKey, String msg) {
@@ -61,6 +66,17 @@ public void checkEndpoint(URI endpoint, String offendingKey, String name) {
     check(endpoint.getFragment() == null, offendingKey, name + " must not have a fragment part");
   }
 
+  public void checkAlgorithm(JWSAlgorithm algorithm) {
+    if (algorithm.equals(JWSAlgorithm.RS256)
+        || algorithm.equals(JWSAlgorithm.RS384)
+        || algorithm.equals(JWSAlgorithm.RS512)) {
+      LOGGER.warn(
+          "JWS algorithm '{}' uses legacy PKCS#1 v1.5 RSA padding; "
+              + "consider using PS256, PS384, or PS512 (RSASSA-PSS) instead",
+          algorithm.getName());
+    }
+  }
+
   public void validate() {
     if (!violations.isEmpty()) {
       throw new IllegalArgumentException(

oauth2/core/src/main/java/com/dremio/iceberg/authmgr/oauth2/flow/AbstractFlow.java

@@ -199,7 +199,7 @@ ClientAuthentication createClientAuthentication() {
     } else if (method.equals(CLIENT_SECRET_JWT)) {
       JWTAssertionDetails details = createJwtAssertionDetails(tokenEndpoint);
       JWSAlgorithm algorithm =
-          getConfig().getJwtClientAuthConfig().getAlgorithm().orElse(JWSAlgorithm.HS256);
+          getConfig().getJwtClientAuthConfig().getAlgorithm().orElse(JWSAlgorithm.HS512);
       Secret secret = getConfig().getBasicConfig().getClientSecret().orElseThrow();
       try {
         SignedJWT assertion = JWTAssertionFactory.create(details, algorithm, secret);
@@ -211,7 +211,7 @@ ClientAuthentication createClientAuthentication() {
     } else if (method.equals(PRIVATE_KEY_JWT)) {
       JWTAssertionDetails details = createJwtAssertionDetails(tokenEndpoint);
       JWSAlgorithm algorithm =
-          getConfig().getJwtClientAuthConfig().getAlgorithm().orElse(JWSAlgorithm.RS256);
+          getConfig().getJwtClientAuthConfig().getAlgorithm().orElse(JWSAlgorithm.PS512);
       Path privateKeyPath = getConfig().getJwtClientAuthConfig().getPrivateKey().orElseThrow();
       PrivateKey privateKey = PemReader.getInstance().readPrivateKey(privateKeyPath);
       try {

oauth2/core/src/testFixtures/java/com/dremio/iceberg/authmgr/oauth2/test/TestCertificates.java

@@ -41,8 +41,8 @@
  * Generates test certificates and keystores at runtime. All materials are generated lazily (once
  * per JVM) and stored in a temp directory.
  *
- * <p>The material is divided in 2 main groups: one is based on RSA keys, the second on ECDSA keys.
- * Each group exposes: a key pair, a self-signed certificate, PEM-encoded files, and a PKCS#12
+ * <p>The material is divided in 2 main groups: one is based on RSA-PSS keys, the second on ECDSA
+ * keys. Each group exposes: a key pair, a self-signed certificate, PEM-encoded files, and a PKCS#12
  * keystore containing all the material.
  *
  * <p>Most of the material (RSA/ECDSA key pairs, PKCS#8 PEM, PKCS#12 keystores) is generated using
@@ -120,7 +120,7 @@ private TestCertificates() {
 
       // Generate RSA material
       rsaKeyStoreP12 = baseDir.resolve("keystore.p12");
-      runKeytool(rsaKeyStoreP12, "RSA", "2048", "SHA256withRSA");
+      runKeytool(rsaKeyStoreP12, "RSA", "2048", "RSASSA-PSS");
       KeyStore rsaKs = loadKeyStore(rsaKeyStoreP12);
 
       rsaCertificate = (X509Certificate) rsaKs.getCertificate(ALIAS);
@@ -364,9 +364,9 @@ private void runKeytool(Path keyStorePath, String keyAlg, String keySpec, String
     command.add("BC=ca:true");
 
     Process process = new ProcessBuilder(command).redirectErrorStream(true).start();
-    String output = new String(process.getInputStream().readAllBytes(), StandardCharsets.UTF_8);
     int exitCode = process.waitFor();
     if (exitCode != 0) {
+      String output = new String(process.getInputStream().readAllBytes(), StandardCharsets.UTF_8);
       throw new RuntimeException("keytool failed (exit code " + exitCode + "): " + output);
     }
   }

oauth2/core/src/testFixtures/java/com/dremio/iceberg/authmgr/oauth2/test/junit/KeycloakExtension.java

@@ -27,7 +27,6 @@
 import com.dremio.iceberg.authmgr.oauth2.test.TestEnvironmentExtension;
 import com.dremio.iceberg.authmgr.oauth2.test.container.KeycloakContainer;
 import com.google.common.base.Preconditions;
-import com.google.common.base.Strings;
 import com.nimbusds.jose.JWSAlgorithm;
 import com.nimbusds.jose.JWSHeader;
 import com.nimbusds.jose.JWSSigner;
@@ -36,10 +35,12 @@
 import com.nimbusds.jwt.JWTClaimsSet;
 import com.nimbusds.jwt.SignedJWT;
 import java.security.PrivateKey;
+import java.security.SecureRandom;
 import java.security.interfaces.ECPrivateKey;
 import java.security.interfaces.RSAPrivateKey;
 import java.time.Duration;
 import java.time.Instant;
+import java.util.Base64;
 import java.util.Date;
 import java.util.UUID;
 import org.junit.jupiter.api.extension.AfterAllCallback;
@@ -62,7 +63,7 @@ public class KeycloakExtension extends TestEnvironmentExtension
 
   // Client3 is used for client_secret_jwt authentication
   public static final String CLIENT_ID3 = "Client3";
-  public static final String CLIENT_SECRET3 = Strings.repeat("S3CR3T", 10);
+  public static final String CLIENT_SECRET3 = generateSecret();
   public static final String CLIENT_AUTH3 = CLIENT_SECRET_JWT.getValue();
 
   // Client4 is used for private_key_jwt authentication (RSA)
@@ -194,6 +195,12 @@ protected ImmutableTestEnvironment.Builder newTestEnvironmentBuilder(ExtensionCo
         .resource(null);
   }
 
+  private static String generateSecret() {
+    byte[] raw = new byte[64];
+    new SecureRandom().nextBytes(raw);
+    return Base64.getUrlEncoder().withoutPadding().encodeToString(raw);
+  }
+
   /**
    * Creates a JWT bearer assertion signed with the given algorithm and private key, suitable for
    * use with the JWT bearer grant against this Keycloak instance.