|
| 1 | +From 306d003174cb4e5994734b20d741867aeeebf918 Mon Sep 17 00:00:00 2001 |
| 2 | +From: Adam Langley <agl@chromium.org> |
| 3 | +Date: Wed, 16 Jan 2013 11:02:35 -0500 |
| 4 | +Subject: [PATCH 1/2] Add and use a constant-time memcmp. |
| 5 | + |
| 6 | +This change adds CRYPTO_memcmp, which compares two vectors of bytes in |
| 7 | +an amount of time that's independent of their contents. It also changes |
| 8 | +several MAC compares in the code to use this over the standard memcmp, |
| 9 | +which may leak information about the size of a matching prefix. |
| 10 | +--- |
| 11 | + crypto/cryptlib.c | 13 +++++++++++++ |
| 12 | + crypto/crypto.h | 7 +++++++ |
| 13 | + crypto/rsa/rsa_oaep.c | 2 +- |
| 14 | + ssl/d1_pkt.c | 2 +- |
| 15 | + ssl/s2_clnt.c | 2 +- |
| 16 | + ssl/s2_pkt.c | 3 +-- |
| 17 | + ssl/s3_both.c | 2 +- |
| 18 | + ssl/s3_pkt.c | 2 +- |
| 19 | + ssl/t1_lib.c | 2 +- |
| 20 | + 9 files changed, 27 insertions(+), 8 deletions(-) |
| 21 | + |
| 22 | +diff --git a/crypto/cryptlib.c b/crypto/cryptlib.c |
| 23 | +index a7cb420..304c6b7 100644 |
| 24 | +--- a/crypto/cryptlib.c |
| 25 | ++++ b/crypto/cryptlib.c |
| 26 | +@@ -925,3 +925,16 @@ void OpenSSLDie(const char *file,int line,const char *assertion) |
| 27 | + } |
| 28 | + |
| 29 | + void *OPENSSL_stderr(void) { return stderr; } |
| 30 | ++ |
| 31 | ++int CRYPTO_memcmp(const void *in_a, const void *in_b, size_t len) |
| 32 | ++ { |
| 33 | ++ size_t i; |
| 34 | ++ const unsigned char *a = in_a; |
| 35 | ++ const unsigned char *b = in_b; |
| 36 | ++ unsigned char x = 0; |
| 37 | ++ |
| 38 | ++ for (i = 0; i < len; i++) |
| 39 | ++ x |= a[i] ^ b[i]; |
| 40 | ++ |
| 41 | ++ return x; |
| 42 | ++ } |
| 43 | +diff --git a/crypto/crypto.h b/crypto/crypto.h |
| 44 | +index 6160576..f92fc51 100644 |
| 45 | +--- a/crypto/crypto.h |
| 46 | ++++ b/crypto/crypto.h |
| 47 | +@@ -574,6 +574,13 @@ void OPENSSL_init(void); |
| 48 | + #define fips_cipher_abort(alg) while(0) |
| 49 | + #endif |
| 50 | + |
| 51 | ++/* CRYPTO_memcmp returns zero iff the |len| bytes at |a| and |b| are equal. It |
| 52 | ++ * takes an amount of time dependent on |len|, but independent of the contents |
| 53 | ++ * of |a| and |b|. Unlike memcmp, it cannot be used to put elements into a |
| 54 | ++ * defined order as the return value when a != b is undefined, other than to be |
| 55 | ++ * non-zero. */ |
| 56 | ++int CRYPTO_memcmp(const void *a, const void *b, size_t len); |
| 57 | ++ |
| 58 | + /* BEGIN ERROR CODES */ |
| 59 | + /* The following lines are auto generated by the script mkerr.pl. Any changes |
| 60 | + * made after this point may be overwritten when the script is next run. |
| 61 | +diff --git a/crypto/rsa/rsa_oaep.c b/crypto/rsa/rsa_oaep.c |
| 62 | +index 553d212..af4d24a 100644 |
| 63 | +--- a/crypto/rsa/rsa_oaep.c |
| 64 | ++++ b/crypto/rsa/rsa_oaep.c |
| 65 | +@@ -149,7 +149,7 @@ int RSA_padding_check_PKCS1_OAEP(unsigned char *to, int tlen, |
| 66 | + if (!EVP_Digest((void *)param, plen, phash, NULL, EVP_sha1(), NULL)) |
| 67 | + return -1; |
| 68 | + |
| 69 | +- if (memcmp(db, phash, SHA_DIGEST_LENGTH) != 0 || bad) |
| 70 | ++ if (CRYPTO_memcmp(db, phash, SHA_DIGEST_LENGTH) != 0 || bad) |
| 71 | + goto decoding_err; |
| 72 | + else |
| 73 | + { |
| 74 | +diff --git a/ssl/d1_pkt.c b/ssl/d1_pkt.c |
| 75 | +index 987af60..5e2c56c 100644 |
| 76 | +--- a/ssl/d1_pkt.c |
| 77 | ++++ b/ssl/d1_pkt.c |
| 78 | +@@ -463,7 +463,7 @@ printf("\n"); |
| 79 | + else |
| 80 | + rr->length = 0; |
| 81 | + i=s->method->ssl3_enc->mac(s,md,0); |
| 82 | +- if (i < 0 || mac == NULL || memcmp(md, mac, mac_size) != 0) |
| 83 | ++ if (i < 0 || mac == NULL || CRYPTO_memcmp(md,mac,mac_size) != 0) |
| 84 | + { |
| 85 | + decryption_failed_or_bad_record_mac = 1; |
| 86 | + } |
| 87 | +diff --git a/ssl/s2_clnt.c b/ssl/s2_clnt.c |
| 88 | +index 76b690e..03b6cf9 100644 |
| 89 | +--- a/ssl/s2_clnt.c |
| 90 | ++++ b/ssl/s2_clnt.c |
| 91 | +@@ -939,7 +939,7 @@ static int get_server_verify(SSL *s) |
| 92 | + s->msg_callback(0, s->version, 0, p, len, s, s->msg_callback_arg); /* SERVER-VERIFY */ |
| 93 | + p += 1; |
| 94 | + |
| 95 | +- if (memcmp(p,s->s2->challenge,s->s2->challenge_length) != 0) |
| 96 | ++ if (CRYPTO_memcmp(p,s->s2->challenge,s->s2->challenge_length) != 0) |
| 97 | + { |
| 98 | + ssl2_return_error(s,SSL2_PE_UNDEFINED_ERROR); |
| 99 | + SSLerr(SSL_F_GET_SERVER_VERIFY,SSL_R_CHALLENGE_IS_DIFFERENT); |
| 100 | +diff --git a/ssl/s2_pkt.c b/ssl/s2_pkt.c |
| 101 | +index ac963b2..8bb6ab8 100644 |
| 102 | +--- a/ssl/s2_pkt.c |
| 103 | ++++ b/ssl/s2_pkt.c |
| 104 | +@@ -269,8 +269,7 @@ static int ssl2_read_internal(SSL *s, void *buf, int len, int peek) |
| 105 | + s->s2->ract_data_length-=mac_size; |
| 106 | + ssl2_mac(s,mac,0); |
| 107 | + s->s2->ract_data_length-=s->s2->padding; |
| 108 | +- if ( (memcmp(mac,s->s2->mac_data, |
| 109 | +- (unsigned int)mac_size) != 0) || |
| 110 | ++ if ( (CRYPTO_memcmp(mac,s->s2->mac_data,mac_size) != 0) || |
| 111 | + (s->s2->rlength%EVP_CIPHER_CTX_block_size(s->enc_read_ctx) != 0)) |
| 112 | + { |
| 113 | + SSLerr(SSL_F_SSL2_READ_INTERNAL,SSL_R_BAD_MAC_DECODE); |
| 114 | +diff --git a/ssl/s3_both.c b/ssl/s3_both.c |
| 115 | +index 918da35..ead01c8 100644 |
| 116 | +--- a/ssl/s3_both.c |
| 117 | ++++ b/ssl/s3_both.c |
| 118 | +@@ -265,7 +265,7 @@ int ssl3_get_finished(SSL *s, int a, int b) |
| 119 | + goto f_err; |
| 120 | + } |
| 121 | + |
| 122 | +- if (memcmp(p, s->s3->tmp.peer_finish_md, i) != 0) |
| 123 | ++ if (CRYPTO_memcmp(p, s->s3->tmp.peer_finish_md, i) != 0) |
| 124 | + { |
| 125 | + al=SSL_AD_DECRYPT_ERROR; |
| 126 | + SSLerr(SSL_F_SSL3_GET_FINISHED,SSL_R_DIGEST_CHECK_FAILED); |
| 127 | +diff --git a/ssl/s3_pkt.c b/ssl/s3_pkt.c |
| 128 | +index dca3458..3e11140 100644 |
| 129 | +--- a/ssl/s3_pkt.c |
| 130 | ++++ b/ssl/s3_pkt.c |
| 131 | +@@ -463,7 +463,7 @@ printf("\n"); |
| 132 | + #endif |
| 133 | + } |
| 134 | + i=s->method->ssl3_enc->mac(s,md,0); |
| 135 | +- if (i < 0 || mac == NULL || memcmp(md, mac, (size_t)mac_size) != 0) |
| 136 | ++ if (i < 0 || mac == NULL || CRYPTO_memcmp(md, mac, (size_t)mac_size) != 0) |
| 137 | + { |
| 138 | + decryption_failed_or_bad_record_mac = 1; |
| 139 | + } |
| 140 | +diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c |
| 141 | +index d8df062..27010dd 100644 |
| 142 | +--- a/ssl/t1_lib.c |
| 143 | ++++ b/ssl/t1_lib.c |
| 144 | +@@ -2226,7 +2226,7 @@ static int tls_decrypt_ticket(SSL *s, const unsigned char *etick, int eticklen, |
| 145 | + HMAC_Update(&hctx, etick, eticklen); |
| 146 | + HMAC_Final(&hctx, tick_hmac, NULL); |
| 147 | + HMAC_CTX_cleanup(&hctx); |
| 148 | +- if (memcmp(tick_hmac, etick + eticklen, mlen)) |
| 149 | ++ if (CRYPTO_memcmp(tick_hmac, etick + eticklen, mlen)) |
| 150 | + return 2; |
| 151 | + /* Attempt to decrypt session data */ |
| 152 | + /* Move p after IV to start of encrypted ticket, update length */ |
| 153 | +-- |
| 154 | +1.8.1 |
| 155 | + |
0 commit comments