[java-metadata-aggregator] branch master updated: MDA-224 - array bounds error if no certificates provided

Ian Young ian at iay.org.uk
Fri Sep 27 10:52:01 EDT 2019


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

iay pushed a commit to branch master
in repository java-metadata-aggregator.

View the commit online:
http://git.shibboleth.net/view/?p=java-metadata-aggregator.git;a=commit;h=76edc2fdfc6f29ecfba5057f7a86a8e5b2fc104f

The following commit(s) were added to refs/heads/master by this push:
       new  76edc2f   MDA-224 - array bounds error if no certificates provided
76edc2f is described below

commit 76edc2fdfc6f29ecfba5057f7a86a8e5b2fc104f
Author: Ian Young <ian at iay.org.uk>
AuthorDate: Fri Sep 27 15:05:58 2019 +0100

    MDA-224 - array bounds error if no certificates provided
    
    https://issues.shibboleth.net/jira/browse/MDA-224
---
 .../metadata/dom/XMLSignatureSigningStage.java     | 29 +++++----
 .../metadata/dom/XMLSignatureSigningStageTest.java | 70 ++++++++++++++++++++++
 2 files changed, 88 insertions(+), 11 deletions(-)

diff --git a/aggregator-pipeline/src/main/java/net/shibboleth/metadata/dom/XMLSignatureSigningStage.java b/aggregator-pipeline/src/main/java/net/shibboleth/metadata/dom/XMLSignatureSigningStage.java
index b34f793..bd4976f 100644
--- a/aggregator-pipeline/src/main/java/net/shibboleth/metadata/dom/XMLSignatureSigningStage.java
+++ b/aggregator-pipeline/src/main/java/net/shibboleth/metadata/dom/XMLSignatureSigningStage.java
@@ -168,7 +168,7 @@ public class XMLSignatureSigningStage extends AbstractIteratingStage<Element> {
     private PrivateKey privKey;
 
     /** Public key associated with the given private key. */
-    private PublicKey pubKey;
+    private PublicKey publicKey;
 
     /**
      * Certificate chain, with end entity certificate as element 0, to be included with the signature. Default value:
@@ -215,7 +215,14 @@ public class XMLSignatureSigningStage extends AbstractIteratingStage<Element> {
     /** Whether key names should be included in the signature's KeyInfo. Default value: <code>true</code> */
     private boolean includeKeyNames;
 
-    /** Whether the public key should be included in the signature's KeyInfo. Default value: <code>false</code> */
+    /**
+     * Whether the public key should be included in the signature's KeyInfo.
+     * 
+     * The public key can be sourced from either the {@link #publicKey} property or from the
+     * first provided certificate.
+     * 
+     * Default value: <code>false</code>
+     */
     private boolean includeKeyValue;
 
     /**
@@ -304,7 +311,7 @@ public class XMLSignatureSigningStage extends AbstractIteratingStage<Element> {
      * @return public key associated with private key used to sign the content
      */
     @Nullable public PublicKey getPublicKey() {
-        return pubKey;
+        return publicKey;
     }
 
     /**
@@ -316,7 +323,7 @@ public class XMLSignatureSigningStage extends AbstractIteratingStage<Element> {
         ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
         ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
 
-        pubKey = key;
+        publicKey = key;
     }
 
     /**
@@ -911,13 +918,13 @@ public class XMLSignatureSigningStage extends AbstractIteratingStage<Element> {
             return;
         }
 
-        PublicKey key = pubKey;
-        if (key == null && certificates != null) {
-            final X509Certificate cert = certificates.get(0);
-            if (cert != null) {
-                key = cert.getPublicKey();
-            }
+        PublicKey key = publicKey;
+        
+        // If we have no explicit public key, we can extract one from a certificate, if we have one.
+        if (key == null && !certificates.isEmpty()) {
+            key = certificates.get(0).getPublicKey();
         }
+
         if (key != null) {
             try {
                 keyInfoItems.add(keyInfoFactory.newKeyValue(key));
@@ -971,7 +978,7 @@ public class XMLSignatureSigningStage extends AbstractIteratingStage<Element> {
         xmlSigFactory = null;
         keyInfoFactory = null;
         privKey = null;
-        pubKey = null;
+        publicKey = null;
         certificates = null;
         crls = null;
         sigAlgo = null;
diff --git a/aggregator-pipeline/src/test/java/net/shibboleth/metadata/dom/XMLSignatureSigningStageTest.java b/aggregator-pipeline/src/test/java/net/shibboleth/metadata/dom/XMLSignatureSigningStageTest.java
index b0eaf1e..cda31a3 100644
--- a/aggregator-pipeline/src/test/java/net/shibboleth/metadata/dom/XMLSignatureSigningStageTest.java
+++ b/aggregator-pipeline/src/test/java/net/shibboleth/metadata/dom/XMLSignatureSigningStageTest.java
@@ -23,6 +23,8 @@ import java.util.ArrayList;
 import java.util.List;
 
 import javax.annotation.Nonnull;
+import javax.xml.crypto.dsig.XMLSignature;
+import javax.xml.namespace.QName;
 import javax.xml.transform.Source;
 
 import org.cryptacular.util.CertUtil;
@@ -39,6 +41,7 @@ import net.shibboleth.metadata.AssertSupport;
 import net.shibboleth.metadata.Item;
 import net.shibboleth.utilities.java.support.logic.ConstraintViolationException;
 import net.shibboleth.utilities.java.support.testing.TestSupport;
+import net.shibboleth.utilities.java.support.xml.ElementSupport;
 import net.shibboleth.utilities.java.support.xml.SerializeSupport;
 import net.shibboleth.utilities.java.support.xml.XMLParserException;
 
@@ -244,4 +247,71 @@ public class XMLSignatureSigningStageTest extends BaseDOMTest {
             Assert.assertFalse(diff.hasDifferences(), "results were different, expected same");
         }
     }
+    
+    @Test
+    public void mda224defaultingPublicKeyFromCertificate() throws Exception {
+        final var signingKey = KeyPairUtil.readPrivateKey(XMLSignatureSigningStageTest.class
+                .getResourceAsStream(classRelativeResource("signingKey.pem")));
+        final var md = getInput("input.xml");
+        final var stage = new XMLSignatureSigningStage();
+        
+        final var signingCert = (X509Certificate) CertUtil.readCertificate(XMLSignatureSigningStageTest.class
+                .getResourceAsStream(classRelativeResource("signingCert.pem")));
+        final var certs = List.of(signingCert);
+
+        stage.setId("test");
+        stage.setIncludeKeyValue(true);
+        stage.setPrivateKey(signingKey);
+        stage.setCertificates(certs);
+        stage.initialize();
+
+        stage.execute(md);
+        Assert.assertEquals(md.size(), 1);
+        
+        final var entitiesDescriptor = md.get(0).unwrap(); // document element
+        final var keyInfo = extractKeyInfo(entitiesDescriptor);
+        Assert.assertNotNull(keyInfo);
+
+        // If we had a certificate, expect to see that as an X509Data, and expect to see
+        // its public key as well as a KeyValue
+        Assert.assertTrue(hasChildNamed(keyInfo, new QName(XMLSignature.XMLNS, "X509Data")));
+        Assert.assertTrue(hasChildNamed(keyInfo, new QName(XMLSignature.XMLNS, "KeyValue")));
+    }
+
+    @Test
+    public void mda224defaultingPublicKeyFromAbsentCertificate() throws Exception {
+        final var signingKey = KeyPairUtil.readPrivateKey(XMLSignatureSigningStageTest.class
+                .getResourceAsStream(classRelativeResource("signingKey.pem")));
+        final var md = getInput("input.xml");
+        final var stage = new XMLSignatureSigningStage();
+        
+        stage.setId("test");
+        stage.setIncludeKeyValue(true);
+        stage.setPrivateKey(signingKey);
+        stage.initialize();
+
+        stage.execute(md);
+        Assert.assertEquals(md.size(), 1);
+        
+        final var entitiesDescriptor = md.get(0).unwrap(); // document element
+        final var keyInfo = extractKeyInfo(entitiesDescriptor);
+
+        // If we didn't have a public key or a certificate, we don't expect to see a KeyInfo at all.
+        Assert.assertNull(keyInfo);
+    }
+    
+    private boolean hasChildNamed(@Nonnull final Element element, @Nonnull final QName name) {
+        return !ElementSupport.getChildElements(element, name).isEmpty();
+    }
+
+    private Element extractKeyInfo(@Nonnull final Element root) {
+        final var signature = ElementSupport.getFirstChildElement(root,
+                new QName(XMLSignature.XMLNS, "Signature"));
+        final var keyInfos = signature.getElementsByTagNameNS(XMLSignature.XMLNS, "KeyInfo");
+        Assert.assertNotNull(keyInfos);
+        if (keyInfos.getLength() != 0) {
+            return (Element)keyInfos.item(0);            
+        }
+        return null;
+    }
 }

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


More information about the commits mailing list