From 3a04acab51bae849bd4fe2ec70b744abdf8523e4 Mon Sep 17 00:00:00 2001 From: Steven Hawkins Date: Tue, 13 Feb 2024 09:38:12 -0500 Subject: [PATCH] fix: adds pfx as a recognized extension (#26876) closes #24661 Signed-off-by: Steve Hawkins --- .../keycloak/common/util/KeystoreUtil.java | 27 ++++++++++++------- .../common/util/KeystoreUtilTest.java | 16 +++++++++++ .../FileTruststoreProviderFactory.java | 14 ++-------- .../truststore/TruststoreBuilder.java | 14 ++++------ .../truststore/TruststoreBuilderTest.java | 5 ++-- .../testsuite/util/KeystoreUtils.java | 2 +- .../testsuite/cli/admin/KcAdmTest.java | 4 +-- .../testsuite/cli/registration/KcRegTest.java | 4 +-- 8 files changed, 48 insertions(+), 38 deletions(-) create mode 100644 common/src/test/java/org/keycloak/common/util/KeystoreUtilTest.java diff --git a/common/src/main/java/org/keycloak/common/util/KeystoreUtil.java b/common/src/main/java/org/keycloak/common/util/KeystoreUtil.java index 585776cf83..97645052ad 100755 --- a/common/src/main/java/org/keycloak/common/util/KeystoreUtil.java +++ b/common/src/main/java/org/keycloak/common/util/KeystoreUtil.java @@ -28,6 +28,7 @@ import java.security.KeyStore; import java.security.PrivateKey; import java.security.PublicKey; import java.util.Arrays; +import java.util.List; import java.util.Optional; /** @@ -38,22 +39,30 @@ public class KeystoreUtil { public enum KeystoreFormat { JKS("jks"), - PKCS12("p12"), + PKCS12("p12", "pfx"), BCFKS("bcfks"); // Typical file extension for this keystore format - private final String fileExtension; - KeystoreFormat(String extension) { - this.fileExtension = extension; + private final List fileExtensions; + KeystoreFormat(String... extensions) { + this.fileExtensions = Arrays.asList(extensions); } - public String getFileExtension() { - return fileExtension; + public List getFileExtensions() { + return fileExtensions; + } + + public String getPrimaryExtension() { + return fileExtensions.get(0); } } public static KeyStore loadKeyStore(String filename, String password) throws Exception { - String keystoreType = getKeystoreType(null, filename, KeyStore.getDefaultType()); + return loadKeyStore(filename, password, null); + } + + public static KeyStore loadKeyStore(String filename, String password, String preferedType) throws Exception { + String keystoreType = getKeystoreType(preferedType, filename, KeyStore.getDefaultType()); KeyStore trustStore = KeyStore.getInstance(keystoreType); InputStream trustStream = null; if (filename.startsWith(GenericConstants.PROTOCOL_CLASSPATH)) { @@ -71,7 +80,7 @@ public class KeystoreUtil { trustStream = new FileInputStream(new File(filename)); } try (InputStream is = trustStream) { - trustStore.load(is, password.toCharArray()); + trustStore.load(is, password == null ? null : password.toCharArray()); } return trustStore; } @@ -115,7 +124,7 @@ public class KeystoreUtil { if (lastDotIndex > -1) { String ext = path.substring(lastDotIndex + 1).toLowerCase(); Optional detectedType = Arrays.stream(KeystoreUtil.KeystoreFormat.values()) - .filter(ksFormat -> ksFormat.getFileExtension().equals(ext)) + .filter(ksFormat -> ksFormat.getFileExtensions().contains(ext)) .findFirst(); if (detectedType.isPresent()) return detectedType.get().toString(); } diff --git a/common/src/test/java/org/keycloak/common/util/KeystoreUtilTest.java b/common/src/test/java/org/keycloak/common/util/KeystoreUtilTest.java new file mode 100644 index 0000000000..08828a5dfb --- /dev/null +++ b/common/src/test/java/org/keycloak/common/util/KeystoreUtilTest.java @@ -0,0 +1,16 @@ +package org.keycloak.common.util; + +import org.junit.Test; + +import static org.junit.Assert.assertEquals; + +public class KeystoreUtilTest { + + @Test + public void testGetType() { + assertEquals("x", KeystoreUtil.getKeystoreType("x", "y", "z")); + assertEquals("z", KeystoreUtil.getKeystoreType(null, "y", "z")); + assertEquals(KeystoreUtil.KeystoreFormat.PKCS12.name(), KeystoreUtil.getKeystoreType(null, "y.pfx", "z")); + } + +} \ No newline at end of file diff --git a/services/src/main/java/org/keycloak/truststore/FileTruststoreProviderFactory.java b/services/src/main/java/org/keycloak/truststore/FileTruststoreProviderFactory.java index a2a3917c8c..870c810ad1 100755 --- a/services/src/main/java/org/keycloak/truststore/FileTruststoreProviderFactory.java +++ b/services/src/main/java/org/keycloak/truststore/FileTruststoreProviderFactory.java @@ -27,8 +27,6 @@ import org.keycloak.provider.ProviderConfigProperty; import org.keycloak.provider.ProviderConfigurationBuilder; import java.io.File; -import java.io.FileInputStream; -import java.io.InputStream; import java.security.InvalidKeyException; import java.security.KeyStore; import java.security.KeyStoreException; @@ -100,12 +98,12 @@ public class FileTruststoreProviderFactory implements TruststoreProviderFactory } String type = KeystoreUtil.getKeystoreType(configuredType, storepath, KeyStore.getDefaultType()); try { - truststore = loadStore(storepath, type, pass == null ? null :pass.toCharArray()); + truststore = KeystoreUtil.loadKeyStore(storepath, pass, type); } catch (Exception e) { // in fips mode the default truststore type can be pkcs12, but the cacerts file will still be jks if (system && !"jks".equalsIgnoreCase(type)) { try { - truststore = loadStore(storepath, "jks", pass == null ? null :pass.toCharArray()); + truststore = KeystoreUtil.loadKeyStore(storepath, pass, "jks"); } catch (Exception e1) { } } @@ -130,14 +128,6 @@ public class FileTruststoreProviderFactory implements TruststoreProviderFactory log.debugf("File truststore provider initialized: %s, Truststore type: %s", new File(storepath).getAbsolutePath(), type); } - private KeyStore loadStore(String path, String type, char[] password) throws Exception { - KeyStore ks = KeyStore.getInstance(type); - try (InputStream is = new FileInputStream(path)) { - ks.load(is, password); - return ks; - } - } - @Override public void postInit(KeycloakSessionFactory factory) { } diff --git a/services/src/main/java/org/keycloak/truststore/TruststoreBuilder.java b/services/src/main/java/org/keycloak/truststore/TruststoreBuilder.java index f70131a9d1..e032740c2f 100644 --- a/services/src/main/java/org/keycloak/truststore/TruststoreBuilder.java +++ b/services/src/main/java/org/keycloak/truststore/TruststoreBuilder.java @@ -18,11 +18,11 @@ package org.keycloak.truststore; import org.jboss.logging.Logger; +import org.keycloak.common.util.KeystoreUtil; import java.io.File; import java.io.FileInputStream; import java.io.FileOutputStream; -import java.io.InputStream; import java.security.KeyStore; import java.security.KeyStoreException; import java.security.cert.Certificate; @@ -30,7 +30,6 @@ import java.security.cert.CertificateException; import java.security.cert.CertificateFactory; import java.security.cert.X509Certificate; import java.util.Collections; -import java.util.Optional; import java.util.stream.Stream; /** @@ -155,8 +154,7 @@ public class TruststoreBuilder { if (defaultTrustStore.exists()) { String path = defaultTrustStore.getAbsolutePath(); - mergeTrustStore(truststore, path, - loadStore(path, type, Optional.ofNullable(password).map(String::toCharArray).orElse(null))); + mergeTrustStore(truststore, path, loadStore(path, type, password)); } else { LOGGER.warnf("Default truststore was to be included, but could not be found at: %s", defaultTrustStore); } @@ -173,11 +171,9 @@ public class TruststoreBuilder { return new File(securityDirectory, "cacerts"); } - static KeyStore loadStore(String path, String type, char[] password) { - try (InputStream is = new FileInputStream(path)) { - KeyStore ks = KeyStore.getInstance(type); - ks.load(is, password); - return ks; + static KeyStore loadStore(String path, String type, String password) { + try { + return KeystoreUtil.loadKeyStore(path, password, type); } catch (Exception e) { throw new RuntimeException( "Failed to initialize truststore: " + new File(path).getAbsolutePath() + ", type: " + type, e); diff --git a/services/src/test/java/org/keycloak/truststore/TruststoreBuilderTest.java b/services/src/test/java/org/keycloak/truststore/TruststoreBuilderTest.java index de47a7c19f..164dece653 100644 --- a/services/src/test/java/org/keycloak/truststore/TruststoreBuilderTest.java +++ b/services/src/test/java/org/keycloak/truststore/TruststoreBuilderTest.java @@ -46,10 +46,9 @@ public class TruststoreBuilderTest { assertTrue(storeWithDefaultsAliases.containsAll(storeWithoutDefaultsAliases)); // saving / loading should provide the certs even without a password - char[] password = null; - File saved = TruststoreBuilder.saveTruststore(storeWithDefaults, "target", password); + File saved = TruststoreBuilder.saveTruststore(storeWithDefaults, "target", null); - KeyStore savedLoaded = TruststoreBuilder.loadStore(saved.getAbsolutePath(), TruststoreBuilder.PKCS12, password); + KeyStore savedLoaded = TruststoreBuilder.loadStore(saved.getAbsolutePath(), TruststoreBuilder.PKCS12, null); assertEquals(certs, Collections.list(savedLoaded.aliases()).size()); } diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/KeystoreUtils.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/KeystoreUtils.java index a4a09ab857..4615118050 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/KeystoreUtils.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/KeystoreUtils.java @@ -64,7 +64,7 @@ public class KeystoreUtils { } public static KeystoreInfo generateKeystore(TemporaryFolder folder, KeystoreUtil.KeystoreFormat keystoreType, String subject, String keystorePassword, String keyPassword) throws Exception { - String fileName = "keystore." + keystoreType.getFileExtension(); + String fileName = "keystore." + keystoreType.getPrimaryExtension(); KeyPair keyPair = KeyUtils.generateRsaKeyPair(2048); X509Certificate certificate = CertificateUtils.generateV1SelfSignedCertificate(keyPair, subject); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/cli/admin/KcAdmTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/cli/admin/KcAdmTest.java index 9006965916..8f2e5b4013 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/cli/admin/KcAdmTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/cli/admin/KcAdmTest.java @@ -513,13 +513,13 @@ public class KcAdmTest extends AbstractAdmCliTest { @Test public void testCRUDWithOnTheFlyUserAuthWithSignedJwtClient_JKSKeystore() throws IOException { KeystoreUtils.assumeKeystoreTypeSupported(KeystoreUtil.KeystoreFormat.JKS); - testCRUDWithOnTheFlyUserAuthWithSignedJwtClient(KeystoreUtil.KeystoreFormat.JKS.getFileExtension()); + testCRUDWithOnTheFlyUserAuthWithSignedJwtClient(KeystoreUtil.KeystoreFormat.JKS.getPrimaryExtension()); } @Test public void testCRUDWithOnTheFlyUserAuthWithSignedJwtClient_PKCS12Keystore() throws IOException { KeystoreUtils.assumeKeystoreTypeSupported(KeystoreUtil.KeystoreFormat.PKCS12); - testCRUDWithOnTheFlyUserAuthWithSignedJwtClient(KeystoreUtil.KeystoreFormat.PKCS12.getFileExtension()); + testCRUDWithOnTheFlyUserAuthWithSignedJwtClient(KeystoreUtil.KeystoreFormat.PKCS12.getPrimaryExtension()); } private void testCRUDWithOnTheFlyUserAuthWithSignedJwtClient(String keystoreFileExtension) throws IOException { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/cli/registration/KcRegTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/cli/registration/KcRegTest.java index 93a779c993..478415a4d9 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/cli/registration/KcRegTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/cli/registration/KcRegTest.java @@ -506,13 +506,13 @@ public class KcRegTest extends AbstractRegCliTest { @Test public void testCRUDWithOnTheFlyUserAuthWithSignedJwtClient_JKSKeystore() throws IOException { KeystoreUtils.assumeKeystoreTypeSupported(KeystoreUtil.KeystoreFormat.JKS); - testCRUDWithOnTheFlyUserAuthWithSignedJwtClient(KeystoreUtil.KeystoreFormat.JKS.getFileExtension()); + testCRUDWithOnTheFlyUserAuthWithSignedJwtClient(KeystoreUtil.KeystoreFormat.JKS.getPrimaryExtension()); } @Test public void testCRUDWithOnTheFlyUserAuthWithSignedJwtClient_PKCS12Keystore() throws IOException { KeystoreUtils.assumeKeystoreTypeSupported(KeystoreUtil.KeystoreFormat.PKCS12); - testCRUDWithOnTheFlyUserAuthWithSignedJwtClient(KeystoreUtil.KeystoreFormat.PKCS12.getFileExtension()); + testCRUDWithOnTheFlyUserAuthWithSignedJwtClient(KeystoreUtil.KeystoreFormat.PKCS12.getPrimaryExtension()); } private void testCRUDWithOnTheFlyUserAuthWithSignedJwtClient(String keystoreFileExtension) throws IOException {