commit 384d0eaf1930af1ebc47eda751f0c78dfcba1c03
author Steven Valdez <svaldez@google.com> Tue Nov 06 10:45:36 2018 -0500
committer CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Tue Nov 06 19:04:48 2018 +0000
tree 07240432812c76ed9c0a151552735bebc0b78efd
parent ffbf95ad41086b116111aea9404230f56fd19311

Make SSL_get_current_cipher valid during QUIC callbacks.

Update-Note: This effectively reverts https://boringssl-review.googlesource.com/4733,
which was an attempt at a well-defined story during renegotiation and pre-handshake.
This is a behavior change, though one that matches OpenSSL upstream. It is also more
consistent with other functions, such as SSL_get_curve_id. Renegotiation is now
opt-in, so this is less critical, and, if we change the behavior mid-renegotiation,
we should do it consistently to all getters.

Change-Id: Ica6b386fb7c5ac524395de6650642edd27cac36f
Reviewed-on: https://boringssl-review.googlesource.com/c/32904
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>

ssl/ssl_lib.cc

diff --git a/ssl/ssl_lib.cc b/ssl/ssl_lib.cc
index 5bd2442..fd560ff 100644
--- a/ssl/ssl_lib.cc
+++ b/ssl/ssl_lib.cc
@@ -2278,7 +2278,8 @@
 }
 
 const SSL_CIPHER *SSL_get_current_cipher(const SSL *ssl) {
-  return ssl->s3->aead_write_ctx->cipher();
+  const SSL_SESSION *session = SSL_get_session(ssl);
+  return session == nullptr ? nullptr : session->cipher;
 }
 
 int SSL_session_reused(const SSL *ssl) {

ssl/ssl_test.cc

diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc
index 14b85d4..f945898 100644
--- a/ssl/ssl_test.cc
+++ b/ssl/ssl_test.cc
@@ -4544,7 +4544,8 @@
 
   bool PeerSecretsMatch(ssl_encryption_level_t level) const {
     return levels_[level].write_secret == peer_->levels_[level].read_secret &&
-           levels_[level].read_secret == peer_->levels_[level].write_secret;
+           levels_[level].read_secret == peer_->levels_[level].write_secret &&
+           levels_[level].cipher == peer_->levels_[level].cipher;
   }
 
   bool HasSecrets(ssl_encryption_level_t level) const {
@@ -4554,11 +4555,18 @@
 
   bool SetEncryptionSecrets(ssl_encryption_level_t level,
                             const uint8_t *read_secret,
-                            const uint8_t *write_secret, size_t secret_len) {
+                            const uint8_t *write_secret, size_t secret_len,
+                            const SSL_CIPHER *cipher) {
     if (HasSecrets(level)) {
       ADD_FAILURE() << "duplicate keys configured";
       return false;
     }
+
+    if (cipher == nullptr) {
+      ADD_FAILURE() << "current cipher unavailable";
+      return false;
+    }
+
     if (level != ssl_encryption_early_data &&
         (read_secret == nullptr || write_secret == nullptr)) {
       ADD_FAILURE() << "key was unexpectedly null";
@@ -4571,6 +4579,7 @@
       levels_[level].write_secret.assign(write_secret,
                                          write_secret + secret_len);
     }
+    levels_[level].cipher = SSL_CIPHER_get_id(cipher);
     return true;
   }
 
@@ -4618,6 +4627,10 @@
       ADD_FAILURE() << "peer write key does not match read key";
       return false;
     }
+    if (peer_->levels_[level].cipher != levels_[level].cipher) {
+      ADD_FAILURE() << "peer cipher does not match";
+      return false;
+    }
     std::vector<uint8_t> *peer_data = &peer_->levels_[level].write_data;
     num = std::min(num, peer_data->size());
     out->assign(peer_data->begin(), peer_data->begin() + num);
@@ -4636,6 +4649,7 @@
     std::vector<uint8_t> write_data;
     std::vector<uint8_t> write_secret;
     std::vector<uint8_t> read_secret;
+    uint32_t cipher = 0;
   };
   Level levels_[kNumQUICLevels];
 };
@@ -4721,8 +4735,8 @@
                                           const uint8_t *read_key,
                                           const uint8_t *write_key,
                                           size_t key_len) {
-    return TransportFromSSL(ssl)->SetEncryptionSecrets(level, read_key,
-                                                       write_key, key_len);
+    return TransportFromSSL(ssl)->SetEncryptionSecrets(
+        level, read_key, write_key, key_len, SSL_get_current_cipher(ssl));
   }
 
   static int AddHandshakeDataCallback(SSL *ssl,

ssl/test/test_config.cc

diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc
index ef24ca0..52e6cf7 100644
--- a/ssl/test/test_config.cc
+++ b/ssl/test/test_config.cc
@@ -1638,10 +1638,5 @@
     }
   }
 
-  if (SSL_get_current_cipher(ssl.get()) != nullptr) {
-    fprintf(stderr, "non-null cipher before handshake\n");
-    return nullptr;
-  }
-
   return ssl;
 }