Add Issuer to failed SAML Signature validation logs when available

docs/changelog/126310.yaml

@@ -0,0 +1,6 @@
+pr: 126310
+summary: Add Issuer to failed SAML Signature validation logs when available
+area: Security
+type: enhancement
+issues:
+ - 111022

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlAuthenticator.java

@@ -93,7 +93,7 @@ private SamlAttributes authenticateResponse(Element element, Collection<String>
         }
         final boolean requireSignedAssertions;
         if (response.isSigned()) {
-            validateSignature(response.getSignature());
+            validateSignature(response.getSignature(), response.getIssuer());
             requireSignedAssertions = false;
         } else {
             requireSignedAssertions = true;
@@ -199,7 +199,7 @@ private List<Attribute> processAssertion(Assertion assertion, boolean requireSig
         }
         // Do not further process unsigned Assertions
         if (assertion.isSigned()) {
-            validateSignature(assertion.getSignature());
+            validateSignature(assertion.getSignature(), assertion.getIssuer());
         } else if (requireSignature) {
             throw samlException("Assertion [{}] is not signed, but a signature is required", assertion.getElementQName());
         }

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlLogoutRequestHandler.java

@@ -73,7 +73,7 @@ private Result parseLogout(LogoutRequest logoutRequest, boolean requireSignature
                 throw samlException("Logout request is not signed");
             }
         } else {
-            validateSignature(signature);
+            validateSignature(signature, logoutRequest.getIssuer());
         }
 
         checkIssuer(logoutRequest.getIssuer(), logoutRequest);

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlLogoutResponseHandler.java

@@ -45,7 +45,7 @@ public void handle(boolean httpRedirect, String payload, Collection<String> allo
                 if (logoutResponse.getSignature() == null) {
                     throw samlException("LogoutResponse is not signed, but is required for HTTP-Post binding");
                 }
-                validateSignature(logoutResponse.getSignature());
+                validateSignature(logoutResponse.getSignature(), logoutResponse.getIssuer());
             }
             checkInResponseTo(logoutResponse, allowedSamlRequestIds);
             checkStatus(logoutResponse.getStatus());

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlObjectHandler.java

@@ -161,13 +161,13 @@ protected static String describe(Collection<X509Credential> credentials) {
         return credentials.stream().map(credential -> describe(credential.getEntityCertificate())).collect(Collectors.joining(","));
     }
 
-    void validateSignature(Signature signature) {
+    void validateSignature(Signature signature, Issuer issuer) {
         final String signatureText = text(signature, 32);
         SAMLSignatureProfileValidator profileValidator = new SAMLSignatureProfileValidator();
         try {
             profileValidator.validate(signature);
         } catch (SignatureException e) {
-            throw samlSignatureException(idp.getSigningCredentials(), signatureText, e);
+            throw samlSignatureException(issuer, idp.getSigningCredentials(), signatureText, e);
         }
 
         checkIdpSignature(credential -> {
@@ -207,14 +207,14 @@ void validateSignature(Signature signature) {
             } catch (PrivilegedActionException e) {
                 throw new SecurityException("SecurityException while attempting to validate SAML signature", e);
             }
-        }, signatureText);
+        }, signatureText, issuer);
     }
 
     /**
      * Tests whether the provided function returns {@code true} for any of the IdP's signing credentials.
-     * @throws ElasticsearchSecurityException - A SAML exception if not matching credential is found.
+     * @throws ElasticsearchSecurityException - A SAML exception if no matching credential is found.
      */
-    protected void checkIdpSignature(CheckedFunction<Credential, Boolean, Exception> check, String signatureText) {
+    protected void checkIdpSignature(CheckedFunction<Credential, Boolean, Exception> check, String signatureText, Issuer issuer) {
         final Predicate<Credential> predicate = credential -> {
             try {
                 return check.apply(credential);
@@ -237,29 +237,35 @@ protected void checkIdpSignature(CheckedFunction<Credential, Boolean, Exception>
         };
         final List<Credential> credentials = idp.getSigningCredentials();
         if (credentials.stream().anyMatch(predicate) == false) {
-            throw samlSignatureException(credentials, signatureText);
+            throw samlSignatureException(issuer, credentials, signatureText);
         }
     }
 
     /**
      * Constructs a SAML specific exception with a consistent message regarding SAML Signature validation failures
      */
-    private ElasticsearchSecurityException samlSignatureException(List<Credential> credentials, String signature, Exception cause) {
+    private ElasticsearchSecurityException samlSignatureException(
+        Issuer issuer,
+        List<Credential> credentials,
+        String signature,
+        Exception cause
+    ) {
+        final String issuerValue = issuer != null ? issuer.getValue() : "";
         logger.warn(
-            "The XML Signature of this SAML message cannot be validated. Please verify that the saml realm uses the correct SAML"
-                + "metadata file/URL for this Identity Provider"
+            "The XML Signature of this SAML message cannot be validated. Please verify that the saml realm uses the correct SAML "
+                + "metadata file/URL for this Identity Provider {}",
+            issuerValue
         );
         final String msg = "SAML Signature [{}] could not be validated against [{}]";
-        return samlException(msg, cause, signature, describeCredentials(credentials));
+        if (cause != null) {
+            return samlException(msg, cause, signature, describeCredentials(credentials));
+        } else {
+            return samlException(msg, signature, describeCredentials(credentials));
+        }
     }
 
-    private ElasticsearchSecurityException samlSignatureException(List<Credential> credentials, String signature) {
-        logger.warn(
-            "The XML Signature of this SAML message cannot be validated. Please verify that the saml realm uses the correct SAML"
-                + "metadata file/URL for this Identity Provider"
-        );
-        final String msg = "SAML Signature [{}] could not be validated against [{}]";
-        return samlException(msg, signature, describeCredentials(credentials));
+    private ElasticsearchSecurityException samlSignatureException(Issuer issuer, List<Credential> credentials, String signature) {
+        return samlSignatureException(issuer, credentials, signature, null);
     }
 
     private static String describeCredentials(List<Credential> credentials) {
@@ -423,7 +429,7 @@ private void validateSignature(String inputString, String signatureAlgorithm, St
                 );
                 return false;
             }
-        }, signatureText);
+        }, signatureText, null);
     }
 
     protected byte[] decodeBase64(String content) {

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlAuthenticatorTests.java

@@ -106,6 +106,9 @@ public class SamlAuthenticatorTests extends SamlResponseHandlerTests {
             + "Attributes with a name clash may prevent authentication or interfere will role mapping. "
             + "Change your IdP configuration to use a different attribute *"
             + " that will not clash with any of [*]";
+    private static final String SIGNATURE_VALIDATION_FAILED_LOG_MESSAGE = "The XML Signature of this SAML message cannot be validated. "
+        + "Please verify that the saml realm uses the correct SAML metadata file/URL for this Identity Provider "
+        + "https://idp.saml.elastic.test/";
 
     private SamlAuthenticator authenticator;
 
@@ -741,16 +744,29 @@ public void testIncorrectSigningKeyIsRejected() throws Exception {
         // check that the content is valid when signed by the correct key-pair
         assertThat(authenticator.authenticate(token(signer.transform(xml, idpSigningCertificatePair))), notNullValue());
 
-        // check is rejected when signed by a different key-pair
-        final Tuple<X509Certificate, PrivateKey> wrongKey = readKeyPair("RSA_4096_updated");
-        final ElasticsearchSecurityException exception = expectThrows(
-            ElasticsearchSecurityException.class,
-            () -> authenticator.authenticate(token(signer.transform(xml, wrongKey)))
-        );
-        assertThat(exception.getMessage(), containsString("SAML Signature"));
-        assertThat(exception.getMessage(), containsString("could not be validated"));
-        assertThat(exception.getCause(), nullValue());
-        assertThat(SamlUtils.isSamlException(exception), is(true));
+        try (var mockLog = MockLog.capture(authenticator.getClass())) {
+            mockLog.addExpectation(
+                new MockLog.SeenEventExpectation(
+                    "Invalid Signature",
+                    authenticator.getClass().getName(),
+                    Level.WARN,
+                    SIGNATURE_VALIDATION_FAILED_LOG_MESSAGE
+                )
+            );
+
+            // check is rejected when signed by a different key-pair
+            final Tuple<X509Certificate, PrivateKey> wrongKey = readKeyPair("RSA_4096_updated");
+            final ElasticsearchSecurityException exception = expectThrows(
+                ElasticsearchSecurityException.class,
+                () -> authenticator.authenticate(token(signer.transform(xml, wrongKey)))
+            );
+            assertThat(exception.getMessage(), containsString("SAML Signature"));
+            assertThat(exception.getMessage(), containsString("could not be validated"));
+            assertThat(exception.getCause(), nullValue());
+            assertThat(SamlUtils.isSamlException(exception), is(true));
+
+            mockLog.assertAllExpectationsMatched();
+        }
     }
 
     public void testSigningKeyIsReloadedForEachRequest() throws Exception {
@@ -1301,24 +1317,54 @@ public void testFailureWhenIdPCredentialsAreEmpty() throws Exception {
         authenticator = buildAuthenticator(() -> emptyList(), emptyList());
         final String xml = getSimpleResponseAsString(clock.instant());
         final SamlToken token = token(signResponse(xml));
-        final ElasticsearchSecurityException exception = expectSamlException(() -> authenticator.authenticate(token));
-        assertThat(exception.getCause(), nullValue());
-        assertThat(exception.getMessage(), containsString("SAML Signature"));
-        assertThat(exception.getMessage(), containsString("could not be validated"));
-        // Restore the authenticator with credentials for the rest of the test cases
-        authenticator = buildAuthenticator(() -> buildOpenSamlCredential(idpSigningCertificatePair), emptyList());
+
+        try (var mockLog = MockLog.capture(authenticator.getClass())) {
+            mockLog.addExpectation(
+                new MockLog.SeenEventExpectation(
+                    "Invalid signature",
+                    authenticator.getClass().getName(),
+                    Level.WARN,
+                    SIGNATURE_VALIDATION_FAILED_LOG_MESSAGE
+                )
+            );
+
+            final ElasticsearchSecurityException exception = expectSamlException(() -> authenticator.authenticate(token));
+            assertThat(exception.getCause(), nullValue());
+            assertThat(exception.getMessage(), containsString("SAML Signature"));
+            assertThat(exception.getMessage(), containsString("could not be validated"));
+
+            mockLog.awaitAllExpectationsMatched();
+        } finally {
+            // Restore the authenticator with credentials for the rest of the test cases
+            authenticator = buildAuthenticator(() -> buildOpenSamlCredential(idpSigningCertificatePair), emptyList());
+        }
     }
 
     public void testFailureWhenIdPCredentialsAreNull() throws Exception {
         authenticator = buildAuthenticator(() -> singletonList(null), emptyList());
         final String xml = getSimpleResponseAsString(clock.instant());
         final SamlToken token = token(signResponse(xml));
-        final ElasticsearchSecurityException exception = expectSamlException(() -> authenticator.authenticate(token));
-        assertThat(exception.getCause(), nullValue());
-        assertThat(exception.getMessage(), containsString("SAML Signature"));
-        assertThat(exception.getMessage(), containsString("could not be validated"));
-        // Restore the authenticator with credentials for the rest of the test cases
-        authenticator = buildAuthenticator(() -> buildOpenSamlCredential(idpSigningCertificatePair), emptyList());
+
+        try (var mockLog = MockLog.capture(authenticator.getClass())) {
+            mockLog.addExpectation(
+                new MockLog.SeenEventExpectation(
+                    "Invalid signature",
+                    authenticator.getClass().getName(),
+                    Level.WARN,
+                    SIGNATURE_VALIDATION_FAILED_LOG_MESSAGE
+                )
+            );
+
+            final ElasticsearchSecurityException exception = expectSamlException(() -> authenticator.authenticate(token));
+            assertThat(exception.getCause(), nullValue());
+            assertThat(exception.getMessage(), containsString("SAML Signature"));
+            assertThat(exception.getMessage(), containsString("could not be validated"));
+
+            mockLog.awaitAllExpectationsMatched();
+        } finally {
+            // Restore the authenticator with credentials for the rest of the test cases
+            authenticator = buildAuthenticator(() -> buildOpenSamlCredential(idpSigningCertificatePair), emptyList());
+        }
     }
 
     private interface CryptoTransform {