Skip to content

Commit 7c770d5

Browse files
benlauriesnhenson
authored andcommitted
Add and use a constant-time memcmp.
This change adds CRYPTO_memcmp, which compares two vectors of bytes in an amount of time that's independent of their contents. It also changes several MAC compares in the code to use this over the standard memcmp, which may leak information about the size of a matching prefix. (cherry picked from commit 2ee7988)
1 parent ea34a58 commit 7c770d5

File tree

9 files changed

+27
-8
lines changed

9 files changed

+27
-8
lines changed

crypto/cryptlib.c

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -397,3 +397,16 @@ void OpenSSLDie(const char *file,int line,const char *assertion)
397397
#ifndef OPENSSL_FIPSCANISTER
398398
void *OPENSSL_stderr(void) { return stderr; }
399399
#endif
400+
401+
int CRYPTO_memcmp(const void *in_a, const void *in_b, size_t len)
402+
{
403+
size_t i;
404+
const unsigned char *a = in_a;
405+
const unsigned char *b = in_b;
406+
unsigned char x = 0;
407+
408+
for (i = 0; i < len; i++)
409+
x |= a[i] ^ b[i];
410+
411+
return x;
412+
}

crypto/crypto.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -567,6 +567,13 @@ int FIPS_mode_set(int r);
567567

568568
void OPENSSL_init(void);
569569

570+
/* CRYPTO_memcmp returns zero iff the |len| bytes at |a| and |b| are equal. It
571+
* takes an amount of time dependent on |len|, but independent of the contents
572+
* of |a| and |b|. Unlike memcmp, it cannot be used to put elements into a
573+
* defined order as the return value when a != b is undefined, other than to be
574+
* non-zero. */
575+
int CRYPTO_memcmp(const void *a, const void *b, size_t len);
576+
570577
/* BEGIN ERROR CODES */
571578
/* The following lines are auto generated by the script mkerr.pl. Any changes
572579
* made after this point may be overwritten when the script is next run.

crypto/rsa/rsa_oaep.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ int RSA_padding_check_PKCS1_OAEP(unsigned char *to, int tlen,
151151
if (!EVP_Digest((void *)param, plen, phash, NULL, EVP_sha1(), NULL))
152152
return -1;
153153

154-
if (memcmp(db, phash, SHA_DIGEST_LENGTH) != 0 || bad)
154+
if (CRYPTO_memcmp(db, phash, SHA_DIGEST_LENGTH) != 0 || bad)
155155
goto decoding_err;
156156
else
157157
{

ssl/d1_pkt.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -463,7 +463,7 @@ printf("\n");
463463
else
464464
rr->length = 0;
465465
i=s->method->ssl3_enc->mac(s,md,0);
466-
if (i < 0 || mac == NULL || memcmp(md, mac, mac_size) != 0)
466+
if (i < 0 || mac == NULL || CRYPTO_memcmp(md,mac,mac_size) != 0)
467467
{
468468
decryption_failed_or_bad_record_mac = 1;
469469
}

ssl/s2_clnt.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -939,7 +939,7 @@ static int get_server_verify(SSL *s)
939939
s->msg_callback(0, s->version, 0, p, len, s, s->msg_callback_arg); /* SERVER-VERIFY */
940940
p += 1;
941941

942-
if (memcmp(p,s->s2->challenge,s->s2->challenge_length) != 0)
942+
if (CRYPTO_memcmp(p,s->s2->challenge,s->s2->challenge_length) != 0)
943943
{
944944
ssl2_return_error(s,SSL2_PE_UNDEFINED_ERROR);
945945
SSLerr(SSL_F_GET_SERVER_VERIFY,SSL_R_CHALLENGE_IS_DIFFERENT);

ssl/s2_pkt.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -269,8 +269,7 @@ static int ssl2_read_internal(SSL *s, void *buf, int len, int peek)
269269
s->s2->ract_data_length-=mac_size;
270270
ssl2_mac(s,mac,0);
271271
s->s2->ract_data_length-=s->s2->padding;
272-
if ( (memcmp(mac,s->s2->mac_data,
273-
(unsigned int)mac_size) != 0) ||
272+
if ( (CRYPTO_memcmp(mac,s->s2->mac_data,mac_size) != 0) ||
274273
(s->s2->rlength%EVP_CIPHER_CTX_block_size(s->enc_read_ctx) != 0))
275274
{
276275
SSLerr(SSL_F_SSL2_READ_INTERNAL,SSL_R_BAD_MAC_DECODE);

ssl/s3_both.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,7 @@ int ssl3_get_finished(SSL *s, int a, int b)
265265
goto f_err;
266266
}
267267

268-
if (memcmp(p, s->s3->tmp.peer_finish_md, i) != 0)
268+
if (CRYPTO_memcmp(p, s->s3->tmp.peer_finish_md, i) != 0)
269269
{
270270
al=SSL_AD_DECRYPT_ERROR;
271271
SSLerr(SSL_F_SSL3_GET_FINISHED,SSL_R_DIGEST_CHECK_FAILED);

ssl/s3_pkt.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -465,7 +465,7 @@ printf("\n");
465465
#endif
466466
}
467467
i=s->method->ssl3_enc->mac(s,md,0);
468-
if (i < 0 || mac == NULL || memcmp(md, mac, (size_t)mac_size) != 0)
468+
if (i < 0 || mac == NULL || CRYPTO_memcmp(md, mac, (size_t)mac_size) != 0)
469469
{
470470
decryption_failed_or_bad_record_mac = 1;
471471
}

ssl/t1_lib.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3125,7 +3125,7 @@ static int tls_decrypt_ticket(SSL *s, const unsigned char *etick, int eticklen,
31253125
HMAC_Update(&hctx, etick, eticklen);
31263126
HMAC_Final(&hctx, tick_hmac, NULL);
31273127
HMAC_CTX_cleanup(&hctx);
3128-
if (memcmp(tick_hmac, etick + eticklen, mlen))
3128+
if (CRYPTO_memcmp(tick_hmac, etick + eticklen, mlen))
31293129
return 2;
31303130
/* Attempt to decrypt session data */
31313131
/* Move p after IV to start of encrypted ticket, update length */

0 commit comments

Comments
 (0)