[java-opensaml] branch master updated: OSJ-188: Enhance SignatureValidationFilter to make operation ...

Brent Putman putmanb at georgetown.edu
Wed Aug 14 21:37:43 EDT 2019


This is an automated email from the git hooks/post-receive script.

putmanb 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=ebdd576c1fc265ba3d1bbcc79724d4ce1a2a0528

The following commit(s) were added to refs/heads/master by this push:
       new  ebdd576   OSJ-188: Enhance SignatureValidationFilter to make operation ...
ebdd576 is described below

commit ebdd576c1fc265ba3d1bbcc79724d4ce1a2a0528
Author: Brent Putman <putmanb at georgetown.edu>
AuthorDate: Wed Aug 14 21:34:03 2019 -0400

    OSJ-188: Enhance SignatureValidationFilter to make operation ...
    
    Enhance SignatureValidationFilter to make operation optional at runtime
---
 .../resolver/filter/data/impl/MetadataSource.java  | 50 +++++++++++++++++
 .../resolver/filter/data/impl/package-info.java    | 22 ++++++++
 .../filter/impl/SignatureValidationFilter.java     | 50 ++++++++++++++---
 .../impl/FileBackedHTTPMetadataResolver.java       | 16 ++++++
 .../SignatureValidationFilterExplicitKeyTest.java  | 62 ++++++++++++++++++++++
 .../impl/FileBackedHTTPMetadataResolverTest.java   | 37 ++++++++++---
 6 files changed, 223 insertions(+), 14 deletions(-)

diff --git a/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/filter/data/impl/MetadataSource.java b/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/filter/data/impl/MetadataSource.java
new file mode 100644
index 0000000..ac0ad2d
--- /dev/null
+++ b/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/filter/data/impl/MetadataSource.java
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the University Corporation for Advanced Internet Development,
+ * Inc. (UCAID) under one or more contributor license agreements.  See the
+ * NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The UCAID licenses this file to You under the Apache
+ * License, Version 2.0 (the "License"); you may not use this file except in
+ * compliance with the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.opensaml.saml.metadata.resolver.filter.data.impl;
+
+import org.opensaml.saml.metadata.resolver.filter.MetadataFilterContext;
+import org.opensaml.saml.metadata.resolver.filter.MetadataFilterContext.Data;
+
+/**
+ * Data object for {@link MetadataFilterContext} intended to hold information about the source of the
+ * metadata currently being processed.
+ */
+public class MetadataSource implements Data {
+
+    /** Flag indicating whether the metadata source is trusted. */
+    private boolean trusted;
+
+    /**
+     * Get whether the metadata source is trusted.
+     *
+     * @return true if trusted, false if not
+     */
+    public boolean isTrusted() {
+        return trusted;
+    }
+
+    /**
+     * Set whether the metadata source is trusted.
+     *
+     * @param flag true if trusted, false if not
+     */
+    public void setTrusted(final boolean flag) {
+        this.trusted = flag;
+    }
+
+}
diff --git a/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/filter/data/impl/package-info.java b/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/filter/data/impl/package-info.java
new file mode 100644
index 0000000..8a26849
--- /dev/null
+++ b/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/filter/data/impl/package-info.java
@@ -0,0 +1,22 @@
+/*
+ * Licensed to the University Corporation for Advanced Internet Development,
+ * Inc. (UCAID) under one or more contributor license agreements.  See the
+ * NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The UCAID licenses this file to You under the Apache
+ * License, Version 2.0 (the "License"); you may not use this file except in
+ * compliance with the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/**
+ * Data classes for use with {@link org.opensaml.saml.metadata.resolver.filter.MetadataFilterContext}.
+ */
+
+package org.opensaml.saml.metadata.resolver.filter.data.impl;
diff --git a/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/filter/impl/SignatureValidationFilter.java b/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/filter/impl/SignatureValidationFilter.java
index a713cb3..c5e0bda 100644
--- a/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/filter/impl/SignatureValidationFilter.java
+++ b/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/filter/impl/SignatureValidationFilter.java
@@ -29,6 +29,7 @@ import org.opensaml.core.xml.XMLObject;
 import org.opensaml.saml.metadata.resolver.filter.FilterException;
 import org.opensaml.saml.metadata.resolver.filter.MetadataFilter;
 import org.opensaml.saml.metadata.resolver.filter.MetadataFilterContext;
+import org.opensaml.saml.metadata.resolver.filter.data.impl.MetadataSource;
 import org.opensaml.saml.saml2.metadata.AffiliationDescriptor;
 import org.opensaml.saml.saml2.metadata.EntitiesDescriptor;
 import org.opensaml.saml.saml2.metadata.EntityDescriptor;
@@ -245,9 +246,9 @@ public class SignatureValidationFilter implements MetadataFilter {
         }
         
         if (signableMetadata instanceof EntityDescriptor) {
-            processEntityDescriptor((EntityDescriptor) signableMetadata);
+            processEntityDescriptor((EntityDescriptor) signableMetadata, context, true);
         } else if (signableMetadata instanceof EntitiesDescriptor) {
-            processEntityGroup((EntitiesDescriptor) signableMetadata);
+            processEntityGroup((EntitiesDescriptor) signableMetadata, context, true);
         } else {
             log.error("Internal error, metadata object was of an unsupported type: {}", metadata.getClass().getName());
         }
@@ -261,15 +262,24 @@ public class SignatureValidationFilter implements MetadataFilter {
      * If signature verification fails on a child, it will be removed from the entity descriptor.
      * 
      * @param entityDescriptor the EntityDescriptor to be processed
+     * @param context the current filter context
+     * @param isRoot true if the element being processed is the XML document root, false if not
      * @throws FilterException thrown if an error occurs during the signature verification process
      *                          on the root EntityDescriptor specified
      */
-    protected void processEntityDescriptor(@Nonnull final EntityDescriptor entityDescriptor) throws FilterException {
+    protected void processEntityDescriptor(@Nonnull final EntityDescriptor entityDescriptor,
+            @Nonnull final MetadataFilterContext context, final boolean isRoot) throws FilterException {
+
         final String entityID = entityDescriptor.getEntityID();
         log.trace("Processing EntityDescriptor: {}", entityID);
         
         if (entityDescriptor.isSigned()) {
-            verifySignature(entityDescriptor, entityID, false);
+            if (isRoot && isSkipRootSignature(context)) {
+                log.trace("Skipping root signature validation of EntityDescriptor based on filter context data");
+            } else {
+                log.trace("Proceeding with signature validation of EntityDescriptor");
+                verifySignature(entityDescriptor, entityID, false);
+            }
         }
         
         final Iterator<RoleDescriptor> roleIter = entityDescriptor.getRoleDescriptors().iterator();
@@ -323,15 +333,24 @@ public class SignatureValidationFilter implements MetadataFilter {
      * If signature verification fails on a child, it will be removed from the entities descriptor group.
      * 
      * @param entitiesDescriptor the EntitiesDescriptor to be processed
+     * @param context the current filter context
+     * @param isRoot true if the element being processed is the XML document root, false if not
      * @throws FilterException thrown if an error occurs during the signature verification process
      *                          on the root EntitiesDescriptor specified
      */
-    protected void processEntityGroup(@Nonnull final EntitiesDescriptor entitiesDescriptor) throws FilterException {
+    protected void processEntityGroup(@Nonnull final EntitiesDescriptor entitiesDescriptor,
+            @Nonnull final MetadataFilterContext context, final boolean isRoot) throws FilterException {
+
         final String name = getGroupName(entitiesDescriptor);
         log.trace("Processing EntitiesDescriptor group: {}", name);
         
         if (entitiesDescriptor.isSigned()) {
-            verifySignature(entitiesDescriptor, name, true);
+            if (isRoot && isSkipRootSignature(context)) {
+                log.trace("Skipping root signature validation of EntitiesDescriptor based on filter context data");
+            } else {
+                log.trace("Proceeding with signature validation of EntitiesDescriptor");
+                verifySignature(entitiesDescriptor, name, true);
+            }
         }
         
         // Can't use IndexedXMLObjectChildrenList sublist iterator remove() to remove members,
@@ -350,7 +369,7 @@ public class SignatureValidationFilter implements MetadataFilter {
             }
             
             try {
-                processEntityDescriptor(entityChild);
+                processEntityDescriptor(entityChild, context, false);
             } catch (final FilterException e) {
                log.error("EntityDescriptor '{}' failed signature verification, removing from metadata provider", 
                        entityChild.getEntityID()); 
@@ -369,7 +388,7 @@ public class SignatureValidationFilter implements MetadataFilter {
             final String childName = getGroupName(entitiesChild);
             log.trace("Processing EntitiesDescriptor member: {}", childName);
             try {
-                processEntityGroup(entitiesChild);
+                processEntityGroup(entitiesChild, context, false);
             } catch (final FilterException e) {
                log.error("EntitiesDescriptor '{}' failed signature verification, removing from metadata provider", 
                        childName); 
@@ -521,4 +540,19 @@ public class SignatureValidationFilter implements MetadataFilter {
         return "(unnamed)";
     }
     
+    /**
+     * Determine whether validation of signature on the document root should be skipped.
+     *
+     * @param context the metadata filter context
+     * @return true if root signature validation should be skipped, false if not
+     */
+    protected boolean isSkipRootSignature(@Nonnull final MetadataFilterContext context) {
+        final MetadataSource metadataSource = context.get(MetadataSource.class);
+        if (metadataSource != null) {
+            return metadataSource.isTrusted();
+        } else {
+            return false;
+        }
+    }
+
 }
\ No newline at end of file
diff --git a/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/impl/FileBackedHTTPMetadataResolver.java b/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/impl/FileBackedHTTPMetadataResolver.java
index b19e4b2..1902dfe 100644
--- a/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/impl/FileBackedHTTPMetadataResolver.java
+++ b/opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/impl/FileBackedHTTPMetadataResolver.java
@@ -34,6 +34,8 @@ import net.shibboleth.utilities.java.support.resolver.ResolverException;
 
 import org.apache.http.client.HttpClient;
 import org.opensaml.core.xml.XMLObject;
+import org.opensaml.saml.metadata.resolver.filter.MetadataFilterContext;
+import org.opensaml.saml.metadata.resolver.filter.data.impl.MetadataSource;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.w3c.dom.Document;
@@ -320,6 +322,20 @@ public class FileBackedHTTPMetadataResolver extends HTTPMetadataResolver {
     }
 
     /** {@inheritDoc} */
+    protected MetadataFilterContext newFilterContext() {
+        final MetadataFilterContext context = super.newFilterContext();
+        if (initializing && initializedFromBackupFile) {
+            MetadataSource metadataSource = context.get(MetadataSource.class);
+            if (metadataSource == null) {
+                metadataSource = new MetadataSource();
+                context.add(metadataSource);
+            }
+            metadataSource.setTrusted(true);
+        }
+        return context;
+    }
+
+    /** {@inheritDoc} */
     @Override
     @Nonnull protected Duration computeNextRefreshDelay(@Nullable final Instant expectedExpiration) {
         if (initializing && initializedFromBackupFile) {
diff --git a/opensaml-saml-impl/src/test/java/org/opensaml/saml/metadata/resolver/filter/impl/SignatureValidationFilterExplicitKeyTest.java b/opensaml-saml-impl/src/test/java/org/opensaml/saml/metadata/resolver/filter/impl/SignatureValidationFilterExplicitKeyTest.java
index 0a3661a..d0b43ad 100644
--- a/opensaml-saml-impl/src/test/java/org/opensaml/saml/metadata/resolver/filter/impl/SignatureValidationFilterExplicitKeyTest.java
+++ b/opensaml-saml-impl/src/test/java/org/opensaml/saml/metadata/resolver/filter/impl/SignatureValidationFilterExplicitKeyTest.java
@@ -30,6 +30,7 @@ import org.opensaml.core.xml.XMLObjectBaseTestCase;
 import org.opensaml.core.xml.io.UnmarshallingException;
 import org.opensaml.saml.metadata.resolver.filter.FilterException;
 import org.opensaml.saml.metadata.resolver.filter.MetadataFilterContext;
+import org.opensaml.saml.metadata.resolver.filter.data.impl.MetadataSource;
 import org.opensaml.saml.metadata.resolver.impl.DOMMetadataResolver;
 import org.opensaml.saml.saml2.metadata.EntityDescriptor;
 import org.opensaml.security.credential.CredentialSupport;
@@ -157,6 +158,24 @@ public class SignatureValidationFilterExplicitKeyTest extends XMLObjectBaseTestC
             // do nothing, should fail
         }
     }
+
+    @Test
+    public void testInvalidSWITCHStandaloneWithRootSkip() throws UnmarshallingException {
+        // Goal here is to test the root signature skip (indicated by filter context data) by using a known invalid root signature.
+        XMLObject xmlObject = unmarshallerFactory.getUnmarshaller(switchMDDocumentInvalid
+                .getDocumentElement()).unmarshall(switchMDDocumentInvalid.getDocumentElement());
+
+        MetadataSource metadataSource = new MetadataSource();
+        metadataSource.setTrusted(true);
+        filterContext.add(metadataSource);
+
+        SignatureValidationFilter filter = new SignatureValidationFilter(switchSigTrustEngine);
+        try {
+            filter.filter(xmlObject, filterContext);
+        } catch (FilterException e) {
+            Assert.fail("Filter failed validation, should have passed b/c we implicitly said to skip root signature");
+        }
+    }
     
     @Test
     public void testEntityDescriptor() throws UnmarshallingException, CertificateException, XMLParserException {
@@ -204,6 +223,34 @@ public class SignatureValidationFilterExplicitKeyTest extends XMLObjectBaseTestC
             // do nothing, should fail
         }
     }
+
+    @Test
+    public void testEntityDescriptorInvalidWithRootSkip() throws UnmarshallingException, CertificateException, XMLParserException {
+        // Goal here is to test the root signature skip (indicated by filter context data) by using a known invalid root signature.
+        X509Certificate cert = X509Support.decodeCertificate(openIDCertBase64);
+        X509Credential cred = CredentialSupport.getSimpleCredential(cert, null);
+        StaticCredentialResolver credResolver = new StaticCredentialResolver(cred);
+        SignatureTrustEngine trustEngine = new ExplicitKeySignatureTrustEngine(credResolver, kiResolver);
+
+        Document mdDoc = parserPool.parse(SignatureValidationFilterExplicitKeyTest.class.getResourceAsStream(openIDFileInvalid));
+        XMLObject xmlObject =
+            unmarshallerFactory.getUnmarshaller(mdDoc.getDocumentElement()).unmarshall(mdDoc.getDocumentElement());
+        Assert.assertTrue(xmlObject instanceof EntityDescriptor);
+        EntityDescriptor ed = (EntityDescriptor) xmlObject;
+        Assert.assertTrue(ed.isSigned());
+        Assert.assertNotNull(ed.getSignature(), "Signature was null");
+
+        MetadataSource metadataSource = new MetadataSource();
+        metadataSource.setTrusted(true);
+        filterContext.add(metadataSource);
+
+        SignatureValidationFilter filter = new SignatureValidationFilter(trustEngine);
+        try {
+            filter.filter(xmlObject, filterContext);
+        } catch (FilterException e) {
+            Assert.fail("Filter failed validation, should have passed b/c we implicitly said to skip root signature");
+        }
+    }
     
     @Test
     public void testEntityDescriptorWithProvider() throws CertificateException, XMLParserException, UnmarshallingException {
@@ -254,4 +301,19 @@ public class SignatureValidationFilterExplicitKeyTest extends XMLObjectBaseTestC
         }
     }
 
+    @Test
+    public void testIsSkipRootSignatureEval() {
+        MetadataFilterContext context = new MetadataFilterContext();
+        SignatureValidationFilter filter = new SignatureValidationFilter(switchSigTrustEngine);
+        MetadataSource metadataSource = new MetadataSource();
+
+        Assert.assertFalse(filter.isSkipRootSignature(context));
+
+        context.add(metadataSource);
+        Assert.assertFalse(filter.isSkipRootSignature(context));
+
+        metadataSource.setTrusted(true);
+        Assert.assertTrue(filter.isSkipRootSignature(context));
+    }
+
 }
\ No newline at end of file
diff --git a/opensaml-saml-impl/src/test/java/org/opensaml/saml/metadata/resolver/impl/FileBackedHTTPMetadataResolverTest.java b/opensaml-saml-impl/src/test/java/org/opensaml/saml/metadata/resolver/impl/FileBackedHTTPMetadataResolverTest.java
index 0d3c9cb..da3b21a 100644
--- a/opensaml-saml-impl/src/test/java/org/opensaml/saml/metadata/resolver/impl/FileBackedHTTPMetadataResolverTest.java
+++ b/opensaml-saml-impl/src/test/java/org/opensaml/saml/metadata/resolver/impl/FileBackedHTTPMetadataResolverTest.java
@@ -27,7 +27,12 @@ import java.time.Duration;
 import java.time.Instant;
 
 import org.opensaml.core.criterion.EntityIdCriterion;
+import org.opensaml.core.xml.XMLObject;
 import org.opensaml.core.xml.XMLObjectBaseTestCase;
+import org.opensaml.saml.metadata.resolver.filter.FilterException;
+import org.opensaml.saml.metadata.resolver.filter.MetadataFilter;
+import org.opensaml.saml.metadata.resolver.filter.MetadataFilterContext;
+import org.opensaml.saml.metadata.resolver.filter.data.impl.MetadataSource;
 import org.opensaml.saml.saml2.metadata.EntityDescriptor;
 import org.opensaml.security.httpclient.HttpClientSecurityParameters;
 import org.testng.Assert;
@@ -211,26 +216,33 @@ public class FileBackedHTTPMetadataResolverTest extends XMLObjectBaseTestCase {
         Assert.assertTrue(backupFile.exists(), "Backup file was not created");
         Assert.assertTrue(backupFile.length() > 0, "Backup file contains no data");
         
+        MockContextTrackingFilter mockFilter = new MockContextTrackingFilter();
+
         metadataProvider = new FileBackedHTTPMetadataResolver(httpClientBuilder.buildClient(), metadataURLHttp, backupFilePath);
         metadataProvider.setParserPool(parserPool);
         metadataProvider.setFailFastInitialization(true);
         metadataProvider.setId("test");
         metadataProvider.setBackupFileInitNextRefreshDelay(Duration.ofSeconds(1));
+        metadataProvider.setMetadataFilter(mockFilter);
         metadataProvider.initialize();
-        
+
         Assert.assertTrue(metadataProvider.isInitializedFromBackupFile());
-        
+
+        Assert.assertTrue(mockFilter.lastFilterContext.get(MetadataSource.class).isTrusted());
+
         Instant initRefresh = metadataProvider.getLastRefresh();
         Instant initUpdate = metadataProvider.getLastUpdate();
-        
+
         Assert.assertNotNull(metadataProvider.resolveSingle(criteriaSet), "Metadata inited from backing file was null");
         
         // Sleep past the artificial next refresh delay on init from backup file.
         Thread.sleep(metadataProvider.getBackupFileInitNextRefreshDelay().toMillis() + 5000);
-        
+
         Assert.assertTrue(initRefresh.isBefore(metadataProvider.getLastRefresh()));
         Assert.assertTrue(initUpdate.isBefore(metadataProvider.getLastUpdate()));
-        
+
+        Assert.assertNull(mockFilter.lastFilterContext.get(MetadataSource.class));
+
         Assert.assertNotNull(metadataProvider.resolveSingle(criteriaSet), "Metadata retrieved from HTTP refreshed metadata was null");
     }
     
@@ -539,5 +551,18 @@ public class FileBackedHTTPMetadataResolverTest extends XMLObjectBaseTestCase {
         Assert.assertEquals(descriptor.getEntityID(), entityID, "Entity's ID does not match requested ID");
     }
     
-    
+    // Test helpers
+
+    public class MockContextTrackingFilter implements MetadataFilter {
+
+        public MetadataFilterContext lastFilterContext;
+
+        /** {@inheritDoc} */
+        public XMLObject filter(XMLObject metadata, MetadataFilterContext context) throws FilterException {
+            lastFilterContext = context;
+            return metadata;
+        }
+
+    }
+
 }
\ No newline at end of file

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


More information about the commits mailing list