From 536123501171987b5f545ce5f332b667d0d47a12 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Fri, 23 Aug 2024 14:36:52 +0300 Subject: [PATCH] Update apache.santuario.xmlsec dep from 2.1.4 to 2.2.6 (#112022) apache.santuario.xmlsec version 2.1.4 is documented vulnerable. We should update to mitigate the vulnerabilities. But apache.santuario.xmlsec is a dependency of opensaml version 3.*. However, in a patch release of elasticsearch (i.e. 7.17.*) it's best we avoid updating dependencies across major versions (i.e. opensaml from version 3.* to version 4.*), particularly for such a complex dependency as opensaml (we did update the opensaml dep in this way, but in a minor elasticsearch 8.* release, i.e. #98199). The latest opensaml 3.* release (i.e. 3.4.6) still requires a vulnerable apache.santuario.xmlsec dep: https://mvnrepository.com/artifact/org.opensaml/opensaml-xmlsec-impl/3.4.6). In this case, our best hope is to find a non-vulnerable version of apache.santuario.xmlsec that is still on the same major version as the version listed in the deps of opensaml (i.e. 2.*). That's version 2.2.6: https://mvnrepository.com/artifact/org.apache.santuario/xmlsec/2.2.6 , which is not vulnerable This PR updates apache.santuario.xmlsec from the existing 2.1.4 version to the 2.2.6 version. The release notes of the 2.2.0 version from https://santuario.apache.org/javareleasenotes.html look simple, and the dependencies differences (from https://mvnrepository.com/artifact/org.apache.santuario/xmlsec/2.1.4) are minimal as well (hopefully optional dependencies, which we don't pull in, stay optional in the same way in the new version). So, it looks to me that the dep update is relatively safe (and it also passes CI)! --- gradle/verification-metadata.xml | 6 +++--- x-pack/plugin/identity-provider/build.gradle | 2 +- x-pack/plugin/security/build.gradle | 2 +- .../security/authc/saml/SamlObjectHandler.java | 17 +++++++++++++++++ 4 files changed, 22 insertions(+), 5 deletions(-) diff --git a/gradle/verification-metadata.xml b/gradle/verification-metadata.xml index 13b6f4635265e..148f51adcd78f 100644 --- a/gradle/verification-metadata.xml +++ b/gradle/verification-metadata.xml @@ -3137,9 +3137,9 @@ - - - + + + diff --git a/x-pack/plugin/identity-provider/build.gradle b/x-pack/plugin/identity-provider/build.gradle index b3ecff4659d86..b15f286623242 100644 --- a/x-pack/plugin/identity-provider/build.gradle +++ b/x-pack/plugin/identity-provider/build.gradle @@ -33,7 +33,7 @@ dependencies { api "org.opensaml:opensaml-storage-api:3.4.5" api "org.opensaml:opensaml-storage-impl:3.4.5" api "net.shibboleth.utilities:java-support:7.5.1" - api "org.apache.santuario:xmlsec:2.1.4" + api "org.apache.santuario:xmlsec:2.2.6" api "io.dropwizard.metrics:metrics-core:3.2.2" api ("org.cryptacular:cryptacular:1.2.4") { exclude group: 'org.bouncycastle' diff --git a/x-pack/plugin/security/build.gradle b/x-pack/plugin/security/build.gradle index 431a64dd98a85..baaa6b9b74642 100644 --- a/x-pack/plugin/security/build.gradle +++ b/x-pack/plugin/security/build.gradle @@ -53,7 +53,7 @@ dependencies { api "org.opensaml:opensaml-storage-api:3.4.5" api "org.opensaml:opensaml-storage-impl:3.4.5" api "net.shibboleth.utilities:java-support:7.5.1" - api "org.apache.santuario:xmlsec:2.1.4" + api "org.apache.santuario:xmlsec:2.2.6" api "io.dropwizard.metrics:metrics-core:3.2.2" api ("org.cryptacular:cryptacular:1.2.4") { exclude group: 'org.bouncycastle' diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlObjectHandler.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlObjectHandler.java index a110296e0f8ef..073881d05191d 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlObjectHandler.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlObjectHandler.java @@ -11,6 +11,7 @@ import org.apache.logging.log4j.message.ParameterizedMessage; import org.elasticsearch.ElasticsearchSecurityException; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.hash.MessageDigests; import org.elasticsearch.core.CheckedFunction; import org.elasticsearch.core.Nullable; import org.elasticsearch.core.TimeValue; @@ -27,6 +28,7 @@ import org.opensaml.saml.security.impl.SAMLSignatureProfileValidator; import org.opensaml.security.credential.Credential; import org.opensaml.security.x509.X509Credential; +import org.opensaml.xmlsec.algorithm.AlgorithmSupport; import org.opensaml.xmlsec.crypto.XMLSigningUtil; import org.opensaml.xmlsec.encryption.support.ChainingEncryptedKeyResolver; import org.opensaml.xmlsec.encryption.support.EncryptedKeyResolver; @@ -167,6 +169,21 @@ void validateSignature(Signature signature) { checkIdpSignature(credential -> { try { + final String signatureAlg = AlgorithmSupport.getKeyAlgorithm(signature.getSignatureAlgorithm()); + final String keyAlg = credential.getPublicKey().getAlgorithm(); + if (signatureAlg != null && signatureAlg.equals(keyAlg) == false) { + if (logger.isDebugEnabled()) { + String keyFingerprint = "SHA265:" + + MessageDigests.toHexString(MessageDigests.sha256().digest(credential.getPublicKey().getEncoded())); + logger.debug( + "Skipping [{}] key [{}] because it is not compatible with signature algorithm [{}]", + keyAlg, + keyFingerprint, + signatureAlg + ); + } + return false; + } return AccessController.doPrivileged((PrivilegedExceptionAction) () -> { try (RestorableContextClassLoader ignore = new RestorableContextClassLoader(SignatureValidator.class)) { SignatureValidator.validate(signature, credential);