[java-opensaml] branch master updated: OSJ-309 - EntityAttribute metadata filter generates duplicate attributes
Scott Cantor
cantor.2 at osu.edu
Wed Apr 15 17:09:57 EDT 2020
This is an automated email from the git hooks/post-receive script.
scantor pushed a commit to branch master
in repository java-opensaml.
View the commit online:
http://git.shibboleth.net/view/?p=java-opensaml.git;a=commit;h=e0c5af9988a48b5022fdf72ce05c692786adc8cd
The following commit(s) were added to refs/heads/master by this push:
new e0c5af9 OSJ-309 - EntityAttribute metadata filter generates duplicate attributes
e0c5af9 is described below
commit e0c5af9988a48b5022fdf72ce05c692786adc8cd
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Wed Apr 15 17:09:50 2020 -0400
OSJ-309 - EntityAttribute metadata filter generates duplicate attributes
https://issues.shibboleth.net/jira/browse/OSJ-309
Fix AlgorithmFilter with duplicate detection.
---
.../resolver/filter/impl/AlgorithmFilter.java | 72 ++++++++++++++++++----
.../resolver/filter/impl/AlgorithmFilterTest.java | 34 +++++-----
.../filter/impl/EntityDescriptorWithAlgorithms.xml | 39 ++++++++++++
3 files changed, 115 insertions(+), 30 deletions(-)
diff --git a/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/filter/impl/AlgorithmFilter.java b/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/filter/impl/AlgorithmFilter.java
index aa0d246..5c8256f 100644
--- a/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/filter/impl/AlgorithmFilter.java
+++ b/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/filter/impl/AlgorithmFilter.java
@@ -18,9 +18,13 @@
package org.opensaml.saml.metadata.resolver.filter.impl;
import java.util.Collection;
+import java.util.Collections;
import java.util.List;
import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
import java.util.function.Predicate;
+import java.util.stream.Collectors;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
@@ -148,6 +152,7 @@ public class AlgorithmFilter extends AbstractInitializableComponent implements M
return metadata;
}
+// Checkstyle: CyclomaticComplexity OFF
/**
* Filters entity descriptor.
*
@@ -155,24 +160,52 @@ public class AlgorithmFilter extends AbstractInitializableComponent implements M
*/
protected void filterEntityDescriptor(@Nonnull final EntityDescriptor descriptor) {
+ Set<String> existingDigests = Collections.emptySet();
+ Set<String> existingSignings = Collections.emptySet();
+ final Extensions exts = descriptor.getExtensions();
+ if (exts != null) {
+ existingDigests = exts.getUnknownXMLObjects(DigestMethod.DEFAULT_ELEMENT_NAME)
+ .stream()
+ .filter(DigestMethod.class::isInstance)
+ .map(DigestMethod.class::cast)
+ .map(DigestMethod::getAlgorithm)
+ .distinct()
+ .collect(Collectors.toUnmodifiableSet());
+ existingSignings = exts.getUnknownXMLObjects(SigningMethod.DEFAULT_ELEMENT_NAME)
+ .stream()
+ .filter(SigningMethod.class::isInstance)
+ .map(SigningMethod.class::cast)
+ .map(SigningMethod::getAlgorithm)
+ .distinct()
+ .collect(Collectors.toUnmodifiableSet());
+ }
+
for (final Map.Entry<Predicate<EntityDescriptor>,Collection<XMLObject>> entry : applyMap.asMap().entrySet()) {
if (!entry.getValue().isEmpty() && entry.getKey().test(descriptor)) {
for (final XMLObject xmlObject : entry.getValue()) {
try {
if (xmlObject instanceof DigestMethod) {
- log.info("Adding DigestMethod ({}) to EntityDescriptor ({})",
- ((DigestMethod) xmlObject).getAlgorithm(), descriptor.getEntityID());
- getExtensions(descriptor).getUnknownXMLObjects().add(
- XMLObjectSupport.cloneXMLObject(xmlObject));
+ if (existingDigests.contains(((DigestMethod) xmlObject).getAlgorithm())) {
+ log.debug("Skipping pre-existing DigestMethod ({}) on EntityDescriptor ({})",
+ ((DigestMethod) xmlObject).getAlgorithm(), descriptor.getEntityID());
+ } else {
+ log.info("Adding DigestMethod ({}) to EntityDescriptor ({})",
+ ((DigestMethod) xmlObject).getAlgorithm(), descriptor.getEntityID());
+ getExtensions(descriptor).getUnknownXMLObjects().add(
+ XMLObjectSupport.cloneXMLObject(xmlObject));
+ }
} else if (xmlObject instanceof SigningMethod) {
- log.info("Adding SigningMethod ({}) to EntityDescriptor ({})",
- ((SigningMethod) xmlObject).getAlgorithm(), descriptor.getEntityID());
- getExtensions(descriptor).getUnknownXMLObjects().add(
- XMLObjectSupport.cloneXMLObject(xmlObject));
+ if (existingSignings.contains(((SigningMethod) xmlObject).getAlgorithm())) {
+ log.debug("Skipping pre-existing SigningMethod ({}) on EntityDescriptor ({})",
+ ((SigningMethod) xmlObject).getAlgorithm(), descriptor.getEntityID());
+ } else {
+ log.info("Adding SigningMethod ({}) to EntityDescriptor ({})",
+ ((SigningMethod) xmlObject).getAlgorithm(), descriptor.getEntityID());
+ getExtensions(descriptor).getUnknownXMLObjects().add(
+ XMLObjectSupport.cloneXMLObject(xmlObject));
+ }
} else if (xmlObject instanceof EncryptionMethod) {
- log.info("Adding EncryptionMethod ({}) to EntityDescriptor ({})",
- ((EncryptionMethod) xmlObject).getAlgorithm(), descriptor.getEntityID());
addEncryptionMethod(descriptor, (EncryptionMethod) xmlObject);
}
@@ -183,6 +216,8 @@ public class AlgorithmFilter extends AbstractInitializableComponent implements M
}
}
}
+// Checkstyle: CyclomaticComplexity ON
+
/**
* Filters entities descriptor.
@@ -219,7 +254,7 @@ public class AlgorithmFilter extends AbstractInitializableComponent implements M
return extensions;
}
-
+
/**
* Add {@link EncryptionMethod} extension to every {@link KeyDescriptor} found in
* an entity.
@@ -233,8 +268,21 @@ public class AlgorithmFilter extends AbstractInitializableComponent implements M
for (final RoleDescriptor role : descriptor.getRoleDescriptors()) {
for (final KeyDescriptor key : role.getKeyDescriptors()) {
if (key.getUse() == null || key.getUse() != UsageType.SIGNING) {
+
+ // Check if here already.
+ final List<EncryptionMethod> existingMethods = key.getEncryptionMethods();
+ for (final EncryptionMethod method : existingMethods) {
+ if (Objects.equals(method.getAlgorithm(), encryptionMethod.getAlgorithm())) {
+ log.debug("Skipping pre-existing EncryptionMethod ({}) on EntityDescriptor ({})",
+ encryptionMethod.getAlgorithm(), descriptor.getEntityID());
+ return;
+ }
+ }
+
try {
- key.getEncryptionMethods().add(XMLObjectSupport.cloneXMLObject(encryptionMethod));
+ log.info("Adding EncryptionMethod ({}) to EntityDescriptor ({})",
+ encryptionMethod.getAlgorithm(), descriptor.getEntityID());
+ existingMethods.add(XMLObjectSupport.cloneXMLObject(encryptionMethod));
} catch (final MarshallingException|UnmarshallingException e) {
log.error("Error cloning XMLObject", e);
}
diff --git a/opensaml-saml-impl/src/test/java/org/opensaml/saml/metadata/resolver/filter/impl/AlgorithmFilterTest.java b/opensaml-saml-impl/src/test/java/org/opensaml/saml/metadata/resolver/filter/impl/AlgorithmFilterTest.java
index c17f558..6340d7a 100644
--- a/opensaml-saml-impl/src/test/java/org/opensaml/saml/metadata/resolver/filter/impl/AlgorithmFilterTest.java
+++ b/opensaml-saml-impl/src/test/java/org/opensaml/saml/metadata/resolver/filter/impl/AlgorithmFilterTest.java
@@ -59,7 +59,7 @@ public class AlgorithmFilterTest extends XMLObjectBaseTestCase implements Predic
protected void setUp() throws Exception {
URL mdURL = FilesystemMetadataResolverTest.class
- .getResource("/org/opensaml/saml/saml2/metadata/InCommon-metadata.xml");
+ .getResource("/org/opensaml/saml/metadata/resolver/filter/impl/EntityDescriptorWithAlgorithms.xml");
mdFile = new File(mdURL.toURI());
metadataProvider = new FilesystemMetadataResolver(mdFile);
@@ -87,23 +87,25 @@ public class AlgorithmFilterTest extends XMLObjectBaseTestCase implements Predic
final SigningMethod signing3 = buildXMLObject(SigningMethod.DEFAULT_ELEMENT_NAME);
signing3.setAlgorithm("foo");
- final EncryptionMethod enc = buildXMLObject(EncryptionMethod.DEFAULT_ELEMENT_NAME);
- enc.setAlgorithm(EncryptionConstants.ALGO_ID_KEYTRANSPORT_RSAOAEP11);
+ final EncryptionMethod enc1 = buildXMLObject(EncryptionMethod.DEFAULT_ELEMENT_NAME);
+ enc1.setAlgorithm(EncryptionConstants.ALGO_ID_KEYTRANSPORT_RSAOAEP11);
final org.opensaml.xmlsec.signature.DigestMethod embeddedDigest =
buildXMLObject(org.opensaml.xmlsec.signature.DigestMethod.DEFAULT_ELEMENT_NAME);
embeddedDigest.setAlgorithm(SignatureConstants.ALGO_ID_DIGEST_SHA256);
- enc.getUnknownXMLObjects().add(embeddedDigest);
+ enc1.getUnknownXMLObjects().add(embeddedDigest);
final MGF mgf = buildXMLObject(MGF.DEFAULT_ELEMENT_NAME);
mgf.setAlgorithm(EncryptionConstants.ALGO_ID_MGF1_SHA256);
- enc.getUnknownXMLObjects().add(mgf);
+ enc1.getUnknownXMLObjects().add(mgf);
final EncryptionMethod enc2 = buildXMLObject(EncryptionMethod.DEFAULT_ELEMENT_NAME);
enc2.setAlgorithm("foo");
+ final EncryptionMethod enc3 = buildXMLObject(EncryptionMethod.DEFAULT_ELEMENT_NAME);
+ enc3.setAlgorithm(EncryptionConstants.ALGO_ID_BLOCKCIPHER_AES256);
- final Collection<XMLObject> algs = List.of(digest1, digest2, signing1, signing2, enc, digest3, signing3, enc2);
+ final Collection<XMLObject> algs = List.of(digest1, digest2, signing1, signing2, enc1, digest3, signing3, enc2, enc3);
final AlgorithmFilter filter = new AlgorithmFilter();
filter.setRules(Collections.singletonMap(this, algs));
@@ -113,7 +115,7 @@ public class AlgorithmFilterTest extends XMLObjectBaseTestCase implements Predic
metadataProvider.setId("test");
metadataProvider.initialize();
- EntityIdCriterion crit = new EntityIdCriterion("https://carmenwiki.osu.edu/shibboleth");
+ EntityIdCriterion crit = new EntityIdCriterion("https://foo.example.org/sp");
EntityDescriptor entity = metadataProvider.resolveSingle(new CriteriaSet(crit));
assertNotNull(entity);
final Extensions exts = entity.getExtensions();
@@ -138,31 +140,27 @@ public class AlgorithmFilterTest extends XMLObjectBaseTestCase implements Predic
for (final RoleDescriptor role : entity.getRoleDescriptors()) {
for (final KeyDescriptor key : role.getKeyDescriptors()) {
final List<EncryptionMethod> methods = key.getEncryptionMethods();
- assertEquals(methods.size(), 2);
- assertEquals(methods.get(0).getAlgorithm(), EncryptionConstants.ALGO_ID_KEYTRANSPORT_RSAOAEP11);
- assertEquals(methods.get(1).getAlgorithm(), "foo");
+ assertEquals(methods.size(), 3);
+ assertEquals(methods.get(0).getAlgorithm(), EncryptionConstants.ALGO_ID_BLOCKCIPHER_AES256);
+ assertEquals(methods.get(1).getAlgorithm(), EncryptionConstants.ALGO_ID_KEYTRANSPORT_RSAOAEP11);
+ assertEquals(methods.get(2).getAlgorithm(), "foo");
- final List<XMLObject> encDigests = methods.get(0).getUnknownXMLObjects(
+ final List<XMLObject> encDigests = methods.get(1).getUnknownXMLObjects(
org.opensaml.xmlsec.signature.DigestMethod.DEFAULT_ELEMENT_NAME);
assertEquals(encDigests.size(), 1);
assertEquals(((org.opensaml.xmlsec.signature.DigestMethod) encDigests.get(0)).getAlgorithm(),
SignatureConstants.ALGO_ID_DIGEST_SHA256);
- final List<XMLObject> mgfs = methods.get(0).getUnknownXMLObjects(MGF.DEFAULT_ELEMENT_NAME);
+ final List<XMLObject> mgfs = methods.get(1).getUnknownXMLObjects(MGF.DEFAULT_ELEMENT_NAME);
assertEquals(mgfs.size(), 1);
assertEquals(((MGF) mgfs.get(0)).getAlgorithm(), EncryptionConstants.ALGO_ID_MGF1_SHA256);
}
}
-
- crit = new EntityIdCriterion("https://cms.psu.edu/Shibboleth");
- entity = metadataProvider.resolveSingle(new CriteriaSet(crit));
- assertNotNull(entity);
- assertNull(entity.getExtensions());
}
/** {@inheritDoc} */
public boolean test(final EntityDescriptor input) {
- return input.getEntityID().equals("https://carmenwiki.osu.edu/shibboleth");
+ return input.getEntityID().equals("https://foo.example.org/sp");
}
}
diff --git a/opensaml-saml-impl/src/test/resources/org/opensaml/saml/metadata/resolver/filter/impl/EntityDescriptorWithAlgorithms.xml b/opensaml-saml-impl/src/test/resources/org/opensaml/saml/metadata/resolver/filter/impl/EntityDescriptorWithAlgorithms.xml
new file mode 100644
index 0000000..cb24fb4
--- /dev/null
+++ b/opensaml-saml-impl/src/test/resources/org/opensaml/saml/metadata/resolver/filter/impl/EntityDescriptorWithAlgorithms.xml
@@ -0,0 +1,39 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<md:EntityDescriptor xmlns:md="urn:oasis:names:tc:SAML:2.0:metadata" xmlns:alg="urn:oasis:names:tc:SAML:metadata:algsupport"
+ ID="abc123" entityID="https://foo.example.org/sp">
+ <md:Extensions>
+ <alg:DigestMethod Algorithm="http://www.w3.org/2001/04/xmlenc#sha256" />
+ <alg:SigningMethod Algorithm="http://www.w3.org/2001/04/xmldsig-more#rsa-sha256" />
+ </md:Extensions>
+ <md:SPSSODescriptor protocolSupportEnumeration="urn:oasis:names:tc:SAML:2.0:protocol">
+ <md:KeyDescriptor xmlns:md="urn:oasis:names:tc:SAML:2.0:metadata">
+ <md:EncryptionMethod Algorithm="http://www.w3.org/2001/04/xmlenc#aes256-cbc" />
+ <ds:KeyInfo xmlns:ds="http://www.w3.org/2000/09/xmldsig#">
+ <ds:X509Data>
+ <ds:X509Certificate>
+MIIDGzCCAgOgAwIBAgIJANI+yGM0M1N2MA0GCSqGSIb3DQEBBQUAMCcxJTAjBgNV
+BAMTHGx0Y2F3aWtpMDEuaXQub2hpby1zdGF0ZS5lZHUwHhcNMTAwNzA3MjI0MzA1
+WhcNMjAwNzA0MjI0MzA1WjAnMSUwIwYDVQQDExxsdGNhd2lraTAxLml0Lm9oaW8t
+c3RhdGUuZWR1MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA5fsEv25M
+r9wfa48qfjn8m40yB/lwimJ8dSnYw2erd/tfB+sPESw42Is5Lv2B3pI3mj9a0PT0
+Gf1VgUoQW0RCT6L4VOW50WsPFv/RKPfT/AIRl00dTCqb440PgotGbrK9ivZqlvkz
+lSGUKuFcg2gLj+CJlbMcwEneSwn0FE1xKEGpMDUk91lZH1XxmnIDDOQn1G5qul4q
+AbXITMpLi2MlsHAEXxnLrthFFas6zDrviTwHcqGXq9zJJkPHDcbu1qg6AUT7bRJr
+qszxxktSV6mFclkgLPpcVkigMR8RNVMQkWaaWSnfBkFy2iAe3xw3DNp7obtzgItY
+i9N8U6K5qorSkQIDAQABo0owSDAnBgNVHREEIDAeghxsdGNhd2lraTAxLml0Lm9o
+aW8tc3RhdGUuZWR1MB0GA1UdDgQWBBR32XnCliG78DdyTtZhyIQSHChtyjANBgkq
+hkiG9w0BAQUFAAOCAQEAVEweCxPElHGmam4Iv2QeJsGE7m4de7axp3epAJb7uVbN
+Z2P1S/s4GZQhmGsUoGoxwqca3wyQ+C1ZkpQJdyFl5s1tFc26D+Z0KTDo174GzO9i
+I9SeQ4YSp3FNhZqxn4xH3DULzzHwoVSwFr5irLPAVtrqK8H/rzBREhqOse2VSJ/1
+PkI+p7lUiElIzMiObLGjumF2fDOPkXOSMNyC4c5oCCJtcrip/BaLo6bqdqn3DKP8
+onMw/lHZQolyVsupuhGsSX13WVJ0uyGvuA7hiHnGEkpDmskUd3TsriyQAt47RZzY
+tTupO/NdWvz8SvXU1qIOk9CTQ0D2b2OOftfUW+FuAQ==
+ </ds:X509Certificate>
+ </ds:X509Data>
+ </ds:KeyInfo>
+ </md:KeyDescriptor>
+ <md:AssertionConsumerService
+ Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST" Location="https://foo.example.org/acs"
+ index="0" />
+ </md:SPSSODescriptor>
+</md:EntityDescriptor>
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list