Improve security and code quality: SHA-256 defaults, configurable clock drift, and dependency updates

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

core/src/main/java/org/codelibs/saml2/core/settings/Saml2Settings.java

@@ -74,12 +74,13 @@ public class Saml2Settings {
     private List<String> 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<String> 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;
     }

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";

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
      *
+     * <p><b>SECURITY WARNING:</b> 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.</p>
+     *
      * @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
      *
+     * <p><b>DEPRECATED AND INSECURE:</b> 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.</p>
+     *
+     * <p><b>SECURITY WARNING:</b> Certificate fingerprint validation is vulnerable to collision attacks.
+     * It is strongly recommended to use full X.509 certificate validation instead of fingerprint-based
+     * validation.</p>
+     *
      * @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");
     }
 

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());
+    }
+
 }

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());