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:50:32 +0000 (16:50 +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
(cherry picked from commit dc406b59f3169fe191e58906df08dce97edb727c)

Conflicts:
crypto/crypto.h
ssl/d1_pkt.c
ssl/s3_pkt.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 dd74ea8855f49f988497f7019b1fc5ad911b5524..0566ec1f976a9c3154fe6d129311390c71fd2b83 100644 (file)
@@ -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;
+       }
index fc1374fad5f7a5fe4c2f99efc35ffde5b4c28d0a..6161697cdc5f71baa7c2c458a8f7b3e2f0d47acf 100644 (file)
@@ -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.
index 546ae5fcb2edc01a0ff45d5daf357c81f5955a89..b8e3edc000f2a82cb6eb16fbd257ae5d519d6bf9 100644 (file)
@@ -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
                {
index 65b1ef28cdeb8bc89caf1215d9108b76f8c2d940..8e9ee5a77e5fab1a2b1ab8044dca69a87b47c5b8 100644 (file)
@@ -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;
                        }
index 782129cd5d51f334a914f1c2647970641b8992c5..c13a640455114788a03844bb5b71170bbdc185d8 100644 (file)
@@ -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);
index a10929a757f9f865021b983766a472d6c59011b7..7387d8b7c59db88ea3e986b6871d8d797f6d85b3 100644 (file)
@@ -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);
index 869a25d4764237f4c85123909e9cc6c907ca4c80..86ad59858cadcb6acb0fe7cf74b975e12a768eeb 100644 (file)
@@ -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);
index 5e3583c04d428cefeb28ba5d09261f5f0822a54d..0e041c0ce025c0712cc016277618caf7287d10bb 100644 (file)
@@ -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;
                        }
index 00b8286acb88d2fb7df07f8b9882dc472e5039e3..d56456e14dfdd26c057c81d1f37a2c330eb585c3 100644 (file)
@@ -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 */