8368694: PKCS11-NSS generic keys generated by DH have leading zeroes stripped

Reviewed-by: valeriep
This commit is contained in:
Daniel Jeliński 2025-10-09 06:01:25 +00:00
parent 0b81db1d38
commit 914b44e277
3 changed files with 63 additions and 36 deletions

View File

@ -200,6 +200,7 @@ final class P11KeyAgreement extends KeyAgreementSpi {
CK_ATTRIBUTE[] attributes = new CK_ATTRIBUTE[] {
new CK_ATTRIBUTE(CKA_CLASS, CKO_SECRET_KEY),
new CK_ATTRIBUTE(CKA_KEY_TYPE, CKK_GENERIC_SECRET),
new CK_ATTRIBUTE(CKA_VALUE_LEN, secretLen),
};
attributes = token.getAttributes
(O_GENERATE, CKO_SECRET_KEY, CKK_GENERIC_SECRET, attributes);
@ -213,22 +214,11 @@ final class P11KeyAgreement extends KeyAgreementSpi {
token.p11.C_GetAttributeValue(session.id(), keyID, attributes);
byte[] secret = attributes[0].getByteArray();
token.p11.C_DestroyObject(session.id(), keyID);
// Some vendors, e.g. NSS, trim off the leading 0x00 byte(s) from
// the generated secret. Thus, we need to check the secret length
// and trim/pad it so the returned value has the same length as
// the modulus size
if (secret.length == secretLen) {
return secret;
} else {
if (secret.length > secretLen) {
// Shouldn't happen; but check just in case
throw new ProviderException("generated secret is out-of-range");
}
byte[] newSecret = new byte[secretLen];
System.arraycopy(secret, 0, newSecret, secretLen - secret.length,
secret.length);
return newSecret;
if (secret.length != secretLen) {
// Shouldn't happen; but check just in case
throw new ProviderException("generated secret is out-of-range");
}
return secret;
} catch (PKCS11Exception e) {
throw new ProviderException("Could not derive key", e);
} finally {
@ -321,10 +311,20 @@ final class P11KeyAgreement extends KeyAgreementSpi {
long privKeyID = privateKey.getKeyID();
try {
session = token.getObjSession();
CK_ATTRIBUTE[] attributes = new CK_ATTRIBUTE[] {
new CK_ATTRIBUTE(CKA_CLASS, CKO_SECRET_KEY),
new CK_ATTRIBUTE(CKA_KEY_TYPE, keyType),
};
CK_ATTRIBUTE[] attributes;
if ("TlsPremasterSecret".equalsIgnoreCase(algorithm)) {
attributes = new CK_ATTRIBUTE[]{
new CK_ATTRIBUTE(CKA_CLASS, CKO_SECRET_KEY),
new CK_ATTRIBUTE(CKA_KEY_TYPE, keyType),
};
} else {
// keep the leading zeroes
attributes = new CK_ATTRIBUTE[]{
new CK_ATTRIBUTE(CKA_CLASS, CKO_SECRET_KEY),
new CK_ATTRIBUTE(CKA_KEY_TYPE, keyType),
new CK_ATTRIBUTE(CKA_VALUE_LEN, secretLen),
};
}
attributes = token.getAttributes
(O_GENERATE, CKO_SECRET_KEY, keyType, attributes);
long keyID = token.p11.C_DeriveKey(session.id(),
@ -337,19 +337,6 @@ final class P11KeyAgreement extends KeyAgreementSpi {
int keyLen = (int)lenAttributes[0].getLong();
SecretKey key = P11Key.secretKey
(session, keyID, algorithm, keyLen << 3, attributes);
if ("RAW".equals(key.getFormat())
&& algorithm.equalsIgnoreCase("TlsPremasterSecret")) {
// Workaround for Solaris bug 6318543.
// Strip leading zeroes ourselves if possible (key not sensitive).
// This should be removed once the Solaris fix is available
// as here we always retrieve the CKA_VALUE even for tokens
// that do not have that bug.
byte[] keyBytes = key.getEncoded();
byte[] newBytes = KeyUtil.trimZeroes(keyBytes);
if (keyBytes != newBytes) {
key = new SecretKeySpec(newBytes, algorithm);
}
}
return key;
} catch (PKCS11Exception e) {
throw new InvalidKeyException("Could not derive key", e);

View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2013, 2024, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2013, 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
@ -23,7 +23,7 @@
/*
* @test
* @bug 8014618
* @bug 8014618 8368694
* @summary Need to strip leading zeros in TlsPremasterSecret of DHKeyAgreement
* @author Pasi Eronen
*/
@ -88,6 +88,26 @@ public class TestLeadingZeroes {
throw new Exception("First byte is not zero as expected");
}
// generate generic shared secret
aliceKeyAgree.init(alicePrivKey);
aliceKeyAgree.doPhase(bobPubKey, true);
byte[] genericSecret =
aliceKeyAgree.generateSecret("Generic").getEncoded();
System.out.println("generic secret:\n" + HEX_FORMATTER.formatHex(genericSecret));
// verify that leading zero is present
if (genericSecret.length != 256) {
throw new Exception("Unexpected generic secret length");
}
if (genericSecret[0] != 0) {
throw new Exception("First byte is not zero as expected");
}
for (int i = 0; i < genericSecret.length; i++) {
if (genericSecret[i] != sharedSecret[i]) {
throw new Exception("Shared secrets differ");
}
}
// now, test TLS premaster secret
aliceKeyAgree.init(alicePrivKey);
aliceKeyAgree.doPhase(bobPubKey, true);

View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2013, 2024, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2013, 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
@ -23,7 +23,7 @@
/*
* @test
* @bug 8014618
* @bug 8014618 8368694
* @summary Need to strip leading zeros in TlsPremasterSecret of DHKeyAgreement
* @library /test/lib ..
* @author Pasi Eronen
@ -87,6 +87,26 @@ public class TestLeadingZeroesP11 extends PKCS11Test {
throw new Exception("First byte is not zero as expected");
}
// generate generic shared secret
aliceKeyAgree.init(alicePrivKey);
aliceKeyAgree.doPhase(bobPubKey, true);
byte[] genericSecret =
aliceKeyAgree.generateSecret("Generic").getEncoded();
System.out.println("generic secret:\n" + HEX.formatHex(genericSecret));
// verify that leading zero is present
if (genericSecret.length != 128) {
throw new Exception("Unexpected generic secret length");
}
if (genericSecret[0] != 0) {
throw new Exception("First byte is not zero as expected");
}
for (int i = 0; i < genericSecret.length; i++) {
if (genericSecret[i] != sharedSecret[i]) {
throw new Exception("Shared secrets differ");
}
}
// now, test TLS premaster secret
aliceKeyAgree.init(alicePrivKey);
aliceKeyAgree.doPhase(bobPubKey, true);