Support sending KeyUpdate in DTLS 1.3
Bug: 42290594
Change-Id: I38312da6425d25038f59dc5c65a17987d3688048
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/73587
Reviewed-by: Nick Harper <nharper@chromium.org>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/ssl/d1_lib.cc b/ssl/d1_lib.cc
index 5cca499..d0a38b3 100644
--- a/ssl/d1_lib.cc
+++ b/ssl/d1_lib.cc
@@ -77,7 +77,8 @@
handshake_write_overflow(false),
handshake_read_overflow(false),
sending_flight(false),
- sending_ack(false) {}
+ sending_ack(false),
+ queued_key_update(QueuedKeyUpdate::kNone) {}
DTLS1_STATE::~DTLS1_STATE() {}
diff --git a/ssl/d1_pkt.cc b/ssl/d1_pkt.cc
index b4d4c67..88e76b3 100644
--- a/ssl/d1_pkt.cc
+++ b/ssl/d1_pkt.cc
@@ -222,6 +222,26 @@
[](const auto &msg) { return msg.IsFullyAcked(); })) {
dtls1_stop_timer(ssl);
dtls_clear_outgoing_messages(ssl);
+
+ // DTLS 1.3 defers the key update to when the message is ACKed.
+ if (ssl->s3->key_update_pending) {
+ if (!tls13_rotate_traffic_key(ssl, evp_aead_seal)) {
+ return ssl_open_record_error;
+ }
+ ssl->s3->key_update_pending = false;
+ }
+
+ // Check for deferred messages.
+ if (ssl->d1->queued_key_update != QueuedKeyUpdate::kNone) {
+ int request_type =
+ ssl->d1->queued_key_update == QueuedKeyUpdate::kUpdateRequested
+ ? SSL_KEY_UPDATE_REQUESTED
+ : SSL_KEY_UPDATE_NOT_REQUESTED;
+ ssl->d1->queued_key_update = QueuedKeyUpdate::kNone;
+ if (!tls13_add_key_update(ssl, request_type)) {
+ return ssl_open_record_error;
+ }
+ }
} else {
// We may still be able to drop unused write epochs.
dtls_clear_unused_write_epochs(ssl);
diff --git a/ssl/internal.h b/ssl/internal.h
index 463852b..42bf3f3 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -2604,10 +2604,9 @@
const char *tls13_client_handshake_state(SSL_HANDSHAKE *hs);
const char *tls13_server_handshake_state(SSL_HANDSHAKE *hs);
-// tls13_add_key_update queues a KeyUpdate message on |ssl|. The
-// |update_requested| argument must be one of |SSL_KEY_UPDATE_REQUESTED| or
-// |SSL_KEY_UPDATE_NOT_REQUESTED|.
-bool tls13_add_key_update(SSL *ssl, int update_requested);
+// tls13_add_key_update queues a KeyUpdate message on |ssl|. |request_type| must
+// be one of |SSL_KEY_UPDATE_REQUESTED| or |SSL_KEY_UPDATE_NOT_REQUESTED|.
+bool tls13_add_key_update(SSL *ssl, int request_type);
// tls13_post_handshake processes a post-handshake message. It returns true on
// success and false on failure.
@@ -3238,8 +3237,10 @@
// Channel ID and the |channel_id| field is filled in.
bool channel_id_valid : 1;
- // key_update_pending is true if we have a KeyUpdate acknowledgment
- // outstanding.
+ // key_update_pending is true if we are in the process of sending a KeyUpdate
+ // message. As a DoS mitigation (and a requirement in DTLS), we never send
+ // more than one KeyUpdate at once. In DTLS, this tracks whether there is an
+ // unACKed KeyUpdate.
bool key_update_pending : 1;
// early_data_accepted is true if early data was accepted by the server.
@@ -3533,6 +3534,12 @@
uint32_t last_msg_end = 0;
};
+enum class QueuedKeyUpdate {
+ kNone,
+ kUpdateNotRequested,
+ kUpdateRequested,
+};
+
struct DTLS1_STATE {
static constexpr bool kAllowUniquePtr = true;
@@ -3565,6 +3572,10 @@
bool sending_flight : 1;
bool sending_ack : 1;
+ // queued_key_update, if not kNone, indicates we've queued a KeyUpdate message
+ // to send after the current flight is ACKed.
+ QueuedKeyUpdate queued_key_update : 2;
+
uint16_t handshake_write_seq = 0;
uint16_t handshake_read_seq = 0;
diff --git a/ssl/ssl_lib.cc b/ssl/ssl_lib.cc
index 379e628..d601695 100644
--- a/ssl/ssl_lib.cc
+++ b/ssl/ssl_lib.cc
@@ -1121,12 +1121,7 @@
return 0;
}
- if (!ssl->s3->key_update_pending &&
- !tls13_add_key_update(ssl, request_type)) {
- return 0;
- }
-
- return 1;
+ return tls13_add_key_update(ssl, request_type);
}
int SSL_shutdown(SSL *ssl) {
diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc
index fda853c..44a7cfc 100644
--- a/ssl/test/bssl_shim.cc
+++ b/ssl/test/bssl_shim.cc
@@ -1196,6 +1196,12 @@
}
if (!config->shim_shuts_down) {
for (;;) {
+ if (config->key_update_before_read &&
+ !SSL_key_update(ssl, SSL_KEY_UPDATE_NOT_REQUESTED)) {
+ fprintf(stderr, "SSL_key_update failed.\n");
+ return false;
+ }
+
// Read only 512 bytes at a time in TLS to ensure records may be
// returned in multiple reads.
size_t read_size = config->is_dtls ? 16384 : 512;
diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go
index 6e76f23..76195f6 100644
--- a/ssl/test/runner/common.go
+++ b/ssl/test/runner/common.go
@@ -2007,9 +2007,7 @@
// OmitPublicName omits the server name extension from ClientHelloOuter.
OmitPublicName bool
- // AllowEpochOverflow allows the DTLS write epoch to wrap around. The DTLS
- // read epoch is never allowed to wrap around because that would be a bug in
- // the shim.
+ // AllowEpochOverflow allows DTLS epoch numbers to wrap around.
AllowEpochOverflow bool
}
diff --git a/ssl/test/runner/conn.go b/ssl/test/runner/conn.go
index 071e262..d262507 100644
--- a/ssl/test/runner/conn.go
+++ b/ssl/test/runner/conn.go
@@ -1660,6 +1660,20 @@
return c.ackHandshake()
}
+func (c *Conn) processKeyUpdate(keyUpdate *keyUpdateMsg) error {
+ epoch := c.in.epoch.epoch + 1
+ if epoch == 0 && !c.config.Bugs.AllowEpochOverflow {
+ return errors.New("tls: too many KeyUpdates")
+ }
+ if err := c.useInTrafficSecret(epoch, c.in.wireVersion, c.cipherSuite, updateTrafficSecret(c.cipherSuite.hash(), c.wireVersion, c.in.trafficSecret, c.isDTLS)); err != nil {
+ return err
+ }
+ if keyUpdate.keyUpdateRequest == keyUpdateRequested {
+ c.keyUpdateRequested = true
+ }
+ return c.ackHandshake()
+}
+
func (c *Conn) handlePostHandshakeMessage() error {
msg, err := c.readHandshake()
if err != nil {
@@ -1690,21 +1704,10 @@
if keyUpdate, ok := msg.(*keyUpdateMsg); ok {
c.keyUpdateSeen = true
-
if c.config.Bugs.RejectUnsolicitedKeyUpdate {
return errors.New("tls: unexpected KeyUpdate message")
}
- epoch := c.in.epoch.epoch + 1
- if epoch == 0 {
- return errors.New("tls: too many KeyUpdates")
- }
- if err := c.useInTrafficSecret(epoch, c.in.wireVersion, c.cipherSuite, updateTrafficSecret(c.cipherSuite.hash(), c.wireVersion, c.in.trafficSecret, c.isDTLS)); err != nil {
- return err
- }
- if keyUpdate.keyUpdateRequest == keyUpdateRequested {
- c.keyUpdateRequested = true
- }
- return c.ackHandshake()
+ return c.processKeyUpdate(keyUpdate)
}
c.sendAlert(alertUnexpectedMessage)
@@ -1717,27 +1720,16 @@
c.in.Lock()
defer c.in.Unlock()
- msg, err := c.readHandshake()
+ keyUpdate, err := readHandshakeType[keyUpdateMsg](c)
if err != nil {
return err
}
- keyUpdate, ok := msg.(*keyUpdateMsg)
- if !ok {
- c.sendAlert(alertUnexpectedMessage)
- return fmt.Errorf("tls: unexpected message (%T) when reading KeyUpdate", msg)
- }
-
if keyUpdate.keyUpdateRequest != keyUpdateNotRequested {
return errors.New("tls: received invalid KeyUpdate message")
}
- epoch := c.in.epoch.epoch + 1
- if epoch == 0 {
- return errors.New("tls: too many KeyUpdates")
- }
-
- return c.useInTrafficSecret(epoch, c.in.wireVersion, c.cipherSuite, updateTrafficSecret(c.cipherSuite.hash(), c.wireVersion, c.in.trafficSecret, c.isDTLS))
+ return c.processKeyUpdate(keyUpdate)
}
func (c *Conn) Renegotiate() error {
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 25651f6..075f6fd 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -627,6 +627,9 @@
sendKeyUpdates int
// keyUpdateRequest is the KeyUpdateRequest value to send in KeyUpdate messages.
keyUpdateRequest byte
+ // shimSendsKeyUpdateBeforeRead indicates the shim should send a KeyUpdate
+ // message before each read.
+ shimSendsKeyUpdateBeforeRead bool
// expectUnsolicitedKeyUpdate makes the test expect a one or more KeyUpdate
// messages while reading data from the shim. Don't use this in combination
// with any of the fields that send a KeyUpdate otherwise any received
@@ -1149,6 +1152,12 @@
}
}
+ if test.shimSendsKeyUpdateBeforeRead {
+ if err := tlsConn.ReadKeyUpdate(); err != nil {
+ return err
+ }
+ }
+
testMessage := makeTestMessage(j, messageLen)
if _, err := tlsConn.Write(testMessage); err != nil {
return err
@@ -1214,6 +1223,14 @@
}
}
+ // The shim will end attempting to read, sending one last KeyUpdate. Consume
+ // the KeyUpdate before closing the connection.
+ if test.shimSendsKeyUpdateBeforeRead {
+ if err := tlsConn.ReadKeyUpdate(); err != nil {
+ return err
+ }
+ }
+
return nil
}
@@ -1659,6 +1676,10 @@
flags = append(flags, "-tls-unique")
}
+ if test.shimSendsKeyUpdateBeforeRead {
+ flags = append(flags, "-key-update-before-read")
+ }
+
if *waitForDebugger {
flags = append(flags, "-wait-for-debugger")
}
@@ -22248,6 +22269,159 @@
flags: []string{"-async"},
})
+ // In DTLS, we KeyUpdate before read, rather than write, because the
+ // KeyUpdate will not be applied before the shim reads the ACK.
+ testCases = append(testCases, testCase{
+ protocol: dtls,
+ name: "KeyUpdate-FromClient-DTLS",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ },
+ shimSendsKeyUpdateBeforeRead: true,
+ // Perform several message exchanges to update keys several times.
+ messageCount: 10,
+ })
+ testCases = append(testCases, testCase{
+ protocol: dtls,
+ testType: serverTest,
+ name: "KeyUpdate-FromServer-DTLS",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ },
+ shimSendsKeyUpdateBeforeRead: true,
+ // Perform several message exchanges to update keys several times.
+ messageCount: 10,
+ // Avoid NewSessionTicket messages getting in the way of ReadKeyUpdate.
+ flags: []string{"-no-ticket"},
+ })
+
+ // If the shim has a pending unACKed flight, it defers sending KeyUpdate.
+ // BoringSSL does not support multiple outgoing flights at once.
+ testCases = append(testCases, testCase{
+ protocol: dtls,
+ name: "KeyUpdate-DeferredSend-DTLS",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ // Request a client certificate, so the shim has more to send.
+ ClientAuth: RequireAnyClientCert,
+ Bugs: ProtocolBugs{
+ MaxPacketLength: 512,
+ ACKFlightDTLS: func(c *DTLSController, prev, received []DTLSMessage, records []DTLSRecordNumberInfo) {
+ if received[len(received)-1].Type != typeFinished {
+ c.WriteACK(c.OutEpoch(), records)
+ return
+ }
+
+ // This test relies on the Finished flight being multiple
+ // records.
+ if len(records) <= 1 {
+ panic("shim sent Finished flight in one record")
+ }
+
+ // Before ACKing Finished, do some rounds of exchanging
+ // application data. Although the shim has already scheduled
+ // KeyUpdate, it should not send the KeyUpdate until it gets
+ // an ACK. (If it sent KeyUpdate, ReadAppData would report
+ // an unexpected record.)
+ msg := []byte("test")
+ for i := 0; i < 10; i++ {
+ c.WriteAppData(c.OutEpoch(), msg)
+ c.ReadAppData(c.InEpoch(), expectedReply(msg))
+ }
+
+ // ACK some of the Finished flight, but not all of it.
+ c.WriteACK(c.OutEpoch(), records[:1])
+
+ // The shim continues to defer KeyUpdate.
+ for i := 0; i < 10; i++ {
+ c.WriteAppData(c.OutEpoch(), msg)
+ c.ReadAppData(c.InEpoch(), expectedReply(msg))
+ }
+
+ // ACK the remainder.
+ c.WriteACK(c.OutEpoch(), records[1:])
+
+ // The shim should now send KeyUpdate. Return to the test
+ // harness, which will look for it.
+ },
+ },
+ },
+ shimCertificate: &rsaChainCertificate,
+ shimSendsKeyUpdateBeforeRead: true,
+ flags: []string{"-mtu", "512"},
+ })
+
+ // The shim should not switch keys until it receives an ACK.
+ testCases = append(testCases, testCase{
+ protocol: dtls,
+ name: "KeyUpdate-WaitForACK-DTLS",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ Bugs: ProtocolBugs{
+ MaxPacketLength: 512,
+ ACKFlightDTLS: func(c *DTLSController, prev, received []DTLSMessage, records []DTLSRecordNumberInfo) {
+ if received[0].Type != typeKeyUpdate {
+ c.WriteACK(c.OutEpoch(), records)
+ return
+ }
+
+ // Make the shim send application data. We have not yet
+ // ACKed KeyUpdate, so the shim should send at the previous
+ // epoch. Through each of these rounds, the shim will also
+ // try to KeyUpdate again. These calls will be suppressed
+ // because there is still an outstanding KeyUpdate.
+ msg := []byte("test")
+ for i := 0; i < 10; i++ {
+ c.WriteAppData(c.OutEpoch(), msg)
+ c.ReadAppData(c.InEpoch()-1, expectedReply(msg))
+ }
+
+ // ACK the KeyUpdate. Ideally we'd test a partial ACK, but
+ // BoringSSL's minimum MTU is such that KeyUpdate always
+ // fits in one record.
+ c.WriteACK(c.OutEpoch(), records)
+
+ // The shim should now send at the new epoch. Return to the
+ // test harness, which will enforce this.
+ },
+ },
+ },
+ shimSendsKeyUpdateBeforeRead: true,
+ })
+
+ // Test that shim responds to KeyUpdate requests.
+ fixKeyUpdateReply := func(c *DTLSController, prev, received []DTLSMessage, records []DTLSRecordNumberInfo) {
+ c.WriteACK(c.OutEpoch(), records)
+ if received[0].Type != typeKeyUpdate {
+ return
+ }
+ // This works around an awkward testing mismatch. The test
+ // harness expects the shim to immediately change keys, but
+ // the shim writes app data before seeing the ACK. The app
+ // data will be sent at the previous epoch. Consume this and
+ // prime the shim to resend its reply at the new epoch.
+ msg := makeTestMessage(int(received[0].Sequence)-2, 32)
+ c.ReadAppData(c.InEpoch()-1, expectedReply(msg))
+ c.WriteAppData(c.OutEpoch(), msg)
+ }
+ testCases = append(testCases, testCase{
+ protocol: dtls,
+ name: "KeyUpdate-Requested-DTLS",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ Bugs: ProtocolBugs{
+ RejectUnsolicitedKeyUpdate: true,
+ ACKFlightDTLS: fixKeyUpdateReply,
+ },
+ },
+ // Test the shim receiving many KeyUpdates in a row. They will be
+ // combined into one reply KeyUpdate.
+ sendKeyUpdates: 5,
+ messageLen: 32,
+ messageCount: 5,
+ keyUpdateRequest: keyUpdateRequested,
+ })
+
mergeNewSessionTicketAndKeyUpdate := func(f WriteFlightFunc) WriteFlightFunc {
return func(c *DTLSController, prev, received, next []DTLSMessage, records []DTLSRecordNumberInfo) {
// Send NewSessionTicket and the first KeyUpdate all together.
@@ -22358,12 +22532,15 @@
// Test KeyUpdate overflow conditions. Both the epoch number and the message
// number may overflow, in either the read or write direction.
- //
- // TODO(crbug.com/42290594): Test the epoch write number overflowing, once
- // we implement sending KeyUpdates.
- //
- // TODO(crbug.com/42290594): Test the message write number overflowing, once
- // we implement sending KeyUpdates.
+
+ // When the sender is the client, the first KeyUpdate is message 2 at epoch
+ // 3, so the epoch number overflows first.
+ const maxClientKeyUpdates = 0xffff - 3
+
+ // Test that the shim, as a server, rejects KeyUpdates at epoch 0xffff. RFC
+ // 9147 does not prescribe this limit, but we enforce it. See
+ // https://mailarchive.ietf.org/arch/msg/tls/6y8wTv8Q_IPM-PCcbCAmDOYg6bM/
+ // and https://www.rfc-editor.org/errata/eid8050
writeFlightKeyUpdate := func(c *DTLSController, prev, received, next []DTLSMessage, records []DTLSRecordNumberInfo) {
if next[0].Type == typeKeyUpdate {
// Exchange some data to avoid tripping KeyUpdate DoS limits.
@@ -22373,12 +22550,6 @@
}
c.WriteFlight(next)
}
-
- // When the runner is the client, the first KeyUpdate is message 2 at epoch
- // 3, so the epoch number overflows first. Test that the shim, as a server,
- // rejects KeyUpdates at epoch 0xffff. RFC 9147 does not prescribe this
- // limit, but we enforce it. See https://mailarchive.ietf.org/arch/msg/tls/6y8wTv8Q_IPM-PCcbCAmDOYg6bM/
- // and https://www.rfc-editor.org/errata/eid8050
testCases = append(testCases, testCase{
testType: serverTest,
protocol: dtls,
@@ -22391,9 +22562,8 @@
},
},
// Avoid the NewSessionTicket messages interfering with the callback.
- flags: []string{"-no-ticket"},
- // Application data starts at epoch 3. Epoch 0xffff is the limit.
- sendKeyUpdates: 0xffff - 3,
+ flags: []string{"-no-ticket"},
+ sendKeyUpdates: maxClientKeyUpdates,
keyUpdateRequest: keyUpdateNotRequested,
})
testCases = append(testCases, testCase{
@@ -22408,23 +22578,56 @@
},
},
// Avoid the NewSessionTicket messages interfering with the callback.
- flags: []string{"-no-ticket"},
- // Application data starts at epoch 3. Epoch 0xffff is the limit.
- sendKeyUpdates: 0xffff - 3 + 1,
+ flags: []string{"-no-ticket"},
+ sendKeyUpdates: maxClientKeyUpdates + 1,
keyUpdateRequest: keyUpdateNotRequested,
shouldFail: true,
expectedError: ":TOO_MANY_KEY_UPDATES:",
expectedLocalError: "remote error: unexpected message",
})
- // When the runner is a server, the first KeyUpdate is message 7 (SH, EE, C,
- // CV, Fin, NST, NST) at epoch 3, so the message number overflows first.
+ // Test that the shim, as a client, notices its epoch overflow condition
+ // when asked to send too many KeyUpdates. The shim sends KeyUpdate before
+ // every read, including reading connection close, so the number of
+ // KeyUpdates is one more than the message count.
+ testCases = append(testCases, testCase{
+ protocol: dtls,
+ name: "KeyUpdate-MaxWriteEpoch-DTLS",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ },
+ shimSendsKeyUpdateBeforeRead: true,
+ messageCount: maxClientKeyUpdates - 1,
+ })
+ testCases = append(testCases, testCase{
+ protocol: dtls,
+ name: "KeyUpdate-WriteEpochOverflow-DTLS",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ Bugs: ProtocolBugs{
+ // The shim does not notice the overflow until immediately after
+ // sending KeyUpdate, so tolerate the overflow on the runner.
+ AllowEpochOverflow: true,
+ },
+ },
+ shimSendsKeyUpdateBeforeRead: true,
+ messageCount: maxClientKeyUpdates,
+ shouldFail: true,
+ expectedError: ":TOO_MANY_KEY_UPDATES:",
+ })
+
+ // When the sender is a server that doesn't send tickets, the first
+ // KeyUpdate is message 5 (SH, EE, C, CV, Fin) at epoch 3, so the message
+ // number overflows first.
+ const maxServerKeyUpdates = 0xffff - 5
+
// Test that the shim, as a client, does not allow the value to wraparound.
testCases = append(testCases, testCase{
protocol: dtls,
name: "KeyUpdate-ReadMessageOverflow-DTLS",
config: Config{
- MaxVersion: VersionTLS13,
+ MaxVersion: VersionTLS13,
+ SessionTicketsDisabled: true,
Bugs: ProtocolBugs{
AllowEpochOverflow: true,
WriteFlightDTLS: func(c *DTLSController, prev, received, next []DTLSMessage, records []DTLSRecordNumberInfo) {
@@ -22450,9 +22653,38 @@
},
},
},
- sendKeyUpdates: 0xffff - 7 + 1,
+ sendKeyUpdates: maxServerKeyUpdates + 1,
keyUpdateRequest: keyUpdateNotRequested,
- flags: []string{"-async"},
+ flags: []string{"-async", "-expect-no-session"},
+ })
+
+ // Test that the shim, as a server, notices its message overflow condition,
+ // when asked to send too many KeyUpdates.
+ testCases = append(testCases, testCase{
+ protocol: dtls,
+ testType: serverTest,
+ name: "KeyUpdate-MaxWriteMessage-DTLS",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ },
+ shimSendsKeyUpdateBeforeRead: true,
+ messageCount: maxServerKeyUpdates,
+ // Avoid NewSessionTicket messages getting in the way of ReadKeyUpdate.
+ flags: []string{"-no-ticket"},
+ })
+ testCases = append(testCases, testCase{
+ protocol: dtls,
+ testType: serverTest,
+ name: "KeyUpdate-WriteMessageOverflow-DTLS",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ },
+ shimSendsKeyUpdateBeforeRead: true,
+ messageCount: maxServerKeyUpdates + 1,
+ shouldFail: true,
+ expectedError: ":overflow:",
+ // Avoid NewSessionTicket messages getting in the way of ReadKeyUpdate.
+ flags: []string{"-no-ticket"},
})
}
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc
index 6583b74..9471379 100644
--- a/ssl/test/test_config.cc
+++ b/ssl/test/test_config.cc
@@ -471,6 +471,8 @@
BoolFlag("-export-traffic-secrets",
&TestConfig::export_traffic_secrets),
BoolFlag("-key-update", &TestConfig::key_update),
+ BoolFlag("-key-update-before-read",
+ &TestConfig::key_update_before_read),
StringFlag("-expect-early-data-reason",
&TestConfig::expect_early_data_reason),
BoolFlag("-expect-hrr", &TestConfig::expect_hrr),
diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h
index b4b290d..06817b6 100644
--- a/ssl/test/test_config.h
+++ b/ssl/test/test_config.h
@@ -208,6 +208,7 @@
bool server_preference = false;
bool export_traffic_secrets = false;
bool key_update = false;
+ bool key_update_before_read = false;
std::string expect_early_data_reason;
bool expect_hrr = false;
bool expect_no_hrr = false;
diff --git a/ssl/tls13_both.cc b/ssl/tls13_both.cc
index e0d5ad0..b26693b 100644
--- a/ssl/tls13_both.cc
+++ b/ssl/tls13_both.cc
@@ -618,13 +618,31 @@
return true;
}
-bool tls13_add_key_update(SSL *ssl, int update_requested) {
+bool tls13_add_key_update(SSL *ssl, int request_type) {
+ if (ssl->s3->key_update_pending) {
+ return true;
+ }
+
+ // We do not support multiple parallel outgoing flights. If there is an
+ // outgoing flight pending, queue the KeyUpdate for later.
+ if (SSL_is_dtls(ssl) && !ssl->d1->outgoing_messages.empty()) {
+ ssl->d1->queued_key_update = request_type == SSL_KEY_UPDATE_REQUESTED
+ ? QueuedKeyUpdate::kUpdateRequested
+ : QueuedKeyUpdate::kUpdateNotRequested;
+ return true;
+ }
+
ScopedCBB cbb;
CBB body_cbb;
if (!ssl->method->init_message(ssl, cbb.get(), &body_cbb,
SSL3_MT_KEY_UPDATE) ||
- !CBB_add_u8(&body_cbb, update_requested) ||
- !ssl_add_message_cbb(ssl, cbb.get()) ||
+ !CBB_add_u8(&body_cbb, request_type) ||
+ !ssl_add_message_cbb(ssl, cbb.get())) {
+ return false;
+ }
+
+ // In DTLS, the actual key update is deferred until KeyUpdate is ACKed.
+ if (!SSL_is_dtls(ssl) &&
!tls13_rotate_traffic_key(ssl, evp_aead_seal)) {
return false;
}
@@ -654,10 +672,7 @@
}
// Acknowledge the KeyUpdate
- // TODO(crbug.com/42290594): Support sending KeyUpdate in DTLS 1.3.
- if (!SSL_is_dtls(ssl) && //
- key_update_request == SSL_KEY_UPDATE_REQUESTED &&
- !ssl->s3->key_update_pending &&
+ if (key_update_request == SSL_KEY_UPDATE_REQUESTED &&
!tls13_add_key_update(ssl, SSL_KEY_UPDATE_NOT_REQUESTED)) {
return false;
}
