From 7f017f540ed33b1690aff0a1dd17f0d927b68eb4 Mon Sep 17 00:00:00 2001 From: mposolda Date: Fri, 27 Jan 2023 15:30:39 +0100 Subject: [PATCH] BCFIPS approved mode: Some tests failing due the short secret for client-secret-jwt client authentication Closes #16678 --- .../test/java/org/keycloak/jose/HmacTest.java | 26 +++++++++++++++---- .../crypto/elytron/test/ElytronHmacTest.java | 8 ++---- .../crypto/fips/test/FIPS1402HmacTest.java | 25 +++++++++--------- docs/fips.md | 5 +++- .../KcOidcBrokerClientSecretJwtTest.java | 5 +++- .../keycloak/testsuite/client/FAPI1Test.java | 4 +-- .../oauth/ClientAuthSecretSignedJWTTest.java | 18 +++++++------ .../testrealm-jwt-client-secret.json | 2 +- 8 files changed, 56 insertions(+), 37 deletions(-) diff --git a/core/src/test/java/org/keycloak/jose/HmacTest.java b/core/src/test/java/org/keycloak/jose/HmacTest.java index 5b8125cd4c8..b9aedda0df6 100755 --- a/core/src/test/java/org/keycloak/jose/HmacTest.java +++ b/core/src/test/java/org/keycloak/jose/HmacTest.java @@ -17,16 +17,20 @@ package org.keycloak.jose; +import org.jboss.logging.Logger; import org.junit.Assert; import org.junit.ClassRule; import org.junit.Test; +import org.keycloak.common.util.BouncyIntegration; import org.keycloak.jose.jws.JWSBuilder; import org.keycloak.jose.jws.JWSInput; import org.keycloak.jose.jws.crypto.HMACProvider; import org.keycloak.rule.CryptoInitRule; import javax.crypto.SecretKey; +import javax.crypto.SecretKeyFactory; import javax.crypto.spec.SecretKeySpec; + import java.util.UUID; /** @@ -35,17 +39,29 @@ import java.util.UUID; */ public abstract class HmacTest { + private final Logger logger = Logger.getLogger(getClass().getName()); + @ClassRule public static CryptoInitRule cryptoInitRule = new CryptoInitRule(); @Test - public void testHmacSignatures() throws Exception { - SecretKey secret = new SecretKeySpec(UUID.randomUUID().toString().getBytes(), "HmacSHA256"); + public void testHmacSignaturesWithRandomSecretKey() throws Exception { + SecretKey secretKey = new SecretKeySpec(UUID.randomUUID().toString().getBytes(), "HmacSHA256"); + testHMACSignAndVerify(secretKey, "testHmacSignaturesWithRandomSecretKey"); + } + + @Test + public void testHmacSignaturesWithShortSecretKey() throws Exception { + SecretKey secretKey = new SecretKeySpec("secret".getBytes(), "HmacSHA256"); + testHMACSignAndVerify(secretKey, "testHmacSignaturesWithShortSecretKey"); + } + + protected void testHMACSignAndVerify(SecretKey secretKey, String test) throws Exception { String encoded = new JWSBuilder().content("12345678901234567890".getBytes()) - .hmac256(secret); - System.out.println("length: " + encoded.length()); + .hmac256(secretKey); + logger.infof("%s: Length of encoded content: %d, Length of secret key: %d", test, encoded.length(), secretKey.getEncoded().length); JWSInput input = new JWSInput(encoded); - Assert.assertTrue(HMACProvider.verify(input, secret)); + Assert.assertTrue(HMACProvider.verify(input, secretKey)); } } diff --git a/crypto/elytron/src/test/java/org/keycloak/crypto/elytron/test/ElytronHmacTest.java b/crypto/elytron/src/test/java/org/keycloak/crypto/elytron/test/ElytronHmacTest.java index 34212aa892a..7ab4d58ae46 100644 --- a/crypto/elytron/src/test/java/org/keycloak/crypto/elytron/test/ElytronHmacTest.java +++ b/crypto/elytron/src/test/java/org/keycloak/crypto/elytron/test/ElytronHmacTest.java @@ -42,13 +42,9 @@ public class ElytronHmacTest extends HmacTest { SecureRandom random = isWindows() ? SecureRandom.getInstance("Windows-PRNG") : SecureRandom.getInstance("NativePRNG"); random.setSeed(UUID.randomUUID().toString().getBytes()); keygen.init(random); - SecretKey secret = keygen.generateKey(); + SecretKey secretKey = keygen.generateKey(); - String encoded = new JWSBuilder().content("12345678901234567890".getBytes()) - .hmac256(secret); - System.out.println("length: " + encoded.length()); - JWSInput input = new JWSInput(encoded); - Assert.assertTrue(HMACProvider.verify(input, secret)); + testHMACSignAndVerify(secretKey, "testHmacSignaturesUsingKeyGen"); } private boolean isWindows(){ return System.getProperty("os.name").startsWith("Windows"); diff --git a/crypto/fips1402/src/test/java/org/keycloak/crypto/fips/test/FIPS1402HmacTest.java b/crypto/fips1402/src/test/java/org/keycloak/crypto/fips/test/FIPS1402HmacTest.java index b3cb5f6f5cc..13927093230 100644 --- a/crypto/fips1402/src/test/java/org/keycloak/crypto/fips/test/FIPS1402HmacTest.java +++ b/crypto/fips1402/src/test/java/org/keycloak/crypto/fips/test/FIPS1402HmacTest.java @@ -1,21 +1,19 @@ package org.keycloak.crypto.fips.test; + import java.util.UUID; import javax.crypto.SecretKey; import javax.crypto.SecretKeyFactory; import javax.crypto.spec.SecretKeySpec; -import org.junit.Assert; +import org.bouncycastle.crypto.CryptoServicesRegistrar; import org.junit.Assume; import org.junit.Before; import org.junit.Test; import org.keycloak.common.util.BouncyIntegration; import org.keycloak.common.util.Environment; import org.keycloak.jose.HmacTest; -import org.keycloak.jose.jws.JWSBuilder; -import org.keycloak.jose.jws.JWSInput; -import org.keycloak.jose.jws.crypto.HMACProvider; /** @@ -31,15 +29,16 @@ public class FIPS1402HmacTest extends HmacTest { } @Test - public void testHmacSignaturesFIPS() throws Exception { - // - + public void testHmacSignaturesWithRandomSecretKeyCreatedByFactory() throws Exception { SecretKeyFactory skFact = SecretKeyFactory.getInstance("HmacSHA256", BouncyIntegration.PROVIDER ); - SecretKey secret = skFact.generateSecret(new SecretKeySpec(UUID.randomUUID().toString().getBytes(), "HmacSHA256")); - String encoded = new JWSBuilder().content("12345678901234567890".getBytes()) - .hmac256(secret); - System.out.println("length: " + encoded.length()); - JWSInput input = new JWSInput(encoded); - Assert.assertTrue(HMACProvider.verify(input, secret)); + SecretKey secretKey = skFact.generateSecret(new SecretKeySpec(UUID.randomUUID().toString().getBytes(), "HmacSHA256")); + testHMACSignAndVerify(secretKey, "testHmacSignaturesWithRandomSecretKeyCreatedByFactory"); + } + + @Override + public void testHmacSignaturesWithShortSecretKey() throws Exception { + // With BCFIPS approved mode, secret key used for HmacSHA256 must be at least 112 bits long (14 characters). Short key won't work + Assume.assumeFalse(CryptoServicesRegistrar.isInApprovedOnlyMode()); + super.testHmacSignaturesWithShortSecretKey(); } } diff --git a/docs/fips.md b/docs/fips.md index 8474e5e93e7..52c70bd0b7a 100644 --- a/docs/fips.md +++ b/docs/fips.md @@ -89,7 +89,10 @@ which means even stricter security requirements on cryptography and security alg ``` --spi-password-hashing-pbkdf2-sha256-max-padding-length=14 ``` -- RSA keys of 1024 bits don't work (2048 is the minimum) +- RSA keys of 1024 bits don't work (2048 is the minimum). This applies for keys used by Keycloak realm itself (Realm keys from the `Keys` tab), but also client keys and IDP keys +- HMAC SHA-XXX keys must be at least 112 bits (or 14 characters long). For example if you use OIDC clients with the client + authentication `Signed Jwt with Client Secret` (aka `client-secret-jwt`), then your client secrets should be at least 14 characters long. + But anyway, it is recommended to use client secrets generated by Keycloak server, which always matches this requirement. - Also `jks` and `pkcs12` keystores/trustores are not supported. When starting server at startup, you can check that startup log contains `KC` provider contains KC provider with the note about `Approved Mode` like this: diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerClientSecretJwtTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerClientSecretJwtTest.java index 2d201c9c7a7..4194fcbf153 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerClientSecretJwtTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerClientSecretJwtTest.java @@ -1,6 +1,5 @@ package org.keycloak.testsuite.broker; -import static org.keycloak.testsuite.broker.BrokerTestConstants.CLIENT_SECRET; import static org.keycloak.testsuite.broker.BrokerTestConstants.IDP_OIDC_ALIAS; import static org.keycloak.testsuite.broker.BrokerTestConstants.IDP_OIDC_PROVIDER_ID; import static org.keycloak.testsuite.broker.BrokerTestTools.createIdentityProvider; @@ -16,6 +15,9 @@ import org.keycloak.representations.idm.IdentityProviderRepresentation; public class KcOidcBrokerClientSecretJwtTest extends AbstractBrokerTest { + // BCFIPS approved mode requires at least 112 bits (14 characters) long SecretKey for "client-secret-jwt" authentication + private static final String CLIENT_SECRET = "atleast-14chars-password"; + @Override protected BrokerConfiguration getBrokerConfiguration() { return new KcOidcBrokerConfigurationWithJWTAuthentication(); @@ -39,6 +41,7 @@ public class KcOidcBrokerClientSecretJwtTest extends AbstractBrokerTest { IdentityProviderRepresentation idp = createIdentityProvider(IDP_OIDC_ALIAS, IDP_OIDC_PROVIDER_ID); Map config = idp.getConfig(); applyDefaultConfiguration(config, syncMode); + config.put("clientSecret", CLIENT_SECRET); config.put("clientAuthMethod", OIDCLoginProtocol.CLIENT_SECRET_JWT); return idp; } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/FAPI1Test.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/FAPI1Test.java index 3f12bb4ad4d..a367fcc3fc1 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/FAPI1Test.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/FAPI1Test.java @@ -316,7 +316,7 @@ public class FAPI1Test extends AbstractClientPoliciesTest { // Register client (default authenticator) String clientUUID = createClientByAdmin("foo", (ClientRepresentation clientRep) -> { clientRep.setClientAuthenticatorType(JWTClientSecretAuthenticator.PROVIDER_ID); - clientRep.setSecret("secret"); + clientRep.setSecret("atleast-14chars-password"); }); ClientRepresentation client = getClientByAdmin(clientUUID); Assert.assertFalse(client.isPublicClient()); @@ -336,7 +336,7 @@ public class FAPI1Test extends AbstractClientPoliciesTest { // Check PKCE with S256, redirectUri and nonce/state set. Login should be successful successfulLoginAndLogout("foo", false, (String code) -> { - String signedJwt = getClientSecretSignedJWT("secret", Algorithm.HS256); + String signedJwt = getClientSecretSignedJWT("atleast-14chars-password", Algorithm.HS256); return doAccessTokenRequestWithClientSignedJWT(code, signedJwt, codeVerifier, DefaultHttpClient::new); }); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/ClientAuthSecretSignedJWTTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/ClientAuthSecretSignedJWTTest.java index f387ef63806..2f9f4f01555 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/ClientAuthSecretSignedJWTTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/ClientAuthSecretSignedJWTTest.java @@ -92,6 +92,9 @@ public class ClientAuthSecretSignedJWTTest extends AbstractKeycloakTest { private static final String PROFILE_NAME = "ClientSecretRotationProfile"; private static final String POLICY_NAME = "ClientSecretRotationPolicy"; private static final String OIDC = "openid-connect"; + + // BCFIPS approved mode requires at least 112 bits (14 characters) long SecretKey for "client-secret-jwt" authentication + private static final String CLIENT_SECRET = "atleast-14chars-password"; private static final ObjectMapper objectMapper = new ObjectMapper(); @Rule @@ -142,7 +145,7 @@ public class ClientAuthSecretSignedJWTTest extends AbstractKeycloakTest { } }; String algorithm = Algorithm.HS256; - jwtProvider.setClientSecret("password", algorithm); + jwtProvider.setClientSecret(CLIENT_SECRET, algorithm); String jwt = jwtProvider.createSignedRequestToken(oauth.getClientId(), getRealmInfoUrl(), algorithm); OAuthClient.AccessTokenResponse response = doAccessTokenRequest(code, jwt); @@ -180,7 +183,6 @@ public class ClientAuthSecretSignedJWTTest extends AbstractKeycloakTest { ClientRepresentation clientRep = null; final String realmName = "test"; final String clientId = "test-app"; - final String clientSecret = "password"; try { clientResource = ApiUtil.findClientByClientId(adminClient.realm(realmName), clientId); clientRep = clientResource.toRepresentation(); @@ -188,11 +190,11 @@ public class ClientAuthSecretSignedJWTTest extends AbstractKeycloakTest { clientResource.update(clientRep); oauth.clientId(clientId); - oauth.doLogin("test-user@localhost", clientSecret); + oauth.doLogin("test-user@localhost", "password"); events.expectLogin().client(clientId).assertEvent(); String code = oauth.getCurrentQuery().get(OAuth2Constants.CODE); - OAuthClient.AccessTokenResponse response = doAccessTokenRequest(code, getClientSignedJWT(clientSecret, 20, Algorithm.HS256)); + OAuthClient.AccessTokenResponse response = doAccessTokenRequest(code, getClientSignedJWT(CLIENT_SECRET, 20, Algorithm.HS256)); assertEquals(400, response.getStatusCode()); assertEquals("invalid_client", response.getError()); } catch (Exception e) { @@ -213,7 +215,7 @@ public class ClientAuthSecretSignedJWTTest extends AbstractKeycloakTest { .assertEvent(); String code = oauth.getCurrentQuery().get(OAuth2Constants.CODE); - OAuthClient.AccessTokenResponse response = doAccessTokenRequest(code, getClientSignedJWT("password", 20, algorithm)); + OAuthClient.AccessTokenResponse response = doAccessTokenRequest(code, getClientSignedJWT(CLIENT_SECRET, 20, algorithm)); assertEquals(200, response.getStatusCode()); oauth.verifyToken(response.getAccessToken()); @@ -245,7 +247,7 @@ public class ClientAuthSecretSignedJWTTest extends AbstractKeycloakTest { } private void processAuthenticateWithAlgorithm(String algorithm, Integer secretLength) throws Exception{ - String cidConfidential= createClientByAdmin("jwt-client","jwt-client","password",algorithm); + String cidConfidential= createClientByAdmin("jwt-client","jwt-client",CLIENT_SECRET,algorithm); ClientResource clientResource = adminClient.realm(REALM_NAME).clients().get(cidConfidential); configureDefaultProfileAndPolicy(); @@ -292,7 +294,7 @@ public class ClientAuthSecretSignedJWTTest extends AbstractKeycloakTest { .assertEvent(); String code = oauth.getCurrentQuery().get(OAuth2Constants.CODE); - String clientSignedJWT = getClientSignedJWT("password", 20); + String clientSignedJWT = getClientSignedJWT(CLIENT_SECRET, 20); OAuthClient.AccessTokenResponse response = doAccessTokenRequest(code, clientSignedJWT); assertEquals(200, response.getStatusCode()); @@ -329,7 +331,7 @@ public class ClientAuthSecretSignedJWTTest extends AbstractKeycloakTest { */ @Test public void authenticateWithInvalidRotatedClientSecretPolicyIsEnable() throws Exception { - String cidConfidential= createClientByAdmin("jwt-client","jwt-client","password",Algorithm.HS256); + String cidConfidential= createClientByAdmin("jwt-client","jwt-client",CLIENT_SECRET,Algorithm.HS256); ClientResource clientResource = adminClient.realm(REALM_NAME).clients().get(cidConfidential); configureDefaultProfileAndPolicy(); String firstSecret = clientResource.getSecret().getValue(); diff --git a/testsuite/integration-arquillian/tests/base/src/test/resources/client-auth-test/testrealm-jwt-client-secret.json b/testsuite/integration-arquillian/tests/base/src/test/resources/client-auth-test/testrealm-jwt-client-secret.json index 16292d22ffa..80e40774e98 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/resources/client-auth-test/testrealm-jwt-client-secret.json +++ b/testsuite/integration-arquillian/tests/base/src/test/resources/client-auth-test/testrealm-jwt-client-secret.json @@ -29,7 +29,7 @@ ], "adminUrl": "http://localhost:8180/auth/realms/master/app/admin", "clientAuthenticatorType": "client-secret-jwt", - "secret": "password" + "secret": "atleast-14chars-password" } ], "roles" : {