Skip to content

Commit 3d947b5

Browse files
coheigeacoheigea
committed
XMLCipher refactor
1 parent cc6c705 commit 3d947b5

3 files changed

Lines changed: 72 additions & 12 deletions

File tree

‎src/main/java/org/apache/xml/security/encryption/XMLCipher.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1842,6 +1842,14 @@ public byte[] decryptToByteArray(Element element) throws XMLEncryptionException
18421842
EncryptedData encryptedData = factory.newEncryptedData(element);
18431843
String encMethodAlgorithm = encryptedData.getEncryptionMethod().getAlgorithm();
18441844

1845+
// Reject any attempt to decrypt with an algorithm that doesn't match the one specified when the XMLCipher was initialized
1846+
if (algorithm != null && !algorithm.equals(encMethodAlgorithm)) {
1847+
throw new XMLEncryptionException("empty",
1848+
"EncryptionMethod algorithm \"" + encMethodAlgorithm
1849+
+ "\" does not match the algorithm this XMLCipher was initialised with: \""
1850+
+ algorithm + "\"");
1851+
}
1852+
18451853
if (key == null) {
18461854
KeyInfo ki = encryptedData.getKeyInfo();
18471855
if (ki != null) {

‎src/test/java/org/apache/xml/security/test/dom/encryption/XMLCipherTest.java‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151
import org.apache.xml.security.encryption.EncryptionProperties;
5252
import org.apache.xml.security.encryption.EncryptionProperty;
5353
import org.apache.xml.security.encryption.XMLCipher;
54+
import org.apache.xml.security.encryption.XMLEncryptionException;
5455
import org.apache.xml.security.encryption.XMLCipherUtil;
5556
import org.apache.xml.security.encryption.keys.KeyInfoEnc;
5657
import org.apache.xml.security.encryption.params.ConcatKDFParams;
@@ -85,6 +86,7 @@
8586
import static org.junit.jupiter.api.Assertions.assertEquals;
8687
import static org.junit.jupiter.api.Assertions.assertNotNull;
8788
import static org.junit.jupiter.api.Assertions.assertNull;
89+
import static org.junit.jupiter.api.Assertions.assertThrows;
8890
import static org.junit.jupiter.api.Assumptions.assumeFalse;
8991

9092

@@ -1367,6 +1369,56 @@ void testMultipleKEKs() throws Exception {
13671369
}
13681370
}
13691371

1372+
/**
1373+
* Test that you can't substitute an encryption algorithm in the EncryptionMethod and have it be accepted by the decryptor.
1374+
*/
1375+
@Test
1376+
void testAlgorithmSubstitutionNotDetected() throws Exception {
1377+
Assumptions.assumeTrue(haveISOPadding, "ISO padding not available, skipping VULN-1 test");
1378+
1379+
// Fixed 256-bit server key.
1380+
byte[] bits256 = {
1381+
(byte)0x00, (byte)0x01, (byte)0x02, (byte)0x03,
1382+
(byte)0x04, (byte)0x05, (byte)0x06, (byte)0x07,
1383+
(byte)0x08, (byte)0x09, (byte)0x0A, (byte)0x0B,
1384+
(byte)0x0C, (byte)0x0D, (byte)0x0E, (byte)0x0F,
1385+
(byte)0x10, (byte)0x11, (byte)0x12, (byte)0x13,
1386+
(byte)0x14, (byte)0x15, (byte)0x16, (byte)0x17,
1387+
(byte)0x18, (byte)0x19, (byte)0x1A, (byte)0x1B,
1388+
(byte)0x1C, (byte)0x1D, (byte)0x1E, (byte)0x1F
1389+
};
1390+
Key serverKey = new SecretKeySpec(bits256, "AES");
1391+
1392+
Document d = document();
1393+
Element e = (Element) d.getElementsByTagName(element()).item(index());
1394+
1395+
// Step 1 – server encrypts with AES-256-CBC.
1396+
cipher = XMLCipher.getInstance(XMLCipher.AES_256);
1397+
cipher.init(XMLCipher.ENCRYPT_MODE, serverKey);
1398+
Document encryptedDoc = cipher.doFinal(d, e);
1399+
1400+
Element encData = (Element) encryptedDoc.getElementsByTagName("xenc:EncryptedData").item(0);
1401+
Element encMethod = (Element) encData.getElementsByTagName("xenc:EncryptionMethod").item(0);
1402+
assertEquals(XMLCipher.AES_256, encMethod.getAttribute("Algorithm"),
1403+
"Sanity check: encrypted document should advertise AES-256-CBC");
1404+
1405+
// Step 2 – attacker tampers the EncryptionMethod to claim AES-128-CBC.
1406+
encMethod.setAttribute("Algorithm", XMLCipher.AES_128);
1407+
1408+
// Step 3 – server decrypts using its AES-256-CBC XMLCipher.
1409+
XMLCipher serverDecryptor = XMLCipher.getInstance(XMLCipher.AES_256);
1410+
serverDecryptor.init(XMLCipher.DECRYPT_MODE, serverKey);
1411+
1412+
XMLEncryptionException thrown = assertThrows(XMLEncryptionException.class,
1413+
() -> serverDecryptor.doFinal(encryptedDoc, encData),
1414+
"Expected XMLEncryptionException for algorithm substitution AES-256-CBC -> AES-128-CBC");
1415+
1416+
// The error must originate from algorithm validation, not from a downstream
1417+
// JCE operation, so the cause must be null (it is a pure logic rejection).
1418+
assertNull(thrown.getCause(),
1419+
"Algorithm mismatch must be detected upfront, not wrapped around a JCE exception");
1420+
}
1421+
13701422
private String toString (Node n) throws Exception {
13711423
ByteArrayOutputStream baos = new ByteArrayOutputStream();
13721424
Canonicalizer c14n = Canonicalizer.getInstance(Canonicalizer.ALGO_ID_C14N_OMIT_COMMENTS);

‎src/test/java/org/apache/xml/security/test/stax/encryption/EncryptionCreationTest.java‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -402,7 +402,7 @@ void testAES128ElementAES192KWCipherUsingKEKOutbound() throws Exception {
402402

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

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

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

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

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

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

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

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

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

640640
@Test
@@ -686,7 +686,7 @@ void testEncryptedKeyIssuerSerialReference() throws Exception {
686686

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

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

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

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

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

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

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

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

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

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

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

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

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

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

0 commit comments

Comments
 (0)