Skip to content

Reject malformed ML-KEM key-transport metadata with XMLEncryptionException #6

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

Merged
ffang merged 1 commit into from
Aug 26, 2026
Merged

Reject malformed ML-KEM key-transport metadata with XMLEncryptionException #6

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
26 changes: 23 additions & 3 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 @@ -2607,7 +2607,7 @@ KeyInfo newKeyInfo(Element element) throws XMLEncryptionException {
* @param element
* @return a new EncryptionMethod
*/
EncryptionMethod newEncryptionMethod(Element element) {
EncryptionMethod newEncryptionMethod(Element element) throws XMLEncryptionException {
String encAlgorithm = element.getAttributeNS(null, EncryptionConstants._ATT_ALGORITHM);
EncryptionMethod result = newEncryptionMethod(encAlgorithm);

Expand Down Expand Up @@ -2668,7 +2668,7 @@ EncryptionMethod newEncryptionMethod(Element element) {
result.setKeyEncapsulationKeyDerivationMethod(
new KeyDerivationMethodImpl(keyDerivationMethodElement, null));
} catch (XMLSecurityException xse) {
throw new RuntimeException(xse);
throw new XMLEncryptionException(xse);
}
}

Expand All @@ -2678,7 +2678,7 @@ EncryptionMethod newEncryptionMethod(Element element) {
EncryptionConstants._TAG_KEYLEN).item(0);
if (keyLenElement != null) {
result.setKeyEncapsulationKeyLength(
Integer.parseInt(keyLenElement.getFirstChild().getNodeValue()));
parseKeyEncapsulationKeyLength(keyLenElement));
}
}

Expand All @@ -2698,6 +2698,26 @@ EncryptionMethod newEncryptionMethod(Element element) {
return result;
}

/**
* Parses the {@code ghc:KeyLen} element content. The element is read from the
* (untrusted) message, so a missing, empty or non-numeric value is reported as an
* {@link XMLEncryptionException}, the exception type the decrypt API declares, rather
* than escaping as a {@code NullPointerException} or {@code NumberFormatException}.
*/
private int parseKeyEncapsulationKeyLength(Element keyLenElement) throws XMLEncryptionException {
Node child = keyLenElement.getFirstChild();
String text = child == null ? null : child.getNodeValue();
if (text == null || text.trim().isEmpty()) {
throw new XMLEncryptionException("KeyDerivation.InvalidParameter", EncryptionConstants._TAG_KEYLEN);
}
try {
return Integer.parseInt(text.trim());
} catch (NumberFormatException e) {
throw new XMLEncryptionException(e, "KeyDerivation.InvalidParameter",
new Object[]{EncryptionConstants._TAG_KEYLEN});
}
}

/**
* @param element
* @return a new EncryptionProperties
Expand Down
21 changes: 19 additions & 2 deletions src/main/java/org/apache/xml/security/encryption/XMLCipherUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.apache.xml.security.encryption.params.KeyAgreementParameters;
import org.apache.xml.security.encryption.params.KeyDerivationParameters;
import org.apache.xml.security.exceptions.XMLSecurityException;
import org.apache.xml.security.utils.Constants;
import org.apache.xml.security.utils.EncryptionConstants;
import org.apache.xml.security.utils.KeyUtils;
import org.w3c.dom.Document;
Expand Down Expand Up @@ -279,13 +280,29 @@ public static KeyDerivationParameters constructKeyDerivationParameter(KeyDerivat
}
HKDFParamsImpl hKDFParams = (HKDFParamsImpl) kdfParams;
return HKDFParams.createBuilder(keyBitLength, hKDFParams.getPRFAlgorithm())
.salt(hKDFParams.getSalt() != null ? Base64.getDecoder().decode(hKDFParams.getSalt()) : null)
.info(hKDFParams.getInfo() != null ? Base64.getDecoder().decode(hKDFParams.getInfo()) : null)
.salt(decodeBase64Parameter(hKDFParams.getSalt(), Constants._TAG_SALT))
.info(decodeBase64Parameter(hKDFParams.getInfo(), EncryptionConstants._TAG_INFO))
.build();
}
throw new XMLEncryptionException("unknownAlgorithm", keyDerivationAlgorithm);
}

/**
* Base64-decodes an optional key derivation parameter read from the message. Malformed
* base64 is reported as an {@link XMLEncryptionException} rather than escaping as the
* {@link IllegalArgumentException} thrown by {@link Base64.Decoder#decode(String)}.
*/
private static byte[] decodeBase64Parameter(String value, String parameterName) throws XMLEncryptionException {
if (value == null) {
return null;
}
try {
return Base64.getDecoder().decode(value);
} catch (IllegalArgumentException e) {
throw new XMLEncryptionException(e, "KeyDerivation.InvalidParameter", new Object[]{parameterName});
}
}

/**
* Construct a {@code KeyDerivationMethod} DOM element from the given {@link KeyDerivationParameters}.
* The inverse of {@link #constructKeyDerivationParameter(KeyDerivationMethod, int)}. Supports the same
Expand Down
93 changes: 91 additions & 2 deletions src/test/java/org/apache/xml/security/test/dom/encryption/XMLEncryptionMLKEMTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.Assumptions;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.CsvSource;
import org.w3c.dom.Document;
Expand All @@ -53,6 +54,7 @@
import org.w3c.dom.Text;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;

Expand All @@ -79,6 +81,9 @@ class XMLEncryptionMLKEMTest {
private static boolean mlKemAvailable;
private static boolean bcAddedForTheTest;

/** Namespace of the HKDFParams / Salt / Info elements (xmldsig-more, 2021). */
private static final String XMLDSIG_MORE_NS = "http://www.w3.org/2021/04/xmldsig-more#";

private static java.util.Map<String, KeyPair> keyPairs = new java.util.HashMap<>();

@BeforeAll
Expand Down Expand Up @@ -311,6 +316,13 @@ void testMLKEMTruncatedEncapsulationRejected(String keyEncapsulationUri, String
* for tests that want to corrupt or otherwise interfere with the decrypt half.
*/
private byte[] encryptToRecipient(PublicKey pubKey, String keyEncapsulationUri) throws Exception {
int wrapKeyBitLength = KeyUtils.getAESKeyBitSizeForWrapAlgorithm(EncryptionConstants.ALGO_ID_KEYWRAP_AES256);
HKDFParams kdfParams = HKDFParams.createBuilder(wrapKeyBitLength, XMLSignature.ALGO_ID_MAC_HMAC_SHA256).build();
return encryptToRecipient(pubKey, keyEncapsulationUri, kdfParams);
}

private byte[] encryptToRecipient(PublicKey pubKey, String keyEncapsulationUri, HKDFParams kdfParams)
throws Exception {
DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
dbf.setNamespaceAware(true);
Document doc = dbf.newDocumentBuilder().newDocument();
Expand All @@ -323,8 +335,6 @@ private byte[] encryptToRecipient(PublicKey pubKey, String keyEncapsulationUri)
SecretKey cek = kg.generateKey();

String kwAlgorithm = EncryptionConstants.ALGO_ID_KEYWRAP_AES256;
int wrapKeyBitLength = KeyUtils.getAESKeyBitSizeForWrapAlgorithm(kwAlgorithm);
HKDFParams kdfParams = HKDFParams.createBuilder(wrapKeyBitLength, XMLSignature.ALGO_ID_MAC_HMAC_SHA256).build();
AlgorithmParameterSpec keyEncapsulationParameters =
new KeyEncapsulationParameters(keyEncapsulationUri, kdfParams);

Expand All @@ -350,6 +360,85 @@ private byte[] encryptToRecipient(PublicKey pubKey, String keyEncapsulationUri)
return bos.toByteArray();
}


/**
* Malformed key-transport metadata in the EncryptedKey must be rejected as
* {@link XMLEncryptionException}, the decrypt API's declared failure type, rather than
* escaping as a NumberFormatException (non-numeric or empty {@code ghc:KeyLen}) or an
* IllegalArgumentException (malformed base64 in the HKDF {@code Salt} or {@code Info}).
* All of these values are parsed from the untrusted message before any private-key operation.
*/
@ParameterizedTest
@CsvSource({
EncryptionConstants.EncryptionSpecGHCNS + ",KeyLen,notanumber",
EncryptionConstants.EncryptionSpecGHCNS + ",KeyLen,''",
XMLDSIG_MORE_NS + ",Salt,!!!not-base64!!!",
XMLDSIG_MORE_NS + ",Info,@@@@"
})
void testMLKEMMalformedKeyTransportMetadataRejected(String namespace, String localName, String badText)
throws Exception {
Assumptions.assumeTrue(mlKemAvailable, "ML-KEM requires BouncyCastle 1.84+ and Java 21+ (javax.crypto.KEM)");

KeyPair kp = keyPairs.get("ML-KEM-768");
Document encDoc = parse(encryptToRecipientWithHkdfSaltAndInfo(kp.getPublic()));
Element target = (Element) encDoc.getElementsByTagNameNS(namespace, localName).item(0);
assertNotNull(target, "expected a <" + localName + "> element to mutate");
replaceTextContent(encDoc, target, badText);

assertThrows(XMLEncryptionException.class, () -> decryptDocument(encDoc, kp.getPrivate()));
}

/**
* A childless {@code <ghc:KeyLen/>} has no text node at all; reading it must not surface as
* a NullPointerException from the decrypt path.
*/
@Test
void testMLKEMChildlessKeyLenRejected() throws Exception {
Assumptions.assumeTrue(mlKemAvailable, "ML-KEM requires BouncyCastle 1.84+ and Java 21+ (javax.crypto.KEM)");

KeyPair kp = keyPairs.get("ML-KEM-768");
Document encDoc = parse(encryptToRecipient(kp.getPublic(), EncryptionConstants.ALGO_ID_KEYTRANSPORT_MLKEM_768));
Element keyLen = (Element) encDoc.getElementsByTagNameNS(
EncryptionConstants.EncryptionSpecGHCNS, "KeyLen").item(0);
assertNotNull(keyLen, "expected a <KeyLen> element to mutate");
while (keyLen.hasChildNodes()) {
keyLen.removeChild(keyLen.getFirstChild());
}

assertThrows(XMLEncryptionException.class, () -> decryptDocument(encDoc, kp.getPrivate()));
}

/** Encrypts with an HKDF that carries explicit Salt and Info elements, so they exist to mutate. */
private byte[] encryptToRecipientWithHkdfSaltAndInfo(PublicKey pubKey) throws Exception {
int wrapKeyBitLength = KeyUtils.getAESKeyBitSizeForWrapAlgorithm(EncryptionConstants.ALGO_ID_KEYWRAP_AES256);
HKDFParams kdfParams = HKDFParams.createBuilder(wrapKeyBitLength, XMLSignature.ALGO_ID_MAC_HMAC_SHA256)
.salt(new byte[]{1, 2, 3, 4, 5, 6, 7, 8})
.info(new byte[]{9, 10, 11, 12})
.build();
return encryptToRecipient(pubKey, EncryptionConstants.ALGO_ID_KEYTRANSPORT_MLKEM_768, kdfParams);
}

private Document parse(byte[] xml) throws Exception {
DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
dbf.setNamespaceAware(true);
return dbf.newDocumentBuilder().parse(new java.io.ByteArrayInputStream(xml));
}

/** The full decrypt path: load the EncryptedData, unwrap the CEK with the ML-KEM private key, decrypt. */
private void decryptDocument(Document encDoc, PrivateKey privKey) throws Exception {
Element encDataElem = (Element) encDoc.getElementsByTagNameNS(
EncryptionConstants.EncryptionSpecNS, "EncryptedData").item(0);
XMLCipher decryptCipher = XMLCipher.getInstance();
decryptCipher.init(XMLCipher.DECRYPT_MODE, null);
EncryptedData encData = decryptCipher.loadEncryptedData(encDoc, encDataElem);
EncryptedKey ek = encData.getKeyInfo().itemEncryptedKey(0);
XMLCipher unwrapCipher = XMLCipher.getInstance();
unwrapCipher.init(XMLCipher.UNWRAP_MODE, privKey);
Key cek = unwrapCipher.decryptKey(ek, encData.getEncryptionMethod().getAlgorithm());
decryptCipher.init(XMLCipher.DECRYPT_MODE, cek);
decryptCipher.doFinal(encDoc, encDataElem);
}

private void replaceTextContent(Document doc, Element element, String newText) {
NodeList children = element.getChildNodes();
for (int i = children.getLength() - 1; i >= 0; i--) {
Expand Down