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>
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index 0fa10b5..17153c2 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h
@@ -1521,8 +1521,8 @@ // TLS 1.3 was negotiated. Otherwise, it returns zero. OPENSSL_EXPORT int SSL_get_extms_support(const SSL *ssl); -// SSL_get_current_cipher returns the cipher used in the current outgoing -// connection state, or NULL if the null cipher is active. +// SSL_get_current_cipher returns cipher suite used by |ssl|, or NULL if it has +// not been negotiated yet. OPENSSL_EXPORT const SSL_CIPHER *SSL_get_current_cipher(const SSL *ssl); // SSL_session_reused returns one if |ssl| performed an abbreviated handshake
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) {
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,
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; }