From c0de22314614735cfee136cc88b541f4d621b74b Mon Sep 17 00:00:00 2001 From: arpansharma Date: Sat, 22 Aug 2026 15:45:12 -0500 Subject: [PATCH] Reject malformed HKDF Salt and Info with XMLEncryptionException On the decrypt path of an ECDH-ES (or X25519/X448) key agreement, the HKDF Salt and Info elements of the KeyDerivationMethod are read from the message and base64-decoded in XMLCipherUtil.constructKeyDerivationParameter. Malformed base64 there threw the IllegalArgumentException from Base64.Decoder, escaping the XMLEncryptionException the decrypt API declares, so a caller handling the declared type would not catch it. Wrap the decode and report it as XMLEncryptionException using the existing KeyDerivation.InvalidParameter message. The same strict decoder is used, so well-formed input is unaffected. Adds a test with a positive control and the two malformed cases; both fail without this change. --- .../security/encryption/XMLCipherUtil.java | 21 ++- ...erKeyAgreementMalformedHKDFParamsTest.java | 170 ++++++++++++++++++ 2 files changed, 189 insertions(+), 2 deletions(-) create mode 100644 src/test/java/org/apache/xml/security/test/dom/encryption/XMLCipherKeyAgreementMalformedHKDFParamsTest.java diff --git a/src/main/java/org/apache/xml/security/encryption/XMLCipherUtil.java b/src/main/java/org/apache/xml/security/encryption/XMLCipherUtil.java index 95842b139..24a945032 100644 --- a/src/main/java/org/apache/xml/security/encryption/XMLCipherUtil.java +++ b/src/main/java/org/apache/xml/security/encryption/XMLCipherUtil.java @@ -27,6 +27,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; @@ -277,13 +278,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}); + } + } + /** * Method hexStringToByteArray converts hex string to byte array. * diff --git a/src/test/java/org/apache/xml/security/test/dom/encryption/XMLCipherKeyAgreementMalformedHKDFParamsTest.java b/src/test/java/org/apache/xml/security/test/dom/encryption/XMLCipherKeyAgreementMalformedHKDFParamsTest.java new file mode 100644 index 000000000..0ee7a1efe --- /dev/null +++ b/src/test/java/org/apache/xml/security/test/dom/encryption/XMLCipherKeyAgreementMalformedHKDFParamsTest.java @@ -0,0 +1,170 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.xml.security.test.dom.encryption; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.nio.charset.StandardCharsets; +import java.security.Key; +import java.security.KeyPair; +import java.security.KeyPairGenerator; +import java.security.PrivateKey; +import java.security.spec.AlgorithmParameterSpec; +import java.security.spec.ECGenParameterSpec; + +import javax.crypto.KeyGenerator; +import javax.xml.parsers.DocumentBuilderFactory; + +import org.apache.xml.security.encryption.EncryptedData; +import org.apache.xml.security.encryption.EncryptedKey; +import org.apache.xml.security.encryption.XMLCipher; +import org.apache.xml.security.encryption.XMLEncryptionException; +import org.apache.xml.security.encryption.params.HKDFParams; +import org.apache.xml.security.encryption.params.KeyAgreementParameters; +import org.apache.xml.security.keys.KeyInfo; +import org.apache.xml.security.signature.XMLSignature; +import org.apache.xml.security.utils.Constants; +import org.apache.xml.security.utils.EncryptionConstants; +import org.apache.xml.security.utils.KeyUtils; +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; +import org.w3c.dom.Element; +import org.w3c.dom.NodeList; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; + +/** + * The HKDF {@code Salt} and {@code Info} elements of an ECDH-ES {@code KeyDerivationMethod} are + * read from the (untrusted) message and base64-decoded on the decrypt path. Malformed base64 + * there must be rejected as {@link XMLEncryptionException}, the exception type the decrypt API + * declares, rather than escaping as the {@code IllegalArgumentException} thrown by + * {@code Base64.Decoder}, which a caller handling the declared type would not catch. + */ +class XMLCipherKeyAgreementMalformedHKDFParamsTest { + + private static final String PLAINTEXT = "CardNumber:4019111111111111"; + + @BeforeAll + static void setUp() { + org.apache.xml.security.Init.init(); + } + + /** Positive control: the same round trip decrypts when the parameters are untouched. */ + @Test + void testWellFormedHKDFParamsDecrypt() throws Exception { + KeyPair recipient = generateRecipientKeyPair(); + Document encDoc = parse(encryptToRecipient(recipient)); + Document decrypted = decryptDocument(encDoc, recipient.getPrivate()); + assertEquals(PLAINTEXT, decrypted.getDocumentElement().getTextContent()); + } + + @ParameterizedTest + @CsvSource({ + "Salt,!!!not-base64!!!", + "Info,@@@@" + }) + void testMalformedHKDFParamRejected(String localName, String badText) throws Exception { + KeyPair recipient = generateRecipientKeyPair(); + Document encDoc = parse(encryptToRecipient(recipient)); + + Element target = (Element) encDoc.getElementsByTagNameNS(Constants.XML_DSIG_NS_MORE_21_04, localName).item(0); + assertNotNull(target, "expected an HKDF <" + localName + "> element to mutate"); + while (target.hasChildNodes()) { + target.removeChild(target.getFirstChild()); + } + target.appendChild(encDoc.createTextNode(badText)); + + assertThrows(XMLEncryptionException.class, () -> decryptDocument(encDoc, recipient.getPrivate())); + } + + private static KeyPair generateRecipientKeyPair() throws Exception { + KeyPairGenerator kpg = KeyPairGenerator.getInstance("EC"); + kpg.initialize(new ECGenParameterSpec("secp256r1")); + return kpg.generateKeyPair(); + } + + /** ECDH-ES key agreement with HKDF (explicit Salt and Info) wrapping an AES-128 CEK; AES-256-GCM content. */ + private static byte[] encryptToRecipient(KeyPair recipient) throws Exception { + DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance(); + dbf.setNamespaceAware(true); + Document doc = dbf.newDocumentBuilder().newDocument(); + Element root = doc.createElement("PaymentInfo"); + root.setTextContent(PLAINTEXT); + doc.appendChild(root); + + KeyGenerator kg = KeyGenerator.getInstance("AES"); + kg.init(128); + Key cek = kg.generateKey(); + + String keyWrapAlgorithm = XMLCipher.AES_128_KeyWrap; + int keyBitLen = KeyUtils.getAESKeyBitSizeForWrapAlgorithm(keyWrapAlgorithm); + HKDFParams kdf = HKDFParams.createBuilder(keyBitLen, XMLSignature.ALGO_ID_MAC_HMAC_SHA256) + .salt(new byte[]{1, 2, 3, 4, 5, 6, 7, 8}) + .info("test-info-data".getBytes(StandardCharsets.UTF_8)) + .build(); + AlgorithmParameterSpec params = new KeyAgreementParameters( + KeyAgreementParameters.ActorType.ORIGINATOR, + EncryptionConstants.ALGO_ID_KEYAGREEMENT_ECDH_ES, + kdf); + + XMLCipher keyCipher = XMLCipher.getInstance(keyWrapAlgorithm); + keyCipher.init(XMLCipher.WRAP_MODE, recipient.getPublic()); + EncryptedKey encryptedKey = keyCipher.encryptKey(doc, cek, params, null); + + XMLCipher dataCipher = XMLCipher.getInstance(XMLCipher.AES_256_GCM); + dataCipher.init(XMLCipher.ENCRYPT_MODE, cek); + EncryptedData encryptedData = dataCipher.getEncryptedData(); + KeyInfo keyInfo = new KeyInfo(doc); + keyInfo.add(encryptedKey); + encryptedData.setKeyInfo(keyInfo); + doc = dataCipher.doFinal(doc, root, false); + + ByteArrayOutputStream bos = new ByteArrayOutputStream(); + javax.xml.transform.TransformerFactory.newInstance().newTransformer().transform( + new javax.xml.transform.dom.DOMSource(doc), + new javax.xml.transform.stream.StreamResult(bos)); + return bos.toByteArray(); + } + + private static Document parse(byte[] xml) throws Exception { + DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance(); + dbf.setNamespaceAware(true); + return dbf.newDocumentBuilder().parse(new ByteArrayInputStream(xml)); + } + + /** The full decrypt path: load the EncryptedData, derive and unwrap the CEK via the recipient's key, decrypt. */ + private static Document decryptDocument(Document encDoc, PrivateKey recipientKey) throws Exception { + NodeList encDataNodes = encDoc.getElementsByTagNameNS(EncryptionConstants.EncryptionSpecNS, "EncryptedData"); + Element encDataElem = (Element) encDataNodes.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, recipientKey); + Key cek = unwrapCipher.decryptKey(ek, encData.getEncryptionMethod().getAlgorithm()); + decryptCipher.init(XMLCipher.DECRYPT_MODE, cek); + return decryptCipher.doFinal(encDoc, encDataElem); + } +}