seanjmullan commented on code in PR #271: URL: https://github.com/apache/santuario-xml-security-java/pull/271#discussion_r1599989548
########## 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: A few comments/questions: - Why are all of these considered weak/insecure? Ok, I understand HmacSHA1 could be considered weak, but why HmacSHA224, HmacSHA384? - There is already a secureValidation mode implemented in Santuario which restricts weak crypto algorithms; however HmacSHA1 is not disabled yet. We could consider disabling it, but we also need to assess the compatibility risk of doing that, which may be too high. - There may be some more places we should be adding additional secureValidation checks in the HKDF code. Can you explore this further? -- 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