# HG changeset patch # User jp161948 # Date 1179258020 25200 # Node ID daa332a90913fdcdf0547b1ff794517cb5972bde # Parent 77e7b46e3d5ed0bd4155a03d0a744d6159c896be 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 diff -r 77e7b46e3d5e -r daa332a90913 usr/src/common/openssl/crypto/engine/hw_pk11.c --- 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) { diff -r 77e7b46e3d5e -r daa332a90913 usr/src/common/openssl/crypto/engine/hw_pk11_pub.c --- 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: