From 9c00a950604aca819cee977f1dcb4b45f2af3aa6 Mon Sep 17 00:00:00 2001 From: Ben Laurie Date: Mon, 28 Jan 2013 17:30:38 +0000 Subject: [PATCH] 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 2ee798880a246d648ecddadc5b91367bee4a5d98) Conflicts: crypto/crypto.h ssl/t1_lib.c --- crypto/cryptlib.c | 13 +++++++++++++ crypto/crypto.h | 7 +++++++ crypto/rsa/rsa_oaep.c | 2 +- ssl/d1_pkt.c | 2 +- ssl/s2_clnt.c | 2 +- ssl/s2_pkt.c | 3 +-- ssl/s3_both.c | 2 +- ssl/s3_pkt.c | 2 +- ssl/t1_lib.c | 2 +- 9 files changed, 27 insertions(+), 8 deletions(-) diff --git a/crypto/cryptlib.c b/crypto/cryptlib.c index 139bd1272a..fd6e3ccb9a 100644 --- a/crypto/cryptlib.c +++ b/crypto/cryptlib.c @@ -896,3 +896,16 @@ void OpenSSLDie(const char *file,int line,const char *assertion) } void *OPENSSL_stderr(void) { return stderr; } + +int CRYPTO_memcmp(const void *in_a, const void *in_b, size_t len) + { + size_t i; + const unsigned char *a = in_a; + const unsigned char *b = in_b; + unsigned char x = 0; + + for (i = 0; i < len; i++) + x |= a[i] ^ b[i]; + + return x; + } diff --git a/crypto/crypto.h b/crypto/crypto.h index b0360cec51..ab65f69a7e 100644 --- a/crypto/crypto.h +++ b/crypto/crypto.h @@ -547,6 +547,13 @@ unsigned long *OPENSSL_ia32cap_loc(void); #define OPENSSL_ia32cap (*(OPENSSL_ia32cap_loc())) int OPENSSL_isservice(void); +/* CRYPTO_memcmp returns zero iff the |len| bytes at |a| and |b| are equal. It + * takes an amount of time dependent on |len|, but independent of the contents + * of |a| and |b|. Unlike memcmp, it cannot be used to put elements into a + * defined order as the return value when a != b is undefined, other than to be + * non-zero. */ +int CRYPTO_memcmp(const void *a, const void *b, size_t len); + /* BEGIN ERROR CODES */ /* The following lines are auto generated by the script mkerr.pl. Any changes * made after this point may be overwritten when the script is next run. diff --git a/crypto/rsa/rsa_oaep.c b/crypto/rsa/rsa_oaep.c index 18d307ea9e..9b9dba0541 100644 --- a/crypto/rsa/rsa_oaep.c +++ b/crypto/rsa/rsa_oaep.c @@ -147,7 +147,7 @@ int RSA_padding_check_PKCS1_OAEP(unsigned char *to, int tlen, EVP_Digest((void *)param, plen, phash, NULL, EVP_sha1(), NULL); - if (memcmp(db, phash, SHA_DIGEST_LENGTH) != 0 || bad) + if (CRYPTO_memcmp(db, phash, SHA_DIGEST_LENGTH) != 0 || bad) goto decoding_err; else { diff --git a/ssl/d1_pkt.c b/ssl/d1_pkt.c index 6d245736f6..46a195089e 100644 --- a/ssl/d1_pkt.c +++ b/ssl/d1_pkt.c @@ -455,7 +455,7 @@ printf("\n"); else rr->length = 0; i=s->method->ssl3_enc->mac(s,md,0); - if (i < 0 || mac == NULL || memcmp(md, mac, mac_size) != 0) + if (i < 0 || mac == NULL || CRYPTO_memcmp(md,mac,mac_size) != 0) { decryption_failed_or_bad_record_mac = 1; } diff --git a/ssl/s2_clnt.c b/ssl/s2_clnt.c index 76b690ea13..03b6cf9673 100644 --- a/ssl/s2_clnt.c +++ b/ssl/s2_clnt.c @@ -939,7 +939,7 @@ static int get_server_verify(SSL *s) s->msg_callback(0, s->version, 0, p, len, s, s->msg_callback_arg); /* SERVER-VERIFY */ p += 1; - if (memcmp(p,s->s2->challenge,s->s2->challenge_length) != 0) + if (CRYPTO_memcmp(p,s->s2->challenge,s->s2->challenge_length) != 0) { ssl2_return_error(s,SSL2_PE_UNDEFINED_ERROR); SSLerr(SSL_F_GET_SERVER_VERIFY,SSL_R_CHALLENGE_IS_DIFFERENT); diff --git a/ssl/s2_pkt.c b/ssl/s2_pkt.c index ac963b2d47..8bb6ab8baa 100644 --- a/ssl/s2_pkt.c +++ b/ssl/s2_pkt.c @@ -269,8 +269,7 @@ static int ssl2_read_internal(SSL *s, void *buf, int len, int peek) s->s2->ract_data_length-=mac_size; ssl2_mac(s,mac,0); s->s2->ract_data_length-=s->s2->padding; - if ( (memcmp(mac,s->s2->mac_data, - (unsigned int)mac_size) != 0) || + if ( (CRYPTO_memcmp(mac,s->s2->mac_data,mac_size) != 0) || (s->s2->rlength%EVP_CIPHER_CTX_block_size(s->enc_read_ctx) != 0)) { SSLerr(SSL_F_SSL2_READ_INTERNAL,SSL_R_BAD_MAC_DECODE); diff --git a/ssl/s3_both.c b/ssl/s3_both.c index a6d869df59..4801dbfb85 100644 --- a/ssl/s3_both.c +++ b/ssl/s3_both.c @@ -240,7 +240,7 @@ int ssl3_get_finished(SSL *s, int a, int b) goto f_err; } - if (memcmp(p, s->s3->tmp.peer_finish_md, i) != 0) + if (CRYPTO_memcmp(p, s->s3->tmp.peer_finish_md, i) != 0) { al=SSL_AD_DECRYPT_ERROR; SSLerr(SSL_F_SSL3_GET_FINISHED,SSL_R_DIGEST_CHECK_FAILED); diff --git a/ssl/s3_pkt.c b/ssl/s3_pkt.c index f9b3629cf7..85ff7022de 100644 --- a/ssl/s3_pkt.c +++ b/ssl/s3_pkt.c @@ -462,7 +462,7 @@ printf("\n"); #endif } i=s->method->ssl3_enc->mac(s,md,0); - if (i < 0 || mac == NULL || memcmp(md, mac, (size_t)mac_size) != 0) + if (i < 0 || mac == NULL || CRYPTO_memcmp(md, mac, (size_t)mac_size) != 0) { decryption_failed_or_bad_record_mac = 1; } diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index ffd5bf294e..81d1a12820 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -1744,7 +1744,7 @@ static int tls_decrypt_ticket(SSL *s, const unsigned char *etick, int eticklen, HMAC_Update(&hctx, etick, eticklen); HMAC_Final(&hctx, tick_hmac, NULL); HMAC_CTX_cleanup(&hctx); - if (memcmp(tick_hmac, etick + eticklen, mlen)) + if (CRYPTO_memcmp(tick_hmac, etick + eticklen, mlen)) goto tickerr; /* Attempt to decrypt session data */ /* Move p after IV to start of encrypted ticket, update length */ -- 2.25.1