| From 306d003174cb4e5994734b20d741867aeeebf918 Mon Sep 17 00:00:00 2001 |
| From: Adam Langley <agl@chromium.org> |
| Date: Wed, 16 Jan 2013 11:02:35 -0500 |
| Subject: [PATCH 1/2] 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. |
| --- |
| 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 a7cb420..304c6b7 100644 |
| --- a/crypto/cryptlib.c |
| +++ b/crypto/cryptlib.c |
| @@ -925,3 +925,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 6160576..f92fc51 100644 |
| --- a/crypto/crypto.h |
| +++ b/crypto/crypto.h |
| @@ -574,6 +574,13 @@ void OPENSSL_init(void); |
| #define fips_cipher_abort(alg) while(0) |
| #endif |
| |
| +/* 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 553d212..af4d24a 100644 |
| --- a/crypto/rsa/rsa_oaep.c |
| +++ b/crypto/rsa/rsa_oaep.c |
| @@ -149,7 +149,7 @@ int RSA_padding_check_PKCS1_OAEP(unsigned char *to, int tlen, |
| if (!EVP_Digest((void *)param, plen, phash, NULL, EVP_sha1(), NULL)) |
| return -1; |
| |
| - 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 987af60..5e2c56c 100644 |
| --- a/ssl/d1_pkt.c |
| +++ b/ssl/d1_pkt.c |
| @@ -463,7 +463,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 76b690e..03b6cf9 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 ac963b2..8bb6ab8 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 918da35..ead01c8 100644 |
| --- a/ssl/s3_both.c |
| +++ b/ssl/s3_both.c |
| @@ -265,7 +265,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 dca3458..3e11140 100644 |
| --- a/ssl/s3_pkt.c |
| +++ b/ssl/s3_pkt.c |
| @@ -463,7 +463,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 d8df062..27010dd 100644 |
| --- a/ssl/t1_lib.c |
| +++ b/ssl/t1_lib.c |
| @@ -2226,7 +2226,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)) |
| return 2; |
| /* Attempt to decrypt session data */ |
| /* Move p after IV to start of encrypted ticket, update length */ |
| -- |
| 1.8.1 |
| |