[java-metadata-aggregator] 03/03: MDA-216 - remove CRs from signatures made under Java 11
Ian Young
ian at iay.org.uk
Tue Jan 29 09:51:21 EST 2019
This is an automated email from the git hooks/post-receive script.
iay pushed a commit to branch maint-0.9
in repository java-metadata-aggregator.
View the commit online:
http://git.shibboleth.net/view/?p=java-metadata-aggregator.git;a=commit;h=4f05c3e1fef72d84d69cd3062e6ab8e71fe52e56
commit 4f05c3e1fef72d84d69cd3062e6ab8e71fe52e56
Author: Ian Young <ian at iay.org.uk>
AuthorDate: Tue Jan 29 10:58:08 2019 +0000
MDA-216 - remove CRs from signatures made under Java 11
---
aggregator-parent/pom.xml | 8 ++
aggregator-pipeline/pom.xml | 6 +
.../metadata/dom/XMLSignatureSigningStage.java | 53 ++++++++-
.../metadata/dom/XMLSignatureSigningStageTest.java | 131 ++++++++++++++++++++-
4 files changed, 194 insertions(+), 4 deletions(-)
diff --git a/aggregator-parent/pom.xml b/aggregator-parent/pom.xml
index 656e19e..ec7ab9c 100644
--- a/aggregator-parent/pom.xml
+++ b/aggregator-parent/pom.xml
@@ -102,6 +102,14 @@
<type>test-jar</type>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>net.shibboleth.utilities</groupId>
+ <artifactId>java-support</artifactId>
+ <version>${java-support.version}</version>
+ <type>test-jar</type>
+ <scope>test</scope>
+ </dependency>
+
</dependencies>
</dependencyManagement>
diff --git a/aggregator-pipeline/pom.xml b/aggregator-pipeline/pom.xml
index ad726f6..a614d9c 100644
--- a/aggregator-pipeline/pom.xml
+++ b/aggregator-pipeline/pom.xml
@@ -61,6 +61,12 @@
<scope>test</scope>
</dependency>
<dependency>
+ <groupId>net.shibboleth.utilities</groupId>
+ <artifactId>java-support</artifactId>
+ <type>test-jar</type>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-all</artifactId>
<scope>test</scope>
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 1c0563c..feeabf1 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
@@ -62,6 +62,7 @@ import net.shibboleth.utilities.java.support.component.ComponentInitializationEx
import net.shibboleth.utilities.java.support.component.ComponentSupport;
import net.shibboleth.utilities.java.support.logic.Constraint;
import net.shibboleth.utilities.java.support.primitive.StringSupport;
+import net.shibboleth.utilities.java.support.xml.ElementSupport;
import net.shibboleth.utilities.java.support.xml.QNameSupport;
import org.apache.xml.security.Init;
@@ -70,6 +71,8 @@ import org.slf4j.LoggerFactory;
import org.w3c.dom.Attr;
import org.w3c.dom.Element;
import org.w3c.dom.NamedNodeMap;
+import org.w3c.dom.Node;
+import org.w3c.dom.NodeList;
import com.google.common.base.Predicates;
import com.google.common.collect.ImmutableList;
@@ -188,6 +191,9 @@ public class XMLSignatureSigningStage extends BaseIteratingStage<Element> {
/** Whether to include comments in the canonicalized data. Default value: <code>false</code> */
private boolean c14nWithComments;
+ /** Whether to remove CR characters from generated signatures. Default value: <code>true</code>. */
+ private boolean removingCRsFromSignature = true;
+
/**
* Canonicalization algorithm to use. This is determined from the {@link #c14nExclusive} and
* {@link #c14nWithComments} properties.
@@ -640,6 +646,44 @@ public class XMLSignatureSigningStage extends BaseIteratingStage<Element> {
return digestAlgo;
}
+ /**
+ * Gets whether CR characters will be removed from generated signatures.
+ *
+ * @return <code>true</code> if CR characters will be removed from generated signatures.
+ */
+ public boolean isRemovingCRsFromSignature() {
+ return removingCRsFromSignature;
+ }
+
+ /**
+ * Sets whether to remove CR characters from generated signatures.
+ *
+ * @param newValue whether to remove CR characters from generated signatures.
+ */
+ public void setRemovingCRsFromSignature(final boolean newValue) {
+ ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
+ ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+
+ removingCRsFromSignature = newValue;
+ }
+
+ /**
+ * Remove any CRs from the text content of named child elements.
+ *
+ * @param signature The <code>Signature</code> element to process.
+ * @param elementName The element name within the XML DSIG namespace to look for.
+ */
+ private void removeCRsFromNamedChildren(@Nonnull final Element signature, @Nonnull final String elementName) {
+ final NodeList nodes = signature.getElementsByTagNameNS(XML_SIG_NS_URI, elementName);
+ for (int i = 0; i < nodes.getLength(); i++) {
+ final Node node = nodes.item(i);
+ final String text = node.getTextContent();
+ if (text.indexOf('\r') >= 0) {
+ node.setTextContent(text.replaceAll("\\r", ""));
+ }
+ }
+ }
+
/** {@inheritDoc} */
@Override protected boolean doExecute(@Nonnull final Item<Element> item) throws StageProcessingException {
final Element element = item.unwrap();
@@ -655,6 +699,13 @@ public class XMLSignatureSigningStage extends BaseIteratingStage<Element> {
// Perform the signature operation
signature.sign(context);
+ // Remove any CRs from selected signature elements.
+ if (isRemovingCRsFromSignature()) {
+ final Element signatureElement = ElementSupport.getFirstChildElement(element, SIGNATURE_NAME);
+ removeCRsFromNamedChildren(signatureElement, "SignatureValue");
+ removeCRsFromNamedChildren(signatureElement, "X509Certificate");
+ }
+
// Log the pre-digest data for debugging
if (isDebugPreDigest() && log.isDebugEnabled()) {
final Reference ref = (Reference) signature.getSignedInfo().getReferences().get(0);
@@ -982,4 +1033,4 @@ public class XMLSignatureSigningStage extends BaseIteratingStage<Element> {
}
}
-}
\ No newline at end of file
+}
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 8c17ad9..d59b354 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
@@ -22,15 +22,25 @@ import java.security.cert.X509Certificate;
import java.util.ArrayList;
import java.util.List;
-import net.shibboleth.metadata.AssertSupport;
-import net.shibboleth.metadata.Item;
-import net.shibboleth.utilities.java.support.logic.ConstraintViolationException;
+import javax.annotation.Nonnull;
+import javax.xml.transform.Source;
import org.cryptacular.util.CertUtil;
import org.cryptacular.util.KeyPairUtil;
import org.testng.Assert;
import org.testng.annotations.Test;
import org.w3c.dom.Element;
+import org.xmlunit.builder.DiffBuilder;
+import org.xmlunit.builder.Input;
+import org.xmlunit.diff.Diff;
+import org.xmlunit.input.NormalizedSource;
+
+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.SerializeSupport;
+import net.shibboleth.utilities.java.support.xml.XMLParserException;
/** {@link XMLSignatureSigningStage} unit test. */
public class XMLSignatureSigningStageTest extends BaseDOMTest {
@@ -40,6 +50,34 @@ public class XMLSignatureSigningStageTest extends BaseDOMTest {
super(XMLSignatureSigningStage.class);
}
+ /**
+ * Utility to return an item collection made from a single named resource.
+ *
+ * @param fileName name of the resource
+ * @return collection made from the resource
+ * @throws XMLParserException if the resource can't be parsed
+ */
+ private List<Item<Element>> getInput(@Nonnull final String fileName) throws XMLParserException {
+ final Element testInput = readXMLData(fileName);
+ final List<Item<Element>> list = new ArrayList<>();
+ list.add(new DOMElementItem(testInput));
+ return list;
+ }
+
+ /**
+ * Returns whether an element or its children contain a literal CR.
+ *
+ * This operates by serializing the element and seeing whether the result
+ * contains an entity-encoded CR.
+ *
+ * @param element {@link Element} to examine
+ * @return <code>true</code> if the serialized element contains an encoded CR
+ */
+ private boolean containsCRs(@Nonnull final Element element) {
+ final String string = SerializeSupport.nodeToString(element);
+ return string.contains("
");
+ }
+
/** Test signing with and verifying the result against a known good. */
@Test
public void testSigning() throws Exception {
@@ -119,4 +157,91 @@ public class XMLSignatureSigningStageTest extends BaseDOMTest {
}
}
+ @Test
+ public void mda216Default() throws Exception {
+ final XMLSignatureSigningStage stage = new XMLSignatureSigningStage();
+ Assert.assertTrue(stage.isRemovingCRsFromSignature());
+ }
+
+ /**
+ * Test the functionality of the MDA-216 fix.
+ *
+ * This test is way over-specified. It <em>will</em> fail if the Santuario
+ * library ever stops putting CRs into its output, but it wil also fail
+ * if Santuario starts putting CRs into its output in other places.
+ *
+ * @throws Exception if something goes wrong
+ */
+ @Test
+ public void setRemovingCRsFromSignature() throws Exception {
+ PrivateKey signingKey = KeyPairUtil.readPrivateKey(XMLSignatureSigningStageTest.class
+ .getResourceAsStream(classRelativeResource("signingKey.pem")));
+ X509Certificate signingCert = (X509Certificate) CertUtil.readCertificate(XMLSignatureSigningStageTest.class
+ .getResourceAsStream(classRelativeResource("signingCert.pem")));
+ final List<X509Certificate> certs = new ArrayList<>();
+ certs.add(signingCert);
+
+ /*
+ * The first result uses the default value.
+ */
+ final List<Item<Element>> mdCol1 = getInput("input.xml");
+
+ final XMLSignatureSigningStage stage1 = new XMLSignatureSigningStage();
+ stage1.setId("test");
+ stage1.setIncludeKeyValue(false);
+ stage1.setIncludeX509IssuerSerial(true);
+ stage1.setPrivateKey(signingKey);
+ stage1.setCertificates(certs);
+ stage1.initialize();
+
+ stage1.execute(mdCol1);
+ Assert.assertEquals(mdCol1.size(), 1);
+
+ final Item<Element> result1 = mdCol1.iterator().next();
+ AssertSupport.assertValidComponentInfo(result1, 1, XMLSignatureSigningStage.class, "test");
+ Assert.assertFalse(containsCRs(result1.unwrap()));
+
+ /*
+ * The second result disables CR stripping.
+ */
+ final List<Item<Element>> mdCol2 = getInput("input.xml");
+
+ final XMLSignatureSigningStage stage2 = new XMLSignatureSigningStage();
+ stage2.setId("test");
+ stage2.setIncludeKeyValue(false);
+ stage2.setIncludeX509IssuerSerial(true);
+ stage2.setPrivateKey(signingKey);
+ stage2.setCertificates(certs);
+ stage2.setRemovingCRsFromSignature(false);
+ stage2.initialize();
+
+ stage2.execute(mdCol2);
+ Assert.assertEquals(mdCol2.size(), 1);
+
+ final Item<Element> result2 = mdCol2.iterator().next();
+ AssertSupport.assertValidComponentInfo(result2, 1, XMLSignatureSigningStage.class, "test");
+
+ /*
+ * Compare the two results.
+ */
+ final Source source1 = new NormalizedSource(Input.fromNode(result1.unwrap()).build());
+ final Source source2 = new NormalizedSource(Input.fromNode(result2.unwrap()).build());
+ final Diff diff = DiffBuilder.compare(source1).withTest(source2)
+ .checkForIdentical()
+ .build();
+
+ /*
+ * Under Java 11 or later, with the latest Santuario, we expect the two results to be
+ * different. Under previous versions of Java, we expect them to be the same.
+ *
+ * We ascertained in other tests that they are both valid.
+ */
+ if (TestSupport.isJavaV11OrLater()) {
+ Assert.assertTrue(containsCRs(result2.unwrap()), "expected CRs in result");
+ Assert.assertTrue(diff.hasDifferences(), "results were same, expected different");
+ } else {
+ Assert.assertFalse(containsCRs(result2.unwrap()), "did not expect CRs in result");
+ Assert.assertFalse(diff.hasDifferences(), "results were different, expected same");
+ }
+ }
}
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list