You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@santuario.apache.org by co...@apache.org on 2021/08/11 07:57:58 UTC

[santuario-xml-security-java] 01/01: SANTUARIO-572 - Disallow a KeyInfoReference to refer to a RetrievalMethod

This is an automated email from the ASF dual-hosted git repository.

coheigea pushed a commit to branch SANTUARIO-572
in repository https://gitbox.apache.org/repos/asf/santuario-xml-security-java.git

commit b512bd927c6132c80a0df397edd8aee14e3baebc
Author: Colm O hEigeartaigh <co...@apache.org>
AuthorDate: Wed Aug 11 08:57:14 2021 +0100

    SANTUARIO-572 - Disallow a KeyInfoReference to refer to a RetrievalMethod
---
 .../exceptions/XMLSecurityRuntimeException.java    | 177 ---------------------
 .../implementations/KeyInfoReferenceResolver.java  |   3 +-
 .../xml/security/signature/XMLSignatureInput.java  |  11 +-
 .../TransformEnvelopedSignature.java               |   8 +-
 .../transforms/implementations/TransformXPath.java |  15 +-
 .../keyresolver/KeyInfoReferenceResolverTest.java  |  21 +++
 .../KeyInfoReference-RSA-RetrievalMethod.xml       |  22 +++
 7 files changed, 62 insertions(+), 195 deletions(-)

diff --git a/src/main/java/org/apache/xml/security/exceptions/XMLSecurityRuntimeException.java b/src/main/java/org/apache/xml/security/exceptions/XMLSecurityRuntimeException.java
deleted file mode 100644
index b889f77..0000000
--- a/src/main/java/org/apache/xml/security/exceptions/XMLSecurityRuntimeException.java
+++ /dev/null
@@ -1,177 +0,0 @@
-/**
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-package org.apache.xml.security.exceptions;
-
-import java.text.MessageFormat;
-
-import org.apache.xml.security.utils.Constants;
-import org.apache.xml.security.utils.I18n;
-
-/**
- * The mother of all runtime Exceptions in this bundle. It allows exceptions to have
- * their messages translated to the different locales.
- *
- * The <code>xmlsecurity_en.properties</code> file contains this line:
- * <pre>
- * xml.WrongElement = Can't create a {0} from a {1} element
- * </pre>
- *
- * Usage in the Java source is:
- * <pre>
- * {
- *    Object[] exArgs = { Constants._TAG_TRANSFORMS, "BadElement" };
- *
- *    throw new XMLSecurityException("xml.WrongElement", exArgs);
- * }
- * </pre>
- *
- * Additionally, if another Exception has been caught, we can supply it, too
- * <pre>
- * try {
- *    ...
- * } catch (Exception oldEx) {
- *    Object[] exArgs = { Constants._TAG_TRANSFORMS, "BadElement" };
- *
- *    throw new XMLSecurityException("xml.WrongElement", exArgs, oldEx);
- * }
- * </pre>
- *
- *
- */
-public class XMLSecurityRuntimeException extends RuntimeException {
-
-    private static final long serialVersionUID = 1L;
-
-    /** Field msgID */
-    protected String msgID;
-
-    /**
-     * Constructor XMLSecurityRuntimeException
-     *
-     */
-    public XMLSecurityRuntimeException() {
-        super("Missing message string");
-
-        this.msgID = null;
-    }
-
-    /**
-     * Constructor XMLSecurityRuntimeException
-     *
-     * @param msgID
-     */
-    public XMLSecurityRuntimeException(String msgID) {
-        super(I18n.getExceptionMessage(msgID));
-
-        this.msgID = msgID;
-    }
-
-    /**
-     * Constructor XMLSecurityRuntimeException
-     *
-     * @param msgID
-     * @param exArgs
-     */
-    public XMLSecurityRuntimeException(String msgID, Object[] exArgs) {
-        super(MessageFormat.format(I18n.getExceptionMessage(msgID), exArgs));
-
-        this.msgID = msgID;
-    }
-
-    /**
-     * Constructor XMLSecurityRuntimeException
-     *
-     * @param originalException
-     */
-    public XMLSecurityRuntimeException(Exception originalException) {
-        super("Missing message ID to locate message string in resource bundle \""
-              + Constants.exceptionMessagesResourceBundleBase
-              + "\". Original Exception was a "
-              + originalException.getClass().getName() + " and message "
-              + originalException.getMessage(), originalException);
-    }
-
-    /**
-     * Constructor XMLSecurityRuntimeException
-     *
-     * @param msgID
-     * @param originalException
-     */
-    public XMLSecurityRuntimeException(String msgID, Exception originalException) {
-        super(I18n.getExceptionMessage(msgID, originalException), originalException);
-
-        this.msgID = msgID;
-    }
-
-    /**
-     * Constructor XMLSecurityRuntimeException
-     *
-     * @param msgID
-     * @param exArgs
-     * @param originalException
-     */
-    public XMLSecurityRuntimeException(String msgID, Object[] exArgs, Exception originalException) {
-        super(MessageFormat.format(I18n.getExceptionMessage(msgID), exArgs), originalException);
-
-        this.msgID = msgID;
-    }
-
-    /**
-     * Method getMsgID
-     *
-     * @return the messageId
-     */
-    public String getMsgID() {
-        if (msgID == null) {
-            return "Missing message ID";
-        }
-        return msgID;
-    }
-
-    /** {@inheritDoc} */
-    public String toString() {
-        String s = this.getClass().getName();
-        String message = super.getLocalizedMessage();
-
-        if (message != null) {
-            message = s + ": " + message;
-        } else {
-            message = s;
-        }
-
-        if (this.getCause() != null) {
-            message = message + "\nOriginal Exception was " + this.getCause().toString();
-        }
-
-        return message;
-    }
-
-    /**
-     * Method getOriginalException
-     *
-     * @return the original exception
-     */
-    public Exception getOriginalException() {
-        if (this.getCause() instanceof Exception) {
-            return (Exception)this.getCause();
-        }
-        return null;
-    }
-
-}
diff --git a/src/main/java/org/apache/xml/security/keys/keyresolver/implementations/KeyInfoReferenceResolver.java b/src/main/java/org/apache/xml/security/keys/keyresolver/implementations/KeyInfoReferenceResolver.java
index 9d2a8df..9641632 100644
--- a/src/main/java/org/apache/xml/security/keys/keyresolver/implementations/KeyInfoReferenceResolver.java
+++ b/src/main/java/org/apache/xml/security/keys/keyresolver/implementations/KeyInfoReferenceResolver.java
@@ -158,6 +158,7 @@ public class KeyInfoReferenceResolver extends KeyResolverSpi {
         validateReference(referentElement, secureValidation);
 
         KeyInfo referent = new KeyInfo(referentElement, baseURI);
+        referent.setSecureValidation(secureValidation);
         referent.addStorageResolver(storage);
         return referent;
     }
@@ -177,7 +178,7 @@ public class KeyInfoReferenceResolver extends KeyResolverSpi {
         }
 
         KeyInfo referent = new KeyInfo(referentElement, "");
-        if (referent.containsKeyInfoReference()) {
+        if (referent.containsKeyInfoReference() || referent.containsRetrievalMethod()) {
             if (secureValidation) {
                 throw new XMLSecurityException("KeyInfoReferenceResolver.InvalidReferentElement.ReferenceWithSecure");
             } else {
diff --git a/src/main/java/org/apache/xml/security/signature/XMLSignatureInput.java b/src/main/java/org/apache/xml/security/signature/XMLSignatureInput.java
index 491d3f7..9d6ad5c 100644
--- a/src/main/java/org/apache/xml/security/signature/XMLSignatureInput.java
+++ b/src/main/java/org/apache/xml/security/signature/XMLSignatureInput.java
@@ -32,7 +32,6 @@ import org.apache.xml.security.c14n.CanonicalizationException;
 import org.apache.xml.security.c14n.implementations.Canonicalizer11_OmitComments;
 import org.apache.xml.security.c14n.implementations.Canonicalizer20010315OmitComments;
 import org.apache.xml.security.c14n.implementations.CanonicalizerBase;
-import org.apache.xml.security.exceptions.XMLSecurityRuntimeException;
 import org.apache.xml.security.parser.XMLParserException;
 import org.apache.xml.security.utils.JavaUtils;
 import org.apache.xml.security.utils.XMLUtils;
@@ -535,15 +534,9 @@ public class XMLSignatureInput {
     /**
      * @param filter
      */
-    public void addNodeFilter(NodeFilter filter) {
+    public void addNodeFilter(NodeFilter filter) throws XMLParserException, IOException {
         if (isOctetStream()) {
-            try {
-                convertToNodes();
-            } catch (Exception e) {
-                throw new XMLSecurityRuntimeException(
-                    "signature.XMLSignatureInput.nodesetReference", e
-                );
-            }
+            convertToNodes();
         }
         nodeFilters.add(filter);
     }
diff --git a/src/main/java/org/apache/xml/security/transforms/implementations/TransformEnvelopedSignature.java b/src/main/java/org/apache/xml/security/transforms/implementations/TransformEnvelopedSignature.java
index c8987c9..5dab049 100644
--- a/src/main/java/org/apache/xml/security/transforms/implementations/TransformEnvelopedSignature.java
+++ b/src/main/java/org/apache/xml/security/transforms/implementations/TransformEnvelopedSignature.java
@@ -18,8 +18,10 @@
  */
 package org.apache.xml.security.transforms.implementations;
 
+import java.io.IOException;
 import java.io.OutputStream;
 
+import org.apache.xml.security.parser.XMLParserException;
 import org.apache.xml.security.signature.NodeFilter;
 import org.apache.xml.security.signature.XMLSignatureInput;
 import org.apache.xml.security.transforms.TransformSpi;
@@ -67,7 +69,11 @@ public class TransformEnvelopedSignature extends TransformSpi {
 
         Node signatureElement = searchSignatureElement(transformElement);
         input.setExcludeNode(signatureElement);
-        input.addNodeFilter(new EnvelopedNodeFilter(signatureElement));
+        try {
+            input.addNodeFilter(new EnvelopedNodeFilter(signatureElement));
+        } catch (XMLParserException | IOException ex) {
+            throw new TransformationException(ex);
+        }
         return input;
     }
 
diff --git a/src/main/java/org/apache/xml/security/transforms/implementations/TransformXPath.java b/src/main/java/org/apache/xml/security/transforms/implementations/TransformXPath.java
index e1a10b6..c70a4a5 100644
--- a/src/main/java/org/apache/xml/security/transforms/implementations/TransformXPath.java
+++ b/src/main/java/org/apache/xml/security/transforms/implementations/TransformXPath.java
@@ -18,11 +18,12 @@
  */
 package org.apache.xml.security.transforms.implementations;
 
+import java.io.IOException;
 import java.io.OutputStream;
 
 import javax.xml.transform.TransformerException;
 
-import org.apache.xml.security.exceptions.XMLSecurityRuntimeException;
+import org.apache.xml.security.parser.XMLParserException;
 import org.apache.xml.security.signature.NodeFilter;
 import org.apache.xml.security.signature.XMLSignatureInput;
 import org.apache.xml.security.transforms.TransformSpi;
@@ -47,6 +48,9 @@ import org.w3c.dom.Node;
  */
 public class TransformXPath extends TransformSpi {
 
+    private static final org.slf4j.Logger LOG =
+            org.slf4j.LoggerFactory.getLogger(TransformXPath.class);
+
     /**
      * {@inheritDoc}
      */
@@ -98,7 +102,7 @@ public class TransformXPath extends TransformSpi {
             input.addNodeFilter(new XPathNodeFilter(xpathElement, xpathnode, str, xpathAPIInstance));
             input.setNodeSet(true);
             return input;
-        } catch (DOMException ex) {
+        } catch (XMLParserException | IOException | DOMException ex) {
             throw new TransformationException(ex);
         }
     }
@@ -140,11 +144,8 @@ public class TransformXPath extends TransformSpi {
                 }
                 return 0;
             } catch (TransformerException e) {
-                Object[] eArgs = {currentNode};
-                throw new XMLSecurityRuntimeException("signature.Transform.node", eArgs, e);
-            } catch (Exception e) {
-                Object[] eArgs = {currentNode, currentNode.getNodeType()};
-                throw new XMLSecurityRuntimeException("signature.Transform.nodeAndType",eArgs, e);
+                LOG.debug("Error evaluating XPath expression", e);
+                return 0;
             }
         }
 
diff --git a/src/test/java/org/apache/xml/security/test/dom/keys/keyresolver/KeyInfoReferenceResolverTest.java b/src/test/java/org/apache/xml/security/test/dom/keys/keyresolver/KeyInfoReferenceResolverTest.java
index d4f5608..2f81cfd 100644
--- a/src/test/java/org/apache/xml/security/test/dom/keys/keyresolver/KeyInfoReferenceResolverTest.java
+++ b/src/test/java/org/apache/xml/security/test/dom/keys/keyresolver/KeyInfoReferenceResolverTest.java
@@ -125,6 +125,19 @@ public class KeyInfoReferenceResolverTest {
         assertNull(keyInfo.getPublicKey());
     }
 
+    @org.junit.jupiter.api.Test
+    public void testKeyInfoReferenceToRetrievalMethodNotAllowed() throws Exception {
+        Document doc = loadXML("KeyInfoReference-RSA-RetrievalMethod.xml");
+        markKeyInfoIdAttrs(doc);
+        markEncodedKeyValueIdAttrs(doc);
+
+        Element referenceElement = doc.getElementById("theReference");
+        assertNotNull(referenceElement);
+
+        KeyInfo keyInfo = new KeyInfo(referenceElement, "");
+        assertNull(keyInfo.getPublicKey());
+    }
+
     // Utility methods
 
     private String getControlFilePath(String fileName) {
@@ -160,4 +173,12 @@ public class KeyInfoReferenceResolverTest {
         }
     }
 
+    private void markEncodedKeyValueIdAttrs(Document doc) {
+        NodeList nl = doc.getElementsByTagNameNS(Constants.SignatureSpec11NS, Constants._TAG_DERENCODEDKEYVALUE);
+        for (int i = 0; i < nl.getLength(); i++) {
+            Element keyInfoElement = (Element) nl.item(i);
+            keyInfoElement.setIdAttributeNS(null, Constants._ATT_ID, true);
+        }
+    }
+
 }
\ No newline at end of file
diff --git a/src/test/resources/org/apache/xml/security/keyresolver/KeyInfoReference-RSA-RetrievalMethod.xml b/src/test/resources/org/apache/xml/security/keyresolver/KeyInfoReference-RSA-RetrievalMethod.xml
new file mode 100644
index 0000000..f34e3d5
--- /dev/null
+++ b/src/test/resources/org/apache/xml/security/keyresolver/KeyInfoReference-RSA-RetrievalMethod.xml
@@ -0,0 +1,22 @@
+<test:root xmlns:test="http://www.example.org/test">
+
+  <ds:KeyInfo xmlns:ds="http://www.w3.org/2000/09/xmldsig#" Id="theRealKey">
+    <dsig11:DEREncodedKeyValue Id="theRealKey2" xmlns:dsig11="http://www.w3.org/2009/xmldsig11#">
+      MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAmDnHagSzfia3N7jOaMSp4VIZjK2lxZgN
+      X/2z98YLp1XE3cvpP+mOvX3gENWQuX3uoix+2qroZ0BFHzhzf4E7is5Q9+42ZFi5naFk3c/B0Q8A
+      jtHtWUEZ8VPPBZggz6uJ1ttJS7YDP6XVjaw6SN1bJSD4/lWNIVsh95kuhunbOef6x/kyIbBz9wF4
+      S0//G6zPD4GG7/jJ+sDXe+bAgPB1qwhLhrK3N1jGuDZkGGcY/c4b7aba0B0rognwKlygv16GoA/n
+      zWehxih7clhmMTzP2VWa3Q2GcN8ETe00dz68KtS7GF6W15qftjUvRXEKSoPz86ZsP30jIH1tvIrs
+      qSh/kwIDAQAB
+    </dsig11:DEREncodedKeyValue>
+  </ds:KeyInfo>
+
+  <ds:KeyInfo xmlns:ds="http://www.w3.org/2000/09/xmldsig#" Id="retrievalMethod">
+    <ds:RetrievalMethod URI="#theRealKey2"/>
+  </ds:KeyInfo>
+  
+  <ds:KeyInfo xmlns:ds="http://www.w3.org/2000/09/xmldsig#" Id="theReference">
+    <dsig11:KeyInfoReference xmlns:dsig11="http://www.w3.org/2009/xmldsig11#" URI="#retrievalMethod" />
+  </ds:KeyInfo>
+
+</test:root>