From: Ben Laurie Date: Mon, 28 Jan 2013 17:30:38 +0000 (+0000) Subject: Add and use a constant-time memcmp. X-Git-Tag: OpenSSL_0_9_8y~20 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=270881316664396326c461ec7a124aec2c6cc081;p=thirdparty%2Fopenssl.git 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 (cherry picked from commit dc406b59f3169fe191e58906df08dce97edb727c) Conflicts: crypto/crypto.h ssl/d1_pkt.c ssl/s3_pkt.c --- diff --git a/crypto/cryptlib.c b/crypto/cryptlib.c index dd74ea8855f..0566ec1f976 100644 --- a/crypto/cryptlib.c +++ b/crypto/cryptlib.c @@ -542,3 +542,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 fc1374fad5f..6161697cdc5 100644 --- a/crypto/crypto.h +++ b/crypto/crypto.h @@ -591,6 +591,13 @@ int OPENSSL_isservice(void); #define OPENSSL_HAVE_INIT 1 void OPENSSL_init(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 546ae5fcb2e..b8e3edc000f 100644 --- a/crypto/rsa/rsa_oaep.c +++ b/crypto/rsa/rsa_oaep.c @@ -143,7 +143,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 65b1ef28cde..8e9ee5a77e5 100644 --- a/ssl/d1_pkt.c +++ b/ssl/d1_pkt.c @@ -410,8 +410,8 @@ if ( (sess == NULL) || } else rr->length = 0; - s->method->ssl3_enc->mac(s,md,0); - if (mac == NULL || memcmp(md, mac, mac_size) != 0) + i=s->method->ssl3_enc->mac(s,md,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 782129cd5d5..c13a6404551 100644 --- a/ssl/s2_clnt.c +++ b/ssl/s2_clnt.c @@ -935,7 +935,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 a10929a757f..7387d8b7c59 100644 --- a/ssl/s2_pkt.c +++ b/ssl/s2_pkt.c @@ -267,8 +267,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 869a25d4764..86ad59858ca 100644 --- a/ssl/s3_both.c +++ b/ssl/s3_both.c @@ -242,7 +242,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 5e3583c04d4..0e041c0ce02 100644 --- a/ssl/s3_pkt.c +++ b/ssl/s3_pkt.c @@ -414,7 +414,7 @@ printf("\n"); #endif } i=s->method->ssl3_enc->mac(s,md,0); - if (mac == NULL || memcmp(md, mac, 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 00b8286acb8..d56456e14df 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -1012,7 +1012,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 */