From 88f9cf3ddba808b3c51f334d07f8656e4a7dd192 Mon Sep 17 00:00:00 2001 From: Andreas Beeker Date: Wed, 18 Mar 2020 21:06:09 +0000 Subject: [PATCH] #63712 - upgrading xmlsec causes junit tests to fail git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1875392 13f79535-47bb-0310-9956-ffa450edef68 --- .classpath | 2 +- build.gradle | 2 +- build.xml | 8 ++- sonar/ooxml/pom.xml | 6 +- .../poi/poifs/crypt/dsig/SignatureInfo.java | 25 +++---- .../dsig/SignatureMarshalDefaultListener.java | 71 +++++++++++++------ .../crypt/dsig/SignatureMarshalListener.java | 15 +--- .../dsig/facets/XAdESSignatureFacet.java | 16 +++-- 8 files changed, 85 insertions(+), 60 deletions(-) diff --git a/.classpath b/.classpath index 819bb23955..7d8b545565 100644 --- a/.classpath +++ b/.classpath @@ -33,7 +33,7 @@ - + diff --git a/build.gradle b/build.gradle index 5f0123386e..1f471d3e26 100644 --- a/build.gradle +++ b/build.gradle @@ -241,7 +241,7 @@ project('ooxml') { compile 'org.apache.commons:commons-collections4:4.4' compile "org.apache.commons:commons-math3:${commonsMathVersion}" compile "org.apache.commons:commons-compress:${commonsCompressVersion}" - compile 'org.apache.santuario:xmlsec:2.1.2' + compile 'org.apache.santuario:xmlsec:2.1.5' compile "org.bouncycastle:bcpkix-jdk15on:${bouncyCastleVersion}" compile 'com.github.virtuald:curvesapi:1.06' compile 'com.zaxxer:SparseBitSet:1.2' diff --git a/build.xml b/build.xml index 360408d969..c6a6406a67 100644 --- a/build.xml +++ b/build.xml @@ -223,8 +223,8 @@ under the License. value="${repository.m2}/maven2/com/zaxxer/SparseBitSet/1.2/SparseBitSet-1.2.jar"/> - - + + @@ -715,6 +715,10 @@ under the License. + + + + diff --git a/sonar/ooxml/pom.xml b/sonar/ooxml/pom.xml index 4d8811d57b..28308494bb 100644 --- a/sonar/ooxml/pom.xml +++ b/sonar/ooxml/pom.xml @@ -84,7 +84,7 @@ - + @@ -147,7 +147,7 @@ org.apache.santuario xmlsec - 2.1.2 + 2.1.5 org.apache.commons @@ -159,7 +159,7 @@ curvesapi 1.06 - + junit junit diff --git a/src/ooxml/java/org/apache/poi/poifs/crypt/dsig/SignatureInfo.java b/src/ooxml/java/org/apache/poi/poifs/crypt/dsig/SignatureInfo.java index 806ab91d65..7be687a48e 100644 --- a/src/ooxml/java/org/apache/poi/poifs/crypt/dsig/SignatureInfo.java +++ b/src/ooxml/java/org/apache/poi/poifs/crypt/dsig/SignatureInfo.java @@ -153,7 +153,7 @@ import org.w3c.dom.events.MutationEvent; * in the classpath:

*
    *
  • BouncyCastle bcpkix and bcprov (tested against 1.64)
    • - *
  • Apache Santuario "xmlsec" (tested against 2.1.2)
    • + *
  • Apache Santuario "xmlsec" (tested against 2.1.5)
    • *
  • and slf4j-api (tested against 1.7.30)
    • *
*/ @@ -461,27 +461,22 @@ public class SignatureInfo { return; } - EventTarget target = (EventTarget)document; - final EventListener[] el = { null }; - el[0] = (e) -> { - if (!(e instanceof MutationEvent)) { - return; - } + final EventTarget eventTarget = (EventTarget)document; + final String eventType = "DOMSubtreeModified"; + final boolean DONT_USE_CAPTURE = false; - MutationEvent mutEvt = (MutationEvent) e; - EventTarget et = mutEvt.getTarget(); - if (!(et instanceof Element)) { - return; + el[0] = (e) -> { + if (e instanceof MutationEvent && e.getTarget() instanceof Document) { + eventTarget.removeEventListener(eventType, el[0], DONT_USE_CAPTURE); + sml.handleElement(this, document, eventTarget, el[0]); + eventTarget.addEventListener(eventType, el[0], DONT_USE_CAPTURE); } - - sml.handleElement(this, (Element) et, target, el[0]); }; - SignatureMarshalListener.setListener(target, el[0], true); + eventTarget.addEventListener(eventType, el[0], DONT_USE_CAPTURE); } - /** * Helper method for adding informations after the signing. * Normally {@link #confirmSignature()} is sufficient to be used. diff --git a/src/ooxml/java/org/apache/poi/poifs/crypt/dsig/SignatureMarshalDefaultListener.java b/src/ooxml/java/org/apache/poi/poifs/crypt/dsig/SignatureMarshalDefaultListener.java index 24acda22ed..df5c3d88d8 100644 --- a/src/ooxml/java/org/apache/poi/poifs/crypt/dsig/SignatureMarshalDefaultListener.java +++ b/src/ooxml/java/org/apache/poi/poifs/crypt/dsig/SignatureMarshalDefaultListener.java @@ -17,47 +17,76 @@ package org.apache.poi.poifs.crypt.dsig; -import static org.apache.poi.poifs.crypt.dsig.SignatureMarshalListener.setListener; -import static org.apache.poi.poifs.crypt.dsig.facets.SignatureFacet.OO_DIGSIG_NS; +import static org.apache.poi.poifs.crypt.dsig.facets.SignatureFacet.XML_DIGSIG_NS; import static org.apache.poi.poifs.crypt.dsig.facets.SignatureFacet.XML_NS; +import java.util.Arrays; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; + +import org.w3c.dom.Document; import org.w3c.dom.Element; +import org.w3c.dom.NamedNodeMap; import org.w3c.dom.Node; import org.w3c.dom.NodeList; import org.w3c.dom.events.EventListener; import org.w3c.dom.events.EventTarget; +import org.w3c.dom.traversal.DocumentTraversal; +import org.w3c.dom.traversal.NodeFilter; +import org.w3c.dom.traversal.NodeIterator; /** * This listener class is used, to modify the to be digested xml document, * e.g. to register id attributes or set prefixes for registered namespaces */ public class SignatureMarshalDefaultListener implements SignatureMarshalListener { + private final Set IGNORE_NS = new HashSet<>(Arrays.asList(null, XML_NS, XML_DIGSIG_NS)); + private final String OBJECT_TAG = "Object"; + @Override - public void handleElement(SignatureInfo signatureInfo, Element el, EventTarget target, EventListener parentListener) { - if (el.hasAttribute("Id")) { - el.setIdAttribute("Id", true); - } + public void handleElement(SignatureInfo signatureInfo, Document doc, EventTarget target, EventListener parentListener) { + // see POI #63712 : because of Santuario change r1853805 in XmlSec 2.1.3, + // we have to deal with the whole document now - setListener(target, parentListener, false); - if (OO_DIGSIG_NS.equals(el.getNamespaceURI())) { - String parentNS = el.getParentNode().getNamespaceURI(); - if (!OO_DIGSIG_NS.equals(parentNS) && !el.hasAttributeNS(XML_NS, "mdssi")) { - el.setAttributeNS(XML_NS, "xmlns:mdssi", OO_DIGSIG_NS); - } + final DocumentTraversal traversal = (DocumentTraversal) doc; + final Map prefixCfg = signatureInfo.getSignatureConfig().getNamespacePrefixes(); + + final Map prefixUsed = new HashMap<>(); + + NodeList nl = doc.getElementsByTagName(OBJECT_TAG); + final int objLen = nl.getLength(); + for (int i=0; i objNode.setAttributeNS(XML_NS, "xmlns:"+prefix, ns)); } - setPrefix(signatureInfo, el); - setListener(target, parentListener, true); } - protected static void setPrefix(SignatureInfo signatureInfo, Node el) { - String prefix = signatureInfo.getSignatureConfig().getNamespacePrefixes().get(el.getNamespaceURI()); - if (prefix != null && el.getPrefix() == null) { - el.setPrefix(prefix); + private void getAllNamespaces(DocumentTraversal traversal, Element objNode, Map prefixCfg, Map prefixUsed) { + prefixUsed.clear(); + final NodeIterator iter = traversal.createNodeIterator(objNode, NodeFilter.SHOW_ELEMENT, null, false); + try { + for (Element node; (node = (Element)iter.nextNode()) != null; ) { + setPrefix(node, prefixCfg, prefixUsed); + NamedNodeMap nnm = node.getAttributes(); + final int nnmLen = nnm.getLength(); + for (int j=0; j prefixCfg, Map prefixUsed) { + String ns = node.getNamespaceURI(); + String prefix = prefixCfg.get(ns); + if (!IGNORE_NS.contains(prefix)) { + node.setPrefix(prefix); + prefixUsed.put(ns, prefix); } } } diff --git a/src/ooxml/java/org/apache/poi/poifs/crypt/dsig/SignatureMarshalListener.java b/src/ooxml/java/org/apache/poi/poifs/crypt/dsig/SignatureMarshalListener.java index 34210f4cf5..43fd335836 100644 --- a/src/ooxml/java/org/apache/poi/poifs/crypt/dsig/SignatureMarshalListener.java +++ b/src/ooxml/java/org/apache/poi/poifs/crypt/dsig/SignatureMarshalListener.java @@ -17,7 +17,7 @@ package org.apache.poi.poifs.crypt.dsig; -import org.w3c.dom.Element; +import org.w3c.dom.Document; import org.w3c.dom.events.EventListener; import org.w3c.dom.events.EventTarget; @@ -26,16 +26,5 @@ import org.w3c.dom.events.EventTarget; * e.g. to register id attributes or set prefixes for registered namespaces */ public interface SignatureMarshalListener { - void handleElement(SignatureInfo signatureInfo, Element el, EventTarget target, EventListener parentListener); - - // helper method to keep it in one place - static void setListener(EventTarget target, EventListener listener, boolean enabled) { - final String type = "DOMSubtreeModified"; - final boolean DONT_USE_CAPTURE = false; - if (enabled) { - target.addEventListener(type, listener, DONT_USE_CAPTURE); - } else { - target.removeEventListener(type, listener, DONT_USE_CAPTURE); - } - } + void handleElement(SignatureInfo signatureInfo, Document doc, EventTarget target, EventListener parentListener); } \ No newline at end of file diff --git a/src/ooxml/java/org/apache/poi/poifs/crypt/dsig/facets/XAdESSignatureFacet.java b/src/ooxml/java/org/apache/poi/poifs/crypt/dsig/facets/XAdESSignatureFacet.java index ce96f16a91..8ad617f22d 100644 --- a/src/ooxml/java/org/apache/poi/poifs/crypt/dsig/facets/XAdESSignatureFacet.java +++ b/src/ooxml/java/org/apache/poi/poifs/crypt/dsig/facets/XAdESSignatureFacet.java @@ -31,8 +31,8 @@ import static org.apache.poi.poifs.crypt.dsig.facets.SignatureFacetHelper.newTra import java.security.MessageDigest; import java.security.cert.CertificateEncodingException; import java.security.cert.X509Certificate; -import java.util.Arrays; import java.util.Calendar; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Locale; @@ -74,7 +74,9 @@ import org.etsi.uri.x01903.v13.SignerRoleType; import org.w3.x2000.x09.xmldsig.DigestMethodType; import org.w3.x2000.x09.xmldsig.X509IssuerSerialType; import org.w3c.dom.Document; +import org.w3c.dom.Element; import org.w3c.dom.Node; +import org.w3c.dom.NodeList; /** * XAdES Signature Facet. Implements XAdES v1.4.1 which is compatible with XAdES @@ -233,9 +235,15 @@ public class XAdESSignatureFacet implements SignatureFacet { private XMLObject addXadesObject(SignatureInfo signatureInfo, Document document, QualifyingPropertiesType qualifyingProperties) { Node qualDocElSrc = qualifyingProperties.getDomNode(); - Node qualDocEl = document.importNode(qualDocElSrc, true); - List xadesObjectContent = Arrays.asList(new DOMStructure(qualDocEl)); - return signatureInfo.getSignatureFactory().newXMLObject(xadesObjectContent, null, null, null); + Element qualDocEl = (Element)document.importNode(qualDocElSrc, true); + + NodeList nl = qualDocEl.getElementsByTagNameNS(SignatureFacet.XADES_132_NS, "SignedProperties"); + assert(nl.getLength() == 1); + ((Element)nl.item(0)).setIdAttribute("Id", true); + + List xadesObjectContent = Collections.singletonList(new DOMStructure(qualDocEl)); + XMLObject xo = signatureInfo.getSignatureFactory().newXMLObject(xadesObjectContent, null, null, null); + return xo; } private Reference addXadesReference(SignatureInfo signatureInfo) throws XMLSignatureException { -- 2.39.5