commit e58f8a6b9ab6e990f0c273552f7b1411a3c6b0bc
author David Benjamin <davidben@google.com> Thu Sep 21 19:07:15 2017 -0400
committer CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Fri Sep 22 15:18:17 2017 +0000
tree 24ab39db21dbba3a119a0cb192217d0d81a2ea7c
parent 38570b26b88a705064a2753feb9233319f796f7b

Simplify tls1_change_cipher_spec.

Rather than use those weird bitmasks, just pass an evp_aead_direction_t
and figure it out from there.

Change-Id: Ie52c6404bd0728d7d1ef964a3590d9ba0843c1d6
Reviewed-on: https://boringssl-review.googlesource.com/20666
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>

include/openssl/ssl3.h

diff --git a/include/openssl/ssl3.h b/include/openssl/ssl3.h
index 343ea34..719a52d 100644
--- a/include/openssl/ssl3.h
+++ b/include/openssl/ssl3.h
@@ -324,16 +324,6 @@
 
 #define SSL3_MT_CCS 1
 
-// These are used when changing over to a new cipher
-#define SSL3_CC_READ 0x01
-#define SSL3_CC_WRITE 0x02
-#define SSL3_CC_CLIENT 0x10
-#define SSL3_CC_SERVER 0x20
-#define SSL3_CHANGE_CIPHER_CLIENT_WRITE (SSL3_CC_CLIENT | SSL3_CC_WRITE)
-#define SSL3_CHANGE_CIPHER_SERVER_READ (SSL3_CC_SERVER | SSL3_CC_READ)
-#define SSL3_CHANGE_CIPHER_CLIENT_READ (SSL3_CC_CLIENT | SSL3_CC_READ)
-#define SSL3_CHANGE_CIPHER_SERVER_WRITE (SSL3_CC_SERVER | SSL3_CC_WRITE)
-
 
 #ifdef  __cplusplus
 }  // extern C

ssl/handshake_client.cc

diff --git a/ssl/handshake_client.cc b/ssl/handshake_client.cc
index 8e45331..3916692 100644
--- a/ssl/handshake_client.cc
+++ b/ssl/handshake_client.cc
@@ -1493,7 +1493,7 @@
   }
 
   if (!ssl->method->add_change_cipher_spec(ssl) ||
-      !tls1_change_cipher_state(hs, SSL3_CHANGE_CIPHER_CLIENT_WRITE)) {
+      !tls1_change_cipher_state(hs, evp_aead_seal)) {
     return ssl_hs_error;
   }
 
@@ -1646,7 +1646,7 @@
 }
 
 static enum ssl_hs_wait_t do_process_change_cipher_spec(SSL_HANDSHAKE *hs) {
-  if (!tls1_change_cipher_state(hs, SSL3_CHANGE_CIPHER_CLIENT_READ)) {
+  if (!tls1_change_cipher_state(hs, evp_aead_open)) {
     return ssl_hs_error;
   }
 

ssl/handshake_server.cc

diff --git a/ssl/handshake_server.cc b/ssl/handshake_server.cc
index cd99ec9..a38e25f 100644
--- a/ssl/handshake_server.cc
+++ b/ssl/handshake_server.cc
@@ -1397,7 +1397,7 @@
 }
 
 static enum ssl_hs_wait_t do_process_change_cipher_spec(SSL_HANDSHAKE *hs) {
-  if (!tls1_change_cipher_state(hs, SSL3_CHANGE_CIPHER_SERVER_READ)) {
+  if (!tls1_change_cipher_state(hs, evp_aead_open)) {
     return ssl_hs_error;
   }
 
@@ -1525,7 +1525,7 @@
   }
 
   if (!ssl->method->add_change_cipher_spec(ssl) ||
-      !tls1_change_cipher_state(hs, SSL3_CHANGE_CIPHER_SERVER_WRITE) ||
+      !tls1_change_cipher_state(hs, evp_aead_seal) ||
       !ssl3_send_finished(hs)) {
     return ssl_hs_error;
   }

ssl/internal.h

diff --git a/ssl/internal.h b/ssl/internal.h
index 89f9046..d5500bb 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -2297,7 +2297,7 @@
 void dtls1_next_message(SSL *ssl);
 int dtls1_dispatch_alert(SSL *ssl);
 
-int tls1_change_cipher_state(SSL_HANDSHAKE *hs, int which);
+int tls1_change_cipher_state(SSL_HANDSHAKE *hs, evp_aead_direction_t direction);
 int tls1_generate_master_secret(SSL_HANDSHAKE *hs, uint8_t *out,
                                 const uint8_t *premaster, size_t premaster_len);
 

ssl/t1_enc.cc

diff --git a/ssl/t1_enc.cc b/ssl/t1_enc.cc
index 0283c6e..d693007 100644
--- a/ssl/t1_enc.cc
+++ b/ssl/t1_enc.cc
@@ -375,21 +375,19 @@
   return 1;
 }
 
-int tls1_change_cipher_state(SSL_HANDSHAKE *hs, int which) {
+int tls1_change_cipher_state(SSL_HANDSHAKE *hs,
+                             evp_aead_direction_t direction) {
   SSL *const ssl = hs->ssl;
   // Ensure the key block is set up.
   if (!tls1_setup_key_block(hs)) {
     return 0;
   }
 
-  // is_read is true if we have just read a ChangeCipherSpec message - i.e. we
-  // need to update the read cipherspec. Otherwise we have just written one.
-  const char is_read = (which & SSL3_CC_READ) != 0;
   // use_client_keys is true if we wish to use the keys for the "client write"
   // direction. This is the case if we're a client sending a ChangeCipherSpec,
   // or a server reading a client's ChangeCipherSpec.
-  const char use_client_keys = which == SSL3_CHANGE_CIPHER_CLIENT_WRITE ||
-                               which == SSL3_CHANGE_CIPHER_SERVER_READ;
+  const bool use_client_keys =
+      direction == (ssl->server ? evp_aead_open : evp_aead_seal);
 
   size_t mac_secret_len = ssl->s3->tmp.new_mac_secret_len;
   size_t key_len = ssl->s3->tmp.new_key_len;
@@ -422,14 +420,13 @@
   }
 
   UniquePtr<SSLAEADContext> aead_ctx = SSLAEADContext::Create(
-      is_read ? evp_aead_open : evp_aead_seal, ssl->version,
-      SSL_is_dtls(ssl), hs->new_cipher, key, key_len, mac_secret,
-      mac_secret_len, iv, iv_len);
+      direction, ssl->version, SSL_is_dtls(ssl), hs->new_cipher, key, key_len,
+      mac_secret, mac_secret_len, iv, iv_len);
   if (!aead_ctx) {
     return 0;
   }
 
-  if (is_read) {
+  if (direction == evp_aead_open) {
     return ssl->method->set_read_state(ssl, std::move(aead_ctx));
   }