Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Comment thread
adutra marked this conversation as resolved.

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).

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>.
Expand Down Expand Up @@ -136,25 +140,26 @@ public interface JwtClientAuthConfig {
default void validate() {
ConfigValidator validator = new ConfigValidator();
if (getAlgorithm().isPresent()) {
Comment thread
adutra marked this conversation as resolved.
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())) {
"client-auth.jwt: JWS signing algorithm '%s' requires a private key",
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());
"client-auth.jwt: private key must not be set for JWS algorithm '%s'",
algorithm.getName());
} else {
validator.check(
false,
PREFIX + '.' + ALGORITHM,
"client assertion: unsupported JWS algorithm '%s', must be one of: %s",
getAlgorithm().get().getName(),
"client-auth.jwt: unsupported JWS algorithm '%s', must be one of: %s",
algorithm.getName(),
Stream.of(
JWSAlgorithm.Family.HMAC_SHA.stream(),
JWSAlgorithm.Family.RSA.stream(),
Expand All @@ -163,12 +168,13 @@ default void validate() {
.map(JWSAlgorithm::getName)
.collect(Collectors.joining("', '", "'", "'")));
}
validator.checkAlgorithm(algorithm);
}
if (getPrivateKey().isPresent()) {
validator.check(
Files.isReadable(getPrivateKey().get()),
PREFIX + '.' + PRIVATE_KEY,
"client assertion: private key path '%s' is not a file or is not readable",
"client-auth.jwt: private key path '%s' is not a file or is not readable",
getPrivateKey().get());
}
validator.validate();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Comment thread
adutra marked this conversation as resolved.
Secret secret = getConfig().getBasicConfig().getClientSecret().orElseThrow();
try {
SignedJWT assertion = JWTAssertionFactory.create(details, algorithm, secret);
Expand All @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ static Stream<Arguments> testValidate() {
Arguments.of(
Map.of(PREFIX + '.' + JwtClientAuthConfig.ALGORITHM, "RS256"),
List.of(
"client assertion: JWS signing algorithm 'RS256' requires a private key "
"client-auth.jwt: JWS signing algorithm 'RS256' requires a private key "
+ "(rest.auth.oauth2.client-auth.jwt.algorithm / rest.auth.oauth2.client-auth.jwt.private-key)")),
Arguments.of(
Map.of(
Expand All @@ -74,7 +74,7 @@ static Stream<Arguments> testValidate() {
PREFIX + '.' + JwtClientAuthConfig.PRIVATE_KEY,
tempFile.toString()),
List.of(
"client assertion: private key must not be set for JWS algorithm 'HS256' "
"client-auth.jwt: private key must not be set for JWS algorithm 'HS256' "
+ "(rest.auth.oauth2.client-auth.jwt.algorithm / rest.auth.oauth2.client-auth.jwt.private-key)")),
Arguments.of(
Map.of(
Expand All @@ -83,7 +83,7 @@ static Stream<Arguments> testValidate() {
PREFIX + '.' + JwtClientAuthConfig.PRIVATE_KEY,
tempFile.toString()),
List.of(
"client assertion: unsupported JWS algorithm 'RSA_SHA256', must be one of: "
"client-auth.jwt: unsupported JWS algorithm 'RSA_SHA256', must be one of: "
+ "'HS256', 'HS384', 'HS512', 'RS256', 'RS384', 'RS512', 'PS256', 'PS384', 'PS512', 'ES256', 'ES256K', 'ES384', 'ES512' "
+ "(rest.auth.oauth2.client-auth.jwt.algorithm)")),
Arguments.of(
Expand All @@ -93,13 +93,13 @@ static Stream<Arguments> testValidate() {
PREFIX + '.' + JwtClientAuthConfig.PRIVATE_KEY,
tempFile.toString()),
List.of(
"client assertion: unsupported JWS algorithm 'EdDSA', must be one of: "
"client-auth.jwt: unsupported JWS algorithm 'EdDSA', must be one of: "
+ "'HS256', 'HS384', 'HS512', 'RS256', 'RS384', 'RS512', 'PS256', 'PS384', 'PS512', 'ES256', 'ES256K', 'ES384', 'ES512' "
+ "(rest.auth.oauth2.client-auth.jwt.algorithm)")),
Arguments.of(
Map.of(PREFIX + '.' + JwtClientAuthConfig.PRIVATE_KEY, "/invalid/path"),
List.of(
"client assertion: private key path '/invalid/path' is not a file or is not readable "
"client-auth.jwt: private key path '/invalid/path' is not a file or is not readable "
+ "(rest.auth.oauth2.client-auth.jwt.private-key)")));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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();
Comment thread
adutra marked this conversation as resolved.
public static final String CLIENT_AUTH3 = CLIENT_SECRET_JWT.getValue();

// Client4 is used for private_key_jwt authentication (RSA)
Expand Down Expand Up @@ -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.
Expand Down
Loading