Skip to content

8353578: Refactor existing usage of internal HKDF impl to use the KDF API #24393

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

Closed
wants to merge 8 commits into from
Closed

8353578: Refactor existing usage of internal HKDF impl to use the KDF API #24393

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
cfa422e
8353578: Refactor existing usage of internal HKDF impl to use the KDF…
valeriepeng Apr 2, 2025
320c49a
added default deriveData method to SSLKeyDerivation interface and
valeriepeng Apr 5, 2025
eaa0737
address review feedbacks
valeriepeng Apr 11, 2025
38b2da7
Undo the special workaround for JSSE in PKCS11 HKDF impl.
valeriepeng Apr 17, 2025
ebc635c
Address review comments from Weijun.
valeriepeng Apr 25, 2025
cc7fd3a
Add comment about not using the same HKDF instance.
valeriepeng Apr 29, 2025
825aa22
Address review comments from Sean.
valeriepeng May 1, 2025
5693ede
Address review feedbacks from Brad.
valeriepeng May 8, 2025
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
119 changes: 76 additions & 43 deletions src/java.base/share/classes/com/sun/crypto/provider/DHKEM.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2023, 2024, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2023, 2025, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -37,11 +37,13 @@
import java.util.Objects;
import javax.crypto.*;
import javax.crypto.spec.SecretKeySpec;
import javax.crypto.spec.HKDFParameterSpec;

import sun.security.jca.JCAUtil;
import sun.security.ssl.HKDF;
import sun.security.util.*;

import jdk.internal.access.SharedSecrets;

// Implementing DHKEM defined inside https://www.rfc-editor.org/rfc/rfc9180.html,
// without the AuthEncap and AuthDecap functions
public class DHKEM implements KEMSpi {
Expand Down Expand Up @@ -153,19 +155,19 @@ public KeyPair derive(int kem_id) {
private enum Params {

P256(0x10, 32, 32, 2 * 32 + 1,
"ECDH", "EC", CurveDB.P_256, "SHA-256"),
"ECDH", "EC", CurveDB.P_256, "HKDF-SHA256"),

P384(0x11, 48, 48, 2 * 48 + 1,
"ECDH", "EC", CurveDB.P_384, "SHA-384"),
"ECDH", "EC", CurveDB.P_384, "HKDF-SHA384"),

P521(0x12, 64, 66, 2 * 66 + 1,
"ECDH", "EC", CurveDB.P_521, "SHA-512"),
"ECDH", "EC", CurveDB.P_521, "HKDF-SHA512"),

X25519(0x20, 32, 32, 32,
"XDH", "XDH", NamedParameterSpec.X25519, "SHA-256"),
"XDH", "XDH", NamedParameterSpec.X25519, "HKDF-SHA256"),

X448(0x21, 64, 56, 56,
"XDH", "XDH", NamedParameterSpec.X448, "SHA-512"),
"XDH", "XDH", NamedParameterSpec.X448, "HKDF-SHA512"),
;

private final int kem_id;
Expand Down Expand Up @@ -247,10 +249,17 @@ private byte[] DH(PrivateKey skE, PublicKey pkR)

private byte[] ExtractAndExpand(byte[] dh, byte[] kem_context)
throws NoSuchAlgorithmException, InvalidKeyException {
HKDF kdf = new HKDF(hkdfAlgorithm);
SecretKey eae_prk = LabeledExtract(kdf, suiteId, null, EAE_PRK, dh);
return LabeledExpand(kdf, suiteId, eae_prk, SHARED_SECRET,
kem_context, Nsecret);
KDF hkdf = KDF.getInstance(hkdfAlgorithm);
SecretKey eae_prk = LabeledExtract(hkdf, suiteId, EAE_PRK, dh);
try {
return LabeledExpand(hkdf, suiteId, eae_prk, SHARED_SECRET,
kem_context, Nsecret);
} finally {
if (eae_prk instanceof SecretKeySpec s) {
SharedSecrets.getJavaxCryptoSpecAccess()
.clearSecretKeySpec(s);
Copy link
Member

@djelinski djelinski Apr 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wish we could use s.destroy() here instead.

Copy link
Contributor Author

@valeriepeng valeriepeng Apr 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it'd be nice. I reopened https://bugs.openjdk.org/browse/JDK-8160206 and we can address this separately.

Copy link
Contributor

@bradfordwetmore bradfordwetmore May 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or in the meantime:

} finally {
    // Best effort
    if (eae_prk instanceof SecretKeySpec s) {
        SharedSecrets.getJavaxCryptoSpecAccess()
                .clearSecretKeySpec(s);
    } else {
        try {
            eae_prk.destroy();
        } catch (DestroyFailedException e) {
            // swallow
        }
    }
}

Copy link
Contributor Author

@valeriepeng valeriepeng May 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good, thanks for the suggestion.

Copy link
Contributor

@bradfordwetmore bradfordwetmore May 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what I did in my Exporter code. Assuming you go in first, I'll update mine to use your Util method.

}
}
}

private PublicKey getPublicKey(PrivateKey sk)
Expand All @@ -277,31 +286,41 @@ private PublicKey getPublicKey(PrivateKey sk)

// For KAT tests only. See RFC9180DeriveKeyPairSR.
public KeyPair deriveKeyPair(byte[] ikm) throws Exception {
HKDF kdf = new HKDF(hkdfAlgorithm);
SecretKey dkp_prk = LabeledExtract(kdf, suiteId, null, DKP_PRK, ikm);
if (isEC()) {
NamedCurve curve = (NamedCurve) spec;
BigInteger sk = BigInteger.ZERO;
int counter = 0;
while (sk.signum() == 0 || sk.compareTo(curve.getOrder()) >= 0) {
if (counter > 255) {
throw new RuntimeException();
}
byte[] bytes = LabeledExpand(kdf, suiteId, dkp_prk,
CANDIDATE, I2OSP(counter, 1), Nsk);
// bitmask is defined to be 0xFF for P-256 and P-384, and 0x01 for P-521
if (this == Params.P521) {
bytes[0] = (byte) (bytes[0] & 0x01);
KDF hkdf = KDF.getInstance(hkdfAlgorithm);
SecretKey dkp_prk = LabeledExtract(hkdf, suiteId, DKP_PRK, ikm);
try {
if (isEC()) {
NamedCurve curve = (NamedCurve) spec;
BigInteger sk = BigInteger.ZERO;
int counter = 0;
while (sk.signum() == 0 ||
sk.compareTo(curve.getOrder()) >= 0) {
if (counter > 255) {
throw new RuntimeException();
}
byte[] bytes = LabeledExpand(hkdf, suiteId, dkp_prk,
CANDIDATE, I2OSP(counter, 1), Nsk);
// bitmask is defined to be 0xFF for P-256 and P-384,
// and 0x01 for P-521
if (this == Params.P521) {
bytes[0] = (byte) (bytes[0] & 0x01);
}
sk = new BigInteger(1, (bytes));
counter = counter + 1;
}
sk = new BigInteger(1, (bytes));
counter = counter + 1;
PrivateKey k = DeserializePrivateKey(sk.toByteArray());
return new KeyPair(getPublicKey(k), k);
} else {
byte[] sk = LabeledExpand(hkdf, suiteId, dkp_prk, SK, EMPTY,
Nsk);
PrivateKey k = DeserializePrivateKey(sk);
return new KeyPair(getPublicKey(k), k);
}
} finally {
if (dkp_prk instanceof SecretKeySpec s) {
SharedSecrets.getJavaxCryptoSpecAccess()
.clearSecretKeySpec(s);
}
PrivateKey k = DeserializePrivateKey(sk.toByteArray());
return new KeyPair(getPublicKey(k), k);
} else {
byte[] sk = LabeledExpand(kdf, suiteId, dkp_prk, SK, EMPTY, Nsk);
PrivateKey k = DeserializePrivateKey(sk);
return new KeyPair(getPublicKey(k), k);
}
}

Expand Down Expand Up @@ -380,18 +399,32 @@ private static byte[] I2OSP(int n, int w) {
}
}

private static SecretKey LabeledExtract(HKDF kdf, byte[] suite_id,
byte[] salt, byte[] label, byte[] ikm) throws InvalidKeyException {
return kdf.extract(salt,
new SecretKeySpec(concat(HPKE_V1, suite_id, label, ikm), "IKM"),
"HKDF-PRK");
private static SecretKey LabeledExtract(KDF hkdf, byte[] suite_id,
byte[] label, byte[] ikm) throws InvalidKeyException {
SecretKeySpec s = new SecretKeySpec(concat(HPKE_V1, suite_id, label,
ikm), "IKM");
try {
HKDFParameterSpec spec =
HKDFParameterSpec.ofExtract().addIKM(s).extractOnly();
return hkdf.deriveKey("Generic", spec);
Copy link
Contributor

@bradfordwetmore bradfordwetmore May 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't done much with DHKEM yet, but should the returned key have algorithm name of "Generic," or something more descriptive like the previous "HKDF-PRK"?

Copy link
Contributor Author

@valeriepeng valeriepeng May 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Me neither. However, given HKDF-PRK is not a standard algorithm and also not recognized by the SunPKCS11 provider, I changed it to Generic. Existing HKDF impl in the SunPKCS11 provider is quite strict about the derived key algorithms and it will error out unless we add HKDF-PRK to be a recognized key algorithm for key derivation. Given these reasons, it seems Generic is the better choice here.

Copy link
Contributor

@bradfordwetmore bradfordwetmore May 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is any specific salt needed here like in TLS?

Copy link
Contributor

@bradfordwetmore bradfordwetmore May 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should chat next week about an issue Weijun raised and the algorithm names in the Exporters.

} catch (InvalidAlgorithmParameterException |
NoSuchAlgorithmException e) {
throw new InvalidKeyException(e.getMessage(), e);
} finally {
SharedSecrets.getJavaxCryptoSpecAccess().clearSecretKeySpec(s);
}
}

private static byte[] LabeledExpand(HKDF kdf, byte[] suite_id,
private static byte[] LabeledExpand(KDF hkdf, byte[] suite_id,
SecretKey prk, byte[] label, byte[] info, int L)
throws InvalidKeyException {
byte[] labeled_info = concat(I2OSP(L, 2), HPKE_V1,
suite_id, label, info);
return kdf.expand(prk, labeled_info, L, "NONE").getEncoded();
byte[] labeled_info = concat(I2OSP(L, 2), HPKE_V1, suite_id, label,
info);
try {
return hkdf.deriveData(HKDFParameterSpec.expandOnly(
prk, labeled_info, L));
} catch (InvalidAlgorithmParameterException iape) {
throw new InvalidKeyException(iape.getMessage(), iape);
}
}
}
32 changes: 14 additions & 18 deletions src/java.base/share/classes/sun/security/ssl/ChangeCipherSpec.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2015, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2015, 2025, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -77,22 +77,20 @@ public byte[] produce(ConnectionContext context,
try {
writeAuthenticator = Authenticator.valueOf(
hc.negotiatedProtocol, ncs.macAlg,
tkd.getTrafficKey(hc.sslConfig.isClientMode ?
tkd.deriveKey(hc.sslConfig.isClientMode ?
"clientMacKey" : "serverMacKey"));
} catch (NoSuchAlgorithmException | InvalidKeyException e) {
// unlikely
throw new SSLException("Algorithm missing: ", e);
}
}

SecretKey writeKey =
tkd.getTrafficKey(hc.sslConfig.isClientMode ?
"clientWriteKey" : "serverWriteKey");
SecretKey writeIv =
tkd.getTrafficKey(hc.sslConfig.isClientMode ?
"clientWriteIv" : "serverWriteIv");
SecretKey writeKey = tkd.deriveKey(hc.sslConfig.isClientMode ?
"clientWriteKey" : "serverWriteKey");
byte[] writeIv = tkd.deriveData(hc.sslConfig.isClientMode ?
"clientWriteIv" : "serverWriteIv");
IvParameterSpec iv = (writeIv == null) ? null :
new IvParameterSpec(writeIv.getEncoded());
new IvParameterSpec(writeIv);

SSLWriteCipher writeCipher;
try {
writeCipher = ncs.bulkCipher.createWriteCipher(
Expand Down Expand Up @@ -173,22 +171,20 @@ public void consume(ConnectionContext context,
try {
readAuthenticator = Authenticator.valueOf(
hc.negotiatedProtocol, ncs.macAlg,
tkd.getTrafficKey(hc.sslConfig.isClientMode ?
tkd.deriveKey(hc.sslConfig.isClientMode ?
"serverMacKey" : "clientMacKey"));
} catch (NoSuchAlgorithmException | InvalidKeyException e) {
// unlikely
throw new SSLException("Algorithm missing: ", e);
}
}

SecretKey readKey =
tkd.getTrafficKey(hc.sslConfig.isClientMode ?
"serverWriteKey" : "clientWriteKey");
SecretKey readIv =
tkd.getTrafficKey(hc.sslConfig.isClientMode ?
"serverWriteIv" : "clientWriteIv");
SecretKey readKey = tkd.deriveKey(hc.sslConfig.isClientMode ?
"serverWriteKey" : "clientWriteKey");
byte[] readIv = tkd.deriveData(hc.sslConfig.isClientMode ?
"serverWriteIv" : "clientWriteIv");
IvParameterSpec iv = (readIv == null) ? null :
new IvParameterSpec(readIv.getEncoded());
new IvParameterSpec(readIv);
SSLReadCipher readCipher;
try {
readCipher = ncs.bulkCipher.createReadCipher(
Expand Down
4 changes: 3 additions & 1 deletion src/java.base/share/classes/sun/security/ssl/CipherSuite.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2002, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2002, 2025, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -1199,11 +1199,13 @@ enum HashAlg {
final String name;
final int hashLength;
final int blockSize;
final String hkdfAlgorithm;

HashAlg(String hashAlg, int hashLength, int blockSize) {
this.name = hashAlg;
this.hashLength = hashLength;
this.blockSize = blockSize;
this.hkdfAlgorithm = "HKDF-" + hashAlg.replace("-", "");
}

@Override
Expand Down
8 changes: 3 additions & 5 deletions src/java.base/share/classes/sun/security/ssl/DHClientKeyExchange.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2003, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2003, 2025, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -206,8 +206,7 @@ public byte[] produce(ConnectionContext context,
"Not supported key exchange type");
} else {
SSLKeyDerivation masterKD = ke.createKeyDerivation(chc);
SecretKey masterSecret =
masterKD.deriveKey("MasterSecret", null);
SecretKey masterSecret = masterKD.deriveKey("MasterSecret");
chc.handshakeSession.setMasterSecret(masterSecret);

SSLTrafficKeyDerivation kd =
Expand Down Expand Up @@ -302,8 +301,7 @@ public void consume(ConnectionContext context,

// update the states
SSLKeyDerivation masterKD = ke.createKeyDerivation(shc);
SecretKey masterSecret =
masterKD.deriveKey("MasterSecret", null);
SecretKey masterSecret = masterKD.deriveKey("MasterSecret");
shc.handshakeSession.setMasterSecret(masterSecret);

SSLTrafficKeyDerivation kd =
Expand Down
14 changes: 5 additions & 9 deletions src/java.base/share/classes/sun/security/ssl/ECDHClientKeyExchange.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2003, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2003, 2025, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -218,8 +218,7 @@ public byte[] produce(ConnectionContext context,
"Not supported key exchange type");
} else {
SSLKeyDerivation masterKD = ke.createKeyDerivation(chc);
SecretKey masterSecret =
masterKD.deriveKey("MasterSecret", null);
SecretKey masterSecret = masterKD.deriveKey("MasterSecret");
chc.handshakeSession.setMasterSecret(masterSecret);

SSLTrafficKeyDerivation kd =
Expand Down Expand Up @@ -338,8 +337,7 @@ public void consume(ConnectionContext context,

// update the states
SSLKeyDerivation masterKD = ke.createKeyDerivation(shc);
SecretKey masterSecret =
masterKD.deriveKey("MasterSecret", null);
SecretKey masterSecret = masterKD.deriveKey("MasterSecret");
shc.handshakeSession.setMasterSecret(masterSecret);

SSLTrafficKeyDerivation kd =
Expand Down Expand Up @@ -418,8 +416,7 @@ public byte[] produce(ConnectionContext context,
"Not supported key exchange type");
} else {
SSLKeyDerivation masterKD = ke.createKeyDerivation(chc);
SecretKey masterSecret =
masterKD.deriveKey("MasterSecret", null);
SecretKey masterSecret = masterKD.deriveKey("MasterSecret");
chc.handshakeSession.setMasterSecret(masterSecret);

SSLTrafficKeyDerivation kd =
Expand Down Expand Up @@ -522,8 +519,7 @@ public void consume(ConnectionContext context,

// update the states
SSLKeyDerivation masterKD = ke.createKeyDerivation(shc);
SecretKey masterSecret =
masterKD.deriveKey("MasterSecret", null);
SecretKey masterSecret = masterKD.deriveKey("MasterSecret");
shc.handshakeSession.setMasterSecret(masterSecret);

SSLTrafficKeyDerivation kd =
Expand Down
Loading