Replace s->first_packet with a s->s3->have_version bit.

first_packet is a temporary connection-global flag set for the duration of some
call and then queried from other code. This kind of logic is too difficult to
reason through. It also incorrectly treats renegotiate ClientHellos as
pre-version-negotiation records. This eliminates the need to query
enc_write_ctx (which wasn't EVP_AEAD-aware anyway).

Instead, take a leaf from Go TLS's book and add a have_version bit. This is
placed on s->s3 as it is connection state; s->s3 automatically gets reset on
SSL_clear while s doesn't.

This new flag will also be used to determine whether to do the V2ClientHello
sniff when the version-locked methods merge into SSLv23_method. It will also
replace needing to condition s->method against a dummy DTLS_ANY_VERSION value
to determine whether DTLS version negotiation has happened yet.

Change-Id: I5c8bc6258b182ba4ab175a48a84eab6d3a001333
Reviewed-on: https://boringssl-review.googlesource.com/2442
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/d1_clnt.c b/ssl/d1_clnt.c
index 64c1775..88a6af4 100644
--- a/ssl/d1_clnt.c
+++ b/ssl/d1_clnt.c
@@ -552,7 +552,6 @@
 	CBS hello_verify_request, cookie;
 	uint16_t server_version;
 
-	s->first_packet = 1;
 	n=s->method->ssl_get_message(s,
 		DTLS1_ST_CR_HELLO_VERIFY_REQUEST_A,
 		DTLS1_ST_CR_HELLO_VERIFY_REQUEST_B,
@@ -561,7 +560,6 @@
 		20000,
 		SSL_GET_MESSAGE_HASH_MESSAGE,
 		&ok);
-	s->first_packet = 0;
 
 	if (!ok) return((int)n);
 
diff --git a/ssl/d1_pkt.c b/ssl/d1_pkt.c
index 2f75d2f..377b264 100644
--- a/ssl/d1_pkt.c
+++ b/ssl/d1_pkt.c
@@ -583,7 +583,7 @@
 		n2s(p,rr->length);
 
 		/* Lets check version */
-		if (!s->first_packet)
+		if (s->s3->have_version)
 			{
 			if (version != s->version)
 				{
diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c
index 2b39ad1..635606e 100644
--- a/ssl/s3_clnt.c
+++ b/ssl/s3_clnt.c
@@ -777,11 +777,6 @@
 	uint8_t compression_method;
 	unsigned long mask_ssl;
 
-	/* DTLS_ANY_VERSION does not sniff the version ahead of time,
-	 * so disable the version check. */
-	if (SSL_IS_DTLS(s))
-		s->first_packet = 1;
-
 	n=s->method->ssl_get_message(s,
 		SSL3_ST_CR_SRVR_HELLO_A,
 		SSL3_ST_CR_SRVR_HELLO_B,
@@ -790,9 +785,6 @@
 		SSL_GET_MESSAGE_HASH_MESSAGE,
 		&ok);
 
-	if (SSL_IS_DTLS(s))
-		s->first_packet = 0;
-
 	if (!ok) return((int)n);
 
 	CBS_init(&server_hello, s->init_msg, n);
@@ -837,6 +829,12 @@
 		goto f_err;
 		}
 
+	/* At this point, the connection's version is known and s->version is
+	 * fixed. Begin enforcing the record-layer version. Note: SSLv23_method
+	 * currently determines its version sooner, but it will later be moved
+	 * to this point. */
+	s->s3->have_version = 1;
+
 	/* Copy over the server random. */
 	memcpy(s->s3->server_random, CBS_data(&server_random), SSL3_RANDOM_SIZE);
 
diff --git a/ssl/s3_pkt.c b/ssl/s3_pkt.c
index 4cb7e6a..914a58e 100644
--- a/ssl/s3_pkt.c
+++ b/ssl/s3_pkt.c
@@ -341,12 +341,12 @@
 #endif
 
 		/* Lets check version */
-		if (!s->first_packet)
+		if (s->s3->have_version)
 			{
 			if (version != s->version)
 				{
 				OPENSSL_PUT_ERROR(SSL, ssl3_get_record, SSL_R_WRONG_VERSION_NUMBER);
-                                if ((s->version & 0xFF00) == (version & 0xFF00) && !s->enc_write_ctx && !s->write_hash)
+                                if ((s->version & 0xFF00) == (version & 0xFF00))
                                 	/* Send back error using their minor version number :-) */
 					s->version = (unsigned short)version;
 				al=SSL_AD_PROTOCOL_VERSION;
diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c
index 5ba5b42..527850b 100644
--- a/ssl/s3_srvr.c
+++ b/ssl/s3_srvr.c
@@ -715,7 +715,6 @@
 	switch (s->state) {
 	case SSL3_ST_SR_CLNT_HELLO_A:
 	case SSL3_ST_SR_CLNT_HELLO_B:
-		s->first_packet=1;
 		n=s->method->ssl_get_message(s,
 			SSL3_ST_SR_CLNT_HELLO_A,
 			SSL3_ST_SR_CLNT_HELLO_B,
@@ -725,7 +724,6 @@
 			&ok);
 
 		if (!ok) return((int)n);
-		s->first_packet=0;
 
 		/* If we require cookies and this ClientHello doesn't
 		 * contain one, just return since we do not want to
@@ -814,7 +812,7 @@
 		{
 		OPENSSL_PUT_ERROR(SSL, ssl3_get_client_hello, SSL_R_WRONG_VERSION_NUMBER);
 		if ((s->client_version>>8) == SSL3_VERSION_MAJOR &&
-			!s->enc_write_ctx && !s->write_hash)
+			!s->s3->have_version)
 			{
 			/* similar to ssl3_get_record, send alert using remote version number */
 			s->version = s->client_version;
@@ -890,6 +888,12 @@
 			}
 		}
 
+	/* At this point, the connection's version is known and s->version is
+	 * fixed. Begin enforcing the record-layer version. Note: SSLv23_method
+	 * currently determines its version sooner, but it will later be moved
+	 * to this point. */
+	s->s3->have_version = 1;
+
 	s->hit=0;
 	/* Versions before 0.9.7 always allow clients to resume sessions in renegotiation.
 	 * 0.9.7 and later allow this by default, but optionally ignore resumption requests
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 783e8ae..fef0649 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -243,8 +243,6 @@
 	ssl_clear_hash_ctx(&s->read_hash);
 	ssl_clear_hash_ctx(&s->write_hash);
 
-	s->first_packet=0;
-
 	s->method->ssl_clear(s);
 	return(1);
 	}