github-advanced-security[bot] commented on code in PR #271: URL: https://github.com/apache/santuario-xml-security-java/pull/271#discussion_r1593884444
########## src/main/java/org/apache/xml/security/encryption/keys/content/derivedKey/HKDF.java: ########## @@ -0,0 +1,182 @@ +/** + * 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 + * <p> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p> + * 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.encryption.keys.content.derivedKey; + +import org.apache.xml.security.encryption.XMLCipherUtil; +import org.apache.xml.security.encryption.params.HKDFParams; +import org.apache.xml.security.exceptions.XMLSecurityException; +import org.apache.xml.security.utils.I18n; + +import javax.crypto.Mac; +import javax.crypto.spec.SecretKeySpec; +import java.nio.ByteBuffer; +import java.security.InvalidKeyException; +import java.security.NoSuchAlgorithmException; + +import static java.lang.System.Logger.Level.DEBUG; + +/** + * The implementation of the HMAC-based Extract-and-Expand Key Derivation Function (HKDF) + * as defined in <a href="https://datatracker.ietf.org/doc/html/rfc5869";>RFC 5869</a>. + * <p> + * The HKDF algorithm is defined as follows: + * <pre> + * N = ceil(L/HashLen) + * T = T(1) | T(2) | T(3) | ... | T(N) + * OKM = first L bytes of T + * where: + * T(0) = empty string (zero length) + * T(1) = HMAC-Hash(PRK, T(0) | info | 0x01) + * T(2) = HMAC-Hash(PRK, T(1) | info | 0x02) + * T(3) = HMAC-Hash(PRK, T(2) | info | 0x03) + * ... + * </pre> + */ +public class HKDF implements DerivationAlgorithm<HKDFParams> { + + + private static final System.Logger LOG = System.getLogger(HKDF.class.getName()); + + /** + * Derive a key using the HMAC-based Extract-and-Expand Key Derivation Function (HKDF) + * as defined in <a href="https://datatracker.ietf.org/doc/html/rfc5869";>RFC 5869</a>. + * + * @param secret The "shared" secret to use for key derivation + * @param params The key derivation parameters (salt, info, key length, ...) + * @return The derived key of the specified length in bytes defined in the params + * @throws IllegalArgumentException if the parameters are missing + * @throws XMLSecurityException if the hmac hash algorithm is not supported + */ + @Override + public byte[] deriveKey(byte[] secret, HKDFParams params) throws XMLSecurityException { + // check if the parameters are set + if (params == null) { + throw new IllegalArgumentException(I18n.translate("KeyDerivation.MissingParameters")); + } + + String jceAlgorithmName; + try { + jceAlgorithmName = XMLCipherUtil.getJCEMacHashForUri(params.getHmacHashAlgorithm()); + } catch (NoSuchAlgorithmException e) { + throw new XMLSecurityException(e, "KeyDerivation.NotSupportedParameter", new Object[]{params.getHmacHashAlgorithm()}); + } + + byte[] prk = extractKey(jceAlgorithmName, params.getSalt(), secret); + return expandKey(jceAlgorithmName, prk, params.getInfo(), params.getKeyLength()); + } + + /** + * The method "extracts" the pseudo-random key (PRK) based on HMAC-Hash function + * (optional) salt value (a non-secret random value) and the shared secret/input + * keying material (IKM). + * Calculation of the extracted key: + * <pre>PRK = HMAC-Hash(salt, IKM)</pre> + * + * @param jceAlgorithmName the java JCE HMAC algorithm name to use for key derivation + * (e.g. HmacSHA256, HmacSHA384, HmacSHA512) + * @param salt the optional salt value (a non-secret random value); + * @param secret the shared secret/input keying material (IKM) to use for + * key derivation + * @return the pseudo-random key bytes + * @throws XMLSecurityException if the jceAlgorithmName is not supported + */ + public byte[] extractKey(String jceAlgorithmName, byte[] salt, byte[] secret) throws XMLSecurityException { + Mac hMac = initHMac(jceAlgorithmName, salt, true); + hMac.reset(); + return hMac.doFinal(secret); + } + + /** + * The method inits Hash-MAC with given PRK (as salt) and output OKM is calculated as follows: + * <pre> + * T(0) = empty string (zero length) + * T(1) = HMAC-Hash(PRK, T(0) | info | 0x01) + * T(2) = HMAC-Hash(PRK, T(1) | info | 0x02) + * T(3) = HMAC-Hash(PRK, T(2) | info | 0x03) + * ... + * </pre> + * + * @param jceHmacAlgorithmName the java JCE HMAC algorithm name to use to expand + * the key (e.g. HmacSHA256, HmacSHA384, HmacSHA512) + * @param prk pseudo-random key derived from the shared secret + * @param info used to derive the key + * @param keyLength key length in bytes of the derived key + * @return the output keying material (OKM) size of keyLength octets + * @throws XMLSecurityException if the jceHmacAlgorithmName is not supported + */ + public byte[] expandKey(String jceHmacAlgorithmName, byte[] prk, byte[] info, long keyLength) throws XMLSecurityException { + // prepare for expanding the key + Mac hMac = initHMac(jceHmacAlgorithmName, prk, false); + int iMacLength = hMac.getMacLength(); + + int toGenerateSize = (int) keyLength; + ByteBuffer result = ByteBuffer.allocate(toGenerateSize); + byte[] prevResult = new byte[0]; + short counter = 1; + while (toGenerateSize > 0) { + hMac.reset(); + hMac.update(prevResult); + if (info != null && info.length > 0) { + hMac.update(info); + } + hMac.update((byte) counter); + prevResult = hMac.doFinal(); + result.put(prevResult, 0, Math.min(toGenerateSize, iMacLength)); + // get ready for next iteration + toGenerateSize -= iMacLength; + counter++; + } + return result.array(); + } + + /** + * Method initializes a Message Authentication Code (MAC) object using the + * init secret/salt or an empty byte array if initSecret parameter is null or empty. + * + * @param jceAlgorithmName the java JCE HMAC algorithm name to use to init Mac + * @param initSecret the secret/salt to initialize the hmac + * @param initPRK if true, the salt is initialized with a string of zero octets + * as long as the hash function output see [RFC5869] Section 2.2 + * @return Initialized Mac object + * @throws XMLSecurityException if the hmac algorithm is not supported or if it + * fails to initialize + */ + private Mac initHMac(String jceAlgorithmName, byte[] initSecret, boolean initPRK) throws XMLSecurityException { + Mac mac; + try { + LOG.log(DEBUG, "Init Mac with hash algorithm: [{}]", jceAlgorithmName); + mac = Mac.getInstance(jceAlgorithmName); + } catch (NoSuchAlgorithmException e) { + throw new XMLSecurityException(e, "KeyDerivation.NotSupportedParameter", new Object[]{jceAlgorithmName}); + } + + if (initPRK && (initSecret == null || initSecret.length == 0)) { + // If "initSecret"/salt is not provided, a string of zero octets as long as the hash function output is used + LOG.log(DEBUG, "Init Mac with hmac algorithm [{}] and empty salt!", jceAlgorithmName); + initSecret = new byte[mac.getMacLength()]; + } + SecretKeySpec secretKey = new SecretKeySpec(initSecret, jceAlgorithmName); Review Comment: ## Use of a potentially broken or risky cryptographic algorithm Cryptographic algorithm [HmacSHA224](1) may not be secure, consider using a different algorithm. Cryptographic algorithm [HmacSHA384](2) may not be secure, consider using a different algorithm. Cryptographic algorithm [HMACRIPEMD160](3) may not be secure, consider using a different algorithm. [Show more details](https://github.com/apache/santuario-xml-security-java/security/code-scanning/1150) ########## src/main/java/org/apache/xml/security/encryption/keys/content/derivedKey/HKDF.java: ########## @@ -0,0 +1,182 @@ +/** + * 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 + * <p> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p> + * 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.encryption.keys.content.derivedKey; + +import org.apache.xml.security.encryption.XMLCipherUtil; +import org.apache.xml.security.encryption.params.HKDFParams; +import org.apache.xml.security.exceptions.XMLSecurityException; +import org.apache.xml.security.utils.I18n; + +import javax.crypto.Mac; +import javax.crypto.spec.SecretKeySpec; +import java.nio.ByteBuffer; +import java.security.InvalidKeyException; +import java.security.NoSuchAlgorithmException; + +import static java.lang.System.Logger.Level.DEBUG; + +/** + * The implementation of the HMAC-based Extract-and-Expand Key Derivation Function (HKDF) + * as defined in <a href="https://datatracker.ietf.org/doc/html/rfc5869";>RFC 5869</a>. + * <p> + * The HKDF algorithm is defined as follows: + * <pre> + * N = ceil(L/HashLen) + * T = T(1) | T(2) | T(3) | ... | T(N) + * OKM = first L bytes of T + * where: + * T(0) = empty string (zero length) + * T(1) = HMAC-Hash(PRK, T(0) | info | 0x01) + * T(2) = HMAC-Hash(PRK, T(1) | info | 0x02) + * T(3) = HMAC-Hash(PRK, T(2) | info | 0x03) + * ... + * </pre> + */ +public class HKDF implements DerivationAlgorithm<HKDFParams> { + + + private static final System.Logger LOG = System.getLogger(HKDF.class.getName()); + + /** + * Derive a key using the HMAC-based Extract-and-Expand Key Derivation Function (HKDF) + * as defined in <a href="https://datatracker.ietf.org/doc/html/rfc5869";>RFC 5869</a>. + * + * @param secret The "shared" secret to use for key derivation + * @param params The key derivation parameters (salt, info, key length, ...) + * @return The derived key of the specified length in bytes defined in the params + * @throws IllegalArgumentException if the parameters are missing + * @throws XMLSecurityException if the hmac hash algorithm is not supported + */ + @Override + public byte[] deriveKey(byte[] secret, HKDFParams params) throws XMLSecurityException { + // check if the parameters are set + if (params == null) { + throw new IllegalArgumentException(I18n.translate("KeyDerivation.MissingParameters")); + } + + String jceAlgorithmName; + try { + jceAlgorithmName = XMLCipherUtil.getJCEMacHashForUri(params.getHmacHashAlgorithm()); + } catch (NoSuchAlgorithmException e) { + throw new XMLSecurityException(e, "KeyDerivation.NotSupportedParameter", new Object[]{params.getHmacHashAlgorithm()}); + } + + byte[] prk = extractKey(jceAlgorithmName, params.getSalt(), secret); + return expandKey(jceAlgorithmName, prk, params.getInfo(), params.getKeyLength()); + } + + /** + * The method "extracts" the pseudo-random key (PRK) based on HMAC-Hash function + * (optional) salt value (a non-secret random value) and the shared secret/input + * keying material (IKM). + * Calculation of the extracted key: + * <pre>PRK = HMAC-Hash(salt, IKM)</pre> + * + * @param jceAlgorithmName the java JCE HMAC algorithm name to use for key derivation + * (e.g. HmacSHA256, HmacSHA384, HmacSHA512) + * @param salt the optional salt value (a non-secret random value); + * @param secret the shared secret/input keying material (IKM) to use for + * key derivation + * @return the pseudo-random key bytes + * @throws XMLSecurityException if the jceAlgorithmName is not supported + */ + public byte[] extractKey(String jceAlgorithmName, byte[] salt, byte[] secret) throws XMLSecurityException { + Mac hMac = initHMac(jceAlgorithmName, salt, true); + hMac.reset(); + return hMac.doFinal(secret); + } + + /** + * The method inits Hash-MAC with given PRK (as salt) and output OKM is calculated as follows: + * <pre> + * T(0) = empty string (zero length) + * T(1) = HMAC-Hash(PRK, T(0) | info | 0x01) + * T(2) = HMAC-Hash(PRK, T(1) | info | 0x02) + * T(3) = HMAC-Hash(PRK, T(2) | info | 0x03) + * ... + * </pre> + * + * @param jceHmacAlgorithmName the java JCE HMAC algorithm name to use to expand + * the key (e.g. HmacSHA256, HmacSHA384, HmacSHA512) + * @param prk pseudo-random key derived from the shared secret + * @param info used to derive the key + * @param keyLength key length in bytes of the derived key + * @return the output keying material (OKM) size of keyLength octets + * @throws XMLSecurityException if the jceHmacAlgorithmName is not supported + */ + public byte[] expandKey(String jceHmacAlgorithmName, byte[] prk, byte[] info, long keyLength) throws XMLSecurityException { + // prepare for expanding the key + Mac hMac = initHMac(jceHmacAlgorithmName, prk, false); + int iMacLength = hMac.getMacLength(); + + int toGenerateSize = (int) keyLength; + ByteBuffer result = ByteBuffer.allocate(toGenerateSize); + byte[] prevResult = new byte[0]; + short counter = 1; + while (toGenerateSize > 0) { + hMac.reset(); + hMac.update(prevResult); + if (info != null && info.length > 0) { + hMac.update(info); + } + hMac.update((byte) counter); + prevResult = hMac.doFinal(); + result.put(prevResult, 0, Math.min(toGenerateSize, iMacLength)); + // get ready for next iteration + toGenerateSize -= iMacLength; + counter++; + } + return result.array(); + } + + /** + * Method initializes a Message Authentication Code (MAC) object using the + * init secret/salt or an empty byte array if initSecret parameter is null or empty. + * + * @param jceAlgorithmName the java JCE HMAC algorithm name to use to init Mac + * @param initSecret the secret/salt to initialize the hmac + * @param initPRK if true, the salt is initialized with a string of zero octets + * as long as the hash function output see [RFC5869] Section 2.2 + * @return Initialized Mac object + * @throws XMLSecurityException if the hmac algorithm is not supported or if it + * fails to initialize + */ + private Mac initHMac(String jceAlgorithmName, byte[] initSecret, boolean initPRK) throws XMLSecurityException { + Mac mac; + try { + LOG.log(DEBUG, "Init Mac with hash algorithm: [{}]", jceAlgorithmName); + mac = Mac.getInstance(jceAlgorithmName); + } catch (NoSuchAlgorithmException e) { + throw new XMLSecurityException(e, "KeyDerivation.NotSupportedParameter", new Object[]{jceAlgorithmName}); + } + + if (initPRK && (initSecret == null || initSecret.length == 0)) { + // If "initSecret"/salt is not provided, a string of zero octets as long as the hash function output is used + LOG.log(DEBUG, "Init Mac with hmac algorithm [{}] and empty salt!", jceAlgorithmName); + initSecret = new byte[mac.getMacLength()]; + } + SecretKeySpec secretKey = new SecretKeySpec(initSecret, jceAlgorithmName); Review Comment: ## Use of a broken or risky cryptographic algorithm Cryptographic algorithm [HmacSHA1](1) is weak and should not be used. [Show more details](https://github.com/apache/santuario-xml-security-java/security/code-scanning/1147) ########## src/main/java/org/apache/xml/security/utils/KeyUtils.java: ########## @@ -247,34 +245,76 @@ } } - /** - * Derive a key encryption key from a shared secret and keyDerivationParameter. Currently only the ConcatKDF is supported. + * Derive a key encryption key from a shared secret and keyDerivationParameter. + * Currently only the ConcatKDF and HMAC-base Extract-and-Expand Key Derivation + * Function (HKDF) are supported. + * * @param sharedSecret the shared secret * @param keyDerivationParameter the key derivation parameters * @return the derived key encryption key + * @throws IllegalArgumentException if the keyDerivationParameter is null * @throws XMLSecurityException if the key derivation algorithm is not supported */ public static byte[] deriveKeyEncryptionKey(byte[] sharedSecret, KeyDerivationParameters keyDerivationParameter) throws XMLSecurityException { - int iKeySize = keyDerivationParameter.getKeyBitLength()/8; + + if (keyDerivationParameter == null) { + throw new IllegalArgumentException(I18n.translate("KeyDerivation.MissingParameters")); + } + String keyDerivationAlgorithm = keyDerivationParameter.getAlgorithm(); - if (!EncryptionConstants.ALGO_ID_KEYDERIVATION_CONCATKDF.equals(keyDerivationAlgorithm)) { - throw new XMLEncryptionException( "unknownAlgorithm", - keyDerivationAlgorithm); + if (keyDerivationParameter instanceof HKDFParams) { + return deriveKeyEncryptionKey(sharedSecret, (HKDFParams) keyDerivationParameter); + } else if (keyDerivationParameter instanceof ConcatKDFParams) { + return deriveKeyEncryptionKey(sharedSecret, (ConcatKDFParams) keyDerivationParameter); + } + + throw new XMLEncryptionException("KeyDerivation.UnsupportedAlgorithm", keyDerivationAlgorithm, + keyDerivationParameter.getClass().getName()); + } + + /** + * Derive a key using the HMAC-based Extract-and-Expand Key Derivation + * Function (HKDF) with implementation instance {@link HKDFParams}. + * + * @param sharedSecret the shared secret + * @param hkdfParameter the HKDF parameters + * @return the derived key encryption key. + * @throws XMLSecurityException if the key derivation parameters are invalid or + * the hmac algorithm is not supported. + */ + public static byte[] deriveKeyEncryptionKey(byte[] sharedSecret, HKDFParams hkdfParameter) + throws XMLSecurityException { + + if (!EncryptionConstants.ALGO_ID_KEYDERIVATION_HKDF.equals(hkdfParameter.getAlgorithm())){ + throw new XMLEncryptionException("KeyDerivation.UnsupportedAlgorithm", hkdfParameter.getAlgorithm(), + HKDFParams.class.getName()); } - ConcatKDFParams ckdfParameter = (ConcatKDFParams) keyDerivationParameter; - // get parameters - String digestAlgorithm = ckdfParameter.getDigestAlgorithm(); + HKDF kdf = new HKDF(); + return kdf.deriveKey(sharedSecret, hkdfParameter); + } - String algorithmID = ckdfParameter.getAlgorithmID(); - String partyUInfo = ckdfParameter.getPartyUInfo(); - String partyVInfo = ckdfParameter.getPartyVInfo(); - String suppPubInfo = ckdfParameter.getSuppPubInfo(); - String suppPrivInfo = ckdfParameter.getSuppPrivInfo(); + /** + * Derive a key using the Concatenation Key Derivation Function (ConcatKDF) + * with implementation instance {@link ConcatKDFParams}. + * + * @param sharedSecret the shared secret/ input keying material + * @param ckdfParameter the ConcatKDF parameters + * @return the derived key + * @throws XMLSecurityException if the key derivation parameters are invalid or + * the hash algorithm is not supported. + */ + public static byte[] deriveKeyEncryptionKey(byte[] sharedSecret, ConcatKDFParams ckdfParameter) Review Comment: ## Confusing overloading of methods Method KeyUtils.deriveKeyEncryptionKey(..) could be confused with overloaded method [deriveKeyEncryptionKey](1), since dispatch depends on static types. [Show more details](https://github.com/apache/santuario-xml-security-java/security/code-scanning/1148) ########## src/main/java/org/apache/xml/security/utils/KeyUtils.java: ########## @@ -247,34 +245,76 @@ } } - /** - * Derive a key encryption key from a shared secret and keyDerivationParameter. Currently only the ConcatKDF is supported. + * Derive a key encryption key from a shared secret and keyDerivationParameter. + * Currently only the ConcatKDF and HMAC-base Extract-and-Expand Key Derivation + * Function (HKDF) are supported. + * * @param sharedSecret the shared secret * @param keyDerivationParameter the key derivation parameters * @return the derived key encryption key + * @throws IllegalArgumentException if the keyDerivationParameter is null * @throws XMLSecurityException if the key derivation algorithm is not supported */ public static byte[] deriveKeyEncryptionKey(byte[] sharedSecret, KeyDerivationParameters keyDerivationParameter) throws XMLSecurityException { - int iKeySize = keyDerivationParameter.getKeyBitLength()/8; + + if (keyDerivationParameter == null) { + throw new IllegalArgumentException(I18n.translate("KeyDerivation.MissingParameters")); + } + String keyDerivationAlgorithm = keyDerivationParameter.getAlgorithm(); - if (!EncryptionConstants.ALGO_ID_KEYDERIVATION_CONCATKDF.equals(keyDerivationAlgorithm)) { - throw new XMLEncryptionException( "unknownAlgorithm", - keyDerivationAlgorithm); + if (keyDerivationParameter instanceof HKDFParams) { + return deriveKeyEncryptionKey(sharedSecret, (HKDFParams) keyDerivationParameter); + } else if (keyDerivationParameter instanceof ConcatKDFParams) { + return deriveKeyEncryptionKey(sharedSecret, (ConcatKDFParams) keyDerivationParameter); + } + + throw new XMLEncryptionException("KeyDerivation.UnsupportedAlgorithm", keyDerivationAlgorithm, + keyDerivationParameter.getClass().getName()); + } + + /** + * Derive a key using the HMAC-based Extract-and-Expand Key Derivation + * Function (HKDF) with implementation instance {@link HKDFParams}. + * + * @param sharedSecret the shared secret + * @param hkdfParameter the HKDF parameters + * @return the derived key encryption key. + * @throws XMLSecurityException if the key derivation parameters are invalid or + * the hmac algorithm is not supported. + */ + public static byte[] deriveKeyEncryptionKey(byte[] sharedSecret, HKDFParams hkdfParameter) Review Comment: ## Confusing overloading of methods Method KeyUtils.deriveKeyEncryptionKey(..) could be confused with overloaded method [deriveKeyEncryptionKey](1), since dispatch depends on static types. [Show more details](https://github.com/apache/santuario-xml-security-java/security/code-scanning/1149) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@santuario.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
