Skip to content

Commit 96fccba

Browse files
coheigeacoheigea
committed
XMLCipher refactor
1 parent be9040d commit 96fccba

3 files changed

Lines changed: 111 additions & 12 deletions

File tree

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1468,6 +1468,8 @@ public Key decryptKey(EncryptedKey encryptedKey, String algorithm)
14681468
throw new XMLEncryptionException("empty", "Cannot decrypt a key without knowing the algorithm");
14691469
}
14701470

1471+
validateEncryptionMethodAlgorithm(encryptedKey.getEncryptionMethod().getAlgorithm());
1472+
14711473
if (key == null) {
14721474
LOG.log(Level.DEBUG, "Trying to find a KEK via key resolvers");
14731475

@@ -1842,6 +1844,9 @@ public byte[] decryptToByteArray(Element element) throws XMLEncryptionException
18421844
EncryptedData encryptedData = factory.newEncryptedData(element);
18431845
String encMethodAlgorithm = encryptedData.getEncryptionMethod().getAlgorithm();
18441846

1847+
// Reject any attempt to decrypt with an algorithm that doesn't match the one specified when the XMLCipher was initialized
1848+
validateEncryptionMethodAlgorithm(encMethodAlgorithm);
1849+
18451850
if (key == null) {
18461851
KeyInfo ki = encryptedData.getKeyInfo();
18471852
if (ki != null) {
@@ -1914,6 +1919,15 @@ public byte[] decryptToByteArray(Element element) throws XMLEncryptionException
19141919
}
19151920
}
19161921

1922+
private void validateEncryptionMethodAlgorithm(String encryptionMethodAlgorithm) throws XMLEncryptionException {
1923+
if (algorithm != null && !algorithm.equals(encryptionMethodAlgorithm)) {
1924+
throw new XMLEncryptionException("empty",
1925+
"EncryptionMethod algorithm \"" + encryptionMethodAlgorithm
1926+
+ "\" does not match the algorithm this XMLCipher was initialised with: \""
1927+
+ algorithm + "\"");
1928+
}
1929+
}
1930+
19171931
/*
19181932
* Expose the interface for creating XML Encryption objects
19191933
*/

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

Lines changed: 85 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,89 @@ 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+
// Fixed 256-bit server key.
1378+
byte[] bits256 = {
1379+
(byte)0x00, (byte)0x01, (byte)0x02, (byte)0x03,
1380+
(byte)0x04, (byte)0x05, (byte)0x06, (byte)0x07,
1381+
(byte)0x08, (byte)0x09, (byte)0x0A, (byte)0x0B,
1382+
(byte)0x0C, (byte)0x0D, (byte)0x0E, (byte)0x0F,
1383+
(byte)0x10, (byte)0x11, (byte)0x12, (byte)0x13,
1384+
(byte)0x14, (byte)0x15, (byte)0x16, (byte)0x17,
1385+
(byte)0x18, (byte)0x19, (byte)0x1A, (byte)0x1B,
1386+
(byte)0x1C, (byte)0x1D, (byte)0x1E, (byte)0x1F
1387+
};
1388+
Key serverKey = new SecretKeySpec(bits256, "AES");
1389+
1390+
Document d = document();
1391+
Element e = (Element) d.getElementsByTagName(element()).item(index());
1392+
1393+
// Step 1 – server encrypts with AES-256-CBC.
1394+
cipher = XMLCipher.getInstance(XMLCipher.AES_256);
1395+
cipher.init(XMLCipher.ENCRYPT_MODE, serverKey);
1396+
Document encryptedDoc = cipher.doFinal(d, e);
1397+
1398+
Element encData = (Element) encryptedDoc.getElementsByTagName("xenc:EncryptedData").item(0);
1399+
Element encMethod = (Element) encData.getElementsByTagName("xenc:EncryptionMethod").item(0);
1400+
assertEquals(XMLCipher.AES_256, encMethod.getAttribute("Algorithm"),
1401+
"Sanity check: encrypted document should advertise AES-256-CBC");
1402+
1403+
// Step 2 – attacker tampers the EncryptionMethod to claim AES-128-CBC.
1404+
encMethod.setAttribute("Algorithm", XMLCipher.AES_128);
1405+
1406+
// Step 3 – server decrypts using its AES-256-CBC XMLCipher.
1407+
XMLCipher serverDecryptor = XMLCipher.getInstance(XMLCipher.AES_256);
1408+
serverDecryptor.init(XMLCipher.DECRYPT_MODE, serverKey);
1409+
1410+
XMLEncryptionException thrown = assertThrows(XMLEncryptionException.class,
1411+
() -> serverDecryptor.doFinal(encryptedDoc, encData),
1412+
"Expected XMLEncryptionException for algorithm substitution AES-256-CBC -> AES-128-CBC");
1413+
1414+
// The error must originate from algorithm validation, not from a downstream
1415+
// JCE operation, so the cause must be null (it is a pure logic rejection).
1416+
assertNull(thrown.getCause(),
1417+
"Algorithm mismatch must be detected upfront, not wrapped around a JCE exception");
1418+
}
1419+
1420+
@Test
1421+
void testKeyWrapAlgorithmSubstitutionNotDetected() throws Exception {
1422+
KeyGenerator wrapKeyGenerator = KeyGenerator.getInstance("AES");
1423+
wrapKeyGenerator.init(192);
1424+
Key wrappingKey = wrapKeyGenerator.generateKey();
1425+
1426+
KeyGenerator trafficKeyGenerator = KeyGenerator.getInstance("AES");
1427+
trafficKeyGenerator.init(128);
1428+
Key trafficKey = trafficKeyGenerator.generateKey();
1429+
1430+
XMLCipher wrapCipher = XMLCipher.getInstance(XMLCipher.AES_192_KeyWrap);
1431+
wrapCipher.init(XMLCipher.WRAP_MODE, wrappingKey);
1432+
1433+
Document d = document();
1434+
EncryptedKey encryptedKey = wrapCipher.encryptKey(d, trafficKey);
1435+
Element encryptedKeyElement = wrapCipher.martial(encryptedKey);
1436+
1437+
Element encMethod = (Element) encryptedKeyElement.getElementsByTagName("xenc:EncryptionMethod").item(0);
1438+
assertEquals(XMLCipher.AES_192_KeyWrap, encMethod.getAttribute("Algorithm"),
1439+
"Sanity check: encrypted key should advertise AES-192 key wrap");
1440+
1441+
encMethod.setAttribute("Algorithm", XMLCipher.AES_128_KeyWrap);
1442+
1443+
XMLCipher unwrapCipher = XMLCipher.getInstance(XMLCipher.AES_192_KeyWrap);
1444+
unwrapCipher.init(XMLCipher.UNWRAP_MODE, wrappingKey);
1445+
EncryptedKey tamperedEncryptedKey = unwrapCipher.loadEncryptedKey(encryptedKeyElement);
1446+
1447+
XMLEncryptionException thrown = assertThrows(XMLEncryptionException.class,
1448+
() -> unwrapCipher.decryptKey(tamperedEncryptedKey, XMLCipher.AES_128),
1449+
"Expected XMLEncryptionException for key-wrap algorithm substitution AES-192-KW -> AES-128-KW");
1450+
1451+
assertNull(thrown.getCause(),
1452+
"Algorithm mismatch must be detected upfront, not wrapped around a JCE exception");
1453+
}
1454+
13701455
private String toString (Node n) throws Exception {
13711456
ByteArrayOutputStream baos = new ByteArrayOutputStream();
13721457
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)