6540060 race in pkcs#11 engine in multithreaded environment
6554248 OpenSSL pkcs#11 engine doesn't strip leading zeros from a computed Diffie-Hellman shared secret
--- a/usr/src/common/openssl/crypto/engine/hw_pk11.c Tue May 15 12:25:41 2007 -0700
+++ b/usr/src/common/openssl/crypto/engine/hw_pk11.c Tue May 15 12:40:20 2007 -0700
@@ -1,5 +1,5 @@
/*
- * Copyright 2006 Sun Microsystems, Inc. All rights reserved.
+ * Copyright 2007 Sun Microsystems, Inc. All rights reserved.
* Use is subject to license terms.
*/
@@ -1516,11 +1516,19 @@
a_key_template[5].pValue = (void *) key;
a_key_template[5].ulValueLen = (unsigned long) ctx->key_len;
+ /*
+ * As stated in v2.20, 11.7 Object Management Function, in section for
+ * C_FindObjectsInit(), at most one search operation may be active at
+ * a given time in a given session. Therefore, we must group these
+ * three calls in one atomic operation.
+ */
+ CRYPTO_w_lock(CRYPTO_LOCK_PK11_ENGINE);
rv = pFuncList->C_FindObjectsInit(session, a_key_template,
ul_key_attr_count);
if (rv != CKR_OK)
{
+ CRYPTO_w_unlock(CRYPTO_LOCK_PK11_ENGINE);
PK11err(PK11_F_GET_CIPHER_KEY, PK11_R_FINDOBJECTSINIT);
snprintf(tmp_buf, sizeof (tmp_buf), "%lx", rv);
ERR_add_error_data(2, "PK11 CK_RV=0X", tmp_buf);
@@ -1531,6 +1539,7 @@
if (rv != CKR_OK)
{
+ CRYPTO_w_unlock(CRYPTO_LOCK_PK11_ENGINE);
PK11err(PK11_F_GET_CIPHER_KEY, PK11_R_FINDOBJECTS);
snprintf(tmp_buf, sizeof (tmp_buf), "%lx", rv);
ERR_add_error_data(2, "PK11 CK_RV=0X", tmp_buf);
@@ -1538,6 +1547,7 @@
}
rv = pFuncList->C_FindObjectsFinal(session);
+ CRYPTO_w_unlock(CRYPTO_LOCK_PK11_ENGINE);
if (rv != CKR_OK)
{
--- a/usr/src/common/openssl/crypto/engine/hw_pk11_pub.c Tue May 15 12:25:41 2007 -0700
+++ b/usr/src/common/openssl/crypto/engine/hw_pk11_pub.c Tue May 15 12:40:20 2007 -0700
@@ -1,5 +1,5 @@
/*
- * Copyright 2006 Sun Microsystems, Inc. All rights reserved.
+ * Copyright 2007 Sun Microsystems, Inc. All rights reserved.
* Use is subject to license terms.
*/
#pragma ident "%Z%%M% %I% %E% SMI"
@@ -1981,19 +1981,18 @@
static int pk11_DH_compute_key(unsigned char *key,const BIGNUM *pub_key,DH *dh)
{
- CK_ULONG i;
+ int i;
CK_MECHANISM mechanism = {CKM_DH_PKCS_DERIVE, NULL_PTR, 0};
CK_OBJECT_CLASS key_class = CKO_SECRET_KEY;
CK_KEY_TYPE key_type = CKK_GENERIC_SECRET;
CK_OBJECT_HANDLE h_derived_key = CK_INVALID_HANDLE;
CK_OBJECT_HANDLE h_key = CK_INVALID_HANDLE;
- CK_ULONG ul_priv_key_attr_count = 3;
+ CK_ULONG ul_priv_key_attr_count = 2;
CK_ATTRIBUTE priv_key_template[] =
{
{CKA_CLASS, (void*) NULL, sizeof(key_class)},
{CKA_KEY_TYPE, (void*) NULL, sizeof(key_type)},
- {CKA_VALUE_LEN, (void*) NULL, sizeof(i)},
};
CK_ULONG priv_key_attr_result_count = 1;
@@ -2034,8 +2033,6 @@
goto err;
}
- i = mechanism.ulParameterLen;
- priv_key_template[2].pValue = &i;
rv = pFuncList->C_DeriveKey(sp->session,
&mechanism,
h_key,
@@ -2089,9 +2086,23 @@
* length of the public key. It is long enough for the derived key */
if (priv_key_result[0].type == CKA_VALUE)
{
- memcpy(key, priv_key_result[0].pValue,
- priv_key_result[0].ulValueLen);
- ret = priv_key_result[0].ulValueLen;
+ /* CKM_DH_PKCS_DERIVE mechanism is not supposed to strip
+ * leading zeros from a computed shared secret. However,
+ * OpenSSL always did it so we must do the same here. The
+ * vagueness of the spec regarding leading zero bytes was
+ * finally cleared with TLS 1.1 (RFC 4346) saying that leading
+ * zeros are stripped before the computed data is used as the
+ * pre-master secret.
+ */
+ for (i = 0; i < priv_key_result[0].ulValueLen; ++i)
+ {
+ if (((char *) priv_key_result[0].pValue)[i] != 0)
+ break;
+ }
+
+ memcpy(key, ((char *) priv_key_result[0].pValue) + i,
+ priv_key_result[0].ulValueLen - i);
+ ret = priv_key_result[0].ulValueLen - i;
}
err: