[java-opensaml] 04/04: OSJ-244: MetadataProvider should be robust for planed change of ...

Brent Putman putmanb at georgetown.edu
Tue Sep 11 21:38:41 EDT 2018


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=4a3a75abde07f16ca2bbf881226c90610c15c345

commit 4a3a75abde07f16ca2bbf881226c90610c15c345
Author: Brent Putman <putmanb at georgetown.edu>
AuthorDate: Tue Sep 11 20:38:24 2018 -0400

    OSJ-244: MetadataProvider should be robust for planed change of ...
    
    This essentially reverts the changes made in OSJ-121, in commit
    729de8ce7e39a18882a4abeac9cecd65d54d683b.
---
 .../filter/impl/SignatureValidationFilter.java     | 22 +++++--------
 .../SignatureValidationFilterExplicitKeyTest.java  | 36 ++++++++++++++--------
 .../impl/SignatureValidationFilterPKIXTest.java    |  6 ++--
 3 files changed, 33 insertions(+), 31 deletions(-)

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 2b893e4..9039327 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
@@ -239,24 +239,16 @@ public class SignatureValidationFilter implements MetadataFilter {
 
         if (!signableMetadata.isSigned()){
             if (getRequireSignedRoot()) {
-                log.warn("Metadata root element was unsigned and signatures are required, " 
-                        + "metadata will be filtered out.");
-                return null;
+                throw new FilterException("Metadata root element was unsigned and signatures are required.");
             }
         }
         
-        try {
-            if (signableMetadata instanceof EntityDescriptor) {
-                processEntityDescriptor((EntityDescriptor) signableMetadata);
-            } else if (signableMetadata instanceof EntitiesDescriptor) {
-                processEntityGroup((EntitiesDescriptor) signableMetadata);
-            } else {
-                log.error("Internal error, metadata object was of an unsupported type: {}", 
-                        metadata.getClass().getName());
-            }
-        } catch (final Throwable t) {
-            log.warn("Saw fatal error validating metadata signature(s), metadata will be filtered out", t);
-            return null;
+        if (signableMetadata instanceof EntityDescriptor) {
+            processEntityDescriptor((EntityDescriptor) signableMetadata);
+        } else if (signableMetadata instanceof EntitiesDescriptor) {
+            processEntityGroup((EntitiesDescriptor) signableMetadata);
+        } else {
+            log.error("Internal error, metadata object was of an unsupported type: {}", metadata.getClass().getName());
         }
         
         return metadata;
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 7243a0f..c9b2040 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
@@ -124,7 +124,7 @@ public class SignatureValidationFilterExplicitKeyTest extends XMLObjectBaseTestC
         }
     }
     
-    @Test
+    @Test(expectedExceptions=FilterException.class)
     public void testSWITCHStandaloneBlacklistedSignatureAlgorithm() throws UnmarshallingException, FilterException {
         XMLObject xmlObject = unmarshallerFactory.getUnmarshaller(switchMDDocumentValid
                 .getDocumentElement()).unmarshall(switchMDDocumentValid.getDocumentElement());
@@ -136,18 +136,21 @@ public class SignatureValidationFilterExplicitKeyTest extends XMLObjectBaseTestC
         CriteriaSet defaultCriteriaSet = new CriteriaSet(new SignatureValidationParametersCriterion(sigParams));
         filter.setDefaultCriteria(defaultCriteriaSet);
         
-        XMLObject filtered = filter.filter(xmlObject);
-        Assert.assertNull(filtered);
+        filter.filter(xmlObject);
     }
     
     @Test
-    public void testInvalidSWITCHStandalone() throws UnmarshallingException, FilterException {
+    public void testInvalidSWITCHStandalone() throws UnmarshallingException {
         XMLObject xmlObject = unmarshallerFactory.getUnmarshaller(switchMDDocumentInvalid
                 .getDocumentElement()).unmarshall(switchMDDocumentInvalid.getDocumentElement());
         
         SignatureValidationFilter filter = new SignatureValidationFilter(switchSigTrustEngine);
-        XMLObject filtered = filter.filter(xmlObject);
-        Assert.assertNull(filtered);
+        try {
+            filter.filter(xmlObject);
+            Assert.fail("Filter passed validation, should have failed");
+        } catch (FilterException e) {
+            // do nothing, should fail
+        }
     }
     
     @Test
@@ -174,7 +177,7 @@ public class SignatureValidationFilterExplicitKeyTest extends XMLObjectBaseTestC
     }
     
     @Test
-    public void testEntityDescriptorInvalid() throws UnmarshallingException, CertificateException, XMLParserException, FilterException {
+    public void testEntityDescriptorInvalid() throws UnmarshallingException, CertificateException, XMLParserException {
         X509Certificate cert = X509Support.decodeCertificate(openIDCertBase64);
         X509Credential cred = CredentialSupport.getSimpleCredential(cert, null);
         StaticCredentialResolver credResolver = new StaticCredentialResolver(cred);
@@ -189,8 +192,12 @@ public class SignatureValidationFilterExplicitKeyTest extends XMLObjectBaseTestC
         Assert.assertNotNull(ed.getSignature(), "Signature was null");
         
         SignatureValidationFilter filter = new SignatureValidationFilter(trustEngine);
-        XMLObject filtered = filter.filter(xmlObject);
-        Assert.assertNull(filtered);
+        try {
+            filter.filter(xmlObject);
+            Assert.fail("Filter passed validation, should have failed");
+        } catch (FilterException e) {
+            // do nothing, should fail
+        }
     }
     
     @Test
@@ -218,7 +225,7 @@ public class SignatureValidationFilterExplicitKeyTest extends XMLObjectBaseTestC
     }
     
     @Test
-    public void testInvalidEntityDescriptorWithProvider() throws CertificateException, XMLParserException, UnmarshallingException, ComponentInitializationException {
+    public void testInvalidEntityDescriptorWithProvider() throws CertificateException, XMLParserException, UnmarshallingException {
         X509Certificate cert = X509Support.decodeCertificate(openIDCertBase64);
         X509Credential cred = CredentialSupport.getSimpleCredential(cert, null);
         StaticCredentialResolver credResolver = new StaticCredentialResolver(cred);
@@ -234,9 +241,12 @@ public class SignatureValidationFilterExplicitKeyTest extends XMLObjectBaseTestC
         mdProvider.setId("test");
         mdProvider.setMetadataFilter(filter);
         
-        mdProvider.initialize();
-        
-        Assert.assertFalse(mdProvider.iterator().hasNext());
+        try {
+            mdProvider.initialize();
+            Assert.fail("Metadata signature was invalid, provider initialization should have failed");
+        } catch (ComponentInitializationException e) {
+            // do nothing, failure expected
+        }
     }
 
 }
\ No newline at end of file
diff --git a/opensaml-saml-impl/src/test/java/org/opensaml/saml/metadata/resolver/filter/impl/SignatureValidationFilterPKIXTest.java b/opensaml-saml-impl/src/test/java/org/opensaml/saml/metadata/resolver/filter/impl/SignatureValidationFilterPKIXTest.java
index 97f4123..c78f5b0 100644
--- a/opensaml-saml-impl/src/test/java/org/opensaml/saml/metadata/resolver/filter/impl/SignatureValidationFilterPKIXTest.java
+++ b/opensaml-saml-impl/src/test/java/org/opensaml/saml/metadata/resolver/filter/impl/SignatureValidationFilterPKIXTest.java
@@ -31,6 +31,7 @@ import org.opensaml.core.xml.io.MarshallingException;
 import org.opensaml.core.xml.io.UnmarshallingException;
 import org.opensaml.core.xml.util.XMLObjectSupport;
 import org.opensaml.saml.common.SignableSAMLObject;
+import org.opensaml.saml.metadata.resolver.filter.FilterException;
 import org.opensaml.security.SecurityException;
 import org.opensaml.security.credential.Credential;
 import org.opensaml.security.crypto.KeySupport;
@@ -77,7 +78,7 @@ public class SignatureValidationFilterPKIXTest extends XMLObjectBaseTestCase {
         filter.filter(entityDescriptor);
     }
     
-    @Test()
+    @Test(expectedExceptions=FilterException.class)
     public void testEntityDescriptorInvalidEntityID() throws Exception {
         Credential signingCredential = buildSigningCredential("entity.key", "entity.crt", "ca.crt");
         // This metadata file is identical to the success case except the document entityID is changed, so
@@ -85,8 +86,7 @@ public class SignatureValidationFilterPKIXTest extends XMLObjectBaseTestCase {
         // will not match.
         XMLObject entityDescriptor = generateSignedMetadata(signingCredential, "EntityDescriptor-invalid-entityid.xml");
         
-        XMLObject filtered = filter.filter(entityDescriptor);
-        Assert.assertNull(filtered);
+        filter.filter(entityDescriptor);
     }
 
     private XMLObject generateSignedMetadata(Credential signingCredential, String unsignedMetadata) 

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


More information about the commits mailing list