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

Phil Smart philip.smart at jisc.ac.uk
Fri Feb 7 12:11:05 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=a5036cf3f4445cfb3e7eb796f75c4474e55e7baf

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

commit a5036cf3f4445cfb3e7eb796f75c4474e55e7baf
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Fri Feb 7 16:58:45 2020 +0000

    JSPT-93 - Clean up API for BaseXXSupport decoders (ENCODE)
    
     - Add appropriate error handling for the new EncodingException thrown by the
       Base64Support and Base32Support encode methods.
    
    https://issues.shibboleth.net/jira/browse/JSPT-93
---
 .../binding/artifact/AbstractSAMLArtifact.java     | 12 +++-
 .../binding/encoding/impl/HTTPArtifactEncoder.java | 11 +++-
 .../binding/encoding/impl/HTTPPostEncoder.java     |  4 ++
 .../binding/decoding/impl/HTTPArtifactDecoder.java |  6 +-
 .../binding/encoding/impl/HTTPArtifactEncoder.java | 26 +++++++--
 .../binding/encoding/impl/HTTPPostEncoder.java     |  4 ++
 .../encoding/impl/HTTPPostSimpleSignEncoder.java   |  7 +++
 .../encoding/impl/HTTPRedirectDeflateEncoder.java  | 10 +++-
 .../binding/impl/AddGeneratedKeyHeaderHandler.java | 10 ++--
 .../profile/impl/AddGeneratedKeyToAssertions.java  | 24 +++++---
 .../impl/HTTPRedirectDeflateDecoderTest.java       | 10 ++--
 .../impl/AddGeneratedKeyHeaderHandlerTest.java     |  4 +-
 .../impl/AddGeneratedKeyToAssertionsTest.java      | 12 ++--
 .../impl/BaseClientCertAuthSecurityHandler.java    |  6 +-
 .../opensaml/xmlsec/keyinfo/KeyInfoSupport.java    | 64 +++++++++++++++++-----
 .../keyinfo/impl/X509KeyInfoGeneratorFactory.java  |  3 +-
 .../xmlsec/signature/impl/CryptoBinaryImpl.java    | 15 ++++-
 .../xmlsec/keyinfo/KeyInfoSupportTest.java         |  4 +-
 18 files changed, 173 insertions(+), 59 deletions(-)

diff --git a/opensaml-saml-api/src/main/java/org/opensaml/saml/common/binding/artifact/AbstractSAMLArtifact.java b/opensaml-saml-api/src/main/java/org/opensaml/saml/common/binding/artifact/AbstractSAMLArtifact.java
index b4fdeec..10d2582 100644
--- a/opensaml-saml-api/src/main/java/org/opensaml/saml/common/binding/artifact/AbstractSAMLArtifact.java
+++ b/opensaml-saml-api/src/main/java/org/opensaml/saml/common/binding/artifact/AbstractSAMLArtifact.java
@@ -23,6 +23,7 @@ import javax.annotation.Nonnull;
 
 import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
 import net.shibboleth.utilities.java.support.codec.Base64Support;
+import net.shibboleth.utilities.java.support.codec.EncodingException;
 import net.shibboleth.utilities.java.support.logic.Constraint;
 
 import org.apache.commons.codec.binary.Hex;
@@ -85,8 +86,9 @@ public abstract class AbstractSAMLArtifact implements SAMLArtifact {
      * Gets the Base64 encoded artifact.
      * 
      * @return Base64 encoded artifact.
+     * @throws EncodingException if the artifact could not be base64 encoded.
      */
-    @Nonnull @NotEmpty public String base64Encode() {
+    @Nonnull @NotEmpty public String base64Encode() throws EncodingException {
         return Base64Support.encode(getArtifactBytes(), Base64Support.UNCHUNKED);
     }
 
@@ -120,6 +122,12 @@ public abstract class AbstractSAMLArtifact implements SAMLArtifact {
 
     /** {@inheritDoc} */
     public String toString() {
-        return base64Encode();
+        try {
+            return base64Encode();
+        } catch (final EncodingException e) {
+            //very unlikely.
+            //TODO: not clear this is the right thing to do here. could throw RT exception.
+            return e.getMessage()!=null ? this.getClass().getSimpleName()+"#toString() threw "+e.getMessage(): "";
+        }
     }
 }
\ No newline at end of file
diff --git a/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml1/binding/encoding/impl/HTTPArtifactEncoder.java b/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml1/binding/encoding/impl/HTTPArtifactEncoder.java
index 7b28a71..34348bb 100644
--- a/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml1/binding/encoding/impl/HTTPArtifactEncoder.java
+++ b/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml1/binding/encoding/impl/HTTPArtifactEncoder.java
@@ -28,6 +28,7 @@ import javax.servlet.http.HttpServletResponse;
 import net.shibboleth.utilities.java.support.annotation.constraint.NonnullAfterInit;
 import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
 import net.shibboleth.utilities.java.support.codec.Base64Support;
+import net.shibboleth.utilities.java.support.codec.EncodingException;
 import net.shibboleth.utilities.java.support.collection.Pair;
 import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
 import net.shibboleth.utilities.java.support.component.ComponentSupport;
@@ -168,14 +169,20 @@ public class HTTPArtifactEncoder extends BaseSAML1MessageEncoder {
                 throw new MessageEncodingException("Unable to build artifact for message to relying party");
             }
 
-            final String artifactString = Base64Support.encode(artifact.getArtifactBytes(), Base64Support.UNCHUNKED);
+           
             try {
+                final String artifactString = Base64Support.encode(artifact.getArtifactBytes(), 
+                        Base64Support.UNCHUNKED);
                 artifactMap.put(artifactString, requester, issuer, assertion);
+                queryParams.add(new Pair<>("SAMLart", artifactString));
             } catch (final IOException e) {
                 log.error("Unable to store assertion mapping for artifact: {}", e.getMessage());
                 throw new MessageEncodingException("Unable to store assertion mapping for artifact", e);
+            } catch (final EncodingException e) {
+                log.error("Unable to base64 encode artifact for message to relying party: {}", e.getMessage());
+                throw new MessageEncodingException("Unable to base64 encode artifact for message to relying party", e);
             }
-            queryParams.add(new Pair<>("SAMLart", artifactString));
+            
         }
 
         final String encodedEndpoint = urlBuilder.buildURL();
diff --git a/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml1/binding/encoding/impl/HTTPPostEncoder.java b/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml1/binding/encoding/impl/HTTPPostEncoder.java
index d440a8d..ce979f0 100644
--- a/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml1/binding/encoding/impl/HTTPPostEncoder.java
+++ b/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml1/binding/encoding/impl/HTTPPostEncoder.java
@@ -24,6 +24,7 @@ import java.io.Writer;
 import javax.servlet.http.HttpServletResponse;
 
 import net.shibboleth.utilities.java.support.codec.Base64Support;
+import net.shibboleth.utilities.java.support.codec.EncodingException;
 import net.shibboleth.utilities.java.support.codec.HTMLEncoder;
 import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
 import net.shibboleth.utilities.java.support.component.ComponentSupport;
@@ -189,6 +190,9 @@ public class HTTPPostEncoder extends BaseSAML1MessageEncoder {
         } catch (final UnsupportedEncodingException e) {
             log.error("UTF-8 encoding is not supported, this VM is not Java compliant");
             throw new MessageEncodingException("Unable to encode message, UTF-8 encoding is not supported");
+        } catch (final EncodingException e) {
+            log.error("Unable to base64 encode SAML message: {}", e.getMessage());
+            throw new MessageEncodingException("Unable to base64 encode SAML message", e);
         } catch (final Exception e) {
             log.error("Error invoking velocity template: {}", e.getMessage());
             throw new MessageEncodingException("Error creating output document", e);
diff --git a/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/binding/decoding/impl/HTTPArtifactDecoder.java b/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/binding/decoding/impl/HTTPArtifactDecoder.java
index db1b115..46c07f5 100644
--- a/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/binding/decoding/impl/HTTPArtifactDecoder.java
+++ b/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/binding/decoding/impl/HTTPArtifactDecoder.java
@@ -67,6 +67,7 @@ import net.shibboleth.utilities.java.support.annotation.constraint.NonnullAfterI
 import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
 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 net.shibboleth.utilities.java.support.component.ComponentInitializationException;
 import net.shibboleth.utilities.java.support.component.ComponentSupport;
 import net.shibboleth.utilities.java.support.primitive.StringSupport;
@@ -466,7 +467,7 @@ public class HTTPArtifactDecoder extends BaseHttpServletRequestXMLMessageDecoder
             }
             throw new MessageDecodingException("SOAP message payload was not an instance of ArtifactResponse: " 
                     + response.getClass().getName());
-        } catch (final MessageException | SOAPException | SecurityException e) {
+        } catch (final MessageException | SOAPException | SecurityException | EncodingException e) {
             throw new MessageDecodingException("Error dereferencing artifact", e);
         }
     }
@@ -506,10 +507,11 @@ public class HTTPArtifactDecoder extends BaseHttpServletRequestXMLMessageDecoder
      * @param peerRoleDescriptor the peer RoleDescriptor
      * @param selfEntityID the entityID of this party, the issuer of the protocol request message
      * @return the SAML protocol message for artifact resolution
+     * @throws EncodingException if the artifact can not be base64 encoded.
      */
     @Nonnull private ArtifactResolve buildArtifactResolveRequestMessage(@Nonnull final SAML2Artifact artifact, 
             @Nonnull final String endpoint, @Nonnull final RoleDescriptor peerRoleDescriptor, 
-            @Nonnull final String selfEntityID) {
+            @Nonnull final String selfEntityID) throws EncodingException  {
         
         final ArtifactResolve request = 
                 (ArtifactResolve) XMLObjectSupport.buildXMLObject(ArtifactResolve.DEFAULT_ELEMENT_NAME);
diff --git a/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/binding/encoding/impl/HTTPArtifactEncoder.java b/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/binding/encoding/impl/HTTPArtifactEncoder.java
index 703db41..c37681e 100644
--- a/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/binding/encoding/impl/HTTPArtifactEncoder.java
+++ b/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/binding/encoding/impl/HTTPArtifactEncoder.java
@@ -29,6 +29,7 @@ import javax.servlet.http.HttpServletResponse;
 import net.shibboleth.utilities.java.support.annotation.constraint.NonnullAfterInit;
 import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
 import net.shibboleth.utilities.java.support.codec.Base64Support;
+import net.shibboleth.utilities.java.support.codec.EncodingException;
 import net.shibboleth.utilities.java.support.codec.HTMLEncoder;
 import net.shibboleth.utilities.java.support.collection.Pair;
 import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
@@ -239,8 +240,13 @@ public class HTTPArtifactEncoder extends BaseSAML2MessageEncoder {
         final String encodedEndpointURL = HTMLEncoder.encodeForHTMLAttribute(endpointURL);
         log.debug("Setting action parameter to: '{}', encoded as '{}'", endpointURL, encodedEndpointURL);
         context.put("action", encodedEndpointURL);
-        context.put("SAMLArt", 
-                Base64Support.encode(buildArtifact(messageContext).getArtifactBytes(), Base64Support.UNCHUNKED));
+        try {
+            context.put("SAMLArt", 
+                    Base64Support.encode(buildArtifact(messageContext).getArtifactBytes(), Base64Support.UNCHUNKED));
+        } catch (final EncodingException e) {
+            log.warn("Unable to base64 encode SAML 2 artifact when creating POST form: {}",e.getMessage());
+            throw new MessageEncodingException("Unable to base64 encode SAML 2 artifact when creating POST form",e);
+        }
         context.put("binding", getBindingURI());
 
         final String relayState = SAMLBindingSupport.getRelayState(messageContext);
@@ -289,8 +295,13 @@ public class HTTPArtifactEncoder extends BaseSAML2MessageEncoder {
             log.error("Unable to build artifact for message to relying party");
             throw new MessageEncodingException("Unable to build artifact for message to relying party");
         }
-        queryParams.add(new Pair<>("SAMLart", 
-                Base64Support.encode(artifact.getArtifactBytes(), Base64Support.UNCHUNKED)));
+        try {
+            queryParams.add(new Pair<>("SAMLart", 
+                    Base64Support.encode(artifact.getArtifactBytes(), Base64Support.UNCHUNKED)));
+        } catch (final EncodingException e) {
+            log.error("Unable to base64 encode artifact for message to relying party: {}",e.getMessage());
+            throw new MessageEncodingException("Unable to base64 encode artifact for message to relying party",e);
+        }
 
         final String relayState = SAMLBindingSupport.getRelayState(messageContext);
         if (SAMLBindingSupport.checkRelayState(relayState)) {
@@ -344,13 +355,16 @@ public class HTTPArtifactEncoder extends BaseSAML2MessageEncoder {
         if (artifact == null) {
             log.error("Unable to build artifact for message to relying party");
             throw new MessageEncodingException("Unable to build artifact for message to relying party");
-        }
-        final String encodedArtifact = Base64Support.encode(artifact.getArtifactBytes(), Base64Support.UNCHUNKED);
+        }        
         try {
+            final String encodedArtifact = Base64Support.encode(artifact.getArtifactBytes(), Base64Support.UNCHUNKED);
             artifactMap.put(encodedArtifact, requester, issuer, samlObject);
         } catch (final IOException e) {
             log.error("Unable to store message mapping for artifact: {}", e.getMessage());
             throw new MessageEncodingException("Unable to store message mapping for artifact", e);
+        } catch (final EncodingException e) {
+            log.error("Unable to base64 encode artifact: {}", e.getMessage());
+            throw new MessageEncodingException("Unable to base64 encode artifact", e);
         }
 
         return artifact;
diff --git a/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/binding/encoding/impl/HTTPPostEncoder.java b/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/binding/encoding/impl/HTTPPostEncoder.java
index ef3e082..07b65af 100644
--- a/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/binding/encoding/impl/HTTPPostEncoder.java
+++ b/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/binding/encoding/impl/HTTPPostEncoder.java
@@ -24,6 +24,7 @@ import java.io.Writer;
 import javax.servlet.http.HttpServletResponse;
 
 import net.shibboleth.utilities.java.support.codec.Base64Support;
+import net.shibboleth.utilities.java.support.codec.EncodingException;
 import net.shibboleth.utilities.java.support.codec.HTMLEncoder;
 import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
 import net.shibboleth.utilities.java.support.component.ComponentSupport;
@@ -212,6 +213,9 @@ public class HTTPPostEncoder extends BaseSAML2MessageEncoder {
         } catch (final UnsupportedEncodingException e) {
             log.error("UTF-8 encoding is not supported, this VM is not Java compliant");
             throw new MessageEncodingException("Unable to encode message, UTF-8 encoding is not supported");
+        } catch (final EncodingException e) {
+            log.error("Unable to base64 encode SAML message: {}",e.getMessage());
+            throw new MessageEncodingException("Unable to base64 encode SAML message",e);
         }
 
         final String relayState = SAMLBindingSupport.getRelayState(messageContext);
diff --git a/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/binding/encoding/impl/HTTPPostSimpleSignEncoder.java b/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/binding/encoding/impl/HTTPPostSimpleSignEncoder.java
index ef6f65c..7c8f7b3 100644
--- a/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/binding/encoding/impl/HTTPPostSimpleSignEncoder.java
+++ b/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/binding/encoding/impl/HTTPPostSimpleSignEncoder.java
@@ -21,6 +21,7 @@ import java.io.UnsupportedEncodingException;
 
 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 net.shibboleth.utilities.java.support.xml.SerializeSupport;
 
 import org.apache.velocity.VelocityContext;
@@ -127,6 +128,9 @@ public class HTTPPostSimpleSignEncoder extends HTTPPostEncoder {
         } catch (final MarshallingException e) {
             log.error("Error marshalling KeyInfo based on signing credential: {}", e.getMessage());
             throw new MessageEncodingException("Error marshalling KeyInfo based on signing credential", e);
+        } catch (final EncodingException e) {
+            log.error("Error base64 encoding KeyInfo from signing credential: {}", e.getMessage());
+            throw new MessageEncodingException("Error base64 encoding KeyInfo from signing credential", e);
         }
     }
 
@@ -237,6 +241,9 @@ public class HTTPPostSimpleSignEncoder extends HTTPPostEncoder {
         } catch (final UnsupportedEncodingException e) {
             log.error("UTF-8 encoding is not supported, this VM is not Java compliant");
             throw new MessageEncodingException("Unable to encode message, UTF-8 encoding is not supported");
+        } catch (final EncodingException e) {
+            log.error("Error base64 encoding signature of form control data: {}",e.getMessage());
+            throw new MessageEncodingException("Unable to base64 encode signature of form control data",e);
         }
 
         return b64Signature;
diff --git a/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/binding/encoding/impl/HTTPRedirectDeflateEncoder.java b/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/binding/encoding/impl/HTTPRedirectDeflateEncoder.java
index e791f39..263bc85 100644
--- a/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/binding/encoding/impl/HTTPRedirectDeflateEncoder.java
+++ b/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/binding/encoding/impl/HTTPRedirectDeflateEncoder.java
@@ -33,6 +33,7 @@ import javax.annotation.Nonnull;
 import javax.servlet.http.HttpServletResponse;
 
 import net.shibboleth.utilities.java.support.codec.Base64Support;
+import net.shibboleth.utilities.java.support.codec.EncodingException;
 import net.shibboleth.utilities.java.support.collection.Pair;
 import net.shibboleth.utilities.java.support.net.HttpServletSupport;
 import net.shibboleth.utilities.java.support.net.URLBuilder;
@@ -147,10 +148,10 @@ public class HTTPRedirectDeflateEncoder extends BaseSAML2MessageEncoder {
                 deflaterStream.finish();
 
                 return Base64Support.encode(bytesOut.toByteArray(), Base64Support.UNCHUNKED);
-            }
-        } catch (final IOException e) {
+            }            
+        } catch (final IOException | EncodingException e) {
             throw new MessageEncodingException("Unable to DEFLATE and Base64 encode SAML message", e);
-        }
+        } 
     }
 
     /**
@@ -292,6 +293,9 @@ public class HTTPRedirectDeflateEncoder extends BaseSAML2MessageEncoder {
             throw new MessageEncodingException("Unable to sign URL query string", e);
         } catch (final UnsupportedEncodingException e) {
             // UTF-8 encoding is required to be supported by all JVMs
+        } catch (final EncodingException e) {
+            log.error("Error during URL signing process: {}", e.getMessage());
+            throw new MessageEncodingException("Unable to base64 encode signature of URL query string", e);
         }
 
         return b64Signature;
diff --git a/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/binding/impl/AddGeneratedKeyHeaderHandler.java b/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/binding/impl/AddGeneratedKeyHeaderHandler.java
index dfce760..1e85681 100644
--- a/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/binding/impl/AddGeneratedKeyHeaderHandler.java
+++ b/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/binding/impl/AddGeneratedKeyHeaderHandler.java
@@ -59,13 +59,11 @@ public class AddGeneratedKeyHeaderHandler extends AbstractMessageHandler {
         final SAMLObjectBuilder<GeneratedKey> builder = (SAMLObjectBuilder<GeneratedKey>)
                 XMLObjectProviderRegistrySupport.getBuilderFactory().<GeneratedKey>getBuilderOrThrow(
                         GeneratedKey.DEFAULT_ELEMENT_NAME);
-        
-        final GeneratedKey header = builder.buildObject();
-        header.setValue(Base64Support.encode(messageContext.getSubcontext(ECPContext.class).getSessionKey(), false));
-        
-        SOAPSupport.addSOAP11ActorAttribute(header, ActorBearing.SOAP11_ACTOR_NEXT);
-        
         try {
+            final GeneratedKey header = builder.buildObject();
+            header.setValue(Base64Support.encode(messageContext.getSubcontext(ECPContext.class).getSessionKey(),
+                    false));
+            SOAPSupport.addSOAP11ActorAttribute(header, ActorBearing.SOAP11_ACTOR_NEXT);      
             SOAPMessagingSupport.addHeaderBlock(messageContext, header);
         } catch (final Exception e) {
             throw new MessageHandlerException(e);
diff --git a/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/profile/impl/AddGeneratedKeyToAssertions.java b/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/profile/impl/AddGeneratedKeyToAssertions.java
index f9d9eed..d96e3f2 100644
--- a/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/profile/impl/AddGeneratedKeyToAssertions.java
+++ b/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/profile/impl/AddGeneratedKeyToAssertions.java
@@ -23,6 +23,7 @@ import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
 import net.shibboleth.utilities.java.support.codec.Base64Support;
+import net.shibboleth.utilities.java.support.codec.EncodingException;
 import net.shibboleth.utilities.java.support.component.ComponentSupport;
 import net.shibboleth.utilities.java.support.logic.Constraint;
 
@@ -140,16 +141,21 @@ public class AddGeneratedKeyToAssertions extends AbstractConditionalProfileActio
                 XMLObjectProviderRegistrySupport.getBuilderFactory().<GeneratedKey>getBuilderOrThrow(
                         GeneratedKey.DEFAULT_ELEMENT_NAME);
 
-        final String key = Base64Support.encode(ecpContext.getSessionKey(), false);
-        
-        for (final Assertion assertion : response.getAssertions()) {
-            final Advice advice = SAML2ActionSupport.addAdviceToAssertion(this, assertion);
-            final GeneratedKey gk = keyBuilder.buildObject();
-            gk.setValue(key);
-            advice.getChildren().add(gk);
+        try {
+            final String key = Base64Support.encode(ecpContext.getSessionKey(), false);
+            
+            for (final Assertion assertion : response.getAssertions()) {
+                final Advice advice = SAML2ActionSupport.addAdviceToAssertion(this, assertion);
+                final GeneratedKey gk = keyBuilder.buildObject();
+                gk.setValue(key);
+                advice.getChildren().add(gk);
+            }        
+            log.debug("{} Added GeneratedKey to Advice", getLogPrefix());
+        } catch(final EncodingException e) {
+            //should never happen
+            log.error("{} Error, could not add GeneratedKey to Advice", getLogPrefix(),e);
+            ActionSupport.buildEvent(profileRequestContext, EventIds.MESSAGE_PROC_ERROR);
         }
-        
-        log.debug("{} Added GeneratedKey to Advice", getLogPrefix());
     }
 
 }
\ No newline at end of file
diff --git a/opensaml-saml-impl/src/test/java/org/opensaml/saml/saml2/binding/decoding/impl/HTTPRedirectDeflateDecoderTest.java b/opensaml-saml-impl/src/test/java/org/opensaml/saml/saml2/binding/decoding/impl/HTTPRedirectDeflateDecoderTest.java
index 2decd20..13b7be0 100644
--- a/opensaml-saml-impl/src/test/java/org/opensaml/saml/saml2/binding/decoding/impl/HTTPRedirectDeflateDecoderTest.java
+++ b/opensaml-saml-impl/src/test/java/org/opensaml/saml/saml2/binding/decoding/impl/HTTPRedirectDeflateDecoderTest.java
@@ -40,6 +40,7 @@ import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
 
 import net.shibboleth.utilities.java.support.codec.Base64Support;
+import net.shibboleth.utilities.java.support.codec.EncodingException;
 import net.shibboleth.utilities.java.support.xml.SerializeSupport;
 
 /**
@@ -86,7 +87,8 @@ public class HTTPRedirectDeflateDecoderTest extends XMLObjectBaseTestCase {
     }    
    
     @Test
-    public void testRequestDecoding() throws MessageDecodingException, MessageEncodingException, MarshallingException {
+    public void testRequestDecoding() throws MessageDecodingException, MessageEncodingException, 
+                                                            MarshallingException, EncodingException {
         AuthnRequest samlRequest =
                 (AuthnRequest) unmarshallElement("/org/opensaml/saml/saml2/binding/AuthnRequest.xml");
         samlRequest.setDestination(null);
@@ -113,7 +115,7 @@ public class HTTPRedirectDeflateDecoderTest extends XMLObjectBaseTestCase {
 
     @Test
     public void testExplicitDefaultSAMLEncoding() 
-            throws MessageDecodingException, MessageEncodingException, MarshallingException {
+            throws MessageDecodingException, MessageEncodingException, MarshallingException, EncodingException {
         AuthnRequest samlRequest =
                 (AuthnRequest) unmarshallElement("/org/opensaml/saml/saml2/binding/AuthnRequest.xml");
         samlRequest.setDestination(null);
@@ -130,7 +132,7 @@ public class HTTPRedirectDeflateDecoderTest extends XMLObjectBaseTestCase {
 
     @Test(expectedExceptions=MessageDecodingException.class)
     public void testUnsupportedSAMLEncoding() 
-            throws MessageDecodingException, MessageEncodingException, MarshallingException {
+            throws MessageDecodingException, MessageEncodingException, MarshallingException, EncodingException {
         AuthnRequest samlRequest =
                 (AuthnRequest) unmarshallElement("/org/opensaml/saml/saml2/binding/AuthnRequest.xml");
         samlRequest.setDestination(null);
@@ -163,7 +165,7 @@ public class HTTPRedirectDeflateDecoderTest extends XMLObjectBaseTestCase {
         request.setQueryString(url.getQuery());
     }
 
-    protected String encodeMessage(SAMLObject message) throws MessageEncodingException, MarshallingException {
+    protected String encodeMessage(SAMLObject message) throws MessageEncodingException, MarshallingException, EncodingException {
         try {
             marshallerFactory.getMarshaller(message).marshall(message);
             String messageStr = SerializeSupport.nodeToString(message.getDOM());
diff --git a/opensaml-saml-impl/src/test/java/org/opensaml/saml/saml2/binding/impl/AddGeneratedKeyHeaderHandlerTest.java b/opensaml-saml-impl/src/test/java/org/opensaml/saml/saml2/binding/impl/AddGeneratedKeyHeaderHandlerTest.java
index 8fafa2b..7f56c38 100644
--- a/opensaml-saml-impl/src/test/java/org/opensaml/saml/saml2/binding/impl/AddGeneratedKeyHeaderHandlerTest.java
+++ b/opensaml-saml-impl/src/test/java/org/opensaml/saml/saml2/binding/impl/AddGeneratedKeyHeaderHandlerTest.java
@@ -22,6 +22,7 @@ import java.security.SecureRandom;
 import java.util.List;
 
 import net.shibboleth.utilities.java.support.codec.Base64Support;
+import net.shibboleth.utilities.java.support.codec.EncodingException;
 import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
 
 import org.opensaml.core.OpenSAMLInitBaseTestCase;
@@ -93,8 +94,9 @@ public class AddGeneratedKeyHeaderHandlerTest extends OpenSAMLInitBaseTestCase {
      * 
      * @throws MessageHandlerException ...
      * @throws NoSuchAlgorithmException ...
+     * @throws EncodingException on failure to base64 encode the key.
      */
-    @Test public void testSuccess() throws MessageHandlerException, NoSuchAlgorithmException {
+    @Test public void testSuccess() throws MessageHandlerException, NoSuchAlgorithmException, EncodingException {
 
         final byte[] key = new byte[32];
         SecureRandom.getInstance("SHA1prng").nextBytes(key);
diff --git a/opensaml-saml-impl/src/test/java/org/opensaml/saml/saml2/profile/impl/AddGeneratedKeyToAssertionsTest.java b/opensaml-saml-impl/src/test/java/org/opensaml/saml/saml2/profile/impl/AddGeneratedKeyToAssertionsTest.java
index 7f689bb..33af774 100644
--- a/opensaml-saml-impl/src/test/java/org/opensaml/saml/saml2/profile/impl/AddGeneratedKeyToAssertionsTest.java
+++ b/opensaml-saml-impl/src/test/java/org/opensaml/saml/saml2/profile/impl/AddGeneratedKeyToAssertionsTest.java
@@ -18,6 +18,7 @@
 package org.opensaml.saml.saml2.profile.impl;
 
 import net.shibboleth.utilities.java.support.codec.Base64Support;
+import net.shibboleth.utilities.java.support.codec.EncodingException;
 import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
 
 import org.opensaml.core.OpenSAMLInitBaseTestCase;
@@ -85,9 +86,10 @@ public class AddGeneratedKeyToAssertionsTest  extends OpenSAMLInitBaseTestCase {
     /**
      * Test that the advice is properly added if there is a single assertion, without an Advice element, in the
      * response.
+     * @throws EncodingException if the ecp session key can not be base64 encoded.
      */
     @Test
-    public void testSingleAssertion() {
+    public void testSingleAssertion() throws EncodingException {
         final Assertion assertion = SAML2ActionTestingSupport.buildAssertion();
 
         final Response response = SAML2ActionTestingSupport.buildResponse();
@@ -112,9 +114,10 @@ public class AddGeneratedKeyToAssertionsTest  extends OpenSAMLInitBaseTestCase {
     /**
      * Test that the advice is properly added if there is a single assertion, with an Advice element, in the
      * response.
+     * @throws EncodingException if the ecp session key can not be base64 encoded.
      */
     @Test
-    public void testSingleAssertionWithExistingCondition() {
+    public void testSingleAssertionWithExistingCondition() throws EncodingException {
 
         final Assertion assertion = SAML2ActionTestingSupport.buildAssertion();
         SAML2ActionSupport.addAdviceToAssertion(action, assertion);
@@ -135,9 +138,10 @@ public class AddGeneratedKeyToAssertionsTest  extends OpenSAMLInitBaseTestCase {
         Assert.assertEquals(key.getValue(), Base64Support.encode(ecp.getSessionKey(), false));
     }
 
-    /** Test that the advice is properly added if there are multiple assertions in the response. */
+    /** Test that the advice is properly added if there are multiple assertions in the response. 
+     * @throws EncodingException if the ecp session key can not be base64 encoded.*/
     @Test
-    public void testMultipleAssertion() {
+    public void testMultipleAssertion() throws EncodingException {
         final Response response = SAML2ActionTestingSupport.buildResponse();
         response.getAssertions().add(SAML2ActionTestingSupport.buildAssertion());
         response.getAssertions().add(SAML2ActionTestingSupport.buildAssertion());
diff --git a/opensaml-security-impl/src/main/java/org/opensaml/security/messaging/impl/BaseClientCertAuthSecurityHandler.java b/opensaml-security-impl/src/main/java/org/opensaml/security/messaging/impl/BaseClientCertAuthSecurityHandler.java
index 584e652..26fd070 100644
--- a/opensaml-security-impl/src/main/java/org/opensaml/security/messaging/impl/BaseClientCertAuthSecurityHandler.java
+++ b/opensaml-security-impl/src/main/java/org/opensaml/security/messaging/impl/BaseClientCertAuthSecurityHandler.java
@@ -29,6 +29,7 @@ import javax.servlet.http.HttpServletRequest;
 import net.shibboleth.utilities.java.support.annotation.constraint.NonnullAfterInit;
 import net.shibboleth.utilities.java.support.annotation.constraint.NonnullElements;
 import net.shibboleth.utilities.java.support.codec.Base64Support;
+import net.shibboleth.utilities.java.support.codec.EncodingException;
 import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
 import net.shibboleth.utilities.java.support.component.ComponentSupport;
 import net.shibboleth.utilities.java.support.logic.Constraint;
@@ -193,8 +194,9 @@ public abstract class BaseClientCertAuthSecurityHandler extends BaseTrustEngineS
                         getLogPrefix());
                 log.debug(Base64Support.encode(requestCredential.getEntityCertificate().getEncoded(),
                         Base64Support.UNCHUNKED));
-            } catch (final CertificateEncodingException e) {
-                // do nothing
+            } catch (final CertificateEncodingException | EncodingException e) {
+                // do nothing other than complete debug log
+                log.debug("{} Could not encode certificate for debug output",getLogPrefix());
             }
         }
         
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 63fcddf..49ee6d2 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
@@ -46,12 +46,14 @@ import javax.annotation.Nullable;
 
 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 net.shibboleth.utilities.java.support.logic.Constraint;
 
 import org.apache.xml.security.utils.XMLUtils;
 import org.opensaml.core.xml.XMLObjectBuilder;
 import org.opensaml.core.xml.XMLObjectBuilderFactory;
 import org.opensaml.core.xml.config.XMLObjectProviderRegistrySupport;
+import org.opensaml.security.SecurityException;
 import org.opensaml.security.credential.Credential;
 import org.opensaml.security.x509.X509Support;
 import org.opensaml.xmlsec.algorithm.AlgorithmSupport;
@@ -351,14 +353,20 @@ public class KeyInfoSupport {
             throws CertificateEncodingException {
         Constraint.isNotNull(cert, "X.509 certificate cannot be null");
         
+       
         final XMLObjectBuilder<org.opensaml.xmlsec.signature.X509Certificate> xmlCertBuilder =
                     XMLObjectProviderRegistrySupport.getBuilderFactory().getBuilderOrThrow(
                             org.opensaml.xmlsec.signature.X509Certificate.DEFAULT_ELEMENT_NAME);
         final org.opensaml.xmlsec.signature.X509Certificate xmlCert =
                 xmlCertBuilder.buildObject(org.opensaml.xmlsec.signature.X509Certificate.DEFAULT_ELEMENT_NAME);
-        xmlCert.setValue(Base64Support.encode(cert.getEncoded(), Base64Support.CHUNKED));
-
-        return xmlCert;
+        
+        try {
+            xmlCert.setValue(Base64Support.encode(cert.getEncoded(), Base64Support.CHUNKED));  
+            return xmlCert;
+        } catch (final EncodingException e) {
+            throw new CertificateEncodingException("X.509 certificate could not be base64 encoded");
+        }
+        
     }
 
     /**
@@ -378,9 +386,13 @@ public class KeyInfoSupport {
                             org.opensaml.xmlsec.signature.X509CRL.DEFAULT_ELEMENT_NAME);
         final org.opensaml.xmlsec.signature.X509CRL xmlCRL =
                 xmlCRLBuilder.buildObject(org.opensaml.xmlsec.signature.X509CRL.DEFAULT_ELEMENT_NAME);
-        xmlCRL.setValue(Base64Support.encode(crl.getEncoded(), Base64Support.CHUNKED));
-
-        return xmlCRL;
+        
+        try {
+            xmlCRL.setValue(Base64Support.encode(crl.getEncoded(), Base64Support.CHUNKED));
+            return xmlCRL;
+        } catch (final EncodingException e) {
+            throw new CRLException("X.509CRL could not be base64 encoded");
+        } 
     }
 
     /**
@@ -434,10 +446,15 @@ public class KeyInfoSupport {
      * Build an {@link X509SKI} containing the subject key identifier extension value contained within a certificate.
      * 
      * @param javaCert the Java X509Certificate from which to extract the subject key identifier value.
-     * @return a new X509SKI object, or null if the certificate did not contain the subject key identifier extension
+     * @return a new X509SKI object, or null if the certificate did not contain the subject key identifier extension, 
+     *         or the subject key identifier binary can not be base64-encoded.
+     * @throws SecurityException if there is a problem building the subject key identifier. 
      */
-    @Nullable public static X509SKI buildX509SKI(@Nonnull final X509Certificate javaCert) {
+    @Nullable public static X509SKI buildX509SKI(@Nonnull final X509Certificate javaCert) throws SecurityException {
         final byte[] skiPlainValue = X509Support.getSubjectKeyIdentifier(javaCert);
+        
+        final Logger log = getLogger();
+        
         if (skiPlainValue == null || skiPlainValue.length == 0) {
             return null;
         }
@@ -445,9 +462,15 @@ public class KeyInfoSupport {
         final XMLObjectBuilder<X509SKI> xmlSKIBuilder =
                 XMLObjectProviderRegistrySupport.getBuilderFactory().getBuilderOrThrow(X509SKI.DEFAULT_ELEMENT_NAME);
         final X509SKI xmlSKI = xmlSKIBuilder.buildObject(X509SKI.DEFAULT_ELEMENT_NAME);
-        xmlSKI.setValue(Base64Support.encode(skiPlainValue, Base64Support.CHUNKED));
-
-        return xmlSKI;
+        
+        
+        try {
+            xmlSKI.setValue(Base64Support.encode(skiPlainValue, Base64Support.CHUNKED));
+            return xmlSKI;
+        } catch (final EncodingException e) {
+            log.warn("X.509 subject key identifier could not be base64 encoded",e);
+            throw new SecurityException("X.509 subject key identifier could not be base64 encoded",e);
+        }        
     }
 
     /**
@@ -474,9 +497,13 @@ public class KeyInfoSupport {
                 XMLObjectProviderRegistrySupport.getBuilderFactory().getBuilderOrThrow(X509Digest.DEFAULT_ELEMENT_NAME);
         final X509Digest xmlDigest = builder.buildObject(X509Digest.DEFAULT_ELEMENT_NAME);
         xmlDigest.setAlgorithm(algorithmURI);
-        xmlDigest.setValue(Base64Support.encode(hash, Base64Support.CHUNKED));
         
-        return xmlDigest;
+        try {
+            xmlDigest.setValue(Base64Support.encode(hash, Base64Support.CHUNKED));
+            return xmlDigest;
+        } catch (final EncodingException e) {
+            throw new CertificateEncodingException("X509Digest could not be base64 encoded");
+        }
     }    
     
     /**
@@ -601,7 +628,12 @@ public class KeyInfoSupport {
         
         final KeyFactory keyFactory = KeyFactory.getInstance(pk.getAlgorithm());
         final X509EncodedKeySpec keySpec = keyFactory.getKeySpec(pk, X509EncodedKeySpec.class);
-        keyValue.setValue(Base64Support.encode(keySpec.getEncoded(), Base64Support.CHUNKED));
+        
+        try {
+            keyValue.setValue(Base64Support.encode(keySpec.getEncoded(), Base64Support.CHUNKED));
+        } catch (final EncodingException e) {
+           throw new InvalidKeySpecException("X509 Key spec could not be base64 encoded",e);
+        }
         
         keyInfo.getDEREncodedKeyValues().add(keyValue);
     }
@@ -765,8 +797,10 @@ public class KeyInfoSupport {
      * 
      * @param bigInt the BigInteger value
      * @return the encoded CryptoBinary value
+     * @throws EncodingException if the BigInteger as bytes can not be base64 encoded.
      */
-    @Nonnull public static final String encodeCryptoBinaryFromBigInteger(@Nonnull final BigInteger bigInt) {
+    @Nonnull public static final String encodeCryptoBinaryFromBigInteger(@Nonnull final BigInteger bigInt) 
+            throws EncodingException {
         Constraint.isNotNull(bigInt, "BigInteger cannot be null");
         
         // This code is really complicated, for now just use the Apache xmlsec lib code directly.
diff --git a/opensaml-xmlsec-impl/src/main/java/org/opensaml/xmlsec/keyinfo/impl/X509KeyInfoGeneratorFactory.java b/opensaml-xmlsec-impl/src/main/java/org/opensaml/xmlsec/keyinfo/impl/X509KeyInfoGeneratorFactory.java
index bb5eb60..fa2d748 100644
--- a/opensaml-xmlsec-impl/src/main/java/org/opensaml/xmlsec/keyinfo/impl/X509KeyInfoGeneratorFactory.java
+++ b/opensaml-xmlsec-impl/src/main/java/org/opensaml/xmlsec/keyinfo/impl/X509KeyInfoGeneratorFactory.java
@@ -551,9 +551,10 @@ public class X509KeyInfoGeneratorFactory extends BasicKeyInfoGeneratorFactory {
          * 
          * @param x509Data the X509Data element being processed.
          * @param cert the certificate being processed
+         * @throws SecurityException  if there is an error in generating the subject key identifier
          */ 
         protected void processCertX509SKI(@Nonnull final X509Data x509Data,
-                @Nonnull final java.security.cert.X509Certificate cert) {
+                @Nonnull final java.security.cert.X509Certificate cert) throws SecurityException {
             if (options.emitX509SKI) {
                 final X509SKI xmlSKI = KeyInfoSupport.buildX509SKI(cert);
                 if (xmlSKI != null) {
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 15a0981..f836830 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
@@ -27,6 +27,7 @@ import org.opensaml.xmlsec.signature.CryptoBinary;
 import com.google.common.base.Strings;
 
 import net.shibboleth.utilities.java.support.codec.DecodingException;
+import net.shibboleth.utilities.java.support.codec.EncodingException;
 
 /**
  * Concrete implementation of {@link org.opensaml.xmlsec.signature.CryptoBinary}.
@@ -66,7 +67,19 @@ public class CryptoBinaryImpl extends XSBase64BinaryImpl implements CryptoBinary
         if (bigInt == null) {
             setValue(null);
         } else {
-            setValue(KeyInfoSupport.encodeCryptoBinaryFromBigInteger(bigInt));
+            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;
+            }
         }
         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 f03686f..37308ce 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
@@ -40,6 +40,7 @@ import net.shibboleth.utilities.java.support.codec.Base64Support;
 import net.shibboleth.utilities.java.support.codec.DecodingException;
 
 import org.opensaml.core.xml.XMLObjectBaseTestCase;
+import org.opensaml.security.SecurityException;
 import org.opensaml.security.crypto.KeySupport;
 import org.opensaml.security.x509.X509Support;
 import org.opensaml.xmlsec.keyinfo.KeyInfoSupport;
@@ -684,9 +685,10 @@ public class KeyInfoSupportTest extends XMLObjectBaseTestCase {
      * 
      * @throws CertificateException ...
      * @throws DecodingException if an issue base64-decoding SKI values
+     * @throws SecurityException if an issue building X509SKI.
      */
     @Test
-    public void testBuildSubjectKeyIdentifier() throws CertificateException, DecodingException {
+    public void testBuildSubjectKeyIdentifier() throws CertificateException, DecodingException, SecurityException {
         byte[] skiValue = Base64Support.decode(cert1SKIPlainBase64);
         X509SKI xmlSKI = KeyInfoSupport.buildX509SKI(javaCert1);
         Assert.assertNotNull(xmlSKI, "Constructed X509SKI was null");

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


More information about the commits mailing list