[java-opensaml COMMIT] in /trunk/opensaml-xmlsec-impl/src: main/java/org/opensaml/xmlsec/impl/BasicWhitelistBlacklist...

[noreply at shibboleth.net]

Author: putmanb
Date: Fri May 16 21:32:00 2014
New Revision: 3886

URL: http://svn.shibboleth.net/view/java-opensaml?rev=3886&view=rev
Log:
Change default blacklist merge to true, to avoid unintentionally missing blacklists defined at at lower order-of-precedence.

Modified:
    trunk/opensaml-xmlsec-impl/src/main/java/org/opensaml/xmlsec/impl/BasicWhitelistBlacklistConfiguration.java
    trunk/opensaml-xmlsec-impl/src/test/java/org/opensaml/xmlsec/impl/AbstractSecurityParametersResolverTest.java
    trunk/opensaml-xmlsec-impl/src/test/java/org/opensaml/xmlsec/impl/BasicWhitelistBlacklistConfigurationTest.java

Modified: trunk/opensaml-xmlsec-impl/src/main/java/org/opensaml/xmlsec/impl/BasicWhitelistBlacklistConfiguration.java
URL: http://svn.shibboleth.net/view/java-opensaml/trunk/opensaml-xmlsec-impl/src/main/java/org/opensaml/xmlsec/impl/BasicWhitelistBlacklistConfiguration.java?rev=3886&r1=3885&r2=3886&view=diff
==============================================================================
--- trunk/opensaml-xmlsec-impl/src/main/java/org/opensaml/xmlsec/impl/BasicWhitelistBlacklistConfiguration.java (original)
+++ trunk/opensaml-xmlsec-impl/src/main/java/org/opensaml/xmlsec/impl/BasicWhitelistBlacklistConfiguration.java Fri May 16 21:32:00 2014
@@ -66,6 +66,12 @@
         whitelist = Collections.emptySet();
         blacklist = Collections.emptySet();
         precedence = DEFAULT_PRECEDENCE;
+        
+        // These merging defaults are intended to be the more secure/conservative approach:
+        // - do merge blacklists by default since don't want to unintentionally miss blacklist from lower level
+        // - do not merge whitelists by default since don't want to unintentionally include algos from lower level
+        blacklistMerge = true;
+        whitelistMerge = false;
     }
     
     /**
@@ -90,7 +96,11 @@
         whitelist = Sets.newHashSet(StringSupport.normalizeStringCollection(uris));
     }
 
-    /** {@inheritDoc} */
+    /** 
+     * {@inheritDoc}
+     * 
+     * <p>Defaults to: <code>false</code>
+     */
     public boolean isWhitelistMerge() {
         return whitelistMerge;
     }
@@ -98,6 +108,8 @@
     /**
      * Set the flag indicating whether to merge this configuration's whitelist with one of a lower order of precedence,
      * or to treat this whitelist as authoritative.
+     * 
+     * <p>Defaults to: <code>false</code>
      * 
      * @param flag true if should merge, false otherwise
      */
@@ -127,7 +139,11 @@
         blacklist = Sets.newHashSet(StringSupport.normalizeStringCollection(uris));
     }
 
-    /** {@inheritDoc} */
+    /** 
+     * {@inheritDoc}
+     * 
+     * <p>Defaults to: <code>true</code>
+     */
     public boolean isBlacklistMerge() {
         return blacklistMerge;
     }
@@ -135,6 +151,8 @@
     /**
      * Set the flag indicating whether to merge this configuration's blacklist with one of a lower order of precedence,
      * or to treat this blacklist as authoritative.
+     * 
+     * <p>Defaults to: <code>true</code>
      * 
      * @param flag true if should merge, false otherwise
      */

Modified: trunk/opensaml-xmlsec-impl/src/test/java/org/opensaml/xmlsec/impl/AbstractSecurityParametersResolverTest.java
URL: http://svn.shibboleth.net/view/java-opensaml/trunk/opensaml-xmlsec-impl/src/test/java/org/opensaml/xmlsec/impl/AbstractSecurityParametersResolverTest.java?rev=3886&r1=3885&r2=3886&view=diff
==============================================================================
--- trunk/opensaml-xmlsec-impl/src/test/java/org/opensaml/xmlsec/impl/AbstractSecurityParametersResolverTest.java (original)
+++ trunk/opensaml-xmlsec-impl/src/test/java/org/opensaml/xmlsec/impl/AbstractSecurityParametersResolverTest.java Fri May 16 21:32:00 2014
@@ -87,6 +87,22 @@
         
         WhitelistBlacklistParameters params = resolver.resolveSingle(criteriaSet);
         
+        HashSet<String> control = new HashSet<>();
+        control.addAll(set1);
+        control.addAll(set2);
+        
+        Assert.assertEquals(params.getWhitelistedAlgorithms(), Collections.emptySet());
+        Assert.assertEquals(params.getBlacklistedAlgorithms(), control);
+    }
+    
+    @Test
+    public void testBlacklistOnlyNoMerge() throws ResolverException {
+        config1.setBlacklistedAlgorithms(set1);
+        config1.setBlacklistMerge(false);
+        config2.setBlacklistedAlgorithms(set2);
+        
+        WhitelistBlacklistParameters params = resolver.resolveSingle(criteriaSet);
+        
         Assert.assertEquals(params.getWhitelistedAlgorithms(), Collections.emptySet());
         Assert.assertEquals(params.getBlacklistedAlgorithms(), set1);
     }
@@ -294,10 +310,10 @@
         
         blacklist = resolver.resolveEffectiveBlacklist(criteriaSet, criterion.getConfigurations());
         Assert.assertTrue(blacklist.containsAll(set1));
-        Assert.assertFalse(blacklist.containsAll(set2));
-        Assert.assertFalse(blacklist.containsAll(set3));
-        
-        config1.setBlacklistMerge(true);
+        Assert.assertTrue(blacklist.containsAll(set2));

[... 58 lines stripped ...]