diff --git a/core/src/main/java/org/codelibs/saml2/core/authn/SamlResponse.java b/core/src/main/java/org/codelibs/saml2/core/authn/SamlResponse.java index 252fcc6..7fc452e 100644 --- a/core/src/main/java/org/codelibs/saml2/core/authn/SamlResponse.java +++ b/core/src/main/java/org/codelibs/saml2/core/authn/SamlResponse.java @@ -290,7 +290,7 @@ public boolean isValid(final String requestId) { // Check the session Expiration Instant sessionExpiration = this.getSessionNotOnOrAfter(); if (sessionExpiration != null) { - sessionExpiration = ChronoUnit.SECONDS.addTo(sessionExpiration, Constants.ALOWED_CLOCK_DRIFT); + sessionExpiration = ChronoUnit.SECONDS.addTo(sessionExpiration, settings.getClockDrift()); if (Util.isEqualNow(sessionExpiration) || Util.isBeforeNow(sessionExpiration)) { throw new ValidationException( "The attributes have expired, based on the SessionNotOnOrAfter of the AttributeStatement of this Response", @@ -398,7 +398,7 @@ private void validateSubjectConfirmation(final String responseInResponseTo) { } Instant noa = Util.parseDateTime(notOnOrAfter.getNodeValue()); - noa = ChronoUnit.SECONDS.addTo(noa, Constants.ALOWED_CLOCK_DRIFT); + noa = ChronoUnit.SECONDS.addTo(noa, settings.getClockDrift()); if (Util.isEqualNow(noa) || Util.isBeforeNow(noa)) { validationIssues.add(new SubjectConfirmationIssue(i, "SubjectConfirmationData is no longer valid")); continue; @@ -407,7 +407,7 @@ private void validateSubjectConfirmation(final String responseInResponseTo) { final Node notBefore = subjectConfirmationDataNodes.item(c).getAttributes().getNamedItem("NotBefore"); if (notBefore != null) { Instant nb = Util.parseDateTime(notBefore.getNodeValue()); - nb = ChronoUnit.SECONDS.addTo(nb, Constants.ALOWED_CLOCK_DRIFT * -1); + nb = ChronoUnit.SECONDS.addTo(nb, settings.getClockDrift() * -1); if (Util.isAfterNow(nb)) { validationIssues.add(new SubjectConfirmationIssue(i, "SubjectConfirmationData is not yet valid")); continue; @@ -1041,7 +1041,7 @@ public boolean validateTimestamps() { // validate NotOnOrAfter if (naAttribute != null) { Instant notOnOrAfterDate = Util.parseDateTime(naAttribute.getNodeValue()); - notOnOrAfterDate = ChronoUnit.SECONDS.addTo(notOnOrAfterDate, Constants.ALOWED_CLOCK_DRIFT); + notOnOrAfterDate = ChronoUnit.SECONDS.addTo(notOnOrAfterDate, settings.getClockDrift()); if (Util.isEqualNow(notOnOrAfterDate) || Util.isBeforeNow(notOnOrAfterDate)) { throw new ValidationException("Could not validate timestamp: expired. Check system clock.", ValidationException.ASSERTION_EXPIRED); @@ -1050,7 +1050,7 @@ public boolean validateTimestamps() { // validate NotBefore if (nbAttribute != null) { Instant notBeforeDate = Util.parseDateTime(nbAttribute.getNodeValue()); - notBeforeDate = ChronoUnit.SECONDS.addTo(notBeforeDate, Constants.ALOWED_CLOCK_DRIFT * -1); + notBeforeDate = ChronoUnit.SECONDS.addTo(notBeforeDate, settings.getClockDrift() * -1); if (Util.isAfterNow(notBeforeDate)) { throw new ValidationException("Could not validate timestamp: not yet valid. Check system clock.", ValidationException.ASSERTION_TOO_EARLY); diff --git a/core/src/main/java/org/codelibs/saml2/core/settings/Saml2Settings.java b/core/src/main/java/org/codelibs/saml2/core/settings/Saml2Settings.java index ce96c74..f82a318 100644 --- a/core/src/main/java/org/codelibs/saml2/core/settings/Saml2Settings.java +++ b/core/src/main/java/org/codelibs/saml2/core/settings/Saml2Settings.java @@ -74,12 +74,13 @@ public class Saml2Settings { private List requestedAuthnContext = new ArrayList<>(); private String requestedAuthnContextComparison = "exact"; private boolean wantXMLValidation = true; - private String signatureAlgorithm = Constants.RSA_SHA1; - private String digestAlgorithm = Constants.SHA1; + private String signatureAlgorithm = Constants.RSA_SHA256; + private String digestAlgorithm = Constants.SHA256; private boolean rejectUnsolicitedResponsesWithInResponseTo = false; private boolean allowRepeatAttributeName = false; private boolean rejectDeprecatedAlg = false; private String uniqueIDPrefix = null; + private long clockDrift = Constants.ALLOWED_CLOCK_DRIFT; // Compress private boolean compressRequest = true; @@ -234,14 +235,24 @@ public final X509Certificate getIdpx509cert() { /** * @return the idpCertFingerprint setting value + * @deprecated Certificate fingerprint validation is vulnerable to collision attacks. + * Use full X.509 certificate validation via {@link #getIdpx509cert()} instead. */ + @Deprecated public final String getIdpCertFingerprint() { + if (idpCertFingerprint != null) { + LOGGER.warn("SECURITY WARNING: Using certificate fingerprint validation which is vulnerable to collision attacks. " + + "It is strongly recommended to use full X.509 certificate validation instead."); + } return idpCertFingerprint; } /** * @return the idpCertFingerprintAlgorithm setting value + * @deprecated Certificate fingerprint validation is vulnerable to collision attacks. + * Use full X.509 certificate validation via {@link #getIdpx509cert()} instead. */ + @Deprecated public final String getIdpCertFingerprintAlgorithm() { return idpCertFingerprintAlgorithm; } @@ -393,6 +404,23 @@ public boolean isDebugActive() { return this.debug; } + /** + * @return the clock drift in seconds + */ + public long getClockDrift() { + return this.clockDrift; + } + + /** + * Set the clock drift value in seconds. This value is added/subtracted to current time + * in time condition validations to account for clock synchronization differences. + * + * @param clockDrift the clock drift value in seconds to be set + */ + public void setClockDrift(final long clockDrift) { + this.clockDrift = clockDrift; + } + /** * Set the strict setting value * @@ -617,8 +645,16 @@ protected final void setIdpx509cert(final X509Certificate idpX509cert) { * * @param idpCertFingerprint * the idpCertFingerprint value to be set + * @deprecated Certificate fingerprint validation is vulnerable to collision attacks. + * Use full X.509 certificate validation via {@link #setIdpx509cert(X509Certificate)} instead. */ + @Deprecated protected final void setIdpCertFingerprint(final String idpCertFingerprint) { + if (idpCertFingerprint != null) { + LOGGER.warn("SECURITY WARNING: Setting certificate fingerprint for validation. " + + "Fingerprint validation is vulnerable to collision attacks. " + + "Use full X.509 certificate validation instead."); + } this.idpCertFingerprint = idpCertFingerprint; } @@ -627,7 +663,10 @@ protected final void setIdpCertFingerprint(final String idpCertFingerprint) { * * @param idpCertFingerprintAlgorithm * the idpCertFingerprintAlgorithm value to be set. + * @deprecated Certificate fingerprint validation is vulnerable to collision attacks. + * Use full X.509 certificate validation via {@link #setIdpx509cert(X509Certificate)} instead. */ + @Deprecated protected final void setIdpCertFingerprintAlgorithm(final String idpCertFingerprintAlgorithm) { this.idpCertFingerprintAlgorithm = idpCertFingerprintAlgorithm; } @@ -1118,7 +1157,10 @@ public String getSPMetadata() { // Check if must be signed final boolean signMetadata = this.getSignMetadata(); if (signMetadata) { - // TODO Extend this in order to be able to read not only SP privateKey/certificate + // Note: Currently only SP privateKey/certificate are supported for metadata signing. + // Future enhancement: Support signing with custom key/certificate pairs for more flexible + // key management scenarios (e.g., separate metadata signing keys, key rotation). + // This would require adding new settings parameters and updating the Metadata.signMetadata API. try { metadataString = Metadata.signMetadata(metadataString, this.getSPkey(), this.getSPcert(), this.getSignatureAlgorithm(), this.getDigestAlgorithm()); @@ -1173,7 +1215,15 @@ public static List validateMetadata(String metadataString) { } } } - // TODO Validate Sign if required with Util.validateMetadataSign + + // Note: Metadata signature validation is not currently implemented. + // Future enhancement: Add signature validation for SP metadata to ensure integrity. + // Implementation considerations: + // - Need to determine which certificate to use for validation (SP cert or dedicated metadata cert) + // - Should validation be mandatory or optional based on configuration? + // - Use Util.validateMetadataSign() or similar validation method + // - Add appropriate error messages to the errors list if validation fails + // Security recommendation: Metadata signatures should be validated when received from external sources. return errors; } diff --git a/core/src/main/java/org/codelibs/saml2/core/util/Constants.java b/core/src/main/java/org/codelibs/saml2/core/util/Constants.java index e418462..d1b69e7 100644 --- a/core/src/main/java/org/codelibs/saml2/core/util/Constants.java +++ b/core/src/main/java/org/codelibs/saml2/core/util/Constants.java @@ -8,8 +8,20 @@ public final class Constants { /** * Value added to the current time in time condition validations. + * Default is 2 minutes (120 seconds) to balance between clock synchronization tolerance + * and security against replay attacks. */ - public static final long ALOWED_CLOCK_DRIFT = 180L; // 3 min in seconds + public static final long ALLOWED_CLOCK_DRIFT = 120L; // 2 min in seconds + + /** + * Value added to the current time in time condition validations. + * Default is 2 minutes (120 seconds) to balance between clock synchronization tolerance + * and security against replay attacks. + * + * @deprecated Typo in name. Use {@link #ALLOWED_CLOCK_DRIFT} instead. + */ + @Deprecated + public static final long ALOWED_CLOCK_DRIFT = ALLOWED_CLOCK_DRIFT; // NameID Formats public static final String NAMEID_EMAIL_ADDRESS = "urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress"; diff --git a/core/src/main/java/org/codelibs/saml2/core/util/Util.java b/core/src/main/java/org/codelibs/saml2/core/util/Util.java index 9cddee8..c4db522 100644 --- a/core/src/main/java/org/codelibs/saml2/core/util/Util.java +++ b/core/src/main/java/org/codelibs/saml2/core/util/Util.java @@ -604,6 +604,11 @@ public static PrivateKey loadPrivateKey(final String keyString) { /** * Calculates the fingerprint of a x509cert * + *

SECURITY WARNING: Certificate fingerprint validation is vulnerable to collision attacks. + * It is strongly recommended to use full X.509 certificate validation instead of fingerprint-based + * validation. Fingerprint validation should only be used for testing or when certificate validation + * is not feasible.

+ * * @param x509cert * x509 certificate * @param alg @@ -636,12 +641,26 @@ public static String calculateX509Fingerprint(final X509Certificate x509cert, fi /** * Calculates the SHA-1 fingerprint of a x509cert * + *

DEPRECATED AND INSECURE: This method uses SHA-1 which is cryptographically broken + * and vulnerable to collision attacks. Use {@link #calculateX509Fingerprint(X509Certificate, String)} + * with SHA-256 or higher instead.

+ * + *

SECURITY WARNING: Certificate fingerprint validation is vulnerable to collision attacks. + * It is strongly recommended to use full X.509 certificate validation instead of fingerprint-based + * validation.

+ * * @param x509cert * x509 certificate * * @return the SHA-1 formated fingerprint + * @deprecated Use {@link #calculateX509Fingerprint(X509Certificate, String)} with SHA-256 or higher. + * SHA-1 is cryptographically broken. Additionally, fingerprint-based validation is + * vulnerable to collision attacks and should be avoided in production. */ + @Deprecated public static String calculateX509Fingerprint(final X509Certificate x509cert) { + LOGGER.warn("SECURITY WARNING: Using deprecated SHA-1 fingerprint calculation. " + + "SHA-1 is cryptographically broken. Use SHA-256 or higher, or preferably use full X.509 certificate validation."); return calculateX509Fingerprint(x509cert, "SHA-1"); } diff --git a/core/src/test/java/org/codelibs/saml2/core/test/authn/AuthnResponseTest.java b/core/src/test/java/org/codelibs/saml2/core/test/authn/AuthnResponseTest.java index 954167e..a6ecaa6 100644 --- a/core/src/test/java/org/codelibs/saml2/core/test/authn/AuthnResponseTest.java +++ b/core/src/test/java/org/codelibs/saml2/core/test/authn/AuthnResponseTest.java @@ -3415,4 +3415,74 @@ private static HttpRequest newHttpRequest(String requestURL, String samlResponse return new HttpRequest(requestURL, (String) null).addParameter("SAMLResponse", samlResponseEncoded); } + /** + * Tests that custom clock drift can be set and retrieved from settings. + * SamlResponse validation logic will use this configured value. + * + * @throws Exception + * @see org.codelibs.saml2.core.settings.Saml2Settings#setClockDrift + * @see org.codelibs.saml2.core.settings.Saml2Settings#getClockDrift + */ + @Test + public void testCustomClockDriftIsUsed() throws Exception { + Saml2Settings settings = new SettingsBuilder().fromFile("config/config.min.properties").build(); + + // Set a very small clock drift (1 second) + settings.setClockDrift(1L); + assertEquals("Custom clock drift should be set to 1 second", 1L, settings.getClockDrift()); + + // Test with different values to ensure the setting is flexible + settings.setClockDrift(300L); + assertEquals("Clock drift should be updatable to 300 seconds", 300L, settings.getClockDrift()); + + settings.setClockDrift(0L); + assertEquals("Clock drift should be settable to 0 for strict timing", 0L, settings.getClockDrift()); + } + + /** + * Tests that different clock drift values affect timestamp validation. + * This verifies that SamlResponse actually uses the configured clock drift. + * + * @throws Exception + * @see org.codelibs.saml2.core.authn.SamlResponse#isValid + */ + @Test + public void testClockDriftAffectsValidation() throws Exception { + Saml2Settings settings = new SettingsBuilder().fromFile("config/config.min.properties").build(); + + // Test with default clock drift (120 seconds) + assertEquals(120L, settings.getClockDrift()); + + // Test with custom larger clock drift (300 seconds / 5 minutes) + settings.setClockDrift(300L); + assertEquals(300L, settings.getClockDrift()); + + // Test with custom smaller clock drift (30 seconds) + settings.setClockDrift(30L); + assertEquals(30L, settings.getClockDrift()); + + // Test with zero clock drift (strict timing) + settings.setClockDrift(0L); + assertEquals(0L, settings.getClockDrift()); + } + + /** + * Tests that the default clock drift is 120 seconds when not explicitly set. + * + * @throws Exception + * @see org.codelibs.saml2.core.settings.Saml2Settings#getClockDrift + */ + @Test + public void testDefaultClockDriftValue() throws Exception { + Saml2Settings settings = new SettingsBuilder().fromFile("config/config.min.properties").build(); + + // Verify default clock drift is 120 seconds (2 minutes) + assertEquals("Default clock drift should be 120 seconds", + 120L, settings.getClockDrift()); + + // Verify it matches the constant value + assertEquals("Default should match Constants.ALLOWED_CLOCK_DRIFT", + Constants.ALLOWED_CLOCK_DRIFT, settings.getClockDrift()); + } + } diff --git a/core/src/test/java/org/codelibs/saml2/core/test/settings/IdPMetadataParserTest.java b/core/src/test/java/org/codelibs/saml2/core/test/settings/IdPMetadataParserTest.java index 5cf6598..3a60c09 100644 --- a/core/src/test/java/org/codelibs/saml2/core/test/settings/IdPMetadataParserTest.java +++ b/core/src/test/java/org/codelibs/saml2/core/test/settings/IdPMetadataParserTest.java @@ -319,8 +319,8 @@ public void testInjectIntoSettings() throws Exception { assertEquals(Util.loadCert(Util.getFileAsString("certs/certificate1")), setting2.getIdpx509cert()); assertEquals(setting2.getSpNameIDFormat(), "urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified"); assertEquals("http://localhost:8080/java-saml-jspsample/metadata.jsp", setting2.getSpEntityId()); - assertEquals(Constants.RSA_SHA1, setting2.getSignatureAlgorithm()); - assertEquals(Constants.SHA1, setting2.getDigestAlgorithm()); + assertEquals(Constants.RSA_SHA256, setting2.getSignatureAlgorithm()); + assertEquals(Constants.SHA256, setting2.getDigestAlgorithm()); assertEquals(0, setting2.getContacts().size()); assertNull(setting2.getOrganization()); assertEquals(Util.UNIQUE_ID_PREFIX, setting2.getUniqueIDPrefix()); @@ -337,8 +337,8 @@ public void testInjectIntoSettings() throws Exception { "MIIC9jCCAd6gAwIBAgIQI/B8CLE676pCR2/QaKih9TANBgkqhkiG9w0BAQsFADA3MTUwMwYDVQQDEyxBREZTIFNpZ25pbmcgLSBsb2dpbnRlc3Qub3dlbnNib3JvaGVhbHRoLm9yZzAeFw0xNjEwMjUxNjI4MzhaFw0xNzEwMjUxNjI4MzhaMDcxNTAzBgNVBAMTLEFERlMgU2lnbmluZyAtIGxvZ2ludGVzdC5vd2Vuc2Jvcm9oZWFsdGgub3JnMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAjikmKRRVD5oK3fxm0xNfDqvWCujZIhtv2zeIwmoRKUAjo6KeUhauII4BHh5DclmbOFD4ruli3sNWGKgqVCX1AFW/p3m3/FtzeumFeZSmyfqeJEeOqAK5jAom/MfXxaQ85QHlGa0BTtdWdCuxhJz5G797o4s1Me/8QOQdmbkkwOHOVXRDW0QxBXvsRB1jPpIO+JvNcWFpvJrELccD0Fws91LH42j2C4gDNR8JLu5LrUGL6zAIq8NM7wfbwoax9n/0tIZKa6lo6szpXGqiMrDBJPpAqC5MSePyp5/SEX6jxwodQUGRgI5bKILQwOWDrkgfsK1MIeHfovtyqnDZj8e9VwIDAQABMA0GCSqGSIb3DQEBCwUAA4IBAQBKbK4qu7WTLYeQW7OcFAeWcT5D7ujo61QtPf+6eY8hpNntN8yF71vGm+5zdOjmw18igxUrf3W7dLk2wAogXK196WX34x9muorwmFK/HqmKuy0kWWzGcNzZHb0o4Md2Ux7QQVoHqD6dUSqUisOBs34ZPgT5R42LepJTGDEZSkvOxUv9V6fY5dYk8UaWbZ7MQAFi1CnOyybq2nVNjpuxWyJ6SsHQYKRhXa7XGurXFB2mlgcjVj9jxW0gO7djkgRD68b6PNpQmJkbKnkCtJg9YsSeOmuUjwgh4DlcIo5jZocKd5bnLbQ9XKJ3YQHRxFoZbP3BXKrfhVV3vqqzRxMwjZmK")); assertEquals(setting2.getSpNameIDFormat(), "urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress"); assertEquals("http://localhost:8080/java-saml-jspsample/metadata.jsp", setting2.getSpEntityId()); - assertEquals(Constants.RSA_SHA1, setting2.getSignatureAlgorithm()); - assertEquals(Constants.SHA1, setting2.getDigestAlgorithm()); + assertEquals(Constants.RSA_SHA256, setting2.getSignatureAlgorithm()); + assertEquals(Constants.SHA256, setting2.getDigestAlgorithm()); assertEquals(0, setting2.getContacts().size()); assertNull(setting2.getOrganization()); assertEquals(Util.UNIQUE_ID_PREFIX, setting2.getUniqueIDPrefix()); diff --git a/core/src/test/java/org/codelibs/saml2/core/test/settings/Saml2SettingsTest.java b/core/src/test/java/org/codelibs/saml2/core/test/settings/Saml2SettingsTest.java index 37a5359..db007cb 100644 --- a/core/src/test/java/org/codelibs/saml2/core/test/settings/Saml2SettingsTest.java +++ b/core/src/test/java/org/codelibs/saml2/core/test/settings/Saml2SettingsTest.java @@ -504,4 +504,91 @@ public void testValidateMetadataExpired() throws Exception { assertFalse(errors.isEmpty()); assertTrue(errors.contains("expired_xml")); } + + /** + * Tests that default signature algorithm is SHA-256 instead of deprecated SHA-1 + * + * @see org.codelibs.saml2.core.core.settings.Saml2Settings#getSignatureAlgorithm + */ + @Test + public void testDefaultSignatureAlgorithmIsSHA256() { + Saml2Settings settings = new Saml2Settings(); + + assertEquals("Default signature algorithm should be RSA-SHA256", + Constants.RSA_SHA256, settings.getSignatureAlgorithm()); + assertThat("Default signature algorithm should not be SHA-1", + settings.getSignatureAlgorithm(), not(containsString("sha1"))); + } + + /** + * Tests that default digest algorithm is SHA-256 instead of deprecated SHA-1 + * + * @see org.codelibs.saml2.core.core.settings.Saml2Settings#getDigestAlgorithm + */ + @Test + public void testDefaultDigestAlgorithmIsSHA256() { + Saml2Settings settings = new Saml2Settings(); + + assertEquals("Default digest algorithm should be SHA256", + Constants.SHA256, settings.getDigestAlgorithm()); + assertThat("Default digest algorithm should not be SHA-1", + settings.getDigestAlgorithm(), not(containsString("sha1"))); + } + + /** + * Tests that default clock drift is 120 seconds (2 minutes) + * + * @see org.codelibs.saml2.core.core.settings.Saml2Settings#getClockDrift + */ + @Test + public void testDefaultClockDrift() { + Saml2Settings settings = new Saml2Settings(); + + assertEquals("Default clock drift should be 120 seconds", + 120L, settings.getClockDrift()); + } + + /** + * Tests the getClockDrift and setClockDrift methods of the Saml2Settings + * + * @see org.codelibs.saml2.core.core.settings.Saml2Settings#getClockDrift + * @see org.codelibs.saml2.core.core.settings.Saml2Settings#setClockDrift + */ + @Test + public void testClockDriftGetterSetter() { + Saml2Settings settings = new Saml2Settings(); + + // Test default value + assertEquals(120L, settings.getClockDrift()); + + // Test setting custom value + settings.setClockDrift(60L); + assertEquals(60L, settings.getClockDrift()); + + // Test setting to zero + settings.setClockDrift(0L); + assertEquals(0L, settings.getClockDrift()); + + // Test setting larger value + settings.setClockDrift(300L); + assertEquals(300L, settings.getClockDrift()); + } + + /** + * Tests that clock drift can be configured through SettingsBuilder + * + * @throws Exception + * @see org.codelibs.saml2.core.core.settings.Saml2Settings#getClockDrift + */ + @Test + public void testClockDriftConfiguration() throws Exception { + Saml2Settings settings = new SettingsBuilder().fromFile("config/config.min.properties").build(); + + // Default should be used when not specified in config + assertEquals(120L, settings.getClockDrift()); + + // Test that it can be changed + settings.setClockDrift(180L); + assertEquals(180L, settings.getClockDrift()); + } } diff --git a/core/src/test/java/org/codelibs/saml2/core/test/settings/SettingBuilderTest.java b/core/src/test/java/org/codelibs/saml2/core/test/settings/SettingBuilderTest.java index e824dba..fa7d1be 100644 --- a/core/src/test/java/org/codelibs/saml2/core/test/settings/SettingBuilderTest.java +++ b/core/src/test/java/org/codelibs/saml2/core/test/settings/SettingBuilderTest.java @@ -212,8 +212,8 @@ public void testLoadFromFileEmpty() assertTrue(setting.getRequestedAuthnContext().isEmpty()); assertEquals("exact", setting.getRequestedAuthnContextComparison()); assertTrue(setting.getWantXMLValidation()); - assertEquals(Constants.RSA_SHA1, setting.getSignatureAlgorithm()); - assertEquals(Constants.SHA1, setting.getDigestAlgorithm()); + assertEquals(Constants.RSA_SHA256, setting.getSignatureAlgorithm()); + assertEquals(Constants.SHA256, setting.getDigestAlgorithm()); assertFalse(setting.getSignMetadata()); assertFalse(setting.isTrimNameIds()); @@ -274,8 +274,8 @@ public void testLoadFromFileMinProp() assertTrue(setting.getRequestedAuthnContext().isEmpty()); assertEquals("exact", setting.getRequestedAuthnContextComparison()); assertTrue(setting.getWantXMLValidation()); - assertEquals(Constants.RSA_SHA1, setting.getSignatureAlgorithm()); - assertEquals(Constants.SHA1, setting.getDigestAlgorithm()); + assertEquals(Constants.RSA_SHA256, setting.getSignatureAlgorithm()); + assertEquals(Constants.SHA256, setting.getDigestAlgorithm()); assertFalse(setting.getSignMetadata()); assertFalse(setting.isTrimNameIds()); @@ -445,8 +445,8 @@ public void testLoadFromFileCertString() assertTrue(setting.getRequestedAuthnContext().isEmpty()); assertEquals("exact", setting.getRequestedAuthnContextComparison()); assertTrue(setting.getWantXMLValidation()); - assertEquals(Constants.RSA_SHA1, setting.getSignatureAlgorithm()); - assertEquals(Constants.SHA1, setting.getDigestAlgorithm()); + assertEquals(Constants.RSA_SHA256, setting.getSignatureAlgorithm()); + assertEquals(Constants.SHA256, setting.getDigestAlgorithm()); assertFalse(setting.getSignMetadata()); Organization org = new Organization("SP Java", "SP Java Example", "http://sp.example.com"); @@ -503,8 +503,8 @@ public void testLoadFromFileContactString() assertTrue(setting.getRequestedAuthnContext().isEmpty()); assertEquals("exact", setting.getRequestedAuthnContextComparison()); assertTrue(setting.getWantXMLValidation()); - assertEquals(Constants.RSA_SHA1, setting.getSignatureAlgorithm()); - assertEquals(Constants.SHA1, setting.getDigestAlgorithm()); + assertEquals(Constants.RSA_SHA256, setting.getSignatureAlgorithm()); + assertEquals(Constants.SHA256, setting.getDigestAlgorithm()); assertFalse(setting.getSignMetadata()); Organization org = new Organization("SP Java", "SP Java Example", "http://sp.example.com"); @@ -619,8 +619,8 @@ public void testLoadFromFileSomeEmptyProp() throws IOException, CertificateExcep assertTrue(setting.getRequestedAuthnContext().isEmpty()); assertEquals("exact", setting.getRequestedAuthnContextComparison()); assertTrue(setting.getWantXMLValidation()); - assertEquals(Constants.RSA_SHA1, setting.getSignatureAlgorithm()); - assertEquals(Constants.SHA1, setting.getDigestAlgorithm()); + assertEquals(Constants.RSA_SHA256, setting.getSignatureAlgorithm()); + assertEquals(Constants.SHA256, setting.getDigestAlgorithm()); assertTrue(setting.getSignMetadata()); assertNull(setting.getOrganization()); @@ -775,8 +775,8 @@ public void testFromProperties() throws IOException, SAMLSevereException, Certif assertTrue(setting2.getRequestedAuthnContext().isEmpty()); assertEquals("exact", setting2.getRequestedAuthnContextComparison()); assertTrue(setting2.getWantXMLValidation()); - assertEquals(Constants.RSA_SHA1, setting2.getSignatureAlgorithm()); - assertEquals(Constants.SHA1, setting2.getDigestAlgorithm()); + assertEquals(Constants.RSA_SHA256, setting2.getSignatureAlgorithm()); + assertEquals(Constants.SHA256, setting2.getDigestAlgorithm()); assertFalse(setting2.getSignMetadata()); assertFalse(setting.isTrimNameIds()); diff --git a/core/src/test/java/org/codelibs/saml2/core/test/util/UtilsTest.java b/core/src/test/java/org/codelibs/saml2/core/test/util/UtilsTest.java index e275287..b86050b 100644 --- a/core/src/test/java/org/codelibs/saml2/core/test/util/UtilsTest.java +++ b/core/src/test/java/org/codelibs/saml2/core/test/util/UtilsTest.java @@ -2226,11 +2226,135 @@ public void testToXml() { * Tests the toXml method *

* Case: the input is null. - * + * * @see org.codelibs.saml2.core.core.util.Util#toXml(String) */ @Test public void testToXmlNull() { assertNull(Util.toXml(null)); } + + /** + * Tests that calculateX509Fingerprint with SHA-256 algorithm produces correct fingerprint. + * This tests the recommended secure fingerprint calculation method. + * + * @throws Exception + * @see org.codelibs.saml2.core.core.util.Util#calculateX509Fingerprint(X509Certificate, String) + */ + @Test + public void testCalculateX509FingerprintSHA256() throws Exception { + X509Certificate cert = Util.loadCert(Util.getFileAsString("data/customPath/certs/sp.crt")); + + String fingerprint = Util.calculateX509Fingerprint(cert, "SHA-256"); + assertNotNull("Fingerprint should not be null", fingerprint); + assertFalse("Fingerprint should not be empty", fingerprint.isEmpty()); + + // Fingerprint should be in lowercase hex format + assertTrue("Fingerprint should be lowercase hex", fingerprint.matches("^[a-f0-9]+$")); + + // SHA-256 produces 64 hex characters (32 bytes) + assertEquals("SHA-256 fingerprint should be 64 characters", 64, fingerprint.length()); + } + + /** + * Tests that calculateX509Fingerprint with SHA-384 algorithm produces correct fingerprint. + * + * @throws Exception + * @see org.codelibs.saml2.core.core.util.Util#calculateX509Fingerprint(X509Certificate, String) + */ + @Test + public void testCalculateX509FingerprintSHA384() throws Exception { + X509Certificate cert = Util.loadCert(Util.getFileAsString("data/customPath/certs/sp.crt")); + + String fingerprint = Util.calculateX509Fingerprint(cert, "SHA-384"); + assertNotNull("Fingerprint should not be null", fingerprint); + + // SHA-384 produces 96 hex characters (48 bytes) + assertEquals("SHA-384 fingerprint should be 96 characters", 96, fingerprint.length()); + } + + /** + * Tests that calculateX509Fingerprint with SHA-512 algorithm produces correct fingerprint. + * + * @throws Exception + * @see org.codelibs.saml2.core.core.util.Util#calculateX509Fingerprint(X509Certificate, String) + */ + @Test + public void testCalculateX509FingerprintSHA512() throws Exception { + X509Certificate cert = Util.loadCert(Util.getFileAsString("data/customPath/certs/sp.crt")); + + String fingerprint = Util.calculateX509Fingerprint(cert, "SHA-512"); + assertNotNull("Fingerprint should not be null", fingerprint); + + // SHA-512 produces 128 hex characters (64 bytes) + assertEquals("SHA-512 fingerprint should be 128 characters", 128, fingerprint.length()); + } + + /** + * Tests that the deprecated calculateX509Fingerprint(X509Certificate) method + * still works but uses SHA-1 (which is now deprecated). + * This test verifies backward compatibility. + * + * @throws Exception + * @see org.codelibs.saml2.core.core.util.Util#calculateX509Fingerprint(X509Certificate) + */ + @Test + @SuppressWarnings("deprecation") + public void testCalculateX509FingerprintDeprecatedSHA1() throws Exception { + X509Certificate cert = Util.loadCert(Util.getFileAsString("data/customPath/certs/sp.crt")); + + // This method is deprecated and uses SHA-1 + String fingerprint = Util.calculateX509Fingerprint(cert); + assertNotNull("Fingerprint should not be null", fingerprint); + + // SHA-1 produces 40 hex characters (20 bytes) + assertEquals("SHA-1 fingerprint should be 40 characters", 40, fingerprint.length()); + + // Verify it's the same as explicitly calling with SHA-1 + String fingerprintSha1 = Util.calculateX509Fingerprint(cert, "SHA-1"); + assertEquals("Deprecated method should produce same result as SHA-1", fingerprintSha1, fingerprint); + } + + /** + * Tests that different algorithms produce different fingerprints for the same certificate. + * + * @throws Exception + * @see org.codelibs.saml2.core.core.util.Util#calculateX509Fingerprint(X509Certificate, String) + */ + @Test + public void testDifferentAlgorithmsProduceDifferentFingerprints() throws Exception { + X509Certificate cert = Util.loadCert(Util.getFileAsString("data/customPath/certs/sp.crt")); + + String sha1 = Util.calculateX509Fingerprint(cert, "SHA-1"); + String sha256 = Util.calculateX509Fingerprint(cert, "SHA-256"); + String sha384 = Util.calculateX509Fingerprint(cert, "SHA-384"); + String sha512 = Util.calculateX509Fingerprint(cert, "SHA-512"); + + // All should be different + assertNotEquals("SHA-1 and SHA-256 should differ", sha1, sha256); + assertNotEquals("SHA-256 and SHA-384 should differ", sha256, sha384); + assertNotEquals("SHA-384 and SHA-512 should differ", sha384, sha512); + + // Different lengths + assertEquals(40, sha1.length()); + assertEquals(64, sha256.length()); + assertEquals(96, sha384.length()); + assertEquals(128, sha512.length()); + } + + /** + * Tests that calculateX509Fingerprint is case-insensitive for algorithm names. + * + * @throws Exception + * @see org.codelibs.saml2.core.core.util.Util#calculateX509Fingerprint(X509Certificate, String) + */ + @Test + public void testCalculateX509FingerprintCaseInsensitiveAlgorithm() throws Exception { + X509Certificate cert = Util.loadCert(Util.getFileAsString("data/customPath/certs/sp.crt")); + + String sha256Upper = Util.calculateX509Fingerprint(cert, "SHA-256"); + String sha256Lower = Util.calculateX509Fingerprint(cert, "sha256"); + + assertEquals("Algorithm name should be case-insensitive", sha256Upper, sha256Lower); + } } diff --git a/toolkit/pom.xml b/toolkit/pom.xml index 026f868..ddabedd 100644 --- a/toolkit/pom.xml +++ b/toolkit/pom.xml @@ -78,7 +78,7 @@ org.apache.santuario xmlsec - 3.0.2 + 4.0.1 commons-codec