Commit e1080ea3 authored by Emilia Kasper's avatar Emilia Kasper
Browse files

RT3067: simplify patch 



(Original commit adb46dbc)

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

Reviewed-by: default avatarKurt Roeckx <kurt@openssl.org>
(cherry picked from commit 455b65df)
parent 941af48f

crypto/constant_time_locl.h

+15 −0
Original line number Diff line number Diff line
@@ -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)

	{

crypto/constant_time_test.c

+39 −0
Original line number Diff line number Diff line
@@ -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,

ssl/Makefile

+23 −22
Original line number Diff line number Diff line
@@ -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

ssl/s3_srvr.c

+20 −52
Original line number Diff line number Diff line
@@ -154,6 +154,7 @@ 
#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>
@@ -2185,8 +2186,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)
@@ -2250,18 +2251,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
@@ -2272,55 +2273,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=