RT3067: simplify patch
authorEmilia Kasper <emilia@openssl.org>
Thu, 4 Sep 2014 11:04:42 +0000 (13:04 +0200)
committerEmilia Kasper <emilia@openssl.org>
Wed, 24 Sep 2014 13:35:02 +0000 (15:35 +0200)
(Original commit adb46dbc6dd7347750df2468c93e8c34bcb93a4b)

Use the new constant-time methods consistently in s3_srvr.c

Reviewed-by: Kurt Roeckx <kurt@openssl.org>
apps/openssl-vms.cnf
crypto/constant_time_locl.h
crypto/constant_time_test.c
ssl/Makefile
ssl/s3_srvr.c

index abea6d8..c0ded4a 100644 (file)
@@ -44,7 +44,7 @@ certs         = $dir.certs]           # Where the issued certs are kept
 crl_dir                = $dir.crl]             # Where the issued crl are kept
 database       = $dir]index.txt        # database index file.
 #unique_subject        = no                    # Set to 'no' to allow creation of
-                                       # several ctificates with same subject.
+                                       # several certs with same subject.
 new_certs_dir  = $dir.newcerts]                # default place for new certs.
 
 certificate    = $dir]cacert.pem       # The CA certificate
@@ -103,7 +103,7 @@ emailAddress                = optional
 
 ####################################################################
 [ req ]
-default_bits           = 1024
+default_bits           = 2048
 default_keyfile        = privkey.pem
 distinguished_name     = req_distinguished_name
 attributes             = req_attributes
index ccf7b62..c048393 100644 (file)
@@ -106,6 +106,11 @@ static inline unsigned char constant_time_is_zero_8(unsigned int a);
 static inline unsigned int constant_time_eq(unsigned int a, unsigned int b);
 /* Convenience method for getting an 8-bit mask. */
 static inline unsigned char constant_time_eq_8(unsigned int a, unsigned int b);
+/* Signed integers. */
+static inline unsigned int constant_time_eq_int(int a, int b);
+/* Convenience method for getting an 8-bit mask. */
+static inline unsigned char constant_time_eq_int_8(int a, int b);
+
 
 /*
  * Returns (mask & a) | (~mask & b).
@@ -177,6 +182,16 @@ static inline unsigned char constant_time_eq_8(unsigned int a, unsigned int b)
        return (unsigned char)(constant_time_eq(a, b));
        }
 
+static inline unsigned int constant_time_eq_int(int a, int b)
+       {
+       return constant_time_eq((unsigned)(a), (unsigned)(b));
+       }
+
+static inline unsigned char constant_time_eq_int_8(int a, int b)
+       {
+       return constant_time_eq_8((unsigned)(a), (unsigned)(b));
+       }
+
 static inline unsigned int constant_time_select(unsigned int mask,
        unsigned int a, unsigned int b)
        {
index 0e51892..1b4b18d 100644 (file)
@@ -196,6 +196,45 @@ static int test_select_int(int a, int b)
        return 0;
        }
 
+static int test_eq_int(int a, int b)
+       {
+       unsigned int equal = constant_time_eq_int(a, b);
+       if (a == b && equal != CONSTTIME_TRUE)
+               {
+               fprintf(stderr, "Test failed for constant_time_select(%d, %d): "
+                       "expected %du(TRUE), got %du\n",
+                       a, b, CONSTTIME_TRUE, equal);
+               return 1;
+               }
+       else if (a != b && equal != CONSTTIME_FALSE)
+               {
+               fprintf(stderr, "Test failed for constant_time_select(%d, %d): "
+                       "expected %du(FALSE), got %du\n",
+                       a, b, CONSTTIME_FALSE, equal);
+               return 1;
+               }
+       return 0;
+       }
+
+static int test_eq_int_8(int a, int b)
+       {
+       unsigned char equal = constant_time_eq_int_8(a, b);
+       if (a == b && equal != CONSTTIME_TRUE_8)
+               {
+               fprintf(stderr, "Test failed for constant_time_select(%d, %d): "
+                       "expected %u(TRUE), got %u\n",
+                       a, b, CONSTTIME_TRUE_8, equal);
+               return 1;
+               }
+       else if (a != b && equal != CONSTTIME_FALSE_8)
+               {
+               fprintf(stderr, "Test failed for constant_time_select(%d, %d): "
+                       "expected %u(FALSE), got %u\n",
+                       a, b, CONSTTIME_FALSE_8, equal);
+               return 1;
+               }
+       return 0;
+       }
 
 static unsigned int test_values[] = {0, 1, 1024, 12345, 32000, UINT_MAX/2-1,
                                      UINT_MAX/2, UINT_MAX/2+1, UINT_MAX-1,
index f39b88c..262d308 100644 (file)
@@ -671,28 +671,29 @@ s3_pkt.o: ../include/openssl/ssl23.h ../include/openssl/ssl3.h
 s3_pkt.o: ../include/openssl/stack.h ../include/openssl/symhacks.h
 s3_pkt.o: ../include/openssl/tls1.h ../include/openssl/x509.h
 s3_pkt.o: ../include/openssl/x509_vfy.h s3_pkt.c ssl_locl.h
-s3_srvr.o: ../e_os.h ../include/openssl/asn1.h ../include/openssl/bio.h
-s3_srvr.o: ../include/openssl/bn.h ../include/openssl/buffer.h
-s3_srvr.o: ../include/openssl/comp.h ../include/openssl/crypto.h
-s3_srvr.o: ../include/openssl/dh.h ../include/openssl/dsa.h
-s3_srvr.o: ../include/openssl/dtls1.h ../include/openssl/e_os2.h
-s3_srvr.o: ../include/openssl/ec.h ../include/openssl/ecdh.h
-s3_srvr.o: ../include/openssl/ecdsa.h ../include/openssl/err.h
-s3_srvr.o: ../include/openssl/evp.h ../include/openssl/hmac.h
-s3_srvr.o: ../include/openssl/krb5_asn.h ../include/openssl/kssl.h
-s3_srvr.o: ../include/openssl/lhash.h ../include/openssl/md5.h
-s3_srvr.o: ../include/openssl/obj_mac.h ../include/openssl/objects.h
-s3_srvr.o: ../include/openssl/opensslconf.h ../include/openssl/opensslv.h
-s3_srvr.o: ../include/openssl/ossl_typ.h ../include/openssl/pem.h
-s3_srvr.o: ../include/openssl/pem2.h ../include/openssl/pkcs7.h
-s3_srvr.o: ../include/openssl/pqueue.h ../include/openssl/rand.h
-s3_srvr.o: ../include/openssl/rsa.h ../include/openssl/safestack.h
-s3_srvr.o: ../include/openssl/sha.h ../include/openssl/srtp.h
-s3_srvr.o: ../include/openssl/ssl.h ../include/openssl/ssl2.h
-s3_srvr.o: ../include/openssl/ssl23.h ../include/openssl/ssl3.h
-s3_srvr.o: ../include/openssl/stack.h ../include/openssl/symhacks.h
-s3_srvr.o: ../include/openssl/tls1.h ../include/openssl/x509.h
-s3_srvr.o: ../include/openssl/x509_vfy.h kssl_lcl.h s3_srvr.c ssl_locl.h
+s3_srvr.o: ../crypto/constant_time_locl.h ../e_os.h ../include/openssl/asn1.h
+s3_srvr.o: ../include/openssl/bio.h ../include/openssl/bn.h
+s3_srvr.o: ../include/openssl/buffer.h ../include/openssl/comp.h
+s3_srvr.o: ../include/openssl/crypto.h ../include/openssl/dh.h
+s3_srvr.o: ../include/openssl/dsa.h ../include/openssl/dtls1.h
+s3_srvr.o: ../include/openssl/e_os2.h ../include/openssl/ec.h
+s3_srvr.o: ../include/openssl/ecdh.h ../include/openssl/ecdsa.h
+s3_srvr.o: ../include/openssl/err.h ../include/openssl/evp.h
+s3_srvr.o: ../include/openssl/hmac.h ../include/openssl/krb5_asn.h
+s3_srvr.o: ../include/openssl/kssl.h ../include/openssl/lhash.h
+s3_srvr.o: ../include/openssl/md5.h ../include/openssl/obj_mac.h
+s3_srvr.o: ../include/openssl/objects.h ../include/openssl/opensslconf.h
+s3_srvr.o: ../include/openssl/opensslv.h ../include/openssl/ossl_typ.h
+s3_srvr.o: ../include/openssl/pem.h ../include/openssl/pem2.h
+s3_srvr.o: ../include/openssl/pkcs7.h ../include/openssl/pqueue.h
+s3_srvr.o: ../include/openssl/rand.h ../include/openssl/rsa.h
+s3_srvr.o: ../include/openssl/safestack.h ../include/openssl/sha.h
+s3_srvr.o: ../include/openssl/srtp.h ../include/openssl/ssl.h
+s3_srvr.o: ../include/openssl/ssl2.h ../include/openssl/ssl23.h
+s3_srvr.o: ../include/openssl/ssl3.h ../include/openssl/stack.h
+s3_srvr.o: ../include/openssl/symhacks.h ../include/openssl/tls1.h
+s3_srvr.o: ../include/openssl/x509.h ../include/openssl/x509_vfy.h kssl_lcl.h
+s3_srvr.o: s3_srvr.c ssl_locl.h
 ssl_algs.o: ../e_os.h ../include/openssl/asn1.h ../include/openssl/bio.h
 ssl_algs.o: ../include/openssl/buffer.h ../include/openssl/comp.h
 ssl_algs.o: ../include/openssl/crypto.h ../include/openssl/dsa.h
index 440fc13..bef055a 100644 (file)
 #include <stdio.h>
 #include "ssl_locl.h"
 #include "kssl_lcl.h"
+#include "../crypto/constant_time_locl.h"
 #include <openssl/buffer.h>
 #include <openssl/rand.h>
 #include <openssl/objects.h>
@@ -2223,8 +2224,8 @@ int ssl3_get_client_key_exchange(SSL *s)
        if (alg_k & SSL_kRSA)
                {
                unsigned char rand_premaster_secret[SSL_MAX_MASTER_KEY_LENGTH];
-               int decrypt_len, decrypt_good_mask;
-               unsigned char version_good;
+               int decrypt_len;
+               unsigned char decrypt_good, version_good;
 
                /* FIX THIS UP EAY EAY EAY EAY */
                if (s->s3->tmp.use_rsa_tmp)
@@ -2288,18 +2289,18 @@ int ssl3_get_client_key_exchange(SSL *s)
                ERR_clear_error();
 
                /* decrypt_len should be SSL_MAX_MASTER_KEY_LENGTH.
-                * decrypt_good_mask will be zero if so and non-zero otherwise. */
-               decrypt_good_mask = decrypt_len ^ SSL_MAX_MASTER_KEY_LENGTH;
+                * decrypt_good will be 0xff if so and zero otherwise. */
+               decrypt_good = constant_time_eq_int_8(decrypt_len, SSL_MAX_MASTER_KEY_LENGTH);
 
                /* If the version in the decrypted pre-master secret is correct
-                * then version_good will be zero. The Klima-Pokorny-Rosa
-                * extension of Bleichenbacher's attack
+                * then version_good will be 0xff, otherwise it'll be zero.
+                * The Klima-Pokorny-Rosa extension of Bleichenbacher's attack
                 * (http://eprint.iacr.org/2003/052/) exploits the version
                 * number check as a "bad version oracle". Thus version checks
                 * are done in constant time and are treated like any other
                 * decryption error. */
-               version_good = p[0] ^ (s->client_version>>8);
-               version_good |= p[1] ^ (s->client_version&0xff);
+               version_good = constant_time_eq_8(p[0], (unsigned)(s->client_version>>8));
+               version_good &= constant_time_eq_8(p[1], (unsigned)(s->client_version&0xff));
 
                /* The premaster secret must contain the same version number as
                 * the ClientHello to detect version rollback attacks
@@ -2310,55 +2311,22 @@ int ssl3_get_client_key_exchange(SSL *s)
                 * SSL_OP_TLS_ROLLBACK_BUG is set, tolerate such clients. */
                if (s->options & SSL_OP_TLS_ROLLBACK_BUG)
                        {
-                       unsigned char workaround_mask = version_good;
-                       unsigned char workaround;
-
-                       /* workaround_mask will be 0xff if version_good is
-                        * non-zero (i.e. the version match failed). Otherwise
-                        * it'll be 0x00. */
-                       workaround_mask |= workaround_mask >> 4;
-                       workaround_mask |= workaround_mask >> 2;
-                       workaround_mask |= workaround_mask >> 1;
-                       workaround_mask = ~((workaround_mask & 1) - 1);
-
-                       workaround = p[0] ^ (s->version>>8);
-                       workaround |= p[1] ^ (s->version&0xff);
-
-                       /* If workaround_mask is 0xff (i.e. there was a version
-                        * mismatch) then we copy the value of workaround over
-                        * version_good. */
-                       version_good = (workaround & workaround_mask) |
-                                      (version_good & ~workaround_mask);
-                       }
-
-               /* If any bits in version_good are set then they'll poision
-                * decrypt_good_mask and cause rand_premaster_secret to be
-                * used. */
-               decrypt_good_mask |= version_good;
-
-               /* decrypt_good_mask will be zero iff decrypt_len ==
-                * SSL_MAX_MASTER_KEY_LENGTH and the version check passed. We
-                * fold the bottom 32 bits of it with an OR so that the LSB
-                * will be zero iff everything is good. This assumes that we'll
-                * never decrypt a value > 2**31 bytes, which seems safe. */
-               decrypt_good_mask |= decrypt_good_mask >> 16;
-               decrypt_good_mask |= decrypt_good_mask >> 8;
-               decrypt_good_mask |= decrypt_good_mask >> 4;
-               decrypt_good_mask |= decrypt_good_mask >> 2;
-               decrypt_good_mask |= decrypt_good_mask >> 1;
-               /* Now select only the LSB and subtract one. If decrypt_len ==
-                * SSL_MAX_MASTER_KEY_LENGTH and the version check passed then
-                * decrypt_good_mask will be all ones. Otherwise it'll be all
-                * zeros. */
-               decrypt_good_mask &= 1;
-               decrypt_good_mask--;
+                       unsigned char workaround_good;
+                       workaround_good = constant_time_eq_8(p[0], (unsigned)(s->version>>8));
+                       workaround_good &= constant_time_eq_8(p[1], (unsigned)(s->version&0xff));
+                       version_good |= workaround_good;
+                       }
+
+               /* Both decryption and version must be good for decrypt_good
+                * to remain non-zero (0xff). */
+               decrypt_good &= version_good;
 
                /* Now copy rand_premaster_secret over p using
                 * decrypt_good_mask. */
                for (i = 0; i < (int) sizeof(rand_premaster_secret); i++)
                        {
-                       p[i] = (p[i] & decrypt_good_mask) |
-                              (rand_premaster_secret[i] & ~decrypt_good_mask);
+                       p[i] = constant_time_select_8(decrypt_good, p[i],
+                                                     rand_premaster_secret[i]);
                        }
 
                s->session->master_key_length=