[java-opensaml] branch maint-4 updated: OSJ-367: Decrypter does not guard against malicious references ...
Brent Putman
putmanb at georgetown.edu
Tue Jan 10 04:38:12 UTC 2023
This is an automated email from the git hooks/post-receive script.
putmanb pushed a commit to branch maint-4
in repository java-opensaml.
View the commit online:
http://git.shibboleth.net/view/?p=java-opensaml.git;a=commit;h=3e17da09ed11cb7f69eb2b4330e3147d106a2048
The following commit(s) were added to refs/heads/maint-4 by this push:
new 3e17da09e OSJ-367: Decrypter does not guard against malicious references ...
3e17da09e is described below
commit 3e17da09ed11cb7f69eb2b4330e3147d106a2048
Author: Brent Putman <putmanb at georgetown.edu>
AuthorDate: Mon Jan 9 22:26:22 2023 -0500
OSJ-367: Decrypter does not guard against malicious references ...
Decrypter does not guard against malicious references or transforms.
---
.../xmlsec/encryption/support/Decrypter.java | 57 ++++++++++++++-
.../support/DefaultPreDecryptionValidator.java | 65 ++++++++++++++++++
.../support/PreDecryptionValidationException.java | 67 ++++++++++++++++++
.../encryption/support/PreDecryptionValidator.java | 48 +++++++++++++
.../tests/DefaultPreDecryptionValidatorTest.java | 80 ++++++++++++++++++++++
.../support/tests/SimpleDecryptionTest.java | 72 +++++++++++++++++++
6 files changed, 387 insertions(+), 2 deletions(-)
diff --git a/opensaml-xmlsec-api/src/main/java/org/opensaml/xmlsec/encryption/support/Decrypter.java b/opensaml-xmlsec-api/src/main/java/org/opensaml/xmlsec/encryption/support/Decrypter.java
index 1883d4ddd..bdc67715a 100644
--- a/opensaml-xmlsec-api/src/main/java/org/opensaml/xmlsec/encryption/support/Decrypter.java
+++ b/opensaml-xmlsec-api/src/main/java/org/opensaml/xmlsec/encryption/support/Decrypter.java
@@ -222,6 +222,9 @@ public class Decrypter {
* root of a new DOM document. */
private boolean defaultRootInNewDocument;
+ /** The pre-decryption validator instance. */
+ private PreDecryptionValidator preDecryptionValidator;
+
/**
* Constructor.
*
@@ -288,8 +291,28 @@ public class Decrypter {
unmarshallerFactory = XMLObjectProviderRegistrySupport.getUnmarshallerFactory();
defaultRootInNewDocument = false;
+
+ preDecryptionValidator = new DefaultPreDecryptionValidator();
}
+ /**
+ * Get the instance of {@link PreDecryptionValidator}.
+ *
+ * @return the validator, may be null
+ */
+ @Nullable PreDecryptionValidator getPreDecryptionValidator() {
+ return preDecryptionValidator;
+ }
+
+ /**
+ * Set the instance of {@link PreDecryptionValidator}.
+ *
+ * @param validator the validator, may be null
+ */
+ public void setPreDecryptionValidator(@Nullable final PreDecryptionValidator validator) {
+ preDecryptionValidator = validator;
+ }
+
/**
* Get the flag which indicates whether by default the DOM Element which backs a decrypted SAML object
* will be the root of a new DOM document. Defaults to false.
@@ -572,6 +595,9 @@ public class Decrypter {
log.error("Error marshalling EncryptedData for decryption", e);
throw e;
}
+
+ preProcessEncryptedData(encryptedData, dataEncKey);
+
final Element targetElement = encryptedData.getDOM();
final XMLCipher xmlCipher;
@@ -675,6 +701,7 @@ public class Decrypter {
log.error("Error marshalling EncryptedKey for decryption: {}", e.getMessage());
throw e;
}
+
preProcessEncryptedKey(encryptedKey, algorithm, kek);
final XMLCipher xmlCipher;
@@ -717,7 +744,31 @@ public class Decrypter {
}
/**
- * Preprocess the EncryptedKey. For example, check for supported algorithms.
+ * Preprocess the EncryptedData.
+ *
+ * <p>
+ * For example, perform pre-decryption validation of the encrypted type.
+ * </p>
+ *
+ * @param encryptedData encrypted data element containing the encrypted data to be decrypted
+ * @param dataEncKey the key with which to attempt decryption of the encrypted data
+ *
+ * @throws DecryptionException exception indicating a decryption error
+ */
+ protected void preProcessEncryptedData(@Nonnull final EncryptedData encryptedData,
+ @Nonnull final Key dataEncKey) throws DecryptionException {
+
+ if (getPreDecryptionValidator() != null) {
+ getPreDecryptionValidator().validate(encryptedData);
+ }
+ }
+
+ /**
+ * Preprocess the EncryptedKey.
+ *
+ * <p>
+ * For example, perform pre-decryption validation of the encrypted type.
+ * </p>
*
* @param encryptedKey encrypted key element containing the encrypted key to be decrypted
* @param algorithm the algorithm associated with the decrypted key
@@ -728,7 +779,9 @@ public class Decrypter {
protected void preProcessEncryptedKey(@Nonnull final EncryptedKey encryptedKey, @Nonnull final String algorithm,
@Nonnull final Key kek) throws DecryptionException {
- // No-op for now. Subclasses can do things here if they want.
+ if (getPreDecryptionValidator() != null) {
+ getPreDecryptionValidator().validate(encryptedKey);
+ }
}
/**
diff --git a/opensaml-xmlsec-api/src/main/java/org/opensaml/xmlsec/encryption/support/DefaultPreDecryptionValidator.java b/opensaml-xmlsec-api/src/main/java/org/opensaml/xmlsec/encryption/support/DefaultPreDecryptionValidator.java
new file mode 100644
index 000000000..f8a9a239f
--- /dev/null
+++ b/opensaml-xmlsec-api/src/main/java/org/opensaml/xmlsec/encryption/support/DefaultPreDecryptionValidator.java
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the University Corporation for Advanced Internet Development,
+ * Inc. (UCAID) under one or more contributor license agreements. See the
+ * NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The UCAID 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.opensaml.xmlsec.encryption.support;
+
+import javax.annotation.Nonnull;
+
+import org.opensaml.xmlsec.encryption.EncryptedData;
+import org.opensaml.xmlsec.encryption.EncryptedKey;
+import org.opensaml.xmlsec.encryption.EncryptedType;
+
+/**
+ * Default implementation of {@link PreDecryptionValidator}.
+ */
+public class DefaultPreDecryptionValidator implements PreDecryptionValidator {
+
+ /** {@inheritDoc} */
+ @Override
+ public void validate(EncryptedData encryptedData) throws PreDecryptionValidationException {
+ performCommonValidation(encryptedData);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public void validate(EncryptedKey encryptedKey) throws PreDecryptionValidationException {
+ performCommonValidation(encryptedKey);
+ }
+
+ /**
+ * Perform validation common to both {@link EncryptedData} and {@link EncryptedKey}.
+ *
+ * @param encryptedType the target to validate
+ *
+ * @throws PreDecryptionValidationException if the target fails validation
+ */
+ protected void performCommonValidation(@Nonnull final EncryptedType encryptedType)
+ throws PreDecryptionValidationException {
+
+ if (encryptedType.getCipherData() == null) {
+ throw new PreDecryptionValidationException(
+ String.format("%s contains no CipherData child element, which is mandatory",
+ encryptedType.getClass().getSimpleName()));
+ }
+
+ if (encryptedType.getCipherData().getCipherReference() != null) {
+ throw new PreDecryptionValidationException(
+ String.format("%s contains a CipherReference, which is not allowed",
+ encryptedType.getClass().getSimpleName()));
+ }
+ }
+}
diff --git a/opensaml-xmlsec-api/src/main/java/org/opensaml/xmlsec/encryption/support/PreDecryptionValidationException.java b/opensaml-xmlsec-api/src/main/java/org/opensaml/xmlsec/encryption/support/PreDecryptionValidationException.java
new file mode 100644
index 000000000..1a6842b08
--- /dev/null
+++ b/opensaml-xmlsec-api/src/main/java/org/opensaml/xmlsec/encryption/support/PreDecryptionValidationException.java
@@ -0,0 +1,67 @@
+/*
+ * Licensed to the University Corporation for Advanced Internet Development,
+ * Inc. (UCAID) under one or more contributor license agreements. See the
+ * NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The UCAID 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.opensaml.xmlsec.encryption.support;
+
+import javax.annotation.Nullable;
+
+/**
+ * Exception thrown when an error occurs during pre-decryption validation.
+ */
+public class PreDecryptionValidationException extends DecryptionException {
+
+ /**
+ * Serial version UID.
+ */
+ private static final long serialVersionUID = 5697805272721615415L;
+
+ /**
+ * Constructor.
+ */
+ public PreDecryptionValidationException() {
+ super();
+ }
+
+ /**
+ * Constructor.
+ *
+ * @param message exception message
+ */
+ public PreDecryptionValidationException(@Nullable final String message) {
+ super(message);
+ }
+
+ /**
+ * Constructor.
+ *
+ * @param wrappedException exception to be wrapped by this one
+ */
+ public PreDecryptionValidationException(@Nullable final Exception wrappedException) {
+ super(wrappedException);
+ }
+
+ /**
+ * Constructor.
+ *
+ * @param message exception message
+ * @param wrappedException exception to be wrapped by this one
+ */
+ public PreDecryptionValidationException(@Nullable final String message, @Nullable final Exception wrappedException) {
+ super(message, wrappedException);
+ }
+
+}
\ No newline at end of file
diff --git a/opensaml-xmlsec-api/src/main/java/org/opensaml/xmlsec/encryption/support/PreDecryptionValidator.java b/opensaml-xmlsec-api/src/main/java/org/opensaml/xmlsec/encryption/support/PreDecryptionValidator.java
new file mode 100644
index 000000000..58f484390
--- /dev/null
+++ b/opensaml-xmlsec-api/src/main/java/org/opensaml/xmlsec/encryption/support/PreDecryptionValidator.java
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the University Corporation for Advanced Internet Development,
+ * Inc. (UCAID) under one or more contributor license agreements. See the
+ * NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The UCAID 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.opensaml.xmlsec.encryption.support;
+
+import javax.annotation.Nonnull;
+
+import org.opensaml.xmlsec.encryption.EncryptedData;
+import org.opensaml.xmlsec.encryption.EncryptedKey;
+
+/**
+ * Component which performs validation of encrypted types prior to decryption.
+ */
+public interface PreDecryptionValidator {
+
+ /**
+ * Validate an instance of {@link EncryptedData}.
+ *
+ * @param encryptedData the target to validate
+ *
+ * @throws PreDecryptionValidationException if the target fails validation
+ */
+ public void validate(@Nonnull final EncryptedData encryptedData) throws PreDecryptionValidationException;
+
+ /**
+ * Validate an instance of {@link EncryptedKey}.
+ *
+ * @param encryptedKey the target to validate
+ *
+ * @throws PreDecryptionValidationException if the target fails validation
+ */
+ public void validate(@Nonnull final EncryptedKey encryptedKey) throws PreDecryptionValidationException;
+
+}
diff --git a/opensaml-xmlsec-impl/src/test/java/org/opensaml/xmlsec/encryption/support/tests/DefaultPreDecryptionValidatorTest.java b/opensaml-xmlsec-impl/src/test/java/org/opensaml/xmlsec/encryption/support/tests/DefaultPreDecryptionValidatorTest.java
new file mode 100644
index 000000000..8c7f27b54
--- /dev/null
+++ b/opensaml-xmlsec-impl/src/test/java/org/opensaml/xmlsec/encryption/support/tests/DefaultPreDecryptionValidatorTest.java
@@ -0,0 +1,80 @@
+/*
+ * Licensed to the University Corporation for Advanced Internet Development,
+ * Inc. (UCAID) under one or more contributor license agreements. See the
+ * NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The UCAID 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.opensaml.xmlsec.encryption.support.tests;
+
+import org.opensaml.core.testing.XMLObjectBaseTestCase;
+import org.opensaml.core.xml.util.XMLObjectSupport;
+import org.opensaml.xmlsec.encryption.CipherData;
+import org.opensaml.xmlsec.encryption.CipherReference;
+import org.opensaml.xmlsec.encryption.EncryptedData;
+import org.opensaml.xmlsec.encryption.EncryptedKey;
+import org.opensaml.xmlsec.encryption.support.DefaultPreDecryptionValidator;
+import org.opensaml.xmlsec.encryption.support.PreDecryptionValidationException;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+/**
+ *
+ */
+public class DefaultPreDecryptionValidatorTest extends XMLObjectBaseTestCase {
+
+ private DefaultPreDecryptionValidator validator;
+
+ private EncryptedData encryptedData;
+ private EncryptedKey encryptedKey;
+
+ @BeforeMethod
+ public void setup() throws Exception {
+ validator = new DefaultPreDecryptionValidator();
+
+ encryptedData = (EncryptedData) XMLObjectSupport.buildXMLObject(EncryptedData.DEFAULT_ELEMENT_NAME);
+ encryptedData.setCipherData((CipherData) XMLObjectSupport.buildXMLObject(CipherData.DEFAULT_ELEMENT_NAME));
+
+ encryptedKey = (EncryptedKey) XMLObjectSupport.buildXMLObject(EncryptedKey.DEFAULT_ELEMENT_NAME);
+ encryptedKey.setCipherData((CipherData) XMLObjectSupport.buildXMLObject(CipherData.DEFAULT_ELEMENT_NAME));
+ }
+
+ @Test
+ public void goodEncryptedData() throws Exception {
+ validator.validate(encryptedData);
+ }
+
+ @Test
+ public void goodEncryptedKey() throws Exception {
+ validator.validate(encryptedKey);
+ }
+
+ @Test(expectedExceptions=PreDecryptionValidationException.class)
+ public void noCipherData() throws Exception {
+ encryptedData.setCipherData(null);
+ validator.validate(encryptedData);
+ }
+
+ @Test(expectedExceptions=PreDecryptionValidationException.class)
+ public void encryptedDataWithCipherReference() throws Exception {
+ encryptedData.getCipherData().setCipherReference((CipherReference) XMLObjectSupport.buildXMLObject(CipherReference.DEFAULT_ELEMENT_NAME));
+ validator.validate(encryptedData);
+ }
+
+ @Test(expectedExceptions=PreDecryptionValidationException.class)
+ public void encryptedKeyWithCipherReference() throws Exception {
+ encryptedKey.getCipherData().setCipherReference((CipherReference) XMLObjectSupport.buildXMLObject(CipherReference.DEFAULT_ELEMENT_NAME));
+ validator.validate(encryptedKey);
+ }
+
+}
diff --git a/opensaml-xmlsec-impl/src/test/java/org/opensaml/xmlsec/encryption/support/tests/SimpleDecryptionTest.java b/opensaml-xmlsec-impl/src/test/java/org/opensaml/xmlsec/encryption/support/tests/SimpleDecryptionTest.java
index 249b090e9..ba5f2e635 100644
--- a/opensaml-xmlsec-impl/src/test/java/org/opensaml/xmlsec/encryption/support/tests/SimpleDecryptionTest.java
+++ b/opensaml-xmlsec-impl/src/test/java/org/opensaml/xmlsec/encryption/support/tests/SimpleDecryptionTest.java
@@ -31,10 +31,12 @@ import net.shibboleth.utilities.java.support.xml.XMLParserException;
import org.opensaml.core.testing.XMLObjectBaseTestCase;
import org.opensaml.core.xml.XMLObject;
+import org.opensaml.core.xml.util.XMLObjectSupport;
import org.opensaml.security.credential.BasicCredential;
import org.opensaml.security.credential.Credential;
import org.opensaml.security.testing.SecurityProviderTestSupport;
import org.opensaml.xmlsec.algorithm.AlgorithmSupport;
+import org.opensaml.xmlsec.encryption.CipherReference;
import org.opensaml.xmlsec.encryption.EncryptedData;
import org.opensaml.xmlsec.encryption.EncryptedKey;
import org.opensaml.xmlsec.encryption.support.DataEncryptionParameters;
@@ -46,6 +48,7 @@ import org.opensaml.xmlsec.encryption.support.EncryptionConstants;
import org.opensaml.xmlsec.encryption.support.EncryptionException;
import org.opensaml.xmlsec.encryption.support.InlineEncryptedKeyResolver;
import org.opensaml.xmlsec.encryption.support.KeyEncryptionParameters;
+import org.opensaml.xmlsec.encryption.support.PreDecryptionValidationException;
import org.opensaml.xmlsec.encryption.support.RSAOAEPParameters;
import org.opensaml.xmlsec.keyinfo.KeyInfoCredentialResolver;
import org.opensaml.xmlsec.keyinfo.impl.StaticKeyInfoCredentialResolver;
@@ -165,6 +168,75 @@ public class SimpleDecryptionTest extends XMLObjectBaseTestCase {
}
+ /**
+ * Test simple decryption of an EncryptedKey object with a null pre-decryption validator.
+ */
+ @Test
+ public void testEncryptedKeyWithNoPreValidator() {
+ Decrypter decrypter = new Decrypter(null, kekResolver, null);
+
+ decrypter.setPreDecryptionValidator(null);
+
+ Key decryptedKey = null;
+ try {
+ decryptedKey = decrypter.decryptKey(encryptedKey, encURI);
+ } catch (DecryptionException e) {
+ Assert.fail("Error on decryption of EncryptedKey: " + e);
+ }
+
+ Assert.assertEquals(encKey, decryptedKey, "Decrypted EncryptedKey");
+
+ }
+
+ /**
+ * Test simple decryption of an EncryptedData object which is of type Element,
+ * with a null pre-decryption validator.
+ */
+ @Test
+ public void testEncryptedElementWithNoPrevalidator() {
+ Decrypter decrypter = new Decrypter(keyResolver, null, null);
+
+ decrypter.setPreDecryptionValidator(null);
+
+ XMLObject decryptedXMLObject = null;
+ try {
+ decryptedXMLObject = decrypter.decryptData(encryptedData);
+ } catch (DecryptionException e) {
+ Assert.fail("Error on decryption of EncryptedData to element: " + e);
+ }
+
+ assertXMLEquals(targetDOM, decryptedXMLObject);
+
+ }
+
+ /**
+ * Test EncryptedData decryption which should fail due to presence of CipherReference failing
+ * default pre-decryption validator.
+ *
+ * @throws DecryptionException
+ */
+ @Test(expectedExceptions=DecryptionException.class)
+ public void testEncryptedDataWithCipherReference() throws DecryptionException {
+ encryptedData.getCipherData().setCipherReference((CipherReference) XMLObjectSupport.buildXMLObject(CipherReference.DEFAULT_ELEMENT_NAME));
+
+ Decrypter decrypter = new Decrypter(keyResolver, null, null);
+ decrypter.decryptData(encryptedData);
+ }
+
+ /**
+ * Test EncryptedData decryption which should fail due to presence of CipherReference failing
+ * default pre-decryption validator.
+ *
+ * @throws DecryptionException
+ */
+ @Test(expectedExceptions=DecryptionException.class)
+ public void testEncryptedKeyWithCipherReference() throws DecryptionException {
+ encryptedKey.getCipherData().setCipherReference((CipherReference) XMLObjectSupport.buildXMLObject(CipherReference.DEFAULT_ELEMENT_NAME));
+
+ Decrypter decrypter = new Decrypter(null, kekResolver, null);
+ decrypter.decryptKey(encryptedKey, encURI);
+ }
+
/**
* Test EncryptedData decryption which should fail due to blacklist validation.
*
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list