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/09/09 09:47:13 UTC
[santuario-xml-security-java] branch 2.1.x-fixes updated:
SANTUARIO-577 - Introduce a system property to control if file/http
references are allowed from an unsigned context
This is an automated email from the ASF dual-hosted git repository.
coheigea pushed a commit to branch 2.1.x-fixes
in repository https://gitbox.apache.org/repos/asf/santuario-xml-security-java.git
The following commit(s) were added to refs/heads/2.1.x-fixes by this push:
new c3410a7 SANTUARIO-577 - Introduce a system property to control if file/http references are allowed from an unsigned context
c3410a7 is described below
commit c3410a7fbecaa7dca61361d887f801b9e6c66c15
Author: Colm O hEigeartaigh <co...@apache.org>
AuthorDate: Thu Sep 9 10:46:26 2021 +0100
SANTUARIO-577 - Introduce a system property to control if file/http references are allowed from an unsigned context
---
.../xml/dsig/internal/dom/DOMURIDereferencer.java | 23 +++++-----
.../xml/security/encryption/XMLCipherInput.java | 13 ++++--
.../implementations/KeyInfoReferenceResolver.java | 12 +++++-
.../implementations/RetrievalMethodResolver.java | 20 ++++++---
.../security/utils/resolver/ResourceResolver.java | 23 ++++++++++
.../crypto/test/dsig/CreateBaltimore23Test.java | 1 +
.../dom/utils/resolver/ResourceResolverTest.java | 50 ++++++++++++++++++++++
7 files changed, 121 insertions(+), 21 deletions(-)
diff --git a/src/main/java/org/apache/jcp/xml/dsig/internal/dom/DOMURIDereferencer.java b/src/main/java/org/apache/jcp/xml/dsig/internal/dom/DOMURIDereferencer.java
index dc144a5..139c612 100644
--- a/src/main/java/org/apache/jcp/xml/dsig/internal/dom/DOMURIDereferencer.java
+++ b/src/main/java/org/apache/jcp/xml/dsig/internal/dom/DOMURIDereferencer.java
@@ -104,17 +104,20 @@ public final class DOMURIDereferencer implements URIDereferencer {
}
}
- try {
- ResourceResolver apacheResolver =
- ResourceResolver.getInstance(uriAttr, baseURI, secVal);
- XMLSignatureInput in = apacheResolver.resolve(uriAttr, baseURI, secVal);
- if (in.isOctetStream()) {
- return new ApacheOctetStreamData(in);
- } else {
- return new ApacheNodeSetData(in);
+ if (uriRef instanceof javax.xml.crypto.dsig.Reference || ResourceResolver.isURISafeToResolve(uriAttr, baseURI)) {
+ try {
+ ResourceResolver apacheResolver =
+ ResourceResolver.getInstance(uriAttr, baseURI, secVal);
+ XMLSignatureInput in = apacheResolver.resolve(uriAttr, baseURI, secVal);
+ if (in.isOctetStream()) {
+ return new ApacheOctetStreamData(in);
+ } else {
+ return new ApacheNodeSetData(in);
+ }
+ } catch (Exception e) {
+ throw new URIReferenceException(e);
}
- } catch (Exception e) {
- throw new URIReferenceException(e);
}
+ throw new URIReferenceException("URI " + uri + " is forbidden");
}
}
diff --git a/src/main/java/org/apache/xml/security/encryption/XMLCipherInput.java b/src/main/java/org/apache/xml/security/encryption/XMLCipherInput.java
index 806bab8..28dc1d4 100644
--- a/src/main/java/org/apache/xml/security/encryption/XMLCipherInput.java
+++ b/src/main/java/org/apache/xml/security/encryption/XMLCipherInput.java
@@ -123,9 +123,16 @@ public class XMLCipherInput {
XMLSignatureInput input = null;
try {
- ResourceResolver resolver =
- ResourceResolver.getInstance(uriAttr, null, secureValidation);
- input = resolver.resolve(uriAttr, null, secureValidation);
+ if (ResourceResolver.isURISafeToResolve(uriAttr, null)) {
+ ResourceResolver resolver =
+ ResourceResolver.getInstance(uriAttr, null, secureValidation);
+ input = resolver.resolve(uriAttr, null, secureValidation);
+ } else {
+ String uriToResolve = uriAttr != null ? uriAttr.getValue() : null;
+ Object[] exArgs = {uriToResolve != null ? uriToResolve : "null", null};
+
+ throw new ResourceResolverException("utils.resolver.noClass", exArgs, uriToResolve, null);
+ }
} catch (ResourceResolverException ex) {
throw new XMLEncryptionException(ex);
}
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 97b2fcf..b16400e 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
@@ -38,6 +38,7 @@ import org.apache.xml.security.signature.XMLSignatureInput;
import org.apache.xml.security.utils.Constants;
import org.apache.xml.security.utils.XMLUtils;
import org.apache.xml.security.utils.resolver.ResourceResolver;
+import org.apache.xml.security.utils.resolver.ResourceResolverException;
import org.w3c.dom.Attr;
import org.w3c.dom.Element;
import org.xml.sax.SAXException;
@@ -219,8 +220,15 @@ public class KeyInfoReferenceResolver extends KeyResolverSpi {
*/
private XMLSignatureInput resolveInput(Attr uri, String baseURI, boolean secureValidation)
throws XMLSecurityException {
- ResourceResolver resRes = ResourceResolver.getInstance(uri, baseURI, secureValidation);
- return resRes.resolve(uri, baseURI, secureValidation);
+ if (ResourceResolver.isURISafeToResolve(uri, baseURI)) {
+ ResourceResolver resRes = ResourceResolver.getInstance(uri, baseURI, secureValidation);
+ return resRes.resolve(uri, baseURI, secureValidation);
+ }
+
+ String uriToResolve = uri != null ? uri.getValue() : null;
+ Object[] exArgs = { uriToResolve != null ? uriToResolve : "null", baseURI };
+
+ throw new ResourceResolverException("utils.resolver.noClass", exArgs, uriToResolve, baseURI);
}
/**
diff --git a/src/main/java/org/apache/xml/security/keys/keyresolver/implementations/RetrievalMethodResolver.java b/src/main/java/org/apache/xml/security/keys/keyresolver/implementations/RetrievalMethodResolver.java
index cceb63b..aabd7b3 100644
--- a/src/main/java/org/apache/xml/security/keys/keyresolver/implementations/RetrievalMethodResolver.java
+++ b/src/main/java/org/apache/xml/security/keys/keyresolver/implementations/RetrievalMethodResolver.java
@@ -46,6 +46,7 @@ import org.apache.xml.security.transforms.Transforms;
import org.apache.xml.security.utils.Constants;
import org.apache.xml.security.utils.XMLUtils;
import org.apache.xml.security.utils.resolver.ResourceResolver;
+import org.apache.xml.security.utils.resolver.ResourceResolverException;
import org.w3c.dom.Attr;
import org.w3c.dom.Element;
import org.w3c.dom.Node;
@@ -273,13 +274,20 @@ public class RetrievalMethodResolver extends KeyResolverSpi {
Attr uri = rm.getURIAttr();
// Apply the transforms
Transforms transforms = rm.getTransforms();
- ResourceResolver resRes = ResourceResolver.getInstance(uri, baseURI, secureValidation);
- XMLSignatureInput resource = resRes.resolve(uri, baseURI, secureValidation);
- if (transforms != null) {
- LOG.debug("We have Transforms");
- resource = transforms.performTransforms(resource);
+ if (ResourceResolver.isURISafeToResolve(uri, baseURI)) {
+ ResourceResolver resRes = ResourceResolver.getInstance(uri, baseURI, secureValidation);
+ XMLSignatureInput resource = resRes.resolve(uri, baseURI, secureValidation);
+ if (transforms != null) {
+ LOG.debug("We have Transforms");
+ resource = transforms.performTransforms(resource);
+ }
+ return resource;
}
- return resource;
+
+ String uriToResolve = uri != null ? uri.getValue() : null;
+ Object[] exArgs = { uriToResolve != null ? uriToResolve : "null", baseURI };
+
+ throw new ResourceResolverException("utils.resolver.noClass", exArgs, uriToResolve, baseURI);
}
/**
diff --git a/src/main/java/org/apache/xml/security/utils/resolver/ResourceResolver.java b/src/main/java/org/apache/xml/security/utils/resolver/ResourceResolver.java
index 9578f6a..6ef694d 100644
--- a/src/main/java/org/apache/xml/security/utils/resolver/ResourceResolver.java
+++ b/src/main/java/org/apache/xml/security/utils/resolver/ResourceResolver.java
@@ -18,6 +18,8 @@
*/
package org.apache.xml.security.utils.resolver;
+import java.security.AccessController;
+import java.security.PrivilegedAction;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
@@ -39,6 +41,10 @@ import org.w3c.dom.Attr;
*/
public class ResourceResolver {
+ private static boolean allowUnsafeResourceResolving =
+ AccessController.doPrivileged(
+ (PrivilegedAction<Boolean>) () -> Boolean.getBoolean("org.apache.xml.security.allowUnsafeResourceResolving"));
+
private static final org.slf4j.Logger LOG =
org.slf4j.LoggerFactory.getLogger(ResourceResolver.class);
@@ -326,6 +332,23 @@ public class ResourceResolver {
return resolverSpi.understandsProperty(propertyToTest);
}
+ public static boolean isURISafeToResolve(Attr uriAttr, String baseUri) {
+ if (allowUnsafeResourceResolving) {
+ return true;
+ }
+ String uriToResolve = uriAttr != null ? uriAttr.getValue() : null;
+ if (uriToResolve != null) {
+ if (uriToResolve.startsWith("file:") || uriToResolve.startsWith("http:")) {
+ return false;
+ }
+ if (!uriToResolve.isEmpty() && uriToResolve.charAt(0) != '#' &&
+ baseUri != null && (baseUri.startsWith("file:") || baseUri.startsWith("http:"))) {
+ return false;
+ }
+ }
+ return true;
+ }
+
/**
* Method canResolve
*
diff --git a/src/test/java/javax/xml/crypto/test/dsig/CreateBaltimore23Test.java b/src/test/java/javax/xml/crypto/test/dsig/CreateBaltimore23Test.java
index 799d784..acb85a7 100644
--- a/src/test/java/javax/xml/crypto/test/dsig/CreateBaltimore23Test.java
+++ b/src/test/java/javax/xml/crypto/test/dsig/CreateBaltimore23Test.java
@@ -79,6 +79,7 @@ public class CreateBaltimore23Test {
private final URIDereferencer ud;
static {
+ System.setProperty("org.apache.xml.security.allowUnsafeResourceResolving", "true");
Security.insertProviderAt
(new org.apache.jcp.xml.dsig.internal.dom.XMLDSigRI(), 1);
}
diff --git a/src/test/java/org/apache/xml/security/test/dom/utils/resolver/ResourceResolverTest.java b/src/test/java/org/apache/xml/security/test/dom/utils/resolver/ResourceResolverTest.java
index 6a99a30..9c1f067 100644
--- a/src/test/java/org/apache/xml/security/test/dom/utils/resolver/ResourceResolverTest.java
+++ b/src/test/java/org/apache/xml/security/test/dom/utils/resolver/ResourceResolverTest.java
@@ -18,6 +18,8 @@
*/
package org.apache.xml.security.test.dom.utils.resolver;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import java.io.File;
@@ -86,4 +88,52 @@ public class ResourceResolverTest {
}
}
+ @org.junit.Test
+ public void testIsSafeURIToResolveFile() throws Exception {
+ Document doc = XMLUtils.newDocument();
+ Attr uriAttr = doc.createAttribute("URI");
+ String basedir = System.getProperty("basedir");
+ String file = new File(basedir, "pom.xml").toURI().toString();
+ uriAttr.setValue(file);
+
+ assertFalse(ResourceResolver.isURISafeToResolve(uriAttr, null));
+ }
+
+ @org.junit.Test
+ public void testIsSafeURIToResolveFileBaseURI() throws Exception {
+ Document doc = XMLUtils.newDocument();
+ Attr uriAttr = doc.createAttribute("URI");
+ String basedir = System.getProperty("basedir");
+ String file = new File(basedir, "pom.xml").toURI().toString();
+ uriAttr.setValue("xyz");
+
+ assertFalse(ResourceResolver.isURISafeToResolve(uriAttr, file));
+ }
+
+ @org.junit.Test
+ public void testIsSafeURIToResolveHTTP() throws Exception {
+ Document doc = XMLUtils.newDocument();
+ Attr uriAttr = doc.createAttribute("URI");
+ uriAttr.setValue("http://www.apache.org");
+
+ assertFalse(ResourceResolver.isURISafeToResolve(uriAttr, null));
+ }
+
+ @org.junit.Test
+ public void testIsSafeURIToResolveHTTPBaseURI() throws Exception {
+ Document doc = XMLUtils.newDocument();
+ Attr uriAttr = doc.createAttribute("URI");
+ uriAttr.setValue("xyz");
+
+ assertFalse(ResourceResolver.isURISafeToResolve(uriAttr, "http://www.apache.org"));
+ }
+
+ @org.junit.Test
+ public void testIsSafeURIToResolveLocalReference() throws Exception {
+ Document doc = XMLUtils.newDocument();
+ Attr uriAttr = doc.createAttribute("URI");
+ uriAttr.setValue("#1234");
+
+ assertTrue(ResourceResolver.isURISafeToResolve(uriAttr, null));
+ }
}