commit 0623bceb254d0dc25b8b5a1024dd36d2f8ec2be1
author David Benjamin <davidben@chromium.org> Fri Dec 25 15:40:14 2015 -0500
committer Adam Langley <alangley@gmail.com> Fri Jan 15 20:46:45 2016 +0000
tree cf561bbccd8392dd76bdb7e26fd3993d51172c62
parent 4119d42e7c240ad7a651b0871e1f5ac9bbae7e68

Fill in ssl->session->cipher when resumption is resolved.

Doing it at ChangeCipherSpec makes it be set twice and, more
importantly, causes us to touch SSL_SESSION objects on resumption. (With
a no-op change, but this still isn't a good idea.)

This should actually let us get rid of ssl->s3->tmp.new_cipher but some
of external code accesses that field directly.

Change-Id: Ia6b7e0964c1b430f963ad0b1a5417b339b7b19d3
Reviewed-on: https://boringssl-review.googlesource.com/6833
Reviewed-by: Adam Langley <alangley@gmail.com>

ssl/d1_clnt.c

diff --git a/ssl/d1_clnt.c b/ssl/d1_clnt.c
index ad5eb50..4e73bc2 100644
--- a/ssl/d1_clnt.c
+++ b/ssl/d1_clnt.c
@@ -368,7 +368,6 @@
         ssl->state = SSL3_ST_CW_FINISHED_A;
         ssl->init_num = 0;
 
-        ssl->session->cipher = ssl->s3->tmp.new_cipher;
         if (!ssl->enc_method->setup_key_block(ssl) ||
             !ssl->enc_method->change_cipher_state(
                 ssl, SSL3_CHANGE_CIPHER_CLIENT_WRITE)) {

ssl/d1_srvr.c

diff --git a/ssl/d1_srvr.c b/ssl/d1_srvr.c
index 3ba9411..4f2d5ec 100644
--- a/ssl/d1_srvr.c
+++ b/ssl/d1_srvr.c
@@ -393,7 +393,6 @@
 
       case SSL3_ST_SW_CHANGE_A:
       case SSL3_ST_SW_CHANGE_B:
-        ssl->session->cipher = ssl->s3->tmp.new_cipher;
         if (!ssl->enc_method->setup_key_block(ssl)) {
           ret = -1;
           goto end;

ssl/s3_clnt.c

diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c
index 5f68037..0fca13c 100644
--- a/ssl/s3_clnt.c
+++ b/ssl/s3_clnt.c
@@ -388,7 +388,6 @@
         }
         ssl->init_num = 0;
 
-        ssl->session->cipher = ssl->s3->tmp.new_cipher;
         if (!ssl->enc_method->setup_key_block(ssl) ||
             !ssl->enc_method->change_cipher_state(
                 ssl, SSL3_CHANGE_CIPHER_CLIENT_WRITE)) {
@@ -867,6 +866,8 @@
       OPENSSL_PUT_ERROR(SSL, SSL_R_OLD_SESSION_VERSION_NOT_RETURNED);
       goto f_err;
     }
+  } else {
+    ssl->session->cipher = c;
   }
   ssl->s3->tmp.new_cipher = c;
 

ssl/s3_pkt.c

diff --git a/ssl/s3_pkt.c b/ssl/s3_pkt.c
index 4c1133c..e9c9be5 100644
--- a/ssl/s3_pkt.c
+++ b/ssl/s3_pkt.c
@@ -653,7 +653,6 @@
       return 0;
     }
 
-    ssl->session->cipher = ssl->s3->tmp.new_cipher;
     if (!ssl->enc_method->setup_key_block(ssl)) {
       return 0;
     }

ssl/s3_srvr.c

diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c
index 49a1a95..489f585 100644
--- a/ssl/s3_srvr.c
+++ b/ssl/s3_srvr.c
@@ -502,7 +502,6 @@
 
       case SSL3_ST_SW_CHANGE_A:
       case SSL3_ST_SW_CHANGE_B:
-        ssl->session->cipher = ssl->s3->tmp.new_cipher;
         if (!ssl->enc_method->setup_key_block(ssl)) {
           ret = -1;
           goto end;
@@ -1059,6 +1058,7 @@
       OPENSSL_PUT_ERROR(SSL, SSL_R_NO_SHARED_CIPHER);
       goto f_err;
     }
+    ssl->session->cipher = c;
     ssl->s3->tmp.new_cipher = c;
 
     /* Determine whether to request a client certificate. */