[java-opensaml] branch master updated: JSPT-93 - Clean up API for BaseXXSupport decoders

Phil Smart philip.smart at jisc.ac.uk
Sun Feb 23 15:49:50 EST 2020


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

philsmart 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=cb490dbeca816888464fcbdf2050612cba9b9130

The following commit(s) were added to refs/heads/master by this push:
       new  cb490db   JSPT-93 - Clean up API for BaseXXSupport decoders
cb490db is described below

commit cb490dbeca816888464fcbdf2050612cba9b9130
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Sun Feb 23 20:49:07 2020 +0000

    JSPT-93 - Clean up API for BaseXXSupport decoders
    
     - Throw EncodingException from CryptoBinary setValueBigInt
        and propagate error handling to call sites.
    
    https://issues.shibboleth.net/jira/browse/JSPT-93
---
 ...olderOfKeySubjectConfirmationValidatorTest.java | 10 +++++----
 .../impl/MetadataCredentialResolverTest.java       |  4 +++-
 ...MLMetadataEncryptionParametersResolverTest.java |  7 ++++++-
 .../opensaml/xmlsec/keyinfo/KeyInfoSupport.java    | 12 ++++++++---
 .../opensaml/xmlsec/signature/CryptoBinary.java    |  5 ++++-
 .../keyinfo/impl/BasicKeyInfoGeneratorFactory.java |  8 +++++++-
 .../xmlsec/signature/impl/CryptoBinaryImpl.java    | 24 ++++++++--------------
 .../xmlsec/keyinfo/KeyInfoSupportTest.java         | 21 +++++++++++--------
 .../impl/LocalKeyInfoCredentialResolverTest.java   |  5 +++--
 9 files changed, 60 insertions(+), 36 deletions(-)

diff --git a/opensaml-saml-impl/src/test/java/org/opensaml/saml/saml2/assertion/impl/HolderOfKeySubjectConfirmationValidatorTest.java b/opensaml-saml-impl/src/test/java/org/opensaml/saml/saml2/assertion/impl/HolderOfKeySubjectConfirmationValidatorTest.java
index dab5ade..915074e 100644
--- a/opensaml-saml-impl/src/test/java/org/opensaml/saml/saml2/assertion/impl/HolderOfKeySubjectConfirmationValidatorTest.java
+++ b/opensaml-saml-impl/src/test/java/org/opensaml/saml/saml2/assertion/impl/HolderOfKeySubjectConfirmationValidatorTest.java
@@ -42,6 +42,8 @@ import org.testng.annotations.BeforeClass;
 import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
 
+import net.shibboleth.utilities.java.support.codec.EncodingException;
+
 /**
  *
  */
@@ -80,7 +82,7 @@ public class HolderOfKeySubjectConfirmationValidatorTest extends BaseAssertionVa
     }
 
     @Test
-    public void testValidPublicKeyViaKeyValue() throws AssertionValidationException {
+    public void testValidPublicKeyViaKeyValue() throws AssertionValidationException, EncodingException {
         KeyInfoSupport.addPublicKey(keyInfo, publicKey1);
         
         Map<String,Object> staticParams = buildBasicStaticParameters();
@@ -110,7 +112,7 @@ public class HolderOfKeySubjectConfirmationValidatorTest extends BaseAssertionVa
     }
     
     @Test
-    public void testInvalidPublicKey() throws AssertionValidationException {
+    public void testInvalidPublicKey() throws AssertionValidationException, EncodingException {
         KeyInfoSupport.addPublicKey(keyInfo, publicKey1);
         
         Map<String,Object> staticParams = buildBasicStaticParameters();
@@ -170,7 +172,7 @@ public class HolderOfKeySubjectConfirmationValidatorTest extends BaseAssertionVa
     }
     
     @Test
-    public void testMissingPresenterParams() throws AssertionValidationException {
+    public void testMissingPresenterParams() throws AssertionValidationException, EncodingException {
         KeyInfoSupport.addPublicKey(keyInfo, publicKey1);
         
         Map<String,Object> staticParams = buildBasicStaticParameters();
@@ -210,7 +212,7 @@ public class HolderOfKeySubjectConfirmationValidatorTest extends BaseAssertionVa
     }
     
     @Test
-    public void testCertAndKeyParamMismatch() throws AssertionValidationException {
+    public void testCertAndKeyParamMismatch() throws AssertionValidationException, EncodingException {
         KeyInfoSupport.addPublicKey(keyInfo, publicKey1);
         
         Map<String,Object> staticParams = buildBasicStaticParameters();
diff --git a/opensaml-saml-impl/src/test/java/org/opensaml/saml/security/impl/MetadataCredentialResolverTest.java b/opensaml-saml-impl/src/test/java/org/opensaml/saml/security/impl/MetadataCredentialResolverTest.java
index 66c0587..740665a 100644
--- a/opensaml-saml-impl/src/test/java/org/opensaml/saml/security/impl/MetadataCredentialResolverTest.java
+++ b/opensaml-saml-impl/src/test/java/org/opensaml/saml/security/impl/MetadataCredentialResolverTest.java
@@ -26,6 +26,7 @@ import java.util.List;
 
 import javax.xml.namespace.QName;
 
+import net.shibboleth.utilities.java.support.codec.EncodingException;
 import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
 import net.shibboleth.utilities.java.support.resolver.CriteriaSet;
 import net.shibboleth.utilities.java.support.resolver.ResolverException;
@@ -566,9 +567,10 @@ public class MetadataCredentialResolverTest extends XMLObjectBaseTestCase {
      * @throws ComponentInitializationException ...
      * @throws ResolverException ...
      * @throws CertificateEncodingException ...
+     * @throws EncodingException on base64 encoding error
      */
     @Test 
-    public void testDirectResolutionFromRoleDescriptor() throws ComponentInitializationException, ResolverException, CertificateEncodingException {
+    public void testDirectResolutionFromRoleDescriptor() throws ComponentInitializationException, ResolverException, CertificateEncodingException, EncodingException {
         // Use a new instance that does not have a role descriptor resolver, just for good measure.
         mdCredResolver = new MetadataCredentialResolver();
         mdCredResolver.setKeyInfoCredentialResolver(SAMLTestSupport.buildBasicInlineKeyInfoResolver());
diff --git a/opensaml-saml-impl/src/test/java/org/opensaml/saml/security/impl/SAMLMetadataEncryptionParametersResolverTest.java b/opensaml-saml-impl/src/test/java/org/opensaml/saml/security/impl/SAMLMetadataEncryptionParametersResolverTest.java
index c9ea228..1eb255f 100644
--- a/opensaml-saml-impl/src/test/java/org/opensaml/saml/security/impl/SAMLMetadataEncryptionParametersResolverTest.java
+++ b/opensaml-saml-impl/src/test/java/org/opensaml/saml/security/impl/SAMLMetadataEncryptionParametersResolverTest.java
@@ -32,6 +32,7 @@ import java.util.Map;
 
 import javax.annotation.Nullable;
 
+import net.shibboleth.utilities.java.support.codec.EncodingException;
 import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
 import net.shibboleth.utilities.java.support.logic.ConstraintViolationException;
 import net.shibboleth.utilities.java.support.resolver.CriteriaSet;
@@ -712,7 +713,11 @@ public class SAMLMetadataEncryptionParametersResolverTest extends XMLObjectBaseT
         
         for (Object contentItem : contentItems) {
             if (contentItem instanceof PublicKey) {
-                KeyInfoSupport.addPublicKey(keyInfo, (PublicKey) contentItem);
+                try {
+                    KeyInfoSupport.addPublicKey(keyInfo, (PublicKey) contentItem);
+                } catch (EncodingException e) {
+                    throw new RuntimeException("EncodingException adding public key to KeyInfo", e);
+                }
             } else if (contentItem instanceof X509Certificate) {
                 try {
                     KeyInfoSupport.addCertificate(keyInfo, (X509Certificate) contentItem);
diff --git a/opensaml-xmlsec-api/src/main/java/org/opensaml/xmlsec/keyinfo/KeyInfoSupport.java b/opensaml-xmlsec-api/src/main/java/org/opensaml/xmlsec/keyinfo/KeyInfoSupport.java
index 49ee6d2..fdc929a 100644
--- a/opensaml-xmlsec-api/src/main/java/org/opensaml/xmlsec/keyinfo/KeyInfoSupport.java
+++ b/opensaml-xmlsec-api/src/main/java/org/opensaml/xmlsec/keyinfo/KeyInfoSupport.java
@@ -515,8 +515,10 @@ public class KeyInfoSupport {
      * 
      * @param keyInfo the {@link KeyInfo} element to which to add the key
      * @param pk the native Java {@link PublicKey} to add
+     * @throws EncodingException if base64 encoding the components of the public key <code>pk</code> fails
      */
-    public static void addPublicKey(@Nonnull final KeyInfo keyInfo, @Nullable final PublicKey pk) {
+    public static void addPublicKey(@Nonnull final KeyInfo keyInfo, @Nullable final PublicKey pk) 
+            throws EncodingException {
         Constraint.isNotNull(keyInfo, "KeyInfo cannot be null");
         
         final XMLObjectBuilder<KeyValue> keyValueBuilder =
@@ -541,8 +543,10 @@ public class KeyInfoSupport {
      * 
      * @param rsaPubKey a native Java {@link RSAPublicKey}
      * @return an {@link RSAKeyValue} XMLObject
+     * @throws EncodingException if the RSA public key modulus/exponent can not be base64 encoded
      */
-    @Nonnull public static RSAKeyValue buildRSAKeyValue(@Nonnull final RSAPublicKey rsaPubKey) {
+    @Nonnull public static RSAKeyValue buildRSAKeyValue(@Nonnull final RSAPublicKey rsaPubKey) 
+            throws EncodingException {
         Constraint.isNotNull(rsaPubKey, "RSA public key cannot be null");
         
         final XMLObjectBuilderFactory builderFactory = XMLObjectProviderRegistrySupport.getBuilderFactory();
@@ -572,8 +576,10 @@ public class KeyInfoSupport {
      * 
      * @param dsaPubKey a native Java {@link DSAPublicKey}
      * @return an {@link DSAKeyValue} XMLObject
+     * @throws EncodingException if the DSA public key parameters can not be base64 encoded
      */
-    @Nonnull public static DSAKeyValue buildDSAKeyValue(@Nonnull final DSAPublicKey dsaPubKey) {
+    @Nonnull public static DSAKeyValue buildDSAKeyValue(@Nonnull final DSAPublicKey dsaPubKey) 
+            throws EncodingException {
         Constraint.isNotNull(dsaPubKey, "DSA public key cannot be null");
         
         final XMLObjectBuilderFactory builderFactory = XMLObjectProviderRegistrySupport.getBuilderFactory();
diff --git a/opensaml-xmlsec-api/src/main/java/org/opensaml/xmlsec/signature/CryptoBinary.java b/opensaml-xmlsec-api/src/main/java/org/opensaml/xmlsec/signature/CryptoBinary.java
index 05fff29..8b5332f 100644
--- a/opensaml-xmlsec-api/src/main/java/org/opensaml/xmlsec/signature/CryptoBinary.java
+++ b/opensaml-xmlsec-api/src/main/java/org/opensaml/xmlsec/signature/CryptoBinary.java
@@ -25,6 +25,8 @@ import javax.xml.namespace.QName;
 import org.opensaml.core.xml.schema.XSBase64Binary;
 import org.opensaml.xmlsec.signature.support.SignatureConstants;
 
+import net.shibboleth.utilities.java.support.codec.EncodingException;
+
 /**
  * XMLObject representing XML Digital Signature, version 20020212, CryptoBinary simple type.
  */
@@ -48,7 +50,8 @@ public interface CryptoBinary extends XSBase64Binary {
      * Convenience method to set the value of the element as a BigInteger type.
      * 
      * @param bigInt the new BigInteger representation of the element's content
+     * @throws EncodingException if the byte value of the BigInteger can not be base64 encoded.
      */
-    public void setValueBigInt(@Nullable final BigInteger bigInt);
+    public void setValueBigInt(@Nullable final BigInteger bigInt) throws EncodingException;
 
 }
\ No newline at end of file
diff --git a/opensaml-xmlsec-impl/src/main/java/org/opensaml/xmlsec/keyinfo/impl/BasicKeyInfoGeneratorFactory.java b/opensaml-xmlsec-impl/src/main/java/org/opensaml/xmlsec/keyinfo/impl/BasicKeyInfoGeneratorFactory.java
index fd1e325..9e6344c 100644
--- a/opensaml-xmlsec-impl/src/main/java/org/opensaml/xmlsec/keyinfo/impl/BasicKeyInfoGeneratorFactory.java
+++ b/opensaml-xmlsec-impl/src/main/java/org/opensaml/xmlsec/keyinfo/impl/BasicKeyInfoGeneratorFactory.java
@@ -36,6 +36,8 @@ import org.opensaml.xmlsec.signature.KeyInfo;
 
 import com.google.common.base.Strings;
 
+import net.shibboleth.utilities.java.support.codec.EncodingException;
+
 
 /**
  * A factory implementation which produces instances of {@link KeyInfoGenerator} capable of 
@@ -249,7 +251,11 @@ public class BasicKeyInfoGeneratorFactory implements KeyInfoGeneratorFactory {
             throws SecurityException {
             if (credential.getPublicKey() != null) {
                 if (options.emitPublicKeyValue) {
-                    KeyInfoSupport.addPublicKey(keyInfo, credential.getPublicKey());
+                    try {
+                        KeyInfoSupport.addPublicKey(keyInfo, credential.getPublicKey());
+                    } catch (final EncodingException e) {
+                        throw new SecurityException("Can't add public key to key info",e);
+                    }
                 }
                 if (options.emitPublicDEREncodedKeyValue) {
                     try {
diff --git a/opensaml-xmlsec-impl/src/main/java/org/opensaml/xmlsec/signature/impl/CryptoBinaryImpl.java b/opensaml-xmlsec-impl/src/main/java/org/opensaml/xmlsec/signature/impl/CryptoBinaryImpl.java
index f836830..1d7e88e 100644
--- a/opensaml-xmlsec-impl/src/main/java/org/opensaml/xmlsec/signature/impl/CryptoBinaryImpl.java
+++ b/opensaml-xmlsec-impl/src/main/java/org/opensaml/xmlsec/signature/impl/CryptoBinaryImpl.java
@@ -23,6 +23,8 @@ import java.util.Objects;
 import org.opensaml.core.xml.schema.impl.XSBase64BinaryImpl;
 import org.opensaml.xmlsec.keyinfo.KeyInfoSupport;
 import org.opensaml.xmlsec.signature.CryptoBinary;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import com.google.common.base.Strings;
 
@@ -34,6 +36,9 @@ import net.shibboleth.utilities.java.support.codec.EncodingException;
  */
 public class CryptoBinaryImpl extends XSBase64BinaryImpl implements CryptoBinary {
     
+    /** Class logger. */
+    private final Logger log = LoggerFactory.getLogger(CryptoBinaryImpl.class);
+    
     /** The cached BigInteger representation of the element's base64-encoded value. */
     private BigInteger bigIntValue;
 
@@ -55,7 +60,8 @@ public class CryptoBinaryImpl extends XSBase64BinaryImpl implements CryptoBinary
             try {
                 bigIntValue = KeyInfoSupport.decodeBigIntegerFromCryptoBinary(getValue());
             } catch (final DecodingException e) {
-                //can not decode big integer from invalid value, return original even if null.                
+                //can not decode big integer from invalid value, return original even if null.    
+                log.warn("Could not decode big integer value, returning cached value",e);
             }
         }
         return bigIntValue;
@@ -63,23 +69,11 @@ public class CryptoBinaryImpl extends XSBase64BinaryImpl implements CryptoBinary
 
     /** {@inheritDoc} */
     @Override
-    public void setValueBigInt(final BigInteger bigInt) {
+    public void setValueBigInt(final BigInteger bigInt) throws EncodingException{
         if (bigInt == null) {
             setValue(null);
         } else {
-            try {
-                setValue(KeyInfoSupport.encodeCryptoBinaryFromBigInteger(bigInt));
-            } catch (final EncodingException e) { 
-                /*
-                 * Should never happen, but if for some reason the bigInt
-                 * is not null but can not be converted and encoded as 
-                 * base64, set both the string value to null and the
-                 * bigIntValue to null so they are consistent. 
-                 */
-                setValue(null);
-                bigIntValue=null;
-                return;
-            }
+            setValue(KeyInfoSupport.encodeCryptoBinaryFromBigInteger(bigInt));
         }
         bigIntValue = bigInt;
     }
diff --git a/opensaml-xmlsec-impl/src/test/java/org/opensaml/xmlsec/keyinfo/KeyInfoSupportTest.java b/opensaml-xmlsec-impl/src/test/java/org/opensaml/xmlsec/keyinfo/KeyInfoSupportTest.java
index 37308ce..c73df93 100644
--- a/opensaml-xmlsec-impl/src/test/java/org/opensaml/xmlsec/keyinfo/KeyInfoSupportTest.java
+++ b/opensaml-xmlsec-impl/src/test/java/org/opensaml/xmlsec/keyinfo/KeyInfoSupportTest.java
@@ -38,6 +38,7 @@ import javax.security.auth.x500.X500Principal;
 
 import net.shibboleth.utilities.java.support.codec.Base64Support;
 import net.shibboleth.utilities.java.support.codec.DecodingException;
+import net.shibboleth.utilities.java.support.codec.EncodingException;
 
 import org.opensaml.core.xml.XMLObjectBaseTestCase;
 import org.opensaml.security.SecurityException;
@@ -444,9 +445,10 @@ public class KeyInfoSupportTest extends XMLObjectBaseTestCase {
         Assert.assertEquals(rsaKey, javaRSAPubKey1, "Generated key was not the expected value");
     }
 
-    /** Test conversion of DSA public keys from Java security native type to XML. */
+    /** Test conversion of DSA public keys from Java security native type to XML. 
+     * @throws EncodingException on base64 encoding error*/
     @Test
-    public void testDSAConversionJavaToXML() {
+    public void testDSAConversionJavaToXML() throws EncodingException {
         DSAKeyValue dsaKeyValue = KeyInfoSupport.buildDSAKeyValue(javaDSAPubKey1);
         Assert.assertNotNull("Generated DSAKeyValue was null");
         Assert.assertEquals(dsaKeyValue
@@ -459,9 +461,10 @@ public class KeyInfoSupportTest extends XMLObjectBaseTestCase {
                 "Generated DSAKeyValue G component was not the expected value");
     }
 
-    /** Test conversion of RSA public keys from Java security native type to XML. */
+    /** Test conversion of RSA public keys from Java security native type to XML. 
+     * @throws EncodingException on base64 encoding error*/
     @Test
-    public void testRSAConversionJavaToXML() {
+    public void testRSAConversionJavaToXML() throws EncodingException {
         RSAKeyValue rsaKeyValue = KeyInfoSupport.buildRSAKeyValue(javaRSAPubKey1);
         Assert.assertNotNull("Generated RSAKeyValue was null");
         Assert.assertEquals(rsaKeyValue.getModulus().getValueBigInt(), javaRSAPubKey1.getModulus(),
@@ -510,9 +513,10 @@ public class KeyInfoSupportTest extends XMLObjectBaseTestCase {
         keyValue.setRSAKeyValue(null);
     }
 
-    /** Tests adding a public key as a KeyValue to KeyInfo. */
+    /** Tests adding a public key as a KeyValue to KeyInfo. 
+     * @throws EncodingException on base64 encoding error*/
     @Test
-    public void testAddDSAPublicKey() {
+    public void testAddDSAPublicKey() throws EncodingException {
         keyInfo.getKeyValues().clear();
 
         KeyInfoSupport.addPublicKey(keyInfo, javaDSAPubKey1);
@@ -533,9 +537,10 @@ public class KeyInfoSupportTest extends XMLObjectBaseTestCase {
         keyInfo.getKeyValues().clear();
     }    
 
-    /** Tests adding a public key as a KeyValue to KeyInfo. */
+    /** Tests adding a public key as a KeyValue to KeyInfo. 
+     * @throws EncodingException on base64 encoding error*/
     @Test
-    public void testAddRSAPublicKey() {
+    public void testAddRSAPublicKey() throws EncodingException {
         keyInfo.getKeyValues().clear();
 
         KeyInfoSupport.addPublicKey(keyInfo, javaRSAPubKey1);
diff --git a/opensaml-xmlsec-impl/src/test/java/org/opensaml/xmlsec/keyinfo/impl/LocalKeyInfoCredentialResolverTest.java b/opensaml-xmlsec-impl/src/test/java/org/opensaml/xmlsec/keyinfo/impl/LocalKeyInfoCredentialResolverTest.java
index 9773115..5b01b81 100644
--- a/opensaml-xmlsec-impl/src/test/java/org/opensaml/xmlsec/keyinfo/impl/LocalKeyInfoCredentialResolverTest.java
+++ b/opensaml-xmlsec-impl/src/test/java/org/opensaml/xmlsec/keyinfo/impl/LocalKeyInfoCredentialResolverTest.java
@@ -25,6 +25,7 @@ import java.security.NoSuchAlgorithmException;
 import java.security.NoSuchProviderException;
 import java.util.ArrayList;
 
+import net.shibboleth.utilities.java.support.codec.EncodingException;
 import net.shibboleth.utilities.java.support.resolver.CriteriaSet;
 import net.shibboleth.utilities.java.support.resolver.ResolverException;
 
@@ -83,7 +84,7 @@ public class LocalKeyInfoCredentialResolverTest extends XMLObjectBaseTestCase {
     }
 
     @Test
-    public void testKeyInfoWithKnownPublicKey() throws ResolverException {
+    public void testKeyInfoWithKnownPublicKey() throws ResolverException, EncodingException {
         KeyInfoSupport.addPublicKey(keyInfo, keyPair.getPublic());
         
         CriteriaSet criteriaSet = new CriteriaSet( new KeyInfoCriterion(keyInfo) );
@@ -94,7 +95,7 @@ public class LocalKeyInfoCredentialResolverTest extends XMLObjectBaseTestCase {
     
     @Test
     public void testKeyInfoWithUnknownPublicKey() throws IllegalArgumentException,
-        NoSuchAlgorithmException, NoSuchProviderException, ResolverException {
+        NoSuchAlgorithmException, NoSuchProviderException, ResolverException, EncodingException {
         
         KeyInfoSupport.addPublicKey(keyInfo, 
                 KeySupport.generateKeyPair("RSA", 1024, null).getPublic());

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


More information about the commits mailing list