Skip to content

Commit 53387e9

Browse files
coheigeacoheigea
committed
XMLCipher refactor (#587)
1 parent 9fa6d53 commit 53387e9

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
@@ -1467,6 +1467,8 @@ public Key decryptKey(EncryptedKey encryptedKey, String algorithm)
14671467
throw new XMLEncryptionException("empty", "Cannot decrypt a key without knowing the algorithm");
14681468
}
14691469

1470+
validateEncryptionMethodAlgorithm(encryptedKey.getEncryptionMethod().getAlgorithm());
1471+
14701472
if (key == null) {
14711473
LOG.debug("Trying to find a KEK via key resolvers");
14721474

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

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

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

‎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
@@ -49,6 +49,7 @@
4949
import org.apache.xml.security.encryption.EncryptionProperties;
5050
import org.apache.xml.security.encryption.EncryptionProperty;
5151
import org.apache.xml.security.encryption.XMLCipher;
52+
import org.apache.xml.security.encryption.XMLEncryptionException;
5253
import org.apache.xml.security.encryption.XMLCipherUtil;
5354
import org.apache.xml.security.encryption.keys.KeyInfoEnc;
5455
import org.apache.xml.security.encryption.params.ConcatKDFParams;
@@ -83,6 +84,7 @@
8384
import static org.junit.jupiter.api.Assertions.assertEquals;
8485
import static org.junit.jupiter.api.Assertions.assertNotNull;
8586
import static org.junit.jupiter.api.Assertions.assertNull;
87+
import static org.junit.jupiter.api.Assertions.assertThrows;
8688
import static org.junit.jupiter.api.Assumptions.assumeFalse;
8789

8890

@@ -1368,6 +1370,89 @@ void testMultipleKEKs() throws Exception {
13681370
}
13691371
}
13701372

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

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

480480
// Check the CreditCard decrypted ok
481481
nodeList = doc.getElementsByTagNameNS("urn:example:po", "CreditCard");
@@ -544,7 +544,7 @@ public void testAES256ElementRSAKWCipherUsingKEKOutbound() throws Exception {
544544

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

549549
// Check the CreditCard decrypted ok
550550
nodeList = doc.getElementsByTagNameNS("urn:example:po", "CreditCard");
@@ -613,7 +613,7 @@ public void testEncryptedKeyKeyValueReference() throws Exception {
613613

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

618618
// Check the CreditCard decrypted ok
619619
nodeList = doc.getElementsByTagNameNS("urn:example:po", "CreditCard");
@@ -683,7 +683,7 @@ public void testEncryptedKeyKeyNameReference() throws Exception {
683683

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

688688
// Check the CreditCard decrypted ok
689689
nodeList = doc.getElementsByTagNameNS("urn:example:po", "CreditCard");
@@ -751,7 +751,7 @@ public void testEncryptedKeyMultipleElements() throws Exception {
751751
assertEquals(nodeList.getLength(), 2);
752752

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

757757
@Test
@@ -818,7 +818,7 @@ public void testEncryptedKeyIssuerSerialReference() throws Exception {
818818

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

823823
// Check the CreditCard decrypted ok
824824
nodeList = doc.getElementsByTagNameNS("urn:example:po", "CreditCard");
@@ -889,7 +889,7 @@ public void testEncryptedKeyX509CertificateReference() throws Exception {
889889

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

894894
// Check the CreditCard decrypted ok
895895
nodeList = doc.getElementsByTagNameNS("urn:example:po", "CreditCard");
@@ -968,7 +968,7 @@ public void testEncryptedKeySKI() throws Exception {
968968

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

973973
// Check the CreditCard decrypted ok
974974
nodeList = doc.getElementsByTagNameNS("urn:example:po", "CreditCard");
@@ -1039,7 +1039,7 @@ public void testEncryptedKeyX509SubjectName() throws Exception {
10391039

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

10441044
// Check the CreditCard decrypted ok
10451045
nodeList = doc.getElementsByTagNameNS("urn:example:po", "CreditCard");
@@ -1110,7 +1110,7 @@ public void testEncryptedKeyNoKeyInfo() throws Exception {
11101110

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

11151115
// Check the CreditCard decrypted ok
11161116
nodeList = doc.getElementsByTagNameNS("urn:example:po", "CreditCard");
@@ -1177,7 +1177,7 @@ public void testAES192Element3DESKWCipher() throws Exception {
11771177

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

11821182
// Check the CreditCard decrypted ok
11831183
nodeList = doc.getElementsByTagNameNS("urn:example:po", "CreditCard");
@@ -1680,7 +1680,7 @@ public void testTransportKey() throws Exception {
16801680

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

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

0 commit comments

Comments
 (0)