RT3067: simplify patch

(Original commit adb46dbc6dd7347750df2468c93e8c34bcb93a4b)

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

Reviewed-by: Kurt Roeckx <kurt@openssl.org>
(cherry picked from commit 455b65dfab0de51c9f67b3c909311770f2b3f801)

1 files changed, 20 insertions, 52 deletions

diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c
index e3261f6e64..f92084b9f0 100644
--- a/ssl/s3_srvr.c
+++ b/ssl/s3_srvr.c
@@ -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=