[java-opensaml] branch master updated: OSJ-301 - Algorithm MetadataFilter should sanity check algorithm URIs

Scott Cantor cantor.2 at osu.edu
Mon Feb 17 20:41:24 EST 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=2b6426136d7e31d95e64c9de4c225826deb05fec

The following commit(s) were added to refs/heads/master by this push:
       new  2b64261   OSJ-301 - Algorithm MetadataFilter should sanity check algorithm URIs
2b64261 is described below

commit 2b6426136d7e31d95e64c9de4c225826deb05fec
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Mon Feb 17 20:41:22 2020 -0500

    OSJ-301 - Algorithm MetadataFilter should sanity check algorithm URIs
    
    https://issues.shibboleth.net/jira/browse/OSJ-301
---
 .../resolver/filter/impl/AlgorithmFilter.java      | 85 +++++++++++++++++++++-
 .../resolver/filter/impl/AlgorithmFilterTest.java  | 26 +++++--
 2 files changed, 103 insertions(+), 8 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 5136938..8740b16 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
@@ -17,7 +17,6 @@
 
 package org.opensaml.saml.metadata.resolver.filter.impl;
 
-
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
@@ -27,6 +26,7 @@ import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
 import net.shibboleth.utilities.java.support.annotation.constraint.NonnullElements;
+import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
 import net.shibboleth.utilities.java.support.component.AbstractInitializableComponent;
 import net.shibboleth.utilities.java.support.component.ComponentSupport;
 import net.shibboleth.utilities.java.support.logic.Constraint;
@@ -49,6 +49,9 @@ import org.opensaml.saml.saml2.metadata.Extensions;
 import org.opensaml.saml.saml2.metadata.KeyDescriptor;
 import org.opensaml.saml.saml2.metadata.RoleDescriptor;
 import org.opensaml.security.credential.UsageType;
+import org.opensaml.xmlsec.algorithm.AlgorithmDescriptor.AlgorithmType;
+import org.opensaml.xmlsec.algorithm.AlgorithmRegistry;
+import org.opensaml.xmlsec.algorithm.AlgorithmSupport;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -67,6 +70,9 @@ public class AlgorithmFilter extends AbstractInitializableComponent implements M
     /** Class logger. */
     @Nonnull private final Logger log = LoggerFactory.getLogger(AlgorithmFilter.class);
 
+    /** Registry for sanity checking algorithms. */
+    @Nonnull private AlgorithmRegistry registry = AlgorithmSupport.getGlobalAlgorithmRegistry();
+    
     /** Rules for adding algorithms. */
     @Nonnull @NonnullElements private Multimap<Predicate<EntityDescriptor>,XMLObject> applyMap;
     
@@ -81,6 +87,8 @@ public class AlgorithmFilter extends AbstractInitializableComponent implements M
         applyMap = ArrayListMultimap.create();
     }
     
+    
+// Checkstyle: CyclomaticComplexity OFF
     /**
      * Set the mappings from {@link Predicate} to extensions of various types to apply.
      * 
@@ -93,10 +101,36 @@ public class AlgorithmFilter extends AbstractInitializableComponent implements M
         applyMap = ArrayListMultimap.create(rules.size(), 1);
         for (final Map.Entry<Predicate<EntityDescriptor>,Collection<XMLObject>> entry : rules.entrySet()) {
             if (entry.getKey() != null && entry.getValue() != null) {
+                
+                entry.getValue()
+                    .stream()
+                    .filter(DigestMethod.class::isInstance)
+                    .map(DigestMethod.class::cast)
+                    .map(DigestMethod::getAlgorithm)
+                    .distinct()
+                    .forEach(uri -> checkDigestMethod(uri));
+
+                entry.getValue()
+                    .stream()
+                    .filter(SigningMethod.class::isInstance)
+                    .map(SigningMethod.class::cast)
+                    .map(SigningMethod::getAlgorithm)
+                    .distinct()
+                    .forEach(uri -> checkSigningMethod(uri));
+
+                entry.getValue()
+                    .stream()
+                    .filter(EncryptionMethod.class::isInstance)
+                    .map(EncryptionMethod.class::cast)
+                    .map(EncryptionMethod::getAlgorithm)
+                    .distinct()
+                    .forEach(uri -> checkEncryptionMethod(uri));
+                
                 applyMap.putAll(entry.getKey(), List.copyOf(entry.getValue()));
             }
         }
     }
+// Checkstyle: CyclomaticComplexity ON
 
     /** {@inheritDoc} */
     @Override
@@ -210,4 +244,53 @@ public class AlgorithmFilter extends AbstractInitializableComponent implements M
         }
     }
     
+    /**
+     * Check the input method for "known" and "supported" status for logging purposes.
+     * 
+     * @param uri input method
+     */
+    private void checkDigestMethod(@Nonnull @NotEmpty final String uri) {
+        if (registry != null) {
+            if (!registry.getRegisteredURIsByType(AlgorithmType.MessageDigest).contains(uri)) {
+                log.warn("DigestMethod {} unrecognized by algorithm registry", uri);
+            } else if (!registry.isRuntimeSupported(uri)) {
+                log.warn("DigestMethod {} unsupported by runtime", uri);
+            }
+        }
+    }
+    
+    /**
+     * Check the input method for "known" and "supported" status for logging purposes.
+     * 
+     * @param uri input method
+     */
+    private void checkSigningMethod(@Nonnull @NotEmpty final String uri) {
+        if (registry != null) {
+            if (!registry.getRegisteredURIsByType(AlgorithmType.Signature).contains(uri) &&
+                    !registry.getRegisteredURIsByType(AlgorithmType.Mac).contains(uri)) {
+                log.warn("SigningMethod {} unrecognized by algorithm registry", uri);
+            } else if (!registry.isRuntimeSupported(uri)) {
+                log.warn("SigningMethod {} unsupported by runtime", uri);
+            }
+        }
+    }
+
+    /**
+     * Check the input method for "known" and "supported" status for logging purposes.
+     * 
+     * @param uri input method
+     */
+    private void checkEncryptionMethod(@Nonnull @NotEmpty final String uri) {
+        if (registry != null) {
+            if (!registry.getRegisteredURIsByType(AlgorithmType.BlockEncryption).contains(uri) &&
+                    !registry.getRegisteredURIsByType(AlgorithmType.KeyTransport).contains(uri) &&
+                    !registry.getRegisteredURIsByType(AlgorithmType.KeyAgreement).contains(uri) &&
+                    !registry.getRegisteredURIsByType(AlgorithmType.SymmetricKeyWrap).contains(uri)) {
+                log.warn("EncryptionMethod {} unrecognized by algorithm registry", uri);
+            } else if (!registry.isRuntimeSupported(uri)) {
+                log.warn("EncryptionMethod {} unsupported by runtime", uri);
+            }
+        }
+    }
+
 }
\ No newline at end of file
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 75217b4..92f9737 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
@@ -21,7 +21,6 @@ import static org.testng.Assert.*;
 
 import java.io.File;
 import java.net.URL;
-import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Iterator;
@@ -76,15 +75,21 @@ public class AlgorithmFilterTest extends XMLObjectBaseTestCase implements Predic
         final DigestMethod digest2 = buildXMLObject(DigestMethod.DEFAULT_ELEMENT_NAME);
         digest2.setAlgorithm(SignatureConstants.ALGO_ID_DIGEST_SHA512);
 
+        final DigestMethod digest3 = buildXMLObject(DigestMethod.DEFAULT_ELEMENT_NAME);
+        digest3.setAlgorithm("foo");
+
         final SigningMethod signing1 = buildXMLObject(SigningMethod.DEFAULT_ELEMENT_NAME);
         signing1.setAlgorithm(SignatureConstants.ALGO_ID_SIGNATURE_RSA_SHA256);
 
         final SigningMethod signing2 = buildXMLObject(SigningMethod.DEFAULT_ELEMENT_NAME);
         signing2.setAlgorithm(SignatureConstants.ALGO_ID_SIGNATURE_RSA_SHA512);
-        
+
+        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 org.opensaml.xmlsec.signature.DigestMethod embeddedDigest =
                 buildXMLObject(org.opensaml.xmlsec.signature.DigestMethod.DEFAULT_ELEMENT_NAME);
         embeddedDigest.setAlgorithm(SignatureConstants.ALGO_ID_DIGEST_SHA256);
@@ -94,7 +99,11 @@ public class AlgorithmFilterTest extends XMLObjectBaseTestCase implements Predic
         mgf.setAlgorithm(EncryptionConstants.ALGO_ID_MGF1_SHA256);
         enc.getUnknownXMLObjects().add(mgf);
 
-        final Collection<XMLObject> algs = Arrays.asList(digest1, digest2, signing1, signing2, enc);
+        final EncryptionMethod enc2 = buildXMLObject(EncryptionMethod.DEFAULT_ELEMENT_NAME);
+        enc2.setAlgorithm("foo");
+
+
+        final Collection<XMLObject> algs = List.of(digest1, digest2, signing1, signing2, enc, digest3, signing3, enc2);
         
         final AlgorithmFilter filter = new AlgorithmFilter();
         filter.setRules(Collections.<Predicate<EntityDescriptor>,Collection<XMLObject>>singletonMap(this, algs));
@@ -111,24 +120,27 @@ public class AlgorithmFilterTest extends XMLObjectBaseTestCase implements Predic
         assertNotNull(exts);
         
         List<XMLObject> extElements = exts.getUnknownXMLObjects(DigestMethod.DEFAULT_ELEMENT_NAME);
-        assertEquals(extElements.size(), 2);
+        assertEquals(extElements.size(), 3);
         
         Iterator<XMLObject> digests = extElements.iterator();
         assertEquals(((DigestMethod) digests.next()).getAlgorithm(), SignatureConstants.ALGO_ID_DIGEST_SHA256);
         assertEquals(((DigestMethod) digests.next()).getAlgorithm(), SignatureConstants.ALGO_ID_DIGEST_SHA512);
+        assertEquals(((DigestMethod) digests.next()).getAlgorithm(), "foo");
 
         extElements = exts.getUnknownXMLObjects(SigningMethod.DEFAULT_ELEMENT_NAME);
-        assertEquals(extElements.size(), 2);
+        assertEquals(extElements.size(), 3);
         
         Iterator<XMLObject> signings = extElements.iterator();
         assertEquals(((SigningMethod) signings.next()).getAlgorithm(), SignatureConstants.ALGO_ID_SIGNATURE_RSA_SHA256);
         assertEquals(((SigningMethod) signings.next()).getAlgorithm(), SignatureConstants.ALGO_ID_SIGNATURE_RSA_SHA512);
+        assertEquals(((SigningMethod) signings.next()).getAlgorithm(), "foo");
 
         for (final RoleDescriptor role : entity.getRoleDescriptors()) {
             for (final KeyDescriptor key : role.getKeyDescriptors()) {
                 final List<EncryptionMethod> methods = key.getEncryptionMethods();
-                assertEquals(methods.size(), 1);
+                assertEquals(methods.size(), 2);
                 assertEquals(methods.get(0).getAlgorithm(), EncryptionConstants.ALGO_ID_KEYTRANSPORT_RSAOAEP11);
+                assertEquals(methods.get(1).getAlgorithm(), "foo");
                 
                 final List<XMLObject> encDigests = methods.get(0).getUnknownXMLObjects(
                         org.opensaml.xmlsec.signature.DigestMethod.DEFAULT_ELEMENT_NAME);

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the commits mailing list