Handle session resumption in SSLv23_client_method.

This fixes version mismatches on resumption without rewriting the entirety of
OpenSSL's version negotiation logic. (Which still badly needs to happen.)

BUG=chromium:417134

Change-Id: Ifa0c5dd2145e37fcd39eec25dfb3561ddb87c9f0
Reviewed-on: https://boringssl-review.googlesource.com/1823
Reviewed-by: Adam Langley <agl@google.com>

1 files changed, 35 insertions, 26 deletions

diff --git a/ssl/s23_clnt.c b/ssl/s23_clnt.c
index b51ddf7..ccb3e2c 100644
--- a/ssl/s23_clnt.c
+++ b/ssl/s23_clnt.c
@@ -121,16 +121,13 @@ static int ssl23_client_hello(SSL *s);
 static int ssl23_get_server_hello(SSL *s);
 static const SSL_METHOD *ssl23_get_client_method(int ver)
 	{
-	if (ver == SSL3_VERSION)
-		return(SSLv3_client_method());
-	else if (ver == TLS1_VERSION)
-		return(TLSv1_client_method());
-	else if (ver == TLS1_1_VERSION)
-		return(TLSv1_1_client_method());
-	else if (ver == TLS1_2_VERSION)
-		return(TLSv1_2_client_method());
-	else
-		return(NULL);
+	/* When SSL_set_session is called, do NOT switch to the version-specific
+	 * method table. The server may still negotiate a different version when
+	 * rejecting the session.
+	 *
+	 * TODO(davidben): Clean this up. This duplicates logic from the
+	 * version-specific tables. https://crbug.com/403378 */
+	return SSLv23_client_method();
 	}
 
 IMPLEMENT_ssl23_meth_func(SSLv23_client_method,
@@ -167,12 +164,6 @@ int ssl23_connect(SSL *s)
 		case SSL_ST_BEFORE|SSL_ST_CONNECT:
 		case SSL_ST_OK|SSL_ST_CONNECT:
 
-			if (s->session != NULL)
-				{
-				OPENSSL_PUT_ERROR(SSL, ssl23_connect, SSL_R_SSL23_DOING_SESSION_ID_REUSE);
-				ret= -1;
-				goto end;
-				}
 			s->server=0;
 			if (cb != NULL) cb(s,SSL_CB_HANDSHAKE_START,1);
 
@@ -303,13 +294,17 @@ static int ssl23_client_hello(SSL *s)
 	buf=(unsigned char *)s->init_buf->data;
 	if (s->state == SSL23_ST_CW_CLNT_HELLO_A)
 		{
-#if 0
-		/* don't reuse session-id's */
-		if (!ssl_get_new_session(s,0))
+		/* Check if the session is resumable. If not, drop it. */
+		if (s->session != NULL)
 			{
-			return(-1);
+			if (s->session->ssl_version > version ||
+				s->session->session_id_length == 0 ||
+				s->session->not_resumable)
+				{
+				SSL_SESSION_free(s->session);
+				s->session = NULL;
+				}
 			}
-#endif
 
 		p=s->s3->client_random;
 		if (ssl_fill_hello_random(s, 0, p, SSL3_RANDOM_SIZE) <= 0)
@@ -363,8 +358,22 @@ static int ssl23_client_hello(SSL *s)
 		memcpy(p, s->s3->client_random, SSL3_RANDOM_SIZE);
 		p += SSL3_RANDOM_SIZE;
 
-		/* Session ID (zero since there is no reuse) */
-		*(p++) = 0;
+		/* Session ID */
+		if (s->new_session || s->session == NULL)
+			i=0;
+		else
+			i=s->session->session_id_length;
+		*(p++)=i;
+		if (i != 0)
+			{
+			if (i > (int)sizeof(s->session->session_id))
+				{
+				OPENSSL_PUT_ERROR(SSL, ssl23_client_hello, ERR_R_INTERNAL_ERROR);
+				return -1;
+				}
+			memcpy(p,s->session->session_id,i);
+			p+=i;
+			}
 
 		/* Ciphers supported (using SSL 3.0/TLS 1.0 format) */
 		i = ssl_cipher_list_to_bytes(s, SSL_get_ciphers(s), &p[2]);
@@ -561,9 +570,9 @@ static int ssl23_get_server_hello(SSL *s)
 		}
 	s->init_num=0;
 
-	/* Since, if we are sending a ssl23 client hello, we are not
-	 * reusing a session-id */
-	if (!ssl_get_new_session(s,0))
+	/* If there was no session to resume, now that the final version is
+	 * determined, insert a fresh one. */
+	if (s->session == NULL && !ssl_get_new_session(s,0))
 		goto err;
 
 	return(SSL_connect(s));