[java-shib-metadata] 01/03: JSMD-2 - X509CredentialNameEvaluatorFactoryBean illegally returns null

Scott Cantor cantor.2 at osu.edu
Mon Nov 14 16:32:24 UTC 2022


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

scantor pushed a commit to branch main
in repository java-shib-metadata.

View the commit online:
http://git.shibboleth.net/view/?p=java-shib-metadata.git;a=commit;h=8b20073476c9eaf64dd25cfb4f9c20a6319801a3

commit 8b20073476c9eaf64dd25cfb4f9c20a6319801a3
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Mon Nov 14 11:21:53 2022 -0500

    JSMD-2 - X509CredentialNameEvaluatorFactoryBean illegally returns null
    
    https://shibboleth.atlassian.net/browse/JSMD-2
    
    Build instance of Dummy class instead of using null.
---
 .../security/trust/AbstractStaticPKIXParser.java   | 39 +++++++++++++---------
 .../trust/StaticPKIXSignatureParserTest.java       | 13 +++++---
 .../trust/StaticPKIXX509CredentialParserTest.java  |  7 ++--
 3 files changed, 35 insertions(+), 24 deletions(-)

diff --git a/shib-metadata-spring/src/main/java/net/shibboleth/spring/security/trust/AbstractStaticPKIXParser.java b/shib-metadata-spring/src/main/java/net/shibboleth/spring/security/trust/AbstractStaticPKIXParser.java
index f59a3888..f7c1443a 100644
--- a/shib-metadata-spring/src/main/java/net/shibboleth/spring/security/trust/AbstractStaticPKIXParser.java
+++ b/shib-metadata-spring/src/main/java/net/shibboleth/spring/security/trust/AbstractStaticPKIXParser.java
@@ -22,13 +22,13 @@ import java.util.List;
 import javax.annotation.Nonnull;
 import javax.xml.namespace.QName;
 
-import net.shibboleth.shared.primitive.StringSupport;
 import net.shibboleth.shared.spring.util.SpringSupport;
 import net.shibboleth.shared.xml.ElementSupport;
 import net.shibboleth.spring.security.SecurityNamespaceHandler;
 
 import org.opensaml.security.x509.impl.BasicX509CredentialNameEvaluator;
 import org.opensaml.security.x509.impl.CertPathPKIXTrustEvaluator;
+import org.opensaml.security.x509.impl.DummyX509CredentialNameEvaluator;
 import org.opensaml.security.x509.impl.StaticPKIXValidationInformationResolver;
 import org.opensaml.security.x509.impl.X509CredentialNameEvaluator;
 import org.springframework.beans.factory.config.AbstractFactoryBean;
@@ -43,11 +43,11 @@ import org.w3c.dom.Element;
 public abstract class AbstractStaticPKIXParser extends AbstractTrustEngineParser {
 
     /** Validation Information. */
-    public static final QName VALIDATION_INFO = new QName(SecurityNamespaceHandler.SECURITY_NAMESPACE,
+    @Nonnull public static final QName VALIDATION_INFO = new QName(SecurityNamespaceHandler.SECURITY_NAMESPACE,
             "ValidationInfo");
 
     /** Trusted Names Information. */
-    public static final QName TRUSTED_NAMES = new QName(SecurityNamespaceHandler.SECURITY_NAMESPACE,
+    @Nonnull public static final QName TRUSTED_NAMES = new QName(SecurityNamespaceHandler.SECURITY_NAMESPACE,
             "TrustedName");
 
     /**
@@ -58,7 +58,7 @@ public abstract class AbstractStaticPKIXParser extends AbstractTrustEngineParser
      * @param parserContext the context to parse inside
      * @return the definition
      */
-    protected BeanDefinition getPKIXValidationInformationResolver(@Nonnull final Element element,
+    @Nonnull protected BeanDefinition getPKIXValidationInformationResolver(@Nonnull final Element element,
             @Nonnull final ParserContext parserContext) {
 
         final List<Element> validationInfoElements = ElementSupport.getChildElements(element, VALIDATION_INFO);
@@ -82,7 +82,7 @@ public abstract class AbstractStaticPKIXParser extends AbstractTrustEngineParser
      * @param parserContext the context to parse inside
      * @return the definition
      */
-    protected BeanDefinition getPKIXTrustEvaluator(@Nonnull final Element element,
+    @Nonnull protected BeanDefinition getPKIXTrustEvaluator(@Nonnull final Element element,
             @Nonnull final ParserContext parserContext) {
 
         final BeanDefinitionBuilder builder =
@@ -107,21 +107,24 @@ public abstract class AbstractStaticPKIXParser extends AbstractTrustEngineParser
      * @param parserContext the context to parse inside
      * @return an X509CredentialNameEvaluator instance or a BeanDefinition. May be null.
      */
-    protected Object getX509CredentialNameEvaluator(@Nonnull final Element element,
+    @Nonnull protected Object getX509CredentialNameEvaluator(@Nonnull final Element element,
             @Nonnull final ParserContext parserContext) {
 
         final BeanDefinitionBuilder builder =
                 BeanDefinitionBuilder.genericBeanDefinition(X509CredentialNameEvaluatorFactoryBean.class);
-        final String attrValue = StringSupport.trimOrNull(element.getAttributeNS(null, "trustedNameCheckEnabled"));
-        if (attrValue != null) {
-            builder.addPropertyValue("trustedNameCheckEnabled", attrValue);
+
+        if (element.hasAttributeNS(null, "trustedNameCheckEnabled")) {
+            builder.addPropertyValue("trustedNameCheckEnabled", SpringSupport.getStringValueAsBoolean(
+                    element.getAttributeNS(null, "trustedNameCheckEnabled")));
         }
+        
         return builder.getBeanDefinition();
     }
 
     /**
-     * FactoryBean to do a deferred decision on whether to create a {@link X509CredentialNameEvaluator}. This is in a
-     * factory bean to allow for property replacement. The default (no value setting) is true.
+     * FactoryBean to do a deferred decision on whether to create a live or dummy
+     * {@link X509CredentialNameEvaluator}. This is in a factory bean to allow for
+     * property replacement. The default (no value setting) is true.
      */
     protected static class X509CredentialNameEvaluatorFactoryBean extends
             AbstractFactoryBean<X509CredentialNameEvaluator> {
@@ -139,16 +142,20 @@ public abstract class AbstractStaticPKIXParser extends AbstractTrustEngineParser
         }
 
         /** {@inheritDoc} */
-        @Override public Class<?> getObjectType() {
-            return BasicX509CredentialNameEvaluator.class;
+        @Override
+        @Nonnull public Class<?> getObjectType() {
+            return X509CredentialNameEvaluator.class;
         }
 
         /** {@inheritDoc} */
-        @Override protected BasicX509CredentialNameEvaluator createInstance() throws Exception {
+        @Override
+        @Nonnull protected X509CredentialNameEvaluator createInstance() throws Exception {
             if (trustedNameCheckEnabled) {
                 return new BasicX509CredentialNameEvaluator();
+            } else {
+                return new DummyX509CredentialNameEvaluator();
             }
-            return null;
         }
     }
-}
+
+}
\ No newline at end of file
diff --git a/shib-metadata-spring/src/test/java/net/shibboleth/spring/security/trust/StaticPKIXSignatureParserTest.java b/shib-metadata-spring/src/test/java/net/shibboleth/spring/security/trust/StaticPKIXSignatureParserTest.java
index 677fb10b..c1173836 100644
--- a/shib-metadata-spring/src/test/java/net/shibboleth/spring/security/trust/StaticPKIXSignatureParserTest.java
+++ b/shib-metadata-spring/src/test/java/net/shibboleth/spring/security/trust/StaticPKIXSignatureParserTest.java
@@ -31,6 +31,7 @@ import org.opensaml.security.x509.PKIXValidationOptions;
 import org.opensaml.security.x509.impl.BasicPKIXValidationInformation;
 import org.opensaml.security.x509.impl.CertPathPKIXTrustEvaluator;
 import org.opensaml.security.x509.impl.CertPathPKIXValidationOptions;
+import org.opensaml.security.x509.impl.DummyX509CredentialNameEvaluator;
 import org.opensaml.security.x509.impl.StaticPKIXValidationInformationResolver;
 import org.opensaml.xmlsec.signature.support.impl.PKIXSignatureTrustEngine;
 import org.testng.Assert;
@@ -70,7 +71,7 @@ public class StaticPKIXSignatureParserTest extends AbstractSecurityParserTest {
         final PKIXSignatureTrustEngine engine =
                 (PKIXSignatureTrustEngine) getBean(TrustEngine.class, "trust/staticPKIX-nameCheckDisabled.xml");
         
-        Assert.assertNull(engine.getX509CredentialNameEvaluator());
+        Assert.assertTrue(engine.getX509CredentialNameEvaluator() instanceof DummyX509CredentialNameEvaluator);
 
         final StaticPKIXValidationInformationResolver resolver =
                 (StaticPKIXValidationInformationResolver) engine.getPKIXResolver();
@@ -109,9 +110,11 @@ public class StaticPKIXSignatureParserTest extends AbstractSecurityParserTest {
             infos.add(info);
         }
         Assert.assertEquals(infos.size(), 2);
-        final int firstVal = ((BasicPKIXValidationInformation) infos.get(0)).getVerificationDepth().intValue();
-        final int secondVal = ((BasicPKIXValidationInformation) infos.get(1)).getVerificationDepth().intValue();
-
+        final Integer firstVal = ((BasicPKIXValidationInformation) infos.get(0)).getVerificationDepth();
+        final Integer secondVal = ((BasicPKIXValidationInformation) infos.get(1)).getVerificationDepth();
+        assert firstVal != null;
+        assert secondVal != null;
+        
         Assert.assertTrue((98 == firstVal) || (99 == firstVal));
         Assert.assertTrue((98 == secondVal) || (99 == secondVal));
         Assert.assertNotEquals(firstVal, secondVal);
@@ -141,7 +144,7 @@ public class StaticPKIXSignatureParserTest extends AbstractSecurityParserTest {
             infos.add(info);
         }
         Assert.assertEquals(infos.size(), 1);
-        final int value = ((BasicPKIXValidationInformation) infos.get(0)).getVerificationDepth().intValue();
+        final Integer value = ((BasicPKIXValidationInformation) infos.get(0)).getVerificationDepth();
 
         Assert.assertEquals(value, 99);
 
diff --git a/shib-metadata-spring/src/test/java/net/shibboleth/spring/security/trust/StaticPKIXX509CredentialParserTest.java b/shib-metadata-spring/src/test/java/net/shibboleth/spring/security/trust/StaticPKIXX509CredentialParserTest.java
index 86ad3bb7..b892e50e 100644
--- a/shib-metadata-spring/src/test/java/net/shibboleth/spring/security/trust/StaticPKIXX509CredentialParserTest.java
+++ b/shib-metadata-spring/src/test/java/net/shibboleth/spring/security/trust/StaticPKIXX509CredentialParserTest.java
@@ -30,6 +30,7 @@ import org.opensaml.security.x509.PKIXValidationInformation;
 import org.opensaml.security.x509.impl.BasicPKIXValidationInformation;
 import org.opensaml.security.x509.impl.CertPathPKIXTrustEvaluator;
 import org.opensaml.security.x509.impl.CertPathPKIXValidationOptions;
+import org.opensaml.security.x509.impl.DummyX509CredentialNameEvaluator;
 import org.opensaml.security.x509.impl.PKIXX509CredentialTrustEngine;
 import org.opensaml.security.x509.impl.StaticPKIXValidationInformationResolver;
 import org.testng.Assert;
@@ -58,7 +59,7 @@ public class StaticPKIXX509CredentialParserTest extends AbstractSecurityParserTe
             infos.add(info);
         }
         Assert.assertEquals(infos.size(), 1);
-        final int value = ((BasicPKIXValidationInformation) infos.get(0)).getVerificationDepth().intValue();
+        final Integer value = ((BasicPKIXValidationInformation) infos.get(0)).getVerificationDepth();
 
         Assert.assertEquals(value, 99);
 
@@ -81,7 +82,7 @@ public class StaticPKIXX509CredentialParserTest extends AbstractSecurityParserTe
         final PKIXX509CredentialTrustEngine engine =
                 (PKIXX509CredentialTrustEngine) getBean(TrustEngine.class, "trust/staticPKIXCredentials-nameCheckDisabled.xml");
         
-        Assert.assertNull(engine.getX509CredentialNameEvaluator());
+        Assert.assertTrue(engine.getX509CredentialNameEvaluator() instanceof DummyX509CredentialNameEvaluator);
 
         final StaticPKIXValidationInformationResolver resolver =
                 (StaticPKIXValidationInformationResolver) engine.getPKIXResolver();
@@ -93,7 +94,7 @@ public class StaticPKIXX509CredentialParserTest extends AbstractSecurityParserTe
             infos.add(info);
         }
         Assert.assertEquals(infos.size(), 1);
-        final int value = ((BasicPKIXValidationInformation) infos.get(0)).getVerificationDepth().intValue();
+        final Integer value = ((BasicPKIXValidationInformation) infos.get(0)).getVerificationDepth();
 
         Assert.assertEquals(value, 99);
 

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


More information about the commits mailing list