Skip to content

Commit 986e391

Browse files
rlm2002rlm2002
authored
Merge pull request #262 from cconlon/fenrirAug19
Fixes for key material zeroization across JCE and JNI, PBKDF input validation
2 parents 61968eb + 82503a3 commit 986e391

9 files changed

Lines changed: 591 additions & 436 deletions

File tree

‎jni/jni_jce_wolfsslkeystore.c‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,9 @@
2525
#include <wolfssl/options.h>
2626
#endif
2727

28+
#include <wolfssl/version.h>
2829
#include <wolfssl/ssl.h>
30+
#include <wolfssl/wolfcrypt/memory.h>
2931
#include <com_wolfssl_provider_jce_WolfSSLKeyStore.h>
3032
#include <wolfcrypt_jni_error.h>
3133

@@ -40,6 +42,7 @@ JNIEXPORT jboolean JNICALL Java_com_wolfssl_provider_jce_WolfSSLKeyStore_X509Che
4042
int ret = WOLFSSL_SUCCESS;
4143
int certDerSz = 0;
4244
int keyDerSz = 0;
45+
jboolean keyDerIsCopy = JNI_FALSE;
4346
byte* certDer = NULL;
4447
byte* keyDer = NULL;
4548
byte* pkcs8KeyDer = NULL;
@@ -57,7 +60,8 @@ JNIEXPORT jboolean JNICALL Java_com_wolfssl_provider_jce_WolfSSLKeyStore_X509Che
5760
certDer = (byte*)(*env)->GetByteArrayElements(env, certDerArr, NULL);
5861
certDerSz = (*env)->GetArrayLength(env, certDerArr);
5962

60-
keyDer = (byte*)(*env)->GetByteArrayElements(env, pkcs8KeyDerArr, NULL);
63+
keyDer = (byte*)(*env)->GetByteArrayElements(env, pkcs8KeyDerArr,
64+
&keyDerIsCopy);
6165
keyDerSz = (*env)->GetArrayLength(env, pkcs8KeyDerArr);
6266
/* Keep original keyDer pointer for free later, wolfSSL_d2i_PKCS8_PKEY
6367
* will change/advance the pointer. */
@@ -118,8 +122,16 @@ JNIEXPORT jboolean JNICALL Java_com_wolfssl_provider_jce_WolfSSLKeyStore_X509Che
118122
(jbyte*)certDer, JNI_ABORT);
119123
}
120124
if (keyDer != NULL) {
125+
if (keyDerIsCopy == JNI_TRUE) {
126+
#if (LIBWOLFSSL_VERSION_HEX >= 0x05008004) && \
127+
!defined(WOLFSSL_NO_FORCE_ZERO)
128+
wc_ForceZero(keyDer, keyDerSz);
129+
#else
130+
XMEMSET(keyDer, 0, keyDerSz);
131+
#endif
132+
}
121133
(*env)->ReleaseByteArrayElements(env, pkcs8KeyDerArr,
122-
(jbyte*)keyDer, JNI_ABORT);
134+
(jbyte*)keyDer, JNI_ABORT);
123135
}
124136

125137
if (ret == WOLFSSL_SUCCESS) {

‎jni/jni_pwdbased.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ JNIEXPORT jbyteArray JNICALL Java_com_wolfssl_wolfcrypt_Pwdbased_wc_1PKCS12_1PBK
5252
jbyteArray result = NULL;
5353
(void)jcl;
5454

55-
if (env == NULL || kLen == 0) {
55+
if (env == NULL || kLen <= 0) {
5656
throwWolfCryptExceptionFromError(env, BAD_FUNC_ARG);
5757
return NULL;
5858
}
@@ -147,7 +147,7 @@ JNIEXPORT jbyteArray JNICALL Java_com_wolfssl_wolfcrypt_Pwdbased_wc_1PBKDF2
147147
jbyteArray result = NULL;
148148
(void)jcl;
149149

150-
if (env == NULL || kLen == 0) {
150+
if (env == NULL || kLen <= 0) {
151151
throwWolfCryptExceptionFromError(env, BAD_FUNC_ARG);
152152
return NULL;
153153
}

‎src/main/java/com/wolfssl/provider/jce/WolfCryptCipher.java‎

Lines changed: 287 additions & 241 deletions
Large diffs are not rendered by default.

‎src/main/java/com/wolfssl/provider/jce/WolfCryptECKeyFactory.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,9 @@ private PrivateKey generatePrivateFromPKCS8(PKCS8EncodedKeySpec keySpec)
263263
if (ecc != null) {
264264
ecc.releaseNativeStruct();
265265
}
266+
if (pkcs8Der != null) {
267+
Arrays.fill(pkcs8Der, (byte)0);
268+
}
266269
if (privDer != null) {
267270
Arrays.fill(privDer, (byte)0);
268271
}

‎src/main/java/com/wolfssl/provider/jce/WolfCryptKeyGenerator.java‎

Lines changed: 31 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import java.security.InvalidParameterException;
3838
import java.security.InvalidAlgorithmParameterException;
3939
import java.security.spec.AlgorithmParameterSpec;
40+
import java.util.Arrays;
4041

4142
/**
4243
* wolfCrypt JCE KeyGenerator implementation.
@@ -251,6 +252,7 @@ protected void engineInit(int keysize, SecureRandom random)
251252
protected SecretKey engineGenerateKey() {
252253

253254
byte[] keyArr = null;
255+
SecretKey result = null;
254256

255257
try {
256258
if (this.random == null) {
@@ -261,26 +263,36 @@ protected SecretKey engineGenerateKey() {
261263
return null;
262264
}
263265

264-
keyArr = new byte[(this.keySizeBits + 7) / 8];
265-
this.random.nextBytes(keyArr);
266-
267-
log("Generating key: " + keyArr.length + " bytes");
268-
269-
switch (this.algoType) {
270-
case WC_AES:
271-
case WC_HMAC_SHA1:
272-
case WC_HMAC_SHA224:
273-
case WC_HMAC_SHA256:
274-
case WC_HMAC_SHA384:
275-
case WC_HMAC_SHA512:
276-
case WC_HMAC_SHA3_224:
277-
case WC_HMAC_SHA3_256:
278-
case WC_HMAC_SHA3_384:
279-
case WC_HMAC_SHA3_512:
280-
return new SecretKeySpec(keyArr, this.algString);
281-
default:
282-
return null;
266+
try {
267+
keyArr = new byte[(this.keySizeBits + 7) / 8];
268+
this.random.nextBytes(keyArr);
269+
270+
log("Generating key: " + keyArr.length + " bytes");
271+
272+
switch (this.algoType) {
273+
case WC_AES:
274+
case WC_HMAC_SHA1:
275+
case WC_HMAC_SHA224:
276+
case WC_HMAC_SHA256:
277+
case WC_HMAC_SHA384:
278+
case WC_HMAC_SHA512:
279+
case WC_HMAC_SHA3_224:
280+
case WC_HMAC_SHA3_256:
281+
case WC_HMAC_SHA3_384:
282+
case WC_HMAC_SHA3_512:
283+
/* SecretKeySpec clones the key bytes */
284+
result = new SecretKeySpec(keyArr, this.algString);
285+
break;
286+
default:
287+
result = null;
288+
}
289+
} finally {
290+
if (keyArr != null) {
291+
Arrays.fill(keyArr, (byte)0);
292+
}
283293
}
294+
295+
return result;
284296
}
285297

286298
/**

‎src/main/java/com/wolfssl/provider/jce/WolfCryptPBEKey.java‎

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ private synchronized void checkDestroyed()
123123
*
124124
* @throws IllegalStateException if object has been destroyed
125125
*/
126-
public synchronized char[] getPassword() {
126+
public final synchronized char[] getPassword() {
127127

128128
checkDestroyed();
129129

@@ -141,7 +141,7 @@ public synchronized char[] getPassword() {
141141
*
142142
* @throws IllegalStateException if object has been destroyed
143143
*/
144-
public synchronized byte[] getSalt() {
144+
public final synchronized byte[] getSalt() {
145145

146146
checkDestroyed();
147147

@@ -197,7 +197,7 @@ public synchronized String getFormat() {
197197
*
198198
* @throws IllegalStateException if object has been destroyed
199199
*/
200-
public synchronized byte[] getEncoded() {
200+
public final synchronized byte[] getEncoded() {
201201

202202
checkDestroyed();
203203

@@ -312,8 +312,6 @@ public boolean equals(Object obj) {
312312
return false;
313313

314314
} finally {
315-
/* Only our own copies, the other key's accessors may return
316-
* internal references and zeroizing those would destroy it */
317315
if (thisEncoded != null) {
318316
Arrays.fill(thisEncoded, (byte)0);
319317
}
@@ -323,6 +321,20 @@ public boolean equals(Object obj) {
323321
if (thisPass != null) {
324322
Arrays.fill(thisPass, (char)0);
325323
}
324+
/* WolfCryptPBEKey accessors are final and return copies which
325+
* are safe to zero, other implementations may return internal
326+
* references and zeroizing would destroy the key */
327+
if (pKey instanceof WolfCryptPBEKey) {
328+
if (pKeyEncoded != null) {
329+
Arrays.fill(pKeyEncoded, (byte)0);
330+
}
331+
if (pKeySalt != null) {
332+
Arrays.fill(pKeySalt, (byte)0);
333+
}
334+
if (pKeyPass != null) {
335+
Arrays.fill(pKeyPass, (char)0);
336+
}
337+
}
326338
}
327339
}
328340

‎src/main/java/com/wolfssl/provider/jce/WolfCryptRSAKeyFactory.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -378,6 +378,9 @@ private PrivateKey generatePrivateFromPKCS8(PKCS8EncodedKeySpec keySpec)
378378
if (rsa != null) {
379379
rsa.releaseNativeStruct();
380380
}
381+
if (pkcs8Der != null) {
382+
Arrays.fill(pkcs8Der, (byte)0);
383+
}
381384
}
382385
}
383386

0 commit comments

Comments
 (0)