Skip to content

Commit e82110b

Browse files
douzzerdouzzer
committed
fix races around WOLFSSL_CTX.{privateKey,privateKeyMask,altPrivateKey,altPrivateKeyMask} in WOLFSSL_BLIND_PRIVATE_KEY code paths:
* rename wolfssl_priv_der_unblind() to wolfssl_priv_der_blind_toggle(), * add wolfssl_priv_der_unblind() that allocates a temp copy, * add wolfssl_priv_der_unblind_free(), * in wolfssl_priv_der_blind_toggle(), make mask a const arg; restore const attribute to ctx arg to wolfSSL_CTX_get0_privatekey(), and add explanatory comment.
1 parent 50c5028 commit e82110b

File tree

5 files changed

+112
-58
lines changed

5 files changed

+112
-58
lines changed

‎src/internal.c‎

Lines changed: 43 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -293,12 +293,40 @@ int wolfssl_priv_der_blind(WC_RNG* rng, DerBuffer* key, DerBuffer** mask)
293293
return ret;
294294
}
295295

296-
void wolfssl_priv_der_unblind(DerBuffer* key, DerBuffer* mask)
296+
void wolfssl_priv_der_blind_toggle(DerBuffer* key, const DerBuffer* mask)
297297
{
298298
if (key != NULL) {
299299
xorbuf(key->buffer, mask->buffer, mask->length);
300300
}
301301
}
302+
303+
DerBuffer *wolfssl_priv_der_unblind(const DerBuffer* key, const DerBuffer* mask)
304+
{
305+
DerBuffer *ret;
306+
if (key == NULL)
307+
return NULL;
308+
if (mask->length > key->length)
309+
return NULL;
310+
ret = (DerBuffer *)XMALLOC(sizeof(*key) + key->length, key->heap,
311+
DYNAMIC_TYPE_TMP_BUFFER);
312+
if (ret == NULL)
313+
return NULL;
314+
XMEMCPY(ret, key, sizeof(*key));
315+
ret->buffer = (byte *)ret + sizeof(*key);
316+
xorbufout(ret->buffer, key->buffer, mask->buffer, mask->length);
317+
return ret;
318+
}
319+
320+
void wolfssl_priv_der_unblind_free(DerBuffer* key)
321+
{
322+
if (key != NULL) {
323+
void *heap = key->heap;
324+
ForceZero(key->buffer, key->length);
325+
ForceZero(key, sizeof(*key));
326+
XFREE(key, heap, DYNAMIC_TYPE_TMP_BUFFER);
327+
}
328+
}
329+
302330
#endif /* !NO_CERT && WOLFSSL_BLIND_PRIVATE_KEY */
303331

304332

@@ -7090,7 +7118,7 @@ int SetSSL_CTX(WOLFSSL* ssl, WOLFSSL_CTX* ctx, int writeDup)
70907118
}
70917119
ssl->buffers.weOwnKey = 1;
70927120
/* Blind the private key for the SSL with new random mask. */
7093-
wolfssl_priv_der_unblind(ssl->buffers.key, ctx->privateKeyMask);
7121+
wolfssl_priv_der_blind_toggle(ssl->buffers.key, ctx->privateKeyMask);
70947122
ret = wolfssl_priv_der_blind(ssl->rng, ssl->buffers.key,
70957123
&ssl->buffers.keyMask);
70967124
if (ret != 0) {
@@ -7115,7 +7143,8 @@ int SetSSL_CTX(WOLFSSL* ssl, WOLFSSL_CTX* ctx, int writeDup)
71157143
return ret;
71167144
}
71177145
/* Blind the private key for the SSL with new random mask. */
7118-
wolfssl_priv_der_unblind(ssl->buffers.altKey, ctx->altPrivateKeyMask);
7146+
wolfssl_priv_der_blind_toggle(ssl->buffers.altKey,
7147+
ctx->altPrivateKeyMask);
71197148
ret = wolfssl_priv_der_blind(ssl->rng, ssl->buffers.altKey,
71207149
&ssl->buffers.altKeyMask);
71217150
if (ret != 0) {
@@ -30281,7 +30310,7 @@ int DecodeAltPrivateKey(WOLFSSL *ssl, word32* length)
3028130310
}
3028230311

3028330312
#ifdef WOLFSSL_BLIND_PRIVATE_KEY
30284-
wolfssl_priv_der_unblind(ssl->buffers.altKey, ssl->buffers.altKeyMask);
30313+
wolfssl_priv_der_blind_toggle(ssl->buffers.altKey, ssl->buffers.altKeyMask);
3028530314
#endif
3028630315

3028730316
#ifdef WOLF_PRIVATE_KEY_ID
@@ -30691,7 +30720,7 @@ int DecodeAltPrivateKey(WOLFSSL *ssl, word32* length)
3069130720
&ssl->buffers.altKeyMask);
3069230721
}
3069330722
else {
30694-
wolfssl_priv_der_unblind(ssl->buffers.key, ssl->buffers.keyMask);
30723+
wolfssl_priv_der_blind_toggle(ssl->buffers.key, ssl->buffers.keyMask);
3069530724
}
3069630725
#endif
3069730726

@@ -34386,7 +34415,7 @@ int SendCertificateVerify(WOLFSSL* ssl)
3438634415
WOLFSSL_ENTER("SendCertificateVerify");
3438734416

3438834417
#ifdef WOLFSSL_BLIND_PRIVATE_KEY
34389-
wolfssl_priv_der_unblind(ssl->buffers.key, ssl->buffers.keyMask);
34418+
wolfssl_priv_der_blind_toggle(ssl->buffers.key, ssl->buffers.keyMask);
3439034419
#endif
3439134420

3439234421
#ifdef WOLFSSL_ASYNC_IO
@@ -34436,7 +34465,7 @@ int SendCertificateVerify(WOLFSSL* ssl)
3443634465
{
3443734466
if (ssl->options.sendVerify == SEND_BLANK_CERT) {
3443834467
#ifdef WOLFSSL_BLIND_PRIVATE_KEY
34439-
wolfssl_priv_der_unblind(ssl->buffers.key,
34468+
wolfssl_priv_der_blind_toggle(ssl->buffers.key,
3444034469
ssl->buffers.keyMask);
3444134470
#endif
3444234471
return 0; /* sent blank cert, can't verify */
@@ -34864,7 +34893,7 @@ int SendCertificateVerify(WOLFSSL* ssl)
3486434893
&ssl->buffers.keyMask);
3486534894
}
3486634895
else {
34867-
wolfssl_priv_der_unblind(ssl->buffers.key, ssl->buffers.keyMask);
34896+
wolfssl_priv_der_blind_toggle(ssl->buffers.key, ssl->buffers.keyMask);
3486834897
}
3486934898
#endif
3487034899

@@ -35538,7 +35567,7 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
3553835567
WOLFSSL_ENTER("SendServerKeyExchange");
3553935568

3554035569
#ifdef WOLFSSL_BLIND_PRIVATE_KEY
35541-
wolfssl_priv_der_unblind(ssl->buffers.key, ssl->buffers.keyMask);
35570+
wolfssl_priv_der_blind_toggle(ssl->buffers.key, ssl->buffers.keyMask);
3554235571
#endif
3554335572

3554435573
#ifdef WOLFSSL_ASYNC_IO
@@ -37123,7 +37152,8 @@ static int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx,
3712337152
&ssl->buffers.keyMask);
3712437153
}
3712537154
else {
37126-
wolfssl_priv_der_unblind(ssl->buffers.key, ssl->buffers.keyMask);
37155+
wolfssl_priv_der_blind_toggle(ssl->buffers.key,
37156+
ssl->buffers.keyMask);
3712737157
}
3712837158
#endif
3712937159

@@ -40999,7 +41029,7 @@ static int DefTicketEncCb(WOLFSSL* ssl, byte key_name[WOLFSSL_TICKET_NAME_SZ],
4099941029
WOLFSSL_ENTER("DoClientKeyExchange");
4100041030

4100141031
#ifdef WOLFSSL_BLIND_PRIVATE_KEY
41002-
wolfssl_priv_der_unblind(ssl->buffers.key, ssl->buffers.keyMask);
41032+
wolfssl_priv_der_blind_toggle(ssl->buffers.key, ssl->buffers.keyMask);
4100341033
#endif
4100441034

4100541035
#ifdef WOLFSSL_ASYNC_CRYPT
@@ -41806,7 +41836,8 @@ static int DefTicketEncCb(WOLFSSL* ssl, byte key_name[WOLFSSL_TICKET_NAME_SZ],
4180641836
&ssl->buffers.keyMask);
4180741837
}
4180841838
else {
41809-
wolfssl_priv_der_unblind(ssl->buffers.key, ssl->buffers.keyMask);
41839+
wolfssl_priv_der_blind_toggle(ssl->buffers.key,
41840+
ssl->buffers.keyMask);
4181041841
}
4181141842
#endif
4181241843

‎src/ssl.c‎

Lines changed: 60 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -7361,9 +7361,9 @@ static int check_cert_key_dev(word32 keyOID, byte* privKey, word32 privSz,
73617361
*
73627362
* Returns WOLFSSL_SUCCESS on good private key
73637363
* WOLFSSL_FAILURE if mismatched */
7364-
static int check_cert_key(DerBuffer* cert, DerBuffer* key, DerBuffer* altKey,
7365-
void* heap, int devId, int isKeyLabel, int isKeyId, int altDevId,
7366-
int isAltKeyLabel, int isAltKeyId)
7364+
static int check_cert_key(const DerBuffer* cert, const DerBuffer* key,
7365+
const DerBuffer* altKey, void* heap, int devId, int isKeyLabel, int isKeyId,
7366+
int altDevId, int isAltKeyLabel, int isAltKeyId)
73677367
{
73687368
WC_DECLARE_VAR(der, DecodedCert, 1, 0);
73697369
word32 size;
@@ -7498,50 +7498,61 @@ static int check_cert_key(DerBuffer* cert, DerBuffer* key, DerBuffer* altKey,
74987498
* WOLFSSL_FAILURE if mismatched. */
74997499
int wolfSSL_CTX_check_private_key(const WOLFSSL_CTX* ctx)
75007500
{
7501-
int res;
7501+
int res = WOLFSSL_SUCCESS;
7502+
#ifdef WOLFSSL_BLIND_PRIVATE_KEY
7503+
DerBuffer *privateKey;
7504+
#ifdef WOLFSSL_DUAL_ALG_CERTS
7505+
DerBuffer *altPrivateKey;
7506+
#endif
7507+
#else
7508+
const DerBuffer *privateKey;
7509+
#ifdef WOLFSSL_DUAL_ALG_CERTS
7510+
const DerBuffer *altPrivateKey;
7511+
#endif
7512+
#endif
75027513

75037514
if (ctx == NULL) {
75047515
return WOLFSSL_FAILURE;
75057516
}
75067517

75077518
#ifdef WOLFSSL_DUAL_ALG_CERTS
75087519
#ifdef WOLFSSL_BLIND_PRIVATE_KEY
7509-
wolfssl_priv_der_unblind(ctx->privateKey, ctx->privateKeyMask);
7510-
wolfssl_priv_der_unblind(ctx->altPrivateKey, ctx->altPrivateKeyMask);
7520+
privateKey = wolfssl_priv_der_unblind(ctx->privateKey, ctx->privateKeyMask);
7521+
altPrivateKey = wolfssl_priv_der_unblind(ctx->altPrivateKey,
7522+
ctx->altPrivateKeyMask);
7523+
if ((privateKey == NULL) || (altPrivateKey == NULL)) {
7524+
res = WOLFSSL_FAILURE;
7525+
}
7526+
#else
7527+
privateKey = ctx->privateKey;
7528+
altPrivateKey = ctx->altPrivateKey;
75117529
#endif
7512-
res = check_cert_key(ctx->certificate, ctx->privateKey, ctx->altPrivateKey,
7513-
ctx->heap, ctx->privateKeyDevId, ctx->privateKeyLabel,
7514-
ctx->privateKeyId, ctx->altPrivateKeyDevId, ctx->altPrivateKeyLabel,
7515-
ctx->altPrivateKeyId) != 0;
7516-
#ifdef WOLFSSL_BLIND_PRIVATE_KEY
7517-
{
7518-
int ret;
7519-
ret = wolfssl_priv_der_blind(NULL, ctx->privateKey,
7520-
(DerBuffer**)&ctx->privateKeyMask);
7521-
if (ret == 0) {
7522-
ret = wolfssl_priv_der_blind(NULL, ctx->altPrivateKey,
7523-
(DerBuffer**)&ctx->altPrivateKeyMask);
7524-
}
7525-
if (ret != 0) {
7526-
res = WOLFSSL_FAILURE;
7527-
}
7530+
if (res == WOLFSSL_SUCCESS) {
7531+
res = check_cert_key(ctx->certificate, privateKey, altPrivateKey,
7532+
ctx->heap, ctx->privateKeyDevId, ctx->privateKeyLabel,
7533+
ctx->privateKeyId, ctx->altPrivateKeyDevId,
7534+
ctx->altPrivateKeyLabel, ctx->altPrivateKeyId) != 0;
75287535
}
7536+
#ifdef WOLFSSL_BLIND_PRIVATE_KEY
7537+
wolfssl_priv_der_unblind_free(privateKey);
7538+
wolfssl_priv_der_unblind_free(altPrivateKey);
75297539
#endif
75307540
#else
75317541
#ifdef WOLFSSL_BLIND_PRIVATE_KEY
7532-
wolfssl_priv_der_unblind(ctx->privateKey, ctx->privateKeyMask);
7542+
privateKey = wolfssl_priv_der_unblind(ctx->privateKey, ctx->privateKeyMask);
7543+
if (privateKey == NULL) {
7544+
res = WOLFSSL_FAILURE;
7545+
}
7546+
#else
7547+
privateKey = ctx->privateKey;
75337548
#endif
7534-
res = check_cert_key(ctx->certificate, ctx->privateKey, NULL, ctx->heap,
7535-
ctx->privateKeyDevId, ctx->privateKeyLabel, ctx->privateKeyId,
7536-
INVALID_DEVID, 0, 0);
7537-
#ifdef WOLFSSL_BLIND_PRIVATE_KEY
7538-
{
7539-
int ret = wolfssl_priv_der_blind(NULL, ctx->privateKey,
7540-
(DerBuffer**)&ctx->privateKeyMask);
7541-
if (ret != 0) {
7542-
res = WOLFSSL_FAILURE;
7543-
}
7549+
if (res == WOLFSSL_SUCCESS) {
7550+
res = check_cert_key(ctx->certificate, privateKey, NULL, ctx->heap,
7551+
ctx->privateKeyDevId, ctx->privateKeyLabel,
7552+
ctx->privateKeyId, INVALID_DEVID, 0, 0);
75447553
}
7554+
#ifdef WOLFSSL_BLIND_PRIVATE_KEY
7555+
wolfssl_priv_der_unblind_free(privateKey);
75457556
#endif
75467557
#endif
75477558

@@ -7558,8 +7569,12 @@ int wolfSSL_CTX_check_private_key(const WOLFSSL_CTX* ctx)
75587569
/**
75597570
* Return the private key of the WOLFSSL_CTX struct
75607571
* @return WOLFSSL_EVP_PKEY* The caller doesn *NOT*` free the returned object.
7572+
*
7573+
* Note, even though the supplied ctx pointer is designated const, on success
7574+
* ctx->privateKeyPKey is changed by this call. The change is done safely using
7575+
* a hardware-synchronized store.
75617576
*/
7562-
WOLFSSL_EVP_PKEY* wolfSSL_CTX_get0_privatekey(WOLFSSL_CTX* ctx)
7577+
WOLFSSL_EVP_PKEY* wolfSSL_CTX_get0_privatekey(const WOLFSSL_CTX* ctx)
75637578
{
75647579
WOLFSSL_EVP_PKEY* res;
75657580
const unsigned char *key;
@@ -7596,19 +7611,23 @@ WOLFSSL_EVP_PKEY* wolfSSL_CTX_get0_privatekey(WOLFSSL_CTX* ctx)
75967611
return NULL;
75977612
}
75987613

7599-
key = ctx->privateKey->buffer;
7600-
76017614
if (ctx->privateKeyPKey != NULL) {
76027615
res = ctx->privateKeyPKey;
76037616
}
76047617
else {
76057618
#ifdef WOLFSSL_BLIND_PRIVATE_KEY
7606-
wolfssl_priv_der_unblind(ctx->privateKey, ctx->privateKeyMask);
7619+
DerBuffer *unblinded_privateKey =
7620+
wolfssl_priv_der_unblind(ctx->privateKey, ctx->privateKeyMask);
7621+
if (unblinded_privateKey == NULL)
7622+
return NULL;
7623+
key = unblinded_privateKey->buffer;
7624+
#else
7625+
key = ctx->privateKey->buffer;
76077626
#endif
76087627
res = wolfSSL_d2i_PrivateKey(type, NULL, &key,
76097628
(long)ctx->privateKey->length);
76107629
#ifdef WOLFSSL_BLIND_PRIVATE_KEY
7611-
wolfssl_priv_der_unblind(ctx->privateKey, ctx->privateKeyMask);
7630+
wolfssl_priv_der_unblind_free(unblinded_privateKey);
76127631
#endif
76137632
if (res) {
76147633
#ifdef WOLFSSL_ATOMIC_OPS
@@ -7621,7 +7640,7 @@ WOLFSSL_EVP_PKEY* wolfSSL_CTX_get0_privatekey(WOLFSSL_CTX* ctx)
76217640
res = current_pkey;
76227641
}
76237642
#else
7624-
ctx->privateKeyPKey = res;
7643+
((WOLFSSL_CTX *)ctx)->privateKeyPKey = res;
76257644
#endif
76267645
}
76277646
}
@@ -8874,7 +8893,7 @@ int wolfSSL_check_private_key(const WOLFSSL* ssl)
88748893
#endif
88758894
#else
88768895
#ifdef WOLFSSL_BLIND_PRIVATE_KEY
8877-
wolfssl_priv_der_unblind(ssl->buffers.key, ssl->buffers.keyMask);
8896+
wolfssl_priv_der_blind_toggle(ssl->buffers.key, ssl->buffers.keyMask);
88788897
#endif
88798898
res = check_cert_key(ssl->buffers.certificate, ssl->buffers.key, NULL,
88808899
ssl->heap, ssl->buffers.keyDevId, ssl->buffers.keyLabel,
@@ -20971,7 +20990,7 @@ WOLFSSL_CTX* wolfSSL_set_SSL_CTX(WOLFSSL* ssl, WOLFSSL_CTX* ctx)
2097120990
return NULL;
2097220991
}
2097320992
/* Blind the private key for the SSL with new random mask. */
20974-
wolfssl_priv_der_unblind(ssl->buffers.key, ctx->privateKeyMask);
20993+
wolfssl_priv_der_blind_toggle(ssl->buffers.key, ctx->privateKeyMask);
2097520994
ret = wolfssl_priv_der_blind(ssl->rng, ssl->buffers.key,
2097620995
&ssl->buffers.keyMask);
2097720996
if (ret != 0) {

‎src/tls13.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9131,7 +9131,7 @@ static int SendTls13CertificateVerify(WOLFSSL* ssl)
91319131
WOLFSSL_ENTER("SendTls13CertificateVerify");
91329132

91339133
#ifdef WOLFSSL_BLIND_PRIVATE_KEY
9134-
wolfssl_priv_der_unblind(ssl->buffers.key, ssl->buffers.keyMask);
9134+
wolfssl_priv_der_blind_toggle(ssl->buffers.key, ssl->buffers.keyMask);
91359135
#endif
91369136

91379137
ssl->options.buildingMsg = 1;
@@ -9187,7 +9187,7 @@ static int SendTls13CertificateVerify(WOLFSSL* ssl)
91879187
{
91889188
if (ssl->options.sendVerify == SEND_BLANK_CERT) {
91899189
#ifdef WOLFSSL_BLIND_PRIVATE_KEY
9190-
wolfssl_priv_der_unblind(ssl->buffers.key,
9190+
wolfssl_priv_der_blind_toggle(ssl->buffers.key,
91919191
ssl->buffers.keyMask);
91929192
#endif
91939193
return 0; /* sent blank cert, can't verify */
@@ -9882,7 +9882,7 @@ static int SendTls13CertificateVerify(WOLFSSL* ssl)
98829882
&ssl->buffers.keyMask);
98839883
}
98849884
else {
9885-
wolfssl_priv_der_unblind(ssl->buffers.key, ssl->buffers.keyMask);
9885+
wolfssl_priv_der_blind_toggle(ssl->buffers.key, ssl->buffers.keyMask);
98869886
}
98879887
#endif
98889888

‎wolfssl/internal.h‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2183,7 +2183,11 @@ WOLFSSL_LOCAL int CreateDevPrivateKey(void** pkey, byte* data, word32 length,
21832183
#ifdef WOLFSSL_BLIND_PRIVATE_KEY
21842184
WOLFSSL_LOCAL int wolfssl_priv_der_blind(WC_RNG* rng, DerBuffer* key,
21852185
DerBuffer** mask);
2186-
WOLFSSL_LOCAL void wolfssl_priv_der_unblind(DerBuffer* key, DerBuffer* mask);
2186+
WOLFSSL_LOCAL void wolfssl_priv_der_blind_toggle(DerBuffer* key,
2187+
const DerBuffer* mask);
2188+
WOLFSSL_LOCAL WARN_UNUSED_RESULT DerBuffer *wolfssl_priv_der_unblind(
2189+
const DerBuffer* key, const DerBuffer* mask);
2190+
WOLFSSL_LOCAL void wolfssl_priv_der_unblind_free(DerBuffer* key);
21872191
#endif
21882192
WOLFSSL_LOCAL int DecodePrivateKey(WOLFSSL *ssl, word32* length);
21892193
#ifdef WOLFSSL_DUAL_ALG_CERTS

‎wolfssl/ssl.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3228,7 +3228,7 @@ WOLFSSL_API int wolfSSL_want_write(WOLFSSL* ssl);
32283228
#ifdef OPENSSL_EXTRA
32293229
WOLFSSL_API int wolfSSL_want(WOLFSSL* ssl);
32303230

3231-
WOLFSSL_API WOLFSSL_EVP_PKEY* wolfSSL_CTX_get0_privatekey(WOLFSSL_CTX* ctx);
3231+
WOLFSSL_API WOLFSSL_EVP_PKEY* wolfSSL_CTX_get0_privatekey(const WOLFSSL_CTX* ctx);
32323232

32333233
#include <stdarg.h> /* var_arg */
32343234
WOLFSSL_API int wolfSSL_BIO_vprintf(WOLFSSL_BIO* bio, const char* format,

0 commit comments

Comments
 (0)