Add and use a constant-time memcmp.
authorBen Laurie <ben@links.org>
Mon, 28 Jan 2013 17:30:38 +0000 (17:30 +0000)
committerDr. Stephen Henson <steve@openssl.org>
Tue, 5 Feb 2013 16:46:15 +0000 (16:46 +0000)
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
crypto/crypto.h
crypto/rsa/rsa_oaep.c
ssl/d1_pkt.c
ssl/s2_clnt.c
ssl/s2_pkt.c
ssl/s3_both.c
ssl/s3_pkt.c
ssl/t1_lib.c

index 139bd1272a7ffd1797633ddd36e9636e7706bd6f..fd6e3ccb9a3c7e37c071605f5992345194b454f4 100644 (file)
@@ -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;
+       }
index b0360cec5140cf5f44946983e7ed3a67604ac832..ab65f69a7e33b5d623771ee0b1091200f39fdb98 100644 (file)
@@ -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.
index 18d307ea9e1d9e4d4cd6c0a238614a5cd3a272ac..9b9dba05416fd23eb960002ec095706180f0834d 100644 (file)
@@ -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
                {
index 6d245736f6aba323dba9ac1d79d251b7c7756d10..46a195089e046d6f33553f057857e17894b0d6cc 100644 (file)
@@ -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;
                        }
index 76b690ea1340f04bad5c9eeb25e3cc9dc43ed9cb..03b6cf9673809a7582c4c176c30a75b7e0e1b6a4 100644 (file)
@@ -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);
index ac963b2d47d6e369adc012ffac5c8df4ddc7f4bb..8bb6ab8baa33501dcd6f0991d53ad80b3d9361aa 100644 (file)
@@ -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);
index a6d869df59e862be10547a97afd1168b7196ea44..4801dbfb85aef10fe80be11338666e86125d4b61 100644 (file)
@@ -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);
index f9b3629cf789adb30f2649e45980287a976d8355..85ff7022de8c09715b2bbd085f1a4d22518a0332 100644 (file)
@@ -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;
                        }
index ffd5bf294e025123c5079a19a81ae76e29830642..81d1a12820bb8ed78aeef381ea4136e340b31fbb 100644 (file)
@@ -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 */