Skip to content

Commit

Permalink
Update apache.santuario.xmlsec dep from 2.1.4 to 2.2.6 (#112022)
Browse files Browse the repository at this point in the history
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)!
  • Loading branch information
albertzaharovits authored Aug 23, 2024
1 parent cba614a commit 5361235
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 5 deletions.
6 changes: 3 additions & 3 deletions gradle/verification-metadata.xml
Original file line number Diff line number Diff line change
Expand Up @@ -3137,9 +3137,9 @@
<sha256 value="71f61f34db5e6d926cb8050709fed0ef32788d3f4e9dc06e884c6f3b911b21eb" origin="Generated by Gradle"/>
</artifact>
</component>
<component group="org.apache.santuario" name="xmlsec" version="2.1.4">
<artifact name="xmlsec-2.1.4.jar">
<sha256 value="2e2ec8fe0cf873979f630ae4d35e7ede3390321279b7a15de9deed3f3430990c" origin="Generated by Gradle"/>
<component group="org.apache.santuario" name="xmlsec" version="2.2.6">
<artifact name="xmlsec-2.2.6.jar">
<sha256 value="9325edf9e2449a31315b7f0c49204840d465b38f7e83ecc9471da125ea3d5b75" origin="Generated by Gradle"/>
</artifact>
</component>
<component group="org.apache.servicemix.bundles" name="org.apache.servicemix.bundles.antlr" version="2.7.7_5">
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugin/identity-provider/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugin/security/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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<Boolean>) () -> {
try (RestorableContextClassLoader ignore = new RestorableContextClassLoader(SignatureValidator.class)) {
SignatureValidator.validate(signature, credential);
Expand Down

0 comments on commit 5361235

Please sign in to comment.