Skip to content

XMLCipher refactor #587

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
coheigea wants to merge 1 commit into
base: main
Choose a base branch
from
+72 −12
Open

XMLCipher refactor #587

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/main/java/org/apache/xml/security/encryption/XMLCipher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1842,6 +1842,14 @@ public byte[] decryptToByteArray(Element element) throws XMLEncryptionException
EncryptedData encryptedData = factory.newEncryptedData(element);
String encMethodAlgorithm = encryptedData.getEncryptionMethod().getAlgorithm();

// Reject any attempt to decrypt with an algorithm that doesn't match the one specified when the XMLCipher was initialized
if (algorithm != null && !algorithm.equals(encMethodAlgorithm)) {
throw new XMLEncryptionException("empty",
"EncryptionMethod algorithm \"" + encMethodAlgorithm
+ "\" does not match the algorithm this XMLCipher was initialised with: \""
+ algorithm + "\"");
}

if (key == null) {
KeyInfo ki = encryptedData.getKeyInfo();
if (ki != null) {
Expand Down
52 changes: 52 additions & 0 deletions src/test/java/org/apache/xml/security/test/dom/encryption/XMLCipherTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
import org.apache.xml.security.encryption.EncryptionProperties;
import org.apache.xml.security.encryption.EncryptionProperty;
import org.apache.xml.security.encryption.XMLCipher;
import org.apache.xml.security.encryption.XMLEncryptionException;
import org.apache.xml.security.encryption.XMLCipherUtil;
import org.apache.xml.security.encryption.keys.KeyInfoEnc;
import org.apache.xml.security.encryption.params.ConcatKDFParams;
Expand Down Expand Up @@ -85,6 +86,7 @@
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assumptions.assumeFalse;


Expand Down Expand Up @@ -1367,6 +1369,56 @@ void testMultipleKEKs() throws Exception {
}
}

/**
* Test that you can't substitute an encryption algorithm in the EncryptionMethod and have it be accepted by the decryptor.
*/
@Test
void testAlgorithmSubstitutionNotDetected() throws Exception {
Assumptions.assumeTrue(haveISOPadding, "ISO padding not available, skipping VULN-1 test");

// Fixed 256-bit server key.
byte[] bits256 = {
(byte)0x00, (byte)0x01, (byte)0x02, (byte)0x03,
(byte)0x04, (byte)0x05, (byte)0x06, (byte)0x07,
(byte)0x08, (byte)0x09, (byte)0x0A, (byte)0x0B,
(byte)0x0C, (byte)0x0D, (byte)0x0E, (byte)0x0F,
(byte)0x10, (byte)0x11, (byte)0x12, (byte)0x13,
(byte)0x14, (byte)0x15, (byte)0x16, (byte)0x17,
(byte)0x18, (byte)0x19, (byte)0x1A, (byte)0x1B,
(byte)0x1C, (byte)0x1D, (byte)0x1E, (byte)0x1F
};
Key serverKey = new SecretKeySpec(bits256, "AES");

Document d = document();
Element e = (Element) d.getElementsByTagName(element()).item(index());

// Step 1 – server encrypts with AES-256-CBC.
cipher = XMLCipher.getInstance(XMLCipher.AES_256);
cipher.init(XMLCipher.ENCRYPT_MODE, serverKey);
Document encryptedDoc = cipher.doFinal(d, e);

Element encData = (Element) encryptedDoc.getElementsByTagName("xenc:EncryptedData").item(0);
Element encMethod = (Element) encData.getElementsByTagName("xenc:EncryptionMethod").item(0);
assertEquals(XMLCipher.AES_256, encMethod.getAttribute("Algorithm"),
"Sanity check: encrypted document should advertise AES-256-CBC");

// Step 2 – attacker tampers the EncryptionMethod to claim AES-128-CBC.
encMethod.setAttribute("Algorithm", XMLCipher.AES_128);

// Step 3 – server decrypts using its AES-256-CBC XMLCipher.
XMLCipher serverDecryptor = XMLCipher.getInstance(XMLCipher.AES_256);
serverDecryptor.init(XMLCipher.DECRYPT_MODE, serverKey);

XMLEncryptionException thrown = assertThrows(XMLEncryptionException.class,
() -> serverDecryptor.doFinal(encryptedDoc, encData),
"Expected XMLEncryptionException for algorithm substitution AES-256-CBC -> AES-128-CBC");

// The error must originate from algorithm validation, not from a downstream
// JCE operation, so the cause must be null (it is a pure logic rejection).
assertNull(thrown.getCause(),
"Algorithm mismatch must be detected upfront, not wrapped around a JCE exception");
}

private String toString (Node n) throws Exception {
ByteArrayOutputStream baos = new ByteArrayOutputStream();
Canonicalizer c14n = Canonicalizer.getInstance(Canonicalizer.ALGO_ID_C14N_OMIT_COMMENTS);
Expand Down
24 changes: 12 additions & 12 deletions src/test/java/org/apache/xml/security/test/stax/encryption/EncryptionCreationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ void testAES128ElementAES192KWCipherUsingKEKOutbound() throws Exception {

// Decrypt using DOM API
Document doc =
decryptUsingDOM("http://www.w3.org/2001/04/xmlenc#tripledes-cbc", null, transportKey, document);
decryptUsingDOM("http://www.w3.org/2001/04/xmlenc#aes128-cbc", null, transportKey, document);

// Check the CreditCard decrypted ok
nodeList = doc.getElementsByTagNameNS("urn:example:po", "CreditCard");
Expand Down Expand Up @@ -460,7 +460,7 @@ void testAES256ElementRSAKWCipherUsingKEKOutbound() throws Exception {

// Decrypt using DOM API
Document doc =
decryptUsingDOM("http://www.w3.org/2001/04/xmlenc#tripledes-cbc", null, priv, document);
decryptUsingDOM("http://www.w3.org/2001/04/xmlenc#aes256-cbc", null, priv, document);

// Check the CreditCard decrypted ok
nodeList = doc.getElementsByTagNameNS("urn:example:po", "CreditCard");
Expand Down Expand Up @@ -518,7 +518,7 @@ void testEncryptedKeyKeyValueReference() throws Exception {

// Decrypt using DOM API
Document doc =
decryptUsingDOM("http://www.w3.org/2001/04/xmlenc#tripledes-cbc", null, priv, document);
decryptUsingDOM("http://www.w3.org/2001/04/xmlenc#aes256-cbc", null, priv, document);

// Check the CreditCard decrypted ok
nodeList = doc.getElementsByTagNameNS("urn:example:po", "CreditCard");
Expand Down Expand Up @@ -577,7 +577,7 @@ void testEncryptedKeyKeyNameReference() throws Exception {

// Decrypt using DOM API
Document doc =
decryptUsingDOM("http://www.w3.org/2001/04/xmlenc#tripledes-cbc", null, priv, document);
decryptUsingDOM("http://www.w3.org/2001/04/xmlenc#aes256-cbc", null, priv, document);

// Check the CreditCard decrypted ok
nodeList = doc.getElementsByTagNameNS("urn:example:po", "CreditCard");
Expand Down Expand Up @@ -634,7 +634,7 @@ void testEncryptedKeyMultipleElements() throws Exception {
assertEquals(nodeList.getLength(), 2);

// Decrypt using DOM API
decryptUsingDOM("http://www.w3.org/2001/04/xmlenc#tripledes-cbc", null, priv, document);
decryptUsingDOM("http://www.w3.org/2001/04/xmlenc#aes256-cbc", null, priv, document);
}

@Test
Expand Down Expand Up @@ -686,7 +686,7 @@ void testEncryptedKeyIssuerSerialReference() throws Exception {

// Decrypt using DOM API
Document doc =
decryptUsingDOM("http://www.w3.org/2001/04/xmlenc#tripledes-cbc", null, priv, document);
decryptUsingDOM("http://www.w3.org/2001/04/xmlenc#aes256-cbc", null, priv, document);

// Check the CreditCard decrypted ok
nodeList = doc.getElementsByTagNameNS("urn:example:po", "CreditCard");
Expand Down Expand Up @@ -742,7 +742,7 @@ void testEncryptedKeyX509CertificateReference() throws Exception {

// Decrypt using DOM API
Document doc =
decryptUsingDOM("http://www.w3.org/2001/04/xmlenc#tripledes-cbc", null, priv, document);
decryptUsingDOM("http://www.w3.org/2001/04/xmlenc#aes256-cbc", null, priv, document);

// Check the CreditCard decrypted ok
nodeList = doc.getElementsByTagNameNS("urn:example:po", "CreditCard");
Expand Down Expand Up @@ -809,7 +809,7 @@ void testEncryptedKeySKI() throws Exception {

// Decrypt using DOM API
Document doc =
decryptUsingDOM("http://www.w3.org/2001/04/xmlenc#tripledes-cbc", null, priv, document);
decryptUsingDOM("http://www.w3.org/2001/04/xmlenc#aes256-cbc", null, priv, document);

// Check the CreditCard decrypted ok
nodeList = doc.getElementsByTagNameNS("urn:example:po", "CreditCard");
Expand Down Expand Up @@ -865,7 +865,7 @@ void testEncryptedKeyX509SubjectName() throws Exception {

// Decrypt using DOM API
Document doc =
decryptUsingDOM("http://www.w3.org/2001/04/xmlenc#tripledes-cbc", null, priv, document);
decryptUsingDOM("http://www.w3.org/2001/04/xmlenc#aes256-cbc", null, priv, document);

// Check the CreditCard decrypted ok
nodeList = doc.getElementsByTagNameNS("urn:example:po", "CreditCard");
Expand Down Expand Up @@ -921,7 +921,7 @@ void testEncryptedKeyNoKeyInfo() throws Exception {

// Decrypt using DOM API
Document doc =
decryptUsingDOM("http://www.w3.org/2001/04/xmlenc#tripledes-cbc", null, priv, document);
decryptUsingDOM("http://www.w3.org/2001/04/xmlenc#aes256-cbc", null, priv, document);

// Check the CreditCard decrypted ok
nodeList = doc.getElementsByTagNameNS("urn:example:po", "CreditCard");
Expand Down Expand Up @@ -977,7 +977,7 @@ void testAES192Element3DESKWCipher() throws Exception {

// Decrypt using DOM API
Document doc =
decryptUsingDOM("http://www.w3.org/2001/04/xmlenc#tripledes-cbc", null, transportKey, document);
decryptUsingDOM("http://www.w3.org/2001/04/xmlenc#aes192-cbc", null, transportKey, document);

// Check the CreditCard decrypted ok
nodeList = doc.getElementsByTagNameNS("urn:example:po", "CreditCard");
Expand Down Expand Up @@ -1411,7 +1411,7 @@ void testTransportKey() throws Exception {

// Decrypt using DOM API
Document doc =
decryptUsingDOM("http://www.w3.org/2001/04/xmlenc#tripledes-cbc", null, transportKey, document);
decryptUsingDOM("http://www.w3.org/2001/04/xmlenc#aes128-cbc", null, transportKey, document);

// Check the CreditCard decrypted ok
nodeList = doc.getElementsByTagNameNS("urn:example:po", "CreditCard");
Expand Down
Loading