Skip to content

Commit 94caecb

Browse files
committedOct 13, 2022
8294906: Memory leak in PKCS11 NSS TLS server
Reviewed-by: valeriep
1 parent 03e63a2 commit 94caecb

File tree

5 files changed

+116
-11
lines changed

5 files changed

+116
-11
lines changed
 

‎src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11TlsKeyMaterialGenerator.java

+10-4
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2005, 2018, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2005, 2022, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -209,15 +209,19 @@ protected SecretKey engineGenerateKey() {
209209
SecretKey clientMacKey, serverMacKey;
210210

211211
// The MAC size may be zero for GCM mode.
212-
//
213-
// PKCS11 does not support GCM mode as the author made the comment,
214-
// so the macBits is unlikely to be zero. It's only a place holder.
215212
if (macBits != 0) {
216213
clientMacKey = P11Key.secretKey
217214
(session, out.hClientMacSecret, "MAC", macBits, attributes);
218215
serverMacKey = P11Key.secretKey
219216
(session, out.hServerMacSecret, "MAC", macBits, attributes);
220217
} else {
218+
// NSS allocates MAC keys even if macBits is zero
219+
if (out.hClientMacSecret != CK_INVALID_HANDLE) {
220+
token.p11.C_DestroyObject(session.id(), out.hClientMacSecret);
221+
}
222+
if (out.hServerMacSecret != CK_INVALID_HANDLE) {
223+
token.p11.C_DestroyObject(session.id(), out.hServerMacSecret);
224+
}
221225
clientMacKey = null;
222226
serverMacKey = null;
223227
}
@@ -229,6 +233,8 @@ protected SecretKey engineGenerateKey() {
229233
serverCipherKey = P11Key.secretKey(session, out.hServerKey,
230234
cipherAlgorithm, expandedKeyBits, attributes);
231235
} else {
236+
assert out.hClientKey == 0;
237+
assert out.hServerKey == 0;
232238
clientCipherKey = null;
233239
serverCipherKey = null;
234240
}

‎test/jdk/com/sun/crypto/provider/TLS/TestKeyMaterial.java

+28-3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2005, 2022, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -61,6 +61,7 @@ public static void main(String[] args) throws Exception {
6161
byte[] clientRandom = null;
6262
byte[] serverRandom = null;
6363
String cipherAlgorithm = null;
64+
String hashAlgorithm = null; // TLS1.2+ only
6465
int keyLength = 0;
6566
int expandedKeyLength = 0;
6667
int ivLength = 0;
@@ -94,6 +95,8 @@ public static void main(String[] args) throws Exception {
9495
serverRandom = parse(data);
9596
} else if (line.startsWith("km-cipalg:")) {
9697
cipherAlgorithm = data;
98+
} else if (line.startsWith("km-hashalg:")) {
99+
hashAlgorithm = data;
97100
} else if (line.startsWith("km-keylen:")) {
98101
keyLength = Integer.parseInt(data);
99102
} else if (line.startsWith("km-explen:")) {
@@ -119,14 +122,36 @@ public static void main(String[] args) throws Exception {
119122
n++;
120123

121124
KeyGenerator kg =
122-
KeyGenerator.getInstance("SunTlsKeyMaterial", provider);
125+
KeyGenerator.getInstance(minor == 3 ?
126+
"SunTls12KeyMaterial" :
127+
"SunTlsKeyMaterial", provider);
123128
SecretKey masterKey =
124129
new SecretKeySpec(master, "TlsMasterSecret");
130+
int prfHashLength, prfBlockSize;
131+
132+
if (hashAlgorithm != null) {
133+
switch (hashAlgorithm) {
134+
case "SHA-256":
135+
prfHashLength = 32;
136+
prfBlockSize = 64;
137+
break;
138+
case "SHA-384":
139+
prfHashLength = 48;
140+
prfBlockSize = 128;
141+
break;
142+
default:
143+
throw new RuntimeException("Unexpected hashalg: " +
144+
hashAlgorithm);
145+
}
146+
} else {
147+
prfHashLength = -1;
148+
prfBlockSize = -1;
149+
}
125150
TlsKeyMaterialParameterSpec spec =
126151
new TlsKeyMaterialParameterSpec(masterKey, major, minor,
127152
clientRandom, serverRandom, cipherAlgorithm,
128153
keyLength, expandedKeyLength, ivLength, macLength,
129-
null, -1, -1);
154+
hashAlgorithm, prfHashLength, prfBlockSize);
130155

131156
kg.init(spec);
132157
TlsKeyMaterialSpec result =

‎test/jdk/com/sun/crypto/provider/TLS/keymatdata.txt

+34
Original file line numberDiff line numberDiff line change
@@ -3646,3 +3646,37 @@ km-civ: 17:bd:47:89:54:be:04:23
36463646
km-siv: 34:8a:e8:24:84:38:c4:e1
36473647
km-cmackey: e8:f0:b5:7b:a7:cc:2f:5e:43:ef:d3:dd:4e:8c:f9:6f:51:d7:84:df
36483648
km-smackey: fc:0c:77:20:c2:28:d3:11:ba:57:13:d8:0b:2d:f1:30:89:c6:35:69
3649+
km-master: f1:05:15:45:33:be:50:d6:88:0b:03:bb:88:9b:ef:d4:3b:98:aa:40:13:71:3c:1c:d9:df:34:c7:50:75:ad:5c:0a:d4:fe:ed:d5:58:6b:ff:2b:ce:c6:12:bc:6b:7e:dc
3650+
km-major: 3
3651+
km-minor: 3
3652+
km-crandom: 42:f3:36:8e:9d:c9:69:3e:c1:8a:38:d3:e0:ec:2b:58:c2:e0:0c:de:4f:f3:af:51:d2:5c:bc:b2:c3:3b:1e:56
3653+
km-srandom: 42:f3:36:8e:fa:fd:23:3e:fd:f9:bc:88:3c:98:93:f3:c3:1d:9c:2a:4a:3b:02:a7:40:d4:64:04:59:e9:65:97
3654+
km-cipalg: AES
3655+
km-hashalg: SHA-256
3656+
km-keylen: 16
3657+
km-explen: 0
3658+
km-ivlen: 4
3659+
km-maclen: 0
3660+
km-ccipkey: 60:7a:45:a9:6e:76:58:ea:d9:44:c5:25:f8:92:f1:26
3661+
km-scipkey: 42:c0:ed:75:a2:51:21:7c:50:74:9d:78:9a:f7:35:2b
3662+
km-civ: a1:3c:3e:4a
3663+
km-siv: 85:ab:ee:70
3664+
km-cmackey: (null)
3665+
km-smackey: (null)
3666+
km-master: f1:05:15:45:33:be:50:d6:88:0b:03:bb:88:9b:ef:d4:3b:98:aa:40:13:71:3c:1c:d9:df:34:c7:50:75:ad:5c:0a:d4:fe:ed:d5:58:6b:ff:2b:ce:c6:12:bc:6b:7e:dc
3667+
km-major: 3
3668+
km-minor: 3
3669+
km-crandom: 42:f3:36:8e:9d:c9:69:3e:c1:8a:38:d3:e0:ec:2b:58:c2:e0:0c:de:4f:f3:af:51:d2:5c:bc:b2:c3:3b:1e:56
3670+
km-srandom: 42:f3:36:8e:fa:fd:23:3e:fd:f9:bc:88:3c:98:93:f3:c3:1d:9c:2a:4a:3b:02:a7:40:d4:64:04:59:e9:65:97
3671+
km-cipalg: AES
3672+
km-hashalg: SHA-384
3673+
km-keylen: 32
3674+
km-explen: 0
3675+
km-ivlen: 4
3676+
km-maclen: 0
3677+
km-ccipkey: 3c:03:17:61:1e:88:4a:aa:01:4c:ac:6c:f8:bb:91:c3:0e:ec:57:c7:bf:07:ff:eb:49:22:f9:80:12:64:72:2a
3678+
km-scipkey: f8:00:8e:b2:dc:25:98:f1:97:00:55:28:60:a3:65:da:42:89:18:bb:40:94:53:d2:75:2a:29:e5:aa:94:1d:7a
3679+
km-civ: 24:02:76:6f
3680+
km-siv: 3b:6d:33:5a
3681+
km-cmackey: (null)
3682+
km-smackey: (null)

‎test/jdk/sun/security/pkcs11/tls/TestKeyMaterial.java

+10-4
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2005, 2020, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2005, 2022, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -23,7 +23,7 @@
2323

2424
/*
2525
* @test
26-
* @bug 6316539 8136355
26+
* @bug 6316539 8136355 8294906
2727
* @summary Known-answer-test for TlsKeyMaterial generator
2828
* @author Andreas Sterbenz
2929
* @library /test/lib ..
@@ -77,6 +77,7 @@ public void main(Provider provider) throws Exception {
7777
byte[] clientRandom = null;
7878
byte[] serverRandom = null;
7979
String cipherAlgorithm = null;
80+
String hashAlgorithm = null; // TLS1.2+ only
8081
int keyLength = 0;
8182
int expandedKeyLength = 0;
8283
int ivLength = 0;
@@ -110,6 +111,8 @@ public void main(Provider provider) throws Exception {
110111
serverRandom = parse(data);
111112
} else if (line.startsWith("km-cipalg:")) {
112113
cipherAlgorithm = data;
114+
} else if (line.startsWith("km-hashalg:")) {
115+
hashAlgorithm = data;
113116
} else if (line.startsWith("km-keylen:")) {
114117
keyLength = Integer.parseInt(data);
115118
} else if (line.startsWith("km-explen:")) {
@@ -135,14 +138,17 @@ public void main(Provider provider) throws Exception {
135138
n++;
136139

137140
KeyGenerator kg =
138-
KeyGenerator.getInstance("SunTlsKeyMaterial", provider);
141+
KeyGenerator.getInstance(minor == 3 ?
142+
"SunTls12KeyMaterial" :
143+
"SunTlsKeyMaterial", provider);
139144
SecretKey masterKey =
140145
new SecretKeySpec(master, "TlsMasterSecret");
146+
// prfHashLength and prfBlockSize are ignored by PKCS11 provider
141147
TlsKeyMaterialParameterSpec spec =
142148
new TlsKeyMaterialParameterSpec(masterKey, major, minor,
143149
clientRandom, serverRandom, cipherAlgorithm,
144150
keyLength, expandedKeyLength, ivLength, macLength,
145-
null, -1, -1);
151+
hashAlgorithm, -1 /*ignored*/, -1 /*ignored*/);
146152

147153
try {
148154
kg.init(spec);

‎test/jdk/sun/security/pkcs11/tls/keymatdata.txt

+34
Original file line numberDiff line numberDiff line change
@@ -3646,3 +3646,37 @@ km-civ: 17:bd:47:89:54:be:04:23
36463646
km-siv: 34:8a:e8:24:84:38:c4:e1
36473647
km-cmackey: e8:f0:b5:7b:a7:cc:2f:5e:43:ef:d3:dd:4e:8c:f9:6f:51:d7:84:df
36483648
km-smackey: fc:0c:77:20:c2:28:d3:11:ba:57:13:d8:0b:2d:f1:30:89:c6:35:69
3649+
km-master: f1:05:15:45:33:be:50:d6:88:0b:03:bb:88:9b:ef:d4:3b:98:aa:40:13:71:3c:1c:d9:df:34:c7:50:75:ad:5c:0a:d4:fe:ed:d5:58:6b:ff:2b:ce:c6:12:bc:6b:7e:dc
3650+
km-major: 3
3651+
km-minor: 3
3652+
km-crandom: 42:f3:36:8e:9d:c9:69:3e:c1:8a:38:d3:e0:ec:2b:58:c2:e0:0c:de:4f:f3:af:51:d2:5c:bc:b2:c3:3b:1e:56
3653+
km-srandom: 42:f3:36:8e:fa:fd:23:3e:fd:f9:bc:88:3c:98:93:f3:c3:1d:9c:2a:4a:3b:02:a7:40:d4:64:04:59:e9:65:97
3654+
km-cipalg: AES
3655+
km-hashalg: SHA-256
3656+
km-keylen: 16
3657+
km-explen: 0
3658+
km-ivlen: 4
3659+
km-maclen: 0
3660+
km-ccipkey: 60:7a:45:a9:6e:76:58:ea:d9:44:c5:25:f8:92:f1:26
3661+
km-scipkey: 42:c0:ed:75:a2:51:21:7c:50:74:9d:78:9a:f7:35:2b
3662+
km-civ: a1:3c:3e:4a
3663+
km-siv: 85:ab:ee:70
3664+
km-cmackey: (null)
3665+
km-smackey: (null)
3666+
km-master: f1:05:15:45:33:be:50:d6:88:0b:03:bb:88:9b:ef:d4:3b:98:aa:40:13:71:3c:1c:d9:df:34:c7:50:75:ad:5c:0a:d4:fe:ed:d5:58:6b:ff:2b:ce:c6:12:bc:6b:7e:dc
3667+
km-major: 3
3668+
km-minor: 3
3669+
km-crandom: 42:f3:36:8e:9d:c9:69:3e:c1:8a:38:d3:e0:ec:2b:58:c2:e0:0c:de:4f:f3:af:51:d2:5c:bc:b2:c3:3b:1e:56
3670+
km-srandom: 42:f3:36:8e:fa:fd:23:3e:fd:f9:bc:88:3c:98:93:f3:c3:1d:9c:2a:4a:3b:02:a7:40:d4:64:04:59:e9:65:97
3671+
km-cipalg: AES
3672+
km-hashalg: SHA-384
3673+
km-keylen: 32
3674+
km-explen: 0
3675+
km-ivlen: 4
3676+
km-maclen: 0
3677+
km-ccipkey: 3c:03:17:61:1e:88:4a:aa:01:4c:ac:6c:f8:bb:91:c3:0e:ec:57:c7:bf:07:ff:eb:49:22:f9:80:12:64:72:2a
3678+
km-scipkey: f8:00:8e:b2:dc:25:98:f1:97:00:55:28:60:a3:65:da:42:89:18:bb:40:94:53:d2:75:2a:29:e5:aa:94:1d:7a
3679+
km-civ: 24:02:76:6f
3680+
km-siv: 3b:6d:33:5a
3681+
km-cmackey: (null)
3682+
km-smackey: (null)

3 commit comments

Comments
 (3)

openjdk-notifier[bot] commented on Oct 13, 2022

@openjdk-notifier[bot]

GoeLin commented on Apr 16, 2023

@GoeLin
Member

/backport jdk17u-dev

openjdk[bot] commented on Apr 16, 2023

@openjdk[bot]

@GoeLin the backport was successfully created on the branch GoeLin-backport-94caecbe in my personal fork of openjdk/jdk17u-dev. To create a pull request with this backport targeting openjdk/jdk17u-dev:master, just click the following link:

➡️ Create pull request

The title of the pull request is automatically filled in correctly and below you find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit 94caecbe from the openjdk/jdk repository.

The commit being backported was authored by Daniel Jeliński on 13 Oct 2022 and was reviewed by Valerie Peng.

Thanks!

If you need to update the source branch of the pull then run the following commands in a local clone of your personal fork of openjdk/jdk17u-dev:

$ git fetch https://github.com/openjdk-bots/jdk17u-dev.git GoeLin-backport-94caecbe:GoeLin-backport-94caecbe
$ git checkout GoeLin-backport-94caecbe
# make changes
$ git add paths/to/changed/files
$ git commit --message 'Describe additional changes made'
$ git push https://github.com/openjdk-bots/jdk17u-dev.git GoeLin-backport-94caecbe
Please sign in to comment.