commit b0921929650fb0a4a993a697baf8ad4f655748a3
author David Benjamin <davidben@google.com> Thu Feb 20 12:33:28 2020 -0500
committer CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Mon Feb 24 22:16:24 2020 +0000
tree bc8986d93a460fe322443d4a65a5cd36f411e598
parent 6432bb46ab44731567ec923e6c8fc182f13d0070

Keep the encryption state and encryption level in sync.

This is a little bit of internal cleanup. The original intent was so
QUIC could install secrets in set_(read|write)_state, but that was
somewhat annoying, so I've left it just before the call for now.

There is one TLS 1.3 state transition which doesn't carry an encryption
level: switching from 0-RTT keys back to unencrypted on an HRR-based
0-RTT reject. The TCP code doesn't care about write_level and the QUIC
code is currently fine because we never "install" the 0-RTT keys. But we
should get this correct.

This also opens the door for DTLS 1.3, if we ever implement it, because
DTLS 1.3 will need to know which level it is to handle 0-RTT keys funny.
(Clients sending 0-RTT will briefly have handshake and 0-RTT write keys
active simultaneously.)

QUIC has the same property, but we can fudge it because only the caller
is aware of this.

Change-Id: Ia76d787e1b96a058d9818948b6d9a051e8592207
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/40124
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>

ssl/dtls_method.cc

diff --git a/ssl/dtls_method.cc b/ssl/dtls_method.cc
index 620a2e1..ae26de7 100644
--- a/ssl/dtls_method.cc
+++ b/ssl/dtls_method.cc
@@ -77,7 +77,8 @@
   }
 }
 
-static bool dtls1_set_read_state(SSL *ssl, UniquePtr<SSLAEADContext> aead_ctx) {
+static bool dtls1_set_read_state(SSL *ssl, ssl_encryption_level_t level,
+                                 UniquePtr<SSLAEADContext> aead_ctx) {
   // Cipher changes are forbidden if the current epoch has leftover data.
   if (dtls_has_unprocessed_handshake_data(ssl)) {
     OPENSSL_PUT_ERROR(SSL, SSL_R_EXCESS_HANDSHAKE_DATA);
@@ -90,11 +91,12 @@
   OPENSSL_memset(ssl->s3->read_sequence, 0, sizeof(ssl->s3->read_sequence));
 
   ssl->s3->aead_read_ctx = std::move(aead_ctx);
+  ssl->s3->read_level = level;
   ssl->d1->has_change_cipher_spec = 0;
   return true;
 }
 
-static bool dtls1_set_write_state(SSL *ssl,
+static bool dtls1_set_write_state(SSL *ssl, ssl_encryption_level_t level,
                                   UniquePtr<SSLAEADContext> aead_ctx) {
   ssl->d1->w_epoch++;
   OPENSSL_memcpy(ssl->d1->last_write_sequence, ssl->s3->write_sequence,
@@ -103,6 +105,7 @@
 
   ssl->d1->last_aead_write_ctx = std::move(ssl->s3->aead_write_ctx);
   ssl->s3->aead_write_ctx = std::move(aead_ctx);
+  ssl->s3->write_level = level;
   return true;
 }