From 951a6dfa474fc3cae11c06746ff6358dc9f084c0 Mon Sep 17 00:00:00 2001 From: Thomas Nymand Date: Thu, 18 Jun 2026 15:20:22 +0200 Subject: [PATCH] REF-6: Resolve keystore alias case-insensitively (issue #73) Configuring oiosaml.servlet.keystore.alias with any casing other than all-lowercase failed with a misleading "incorrect keystore password" error. Java's PKCS12 KeyStore lowercases aliases on load, so the key-password map was keyed by the lowercased alias while OpenSAML's KeyStoreCredentialResolver looked the password up by the configured alias verbatim. A mixed-case alias therefore resolved no password and the private key could not be decrypted. Key the password map by the configured alias instead; PKCS12 key lookup is already case-insensitive, so alias resolution is now fully case-insensitive. Add CredentialServiceTest with a keystore whose alias is configured in a different case than stored. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../oio/saml/service/CredentialService.java | 14 +++--- .../saml/service/CredentialServiceTest.java | 47 ++++++++++++++++++ .../src/test/resources/mixedcase-alias.p12 | Bin 0 -> 2660 bytes 3 files changed, 55 insertions(+), 6 deletions(-) create mode 100644 oiosaml/src/test/java/dk/gov/oio/saml/service/CredentialServiceTest.java create mode 100644 oiosaml/src/test/resources/mixedcase-alias.p12 diff --git a/oiosaml/src/main/java/dk/gov/oio/saml/service/CredentialService.java b/oiosaml/src/main/java/dk/gov/oio/saml/service/CredentialService.java index b5f1196..8c8900c 100644 --- a/oiosaml/src/main/java/dk/gov/oio/saml/service/CredentialService.java +++ b/oiosaml/src/main/java/dk/gov/oio/saml/service/CredentialService.java @@ -90,13 +90,15 @@ private BasicX509Credential getBasicX509Credential(String keystoreLocation, Stri KeyStore ks = keyStore(keystoreLocation, keystorePassword.toCharArray()); + // OpenSAML's KeyStoreCredentialResolver looks up the key password by the alias + // supplied in the EntityIdCriterion below. Java's PKCS12 keystore lowercases + // aliases on load, so keying the password map by the keystore's stored alias + // broke whenever the configured alias used a different casing, surfacing as a + // misleading "incorrect keystore password" error (issue #73). Key it by the + // configured alias instead; PKCS12 key lookup is already case-insensitive, so + // alias resolution becomes fully case-insensitive. Map passwords = new HashMap<>(); - try { - passwords.put(ks.aliases().nextElement(), keystorePassword); - } - catch (KeyStoreException e) { - throw new InternalException("Keystore not initialized properly", e); - } + passwords.put(alias, keystorePassword); KeyStoreCredentialResolver resolver = new KeyStoreCredentialResolver(ks, passwords); CriteriaSet criteria = new CriteriaSet(); diff --git a/oiosaml/src/test/java/dk/gov/oio/saml/service/CredentialServiceTest.java b/oiosaml/src/test/java/dk/gov/oio/saml/service/CredentialServiceTest.java new file mode 100644 index 0000000..d2d0a3a --- /dev/null +++ b/oiosaml/src/test/java/dk/gov/oio/saml/service/CredentialServiceTest.java @@ -0,0 +1,47 @@ +package dk.gov.oio.saml.service; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.opensaml.core.config.InitializationService; +import org.opensaml.security.x509.BasicX509Credential; + +import dk.gov.oio.saml.config.Configuration; +import dk.gov.oio.saml.util.TestConstants; + +public class CredentialServiceTest { + + // 'mixedcase-alias.p12' holds a single key entry created with the alias + // "TestKeyAlias". Java's PKCS12 keystore lowercases aliases on load, so + // configuring the alias with any other casing used to fail resolving the key + // and surfaced as a misleading "incorrect keystore password" error (issue #73). + private static final String MIXED_CASE_KEYSTORE = "mixedcase-alias.p12"; + private static final String ALIAS_IN_DIFFERENT_CASE = "TestKeyAlias"; + + @BeforeAll + public static void initOpenSAML() throws Exception { + InitializationService.initialize(); + } + + @DisplayName("Keystore alias is resolved case-insensitively (issue #73)") + @Test + public void testKeystoreAliasIsCaseInsensitive() throws Exception { + String keystoreLocation = getClass().getClassLoader().getResource(MIXED_CASE_KEYSTORE).getFile(); + + Configuration config = new Configuration.Builder() + .setSpEntityID(TestConstants.SP_ENTITY_ID) + .setBaseUrl(TestConstants.SP_BASE_URL) + .setIdpEntityID(TestConstants.IDP_ENTITY_ID) + .setIdpMetadataUrl(TestConstants.IDP_METADATA_URL) + .setKeystoreLocation(keystoreLocation) + .setKeystorePassword("Test1234") + .setKeyAlias(ALIAS_IN_DIFFERENT_CASE) + .build(); + + CredentialService credentialService = new CredentialService(config); + BasicX509Credential credential = credentialService.getPrimaryBasicX509Credential(); + + Assertions.assertNotNull(credential, "Key should resolve regardless of the configured alias casing"); + } +} diff --git a/oiosaml/src/test/resources/mixedcase-alias.p12 b/oiosaml/src/test/resources/mixedcase-alias.p12 new file mode 100644 index 0000000000000000000000000000000000000000..4ca5f5d680f46cc81f66d00cd73d86e5d0fd2442 GIT binary patch literal 2660 zcma)8X*3j!8lKgdv1h&{*|LRb#u5|BHpuoN`|e|8%Q8yCSjN!Unrf0QW8amX$WE^8 zv@qGXQnG|3OSta2_w=3n>)vzT<-E`PJnzrvyeKll83Y8Q$Or@zT#BMc`N;-^0kg^Q z0|*)Z6Geu9N0Fh8|FwcXgOH&`Cve_LW-+n+N5zT&g0jg_^%GDP<^Ed%WkcDZl>Z)C zQPL3PDtkBnE-dLQII84^qa`-_rMb&A2qeJ?0t%s6nVA255d?<O2z?}<@>_T_tXNgW12OdGlkP(>;ad3|^DFP$3E7aWJnvOU9LV2F%Rw~Br>3YA> zng3UM^}xJ(?HwetOuThic_qU?B2`#S6RSmDiZaZ{r8LU8%7GX_V zwZp~w{}6t$({K;bTbQ>?TEMb~M~}U@`BkNmwLxQ( zPvP}v<>@RVuNQv7gUsS|oXSZULU6>WYes1071l*}e`U`}XJGEIzHL%9p^LAry4{sLJwfZa(DqH~mhm%z5vjmAh5_1Kqof75yAeOg z&pq#a(|v<}YVV?&w&ny91}tr2e9*Xw38poe5H?Z5v*WVd1b+*|@{}$+-Sc^eiB{O# zHnOK!y@ME#IHI78M{CsT3lovc*eeifB3LopUN1D$X0_7unLUZtG7wWZDdC< zBa1{mEs>ViE^JVP9{!h@U~oXZeWolM-el>SONXl!JGnRE2hWl607+im@ll+L(%PQ0 z)M!&Zym*d>DG+xQ=#`!{WFzDD6+^#0iB7U?Uzd>L*XWu7mpO9IonG!iDpOj4M&HWI z=viW*8o;sxN3W96#Vm1tt+|krvQZ*pg?HfWt)4%3O&`6IG~sp285U`D$AxC*@0vED zdG*Guh8MJ5F--Znn~63h_Kq0yg+y-So?_LLwDU8<2F1jos4p0 zHH|Hm!hKr`+m`v~aG#<)2h=7J zO?CWKX3o5nJ1Pt@zpklZQtOBO(IUy5X3ct5ZtLCtJ?YTtP za6fH4x@0!qIT=$h`;2(^@#od=S(02wuS+@;8bH%&C{lQ9MSDh_$Fdt?DFOwmW%ud>$INSYQHFjdpN>X4b#V49K0WJU^faggKL<#>*`~!`IoHF;kg%d(6DW8P0 zoV(++emQ!4LkY& z$FlwMs@iD}6f*BzuFxh|4Q7(a;OxQF6w1KZ6iGmPK6P2YpI+HIJORGZ~aU=J?(K>U2}C>ls%Xon3KSL*Q%i{?!s&`%u$8 zyQ)7lXM;k!uO);T>cv=jd|+h|^?hNd{0~O4=LW5uI61^uF_Jv~CQq4lS@;S-UuCo< z!fT{g3cjbitB$5j26JDhPmOv8+a54LqdX7eTsTIk%5qMn3~kK5F)`MQSl20zlDhXC znVHjspJ{^$RL)+X_7|zzH@vOOhg`J5O(ba5pA|RT7S-G;HLa1OUDJA!CCW2Zz-X6` zX0++YFOC4ewSRD#3mA@xGQRgzM6mYU)4N5{+xLg)Wy#&iBC-^#^&_huiTcRYPg4_I zc5Zt8g%H;lY_hBUo@yb{CDek;ws<4GTMO3q=T2>IY7M@6c8v$ovRGP&(x8(4zJIz(i>>MNVtC(#J0C!_Hr7sqb>>;Pb}| zuk|EV-w38A?F&vG#-+^1g0S3w#j@Yt&7VRYKx~>Zd#xSPGtoY6pCgosooro`B~;s( z(_}@3(fO>TQv6%mTR1F**HSYtZ7VfV7{vE-4IJqbbwk1!igG{37GlMB<{uw>gcE<5 z$Ja@InHcj(5}Y6O^5?0pB&Z&Uqow1%F7`G(oFQr=XgAPH0{5Ql9Qmcfbo~^ZHVQ^H zK8`bMM2Csa^xZZT-V(-alVlrIE2-Io?A#lbk@uJBi=V?Mb0P|C@U0D|6{lrPmdEu{Meq?=+`|J1