Implement receiving KeyUpdates in DTLS 1.3
Receiving KeyUpdates is easy because as soon as you get to the message,
you can act on it. We just needed to fix up some edge cases around epoch
management.
I have included a couple tests which exercise the absolute highest
possible epoch. On my laptop, they take a couple seconds to run each
with an unoptimized shim. That is not very fast, but will maybe be
bearable. But we may need a flag later to disable those tests.
Bug: 42290594
Change-Id: I4276c1ff4547e31ba3367c74606dcf3a2db386f5
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/73308
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Nick Harper <nharper@chromium.org>
diff --git a/ssl/dtls_method.cc b/ssl/dtls_method.cc
index df0f7ed..54bac2f 100644
--- a/ssl/dtls_method.cc
+++ b/ssl/dtls_method.cc
@@ -82,6 +82,34 @@
}
}
+static bool next_epoch(const SSL *ssl, uint16_t *out,
+ ssl_encryption_level_t level, uint16_t prev) {
+ switch (level) {
+ case ssl_encryption_initial:
+ case ssl_encryption_early_data:
+ case ssl_encryption_handshake:
+ *out = static_cast<uint16_t>(level);
+ return true;
+
+ case ssl_encryption_application:
+ if (prev < ssl_encryption_application &&
+ ssl_protocol_version(ssl) >= TLS1_3_VERSION) {
+ *out = static_cast<uint16_t>(level);
+ return true;
+ }
+
+ if (prev == 0xffff) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_TOO_MANY_KEY_UPDATES);
+ return false;
+ }
+ *out = prev + 1;
+ return true;
+ }
+
+ assert(0);
+ return false;
+}
+
static bool dtls1_set_read_state(SSL *ssl, ssl_encryption_level_t level,
UniquePtr<SSLAEADContext> aead_ctx,
Span<const uint8_t> traffic_secret) {
@@ -94,10 +122,12 @@
DTLSReadEpoch new_epoch;
new_epoch.aead = std::move(aead_ctx);
+ if (!next_epoch(ssl, &new_epoch.epoch, level, ssl->d1->read_epoch.epoch)) {
+ ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_UNEXPECTED_MESSAGE);
+ return false;
+ }
+
if (ssl_protocol_version(ssl) > TLS1_2_VERSION) {
- // TODO(crbug.com/42290594): Handle the additional epochs used for key
- // update.
- new_epoch.epoch = level;
new_epoch.rn_encrypter =
RecordNumberEncrypter::Create(new_epoch.aead->cipher(), traffic_secret);
if (new_epoch.rn_encrypter == nullptr) {
@@ -113,7 +143,6 @@
return false;
}
} else {
- new_epoch.epoch = ssl->d1->read_epoch.epoch + 1;
ssl->d1->read_epoch = std::move(new_epoch);
ssl->d1->has_change_cipher_spec = false;
}
@@ -123,20 +152,21 @@
static bool dtls1_set_write_state(SSL *ssl, ssl_encryption_level_t level,
UniquePtr<SSLAEADContext> aead_ctx,
Span<const uint8_t> traffic_secret) {
+ uint16_t epoch;
+ if (!next_epoch(ssl, &epoch, level, ssl->d1->write_epoch.epoch())) {
+ return false;
+ }
+
DTLSWriteEpoch new_epoch;
+ new_epoch.aead = std::move(aead_ctx);
+ new_epoch.next_record = DTLSRecordNumber(epoch, 0);
if (ssl_protocol_version(ssl) > TLS1_2_VERSION) {
- // TODO(crbug.com/42290594): See above.
- new_epoch.next_record = DTLSRecordNumber(level, 0);
new_epoch.rn_encrypter =
- RecordNumberEncrypter::Create(aead_ctx->cipher(), traffic_secret);
+ RecordNumberEncrypter::Create(new_epoch.aead->cipher(), traffic_secret);
if (new_epoch.rn_encrypter == nullptr) {
return false;
}
- } else {
- new_epoch.next_record =
- DTLSRecordNumber(ssl->d1->write_epoch.epoch() + 1, 0);
}
- new_epoch.aead = std::move(aead_ctx);
auto current = MakeUnique<DTLSWriteEpoch>(std::move(ssl->d1->write_epoch));
if (current == nullptr) {
diff --git a/ssl/dtls_record.cc b/ssl/dtls_record.cc
index 36676ca..5b124d9 100644
--- a/ssl/dtls_record.cc
+++ b/ssl/dtls_record.cc
@@ -243,9 +243,11 @@
return false;
}
- // TODO(crbug.com/42290594): Add a runner test that performs many
- // key updates to verify epoch reconstruction works for epochs larger than 3.
- uint16_t epoch = reconstruct_epoch(out->type, ssl->d1->read_epoch.epoch);
+ uint16_t max_epoch = ssl->d1->read_epoch.epoch;
+ if (ssl->d1->next_read_epoch != nullptr) {
+ max_epoch = std::max(max_epoch, ssl->d1->next_read_epoch->epoch);
+ }
+ uint16_t epoch = reconstruct_epoch(out->type, max_epoch);
size_t seq_len = (out->type & 0x08) ? 2 : 1;
CBS seq_bytes;
if (!CBS_get_bytes(in, &seq_bytes, seq_len)) {
diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go
index e5ba6d4..6e76f23 100644
--- a/ssl/test/runner/common.go
+++ b/ssl/test/runner/common.go
@@ -2006,6 +2006,11 @@
// 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 bool
}
func (c *Config) serverInit() {
diff --git a/ssl/test/runner/conn.go b/ssl/test/runner/conn.go
index 5af9fba..071e262 100644
--- a/ssl/test/runner/conn.go
+++ b/ssl/test/runner/conn.go
@@ -2086,7 +2086,7 @@
return errors.New("tls: attempted to send KeyUpdate before TLS 1.3")
}
epoch := c.out.epoch.epoch + 1
- if epoch == 0 {
+ if epoch == 0 && !c.config.Bugs.AllowEpochOverflow {
return errors.New("tls: too many KeyUpdates")
}
@@ -2096,16 +2096,12 @@
if _, err := c.writeRecord(recordTypeHandshake, m.marshal()); err != nil {
return err
}
- if err := c.flushHandshake(); err != nil {
- return err
- }
- if !c.isDTLS {
- // TODO(crbug.com/42290594): Properly implement KeyUpdate. Right
- // now we only support sending KeyUpdate to test that we drop
- // post-HS messages on the floor (instead of erroring).
- c.useOutTrafficSecret(epoch, c.out.wireVersion, c.cipherSuite, updateTrafficSecret(c.cipherSuite.hash(), c.wireVersion, c.out.trafficSecret, c.isDTLS))
- }
- return nil
+ // In DTLS 1.3, a real implementation would not install the new epoch until
+ // receiving an ACK. Our test transport is ordered and reliable, so this is
+ // not necessary. ACK effects will be simulated in tests by the WriteFlight
+ // callback.
+ c.useOutTrafficSecret(epoch, c.out.wireVersion, c.cipherSuite, updateTrafficSecret(c.cipherSuite.hash(), c.wireVersion, c.out.trafficSecret, c.isDTLS))
+ return c.flushHandshake()
}
func (c *Conn) sendFakeEarlyData(len int) error {
diff --git a/ssl/test/runner/dtls.go b/ssl/test/runner/dtls.go
index 3b08f79..2b91212 100644
--- a/ssl/test/runner/dtls.go
+++ b/ssl/test/runner/dtls.go
@@ -1095,10 +1095,6 @@
continue
}
- if f.ShouldDiscard {
- anyDiscard = true
- }
-
fBytes := f.Bytes()
if n := config.Bugs.SplitFragments; n > 0 {
if len(fBytes) > n {
@@ -1123,6 +1119,9 @@
return
}
}
+ if f.ShouldDiscard {
+ anyDiscard = true
+ }
record = append(record, fBytes...)
}
}
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 1e9ca3c..5ec1382 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -1093,7 +1093,9 @@
if test.damageFirstWrite {
connDamage.setDamage(true)
- tlsConn.Write([]byte("DAMAGED WRITE"))
+ if _, err := tlsConn.Write([]byte("DAMAGED WRITE")); err != nil {
+ return err
+ }
connDamage.setDamage(false)
}
@@ -1118,27 +1120,39 @@
for j := 0; j < messageCount; j++ {
for i := 0; i < test.sendKeyUpdates; i++ {
- tlsConn.SendKeyUpdate(test.keyUpdateRequest)
+ if err := tlsConn.SendKeyUpdate(test.keyUpdateRequest); err != nil {
+ return err
+ }
}
for i := 0; i < test.sendEmptyRecords; i++ {
- tlsConn.Write(nil)
+ if _, err := tlsConn.Write(nil); err != nil {
+ return err
+ }
}
for i := 0; i < test.sendWarningAlerts; i++ {
- tlsConn.SendAlert(alertLevelWarning, alertUnexpectedMessage)
+ if err := tlsConn.SendAlert(alertLevelWarning, alertUnexpectedMessage); err != nil {
+ return err
+ }
}
for i := 0; i < test.sendUserCanceledAlerts; i++ {
- tlsConn.SendAlert(alertLevelWarning, alertUserCanceled)
+ if err := tlsConn.SendAlert(alertLevelWarning, alertUserCanceled); err != nil {
+ return err
+ }
}
if test.sendBogusAlertType {
- tlsConn.SendAlert(0x42, alertUnexpectedMessage)
+ if err := tlsConn.SendAlert(0x42, alertUnexpectedMessage); err != nil {
+ return err
+ }
}
testMessage := makeTestMessage(j, messageLen)
- tlsConn.Write(testMessage)
+ if _, err := tlsConn.Write(testMessage); err != nil {
+ return err
+ }
// Consume the shim prefix if needed.
if shimPrefix != "" {
@@ -3477,101 +3491,6 @@
expectedError: ":WRONG_VERSION_NUMBER:",
},
{
- name: "KeyUpdate-ToClient",
- config: Config{
- MaxVersion: VersionTLS13,
- },
- sendKeyUpdates: 1,
- keyUpdateRequest: keyUpdateNotRequested,
- },
- {
- testType: serverTest,
- name: "KeyUpdate-ToServer",
- config: Config{
- MaxVersion: VersionTLS13,
- },
- sendKeyUpdates: 1,
- keyUpdateRequest: keyUpdateNotRequested,
- },
- {
- protocol: dtls,
- name: "KeyUpdate-ToClient-DTLS",
- config: Config{
- MaxVersion: VersionTLS13,
- },
- sendKeyUpdates: 1,
- keyUpdateRequest: keyUpdateNotRequested,
- },
- {
- protocol: dtls,
- testType: serverTest,
- name: "KeyUpdate-ToServerDTLS",
- config: Config{
- MaxVersion: VersionTLS13,
- },
- sendKeyUpdates: 1,
- keyUpdateRequest: keyUpdateNotRequested,
- },
- {
- name: "KeyUpdate-FromClient",
- config: Config{
- MaxVersion: VersionTLS13,
- },
- expectUnsolicitedKeyUpdate: true,
- flags: []string{"-key-update"},
- },
- {
- testType: serverTest,
- name: "KeyUpdate-FromServer",
- config: Config{
- MaxVersion: VersionTLS13,
- },
- expectUnsolicitedKeyUpdate: true,
- flags: []string{"-key-update"},
- },
- {
- name: "KeyUpdate-InvalidRequestMode",
- config: Config{
- MaxVersion: VersionTLS13,
- },
- sendKeyUpdates: 1,
- keyUpdateRequest: 42,
- shouldFail: true,
- expectedError: ":DECODE_ERROR:",
- },
- {
- // Test that shim responds to KeyUpdate requests.
- name: "KeyUpdate-Requested",
- config: Config{
- MaxVersion: VersionTLS13,
- Bugs: ProtocolBugs{
- RejectUnsolicitedKeyUpdate: true,
- },
- },
- // Test the shim receiving many KeyUpdates in a row.
- sendKeyUpdates: 5,
- messageCount: 5,
- keyUpdateRequest: keyUpdateRequested,
- },
- {
- // Test that shim responds to KeyUpdate requests if
- // peer's KeyUpdate is discovered while a write is
- // pending.
- name: "KeyUpdate-Requested-UnfinishedWrite",
- config: Config{
- MaxVersion: VersionTLS13,
- Bugs: ProtocolBugs{
- RejectUnsolicitedKeyUpdate: true,
- },
- },
- // Test the shim receiving many KeyUpdates in a row.
- sendKeyUpdates: 5,
- messageCount: 5,
- keyUpdateRequest: keyUpdateRequested,
- readWithUnfinishedWrite: true,
- flags: []string{"-async"},
- },
- {
name: "SendSNIWarningAlert",
config: Config{
MaxVersion: VersionTLS12,
@@ -22087,6 +22006,329 @@
}
}
+func addKeyUpdateTests() {
+ // TLS tests.
+ testCases = append(testCases, testCase{
+ name: "KeyUpdate-ToClient",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ },
+ sendKeyUpdates: 10,
+ keyUpdateRequest: keyUpdateNotRequested,
+ })
+ testCases = append(testCases, testCase{
+ testType: serverTest,
+ name: "KeyUpdate-ToServer",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ },
+ sendKeyUpdates: 10,
+ keyUpdateRequest: keyUpdateNotRequested,
+ })
+ testCases = append(testCases, testCase{
+ name: "KeyUpdate-FromClient",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ },
+ expectUnsolicitedKeyUpdate: true,
+ flags: []string{"-key-update"},
+ })
+ testCases = append(testCases, testCase{
+ testType: serverTest,
+ name: "KeyUpdate-FromServer",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ },
+ expectUnsolicitedKeyUpdate: true,
+ flags: []string{"-key-update"},
+ })
+ testCases = append(testCases, testCase{
+ name: "KeyUpdate-InvalidRequestMode",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ },
+ sendKeyUpdates: 1,
+ keyUpdateRequest: 42,
+ shouldFail: true,
+ expectedError: ":DECODE_ERROR:",
+ })
+ testCases = append(testCases, testCase{
+ // Test that shim responds to KeyUpdate requests.
+ name: "KeyUpdate-Requested",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ Bugs: ProtocolBugs{
+ RejectUnsolicitedKeyUpdate: true,
+ },
+ },
+ // Test the shim receiving many KeyUpdates in a row.
+ sendKeyUpdates: 5,
+ messageCount: 5,
+ keyUpdateRequest: keyUpdateRequested,
+ })
+ testCases = append(testCases, testCase{
+ // Test that shim responds to KeyUpdate requests if peer's KeyUpdate is
+ // discovered while a write is pending.
+ name: "KeyUpdate-Requested-UnfinishedWrite",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ Bugs: ProtocolBugs{
+ RejectUnsolicitedKeyUpdate: true,
+ },
+ },
+ // Test the shim receiving many KeyUpdates in a row.
+ sendKeyUpdates: 5,
+ messageCount: 5,
+ keyUpdateRequest: keyUpdateRequested,
+ readWithUnfinishedWrite: true,
+ flags: []string{"-async"},
+ })
+
+ // DTLS tests.
+ testCases = append(testCases, testCase{
+ protocol: dtls,
+ name: "KeyUpdate-ToClient-DTLS",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ },
+ // Send many KeyUpdates to make sure record reassembly can handle it.
+ sendKeyUpdates: 10,
+ keyUpdateRequest: keyUpdateNotRequested,
+ })
+ testCases = append(testCases, testCase{
+ protocol: dtls,
+ testType: serverTest,
+ name: "KeyUpdate-ToServer-DTLS",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ },
+ sendKeyUpdates: 10,
+ keyUpdateRequest: keyUpdateNotRequested,
+ })
+
+ // Test that the shim accounts for packet loss when processing KeyUpdate.
+ testCases = append(testCases, testCase{
+ protocol: dtls,
+ name: "KeyUpdate-ToClient-PacketLoss-DTLS",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ Bugs: ProtocolBugs{
+ WriteFlightDTLS: func(c *DTLSController, prev, received, next []DTLSMessage, records []DTLSRecordNumberInfo) {
+ if next[0].Type != typeKeyUpdate {
+ c.WriteFlight(next)
+ return
+ }
+
+ // Send the KeyUpdate. The shim should ACK it.
+ c.WriteFlight(next)
+ ackTimeout := timeouts[0] / 4
+ c.AdvanceClock(ackTimeout)
+ c.ReadACK(c.InEpoch())
+
+ // The shim should continue reading data at the old epoch.
+ // The ACK may not have come through.
+ msg := []byte("test")
+ c.WriteAppData(c.OutEpoch()-1, msg)
+ c.ReadAppData(c.InEpoch(), expectedReply(msg))
+
+ // Re-send KeyUpdate. The shim should ACK it again. The ACK
+ // may not have come through.
+ c.WriteFlight(next)
+ c.AdvanceClock(ackTimeout)
+ c.ReadACK(c.InEpoch())
+
+ // The shim should be able to read data at the new epoch.
+ c.WriteAppData(c.OutEpoch(), msg)
+ c.ReadAppData(c.InEpoch(), expectedReply(msg))
+
+ // Having received something at the new epoch, the shim
+ // should discard the old epoch. The following writes should
+ // be ignored.
+ f := next[0].Fragment(0, len(next[0].Data))
+ f.ShouldDiscard = true
+ c.WriteFragments([]DTLSFragment{f})
+ c.WriteAppData(c.OutEpoch()-1, msg)
+ },
+ },
+ },
+ sendKeyUpdates: 10,
+ keyUpdateRequest: keyUpdateNotRequested,
+ flags: []string{"-async"},
+ })
+
+ mergeNewSessionTicketAndKeyUpdate := func(f WriteFlightFunc) WriteFlightFunc {
+ return func(c *DTLSController, prev, received, next []DTLSMessage, records []DTLSRecordNumberInfo) {
+ // Send NewSessionTicket and the first KeyUpdate all together.
+ if next[0].Type == typeKeyUpdate {
+ panic("key update should have been merged into NewSessionTicket")
+ }
+ if next[0].Type != typeNewSessionTicket {
+ c.WriteFlight(next)
+ return
+ }
+ if next[0].Type == typeNewSessionTicket && next[len(next)-1].Type != typeKeyUpdate {
+ c.MergeIntoNextFlight()
+ return
+ }
+
+ f(c, prev, received, next, records)
+ }
+ }
+
+ // Test that the shim does not process KeyUpdate until it has processed all
+ // preceding messages.
+ testCases = append(testCases, testCase{
+ protocol: dtls,
+ name: "KeyUpdate-ProcessInOrder-DTLS",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ Bugs: ProtocolBugs{
+ WriteFlightDTLS: mergeNewSessionTicketAndKeyUpdate(func(c *DTLSController, prev, received, next []DTLSMessage, records []DTLSRecordNumberInfo) {
+ // Write the KeyUpdate. The shim should buffer and ACK it.
+ keyUpdate := next[len(next)-1]
+ c.WriteFlight([]DTLSMessage{keyUpdate})
+ ackTimeout := timeouts[0] / 4
+ c.AdvanceClock(ackTimeout)
+ c.ReadACK(c.InEpoch())
+
+ // The shim should not process KeyUpdate yet. It should not
+ // read from the new epoch.
+ msg1, msg2 := []byte("aaaa"), []byte("bbbb")
+ c.WriteAppData(c.OutEpoch(), msg1)
+ c.AdvanceClock(0) // Check there are no messages.
+
+ // It can read from the old epoch, however.
+ c.WriteAppData(c.OutEpoch()-1, msg2)
+ c.ReadAppData(c.InEpoch(), expectedReply(msg2))
+
+ // Write the rest of the flight.
+ c.WriteFlight(next[:len(next)-1])
+ c.AdvanceClock(ackTimeout)
+ c.ReadACK(c.InEpoch())
+
+ // Now the new epoch is functional.
+ c.WriteAppData(c.OutEpoch(), msg1)
+ c.ReadAppData(c.InEpoch(), expectedReply(msg1))
+ }),
+ },
+ },
+ sendKeyUpdates: 1,
+ keyUpdateRequest: keyUpdateNotRequested,
+ flags: []string{"-async"},
+ })
+
+ // Messages after a KeyUpdate are not allowed.
+ testCases = append(testCases, testCase{
+ protocol: dtls,
+ name: "KeyUpdate-ExtraMessage-DTLS",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ Bugs: ProtocolBugs{
+ WriteFlightDTLS: mergeNewSessionTicketAndKeyUpdate(func(c *DTLSController, prev, received, next []DTLSMessage, records []DTLSRecordNumberInfo) {
+ extra := next[0]
+ extra.Sequence = next[len(next)-1].Sequence + 1
+ next = append(slices.Clip(next), extra)
+ c.WriteFlight(next)
+ }),
+ },
+ },
+ sendKeyUpdates: 1,
+ keyUpdateRequest: keyUpdateNotRequested,
+ shouldFail: true,
+ expectedError: ":EXCESS_HANDSHAKE_DATA:",
+ expectedLocalError: "remote error: unexpected message",
+ })
+ testCases = append(testCases, testCase{
+ protocol: dtls,
+ name: "KeyUpdate-ExtraMessageBuffered-DTLS",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ Bugs: ProtocolBugs{
+ WriteFlightDTLS: mergeNewSessionTicketAndKeyUpdate(func(c *DTLSController, prev, received, next []DTLSMessage, records []DTLSRecordNumberInfo) {
+ // Send the extra message first. The shim should accept and
+ // buffer it.
+ extra := next[0]
+ extra.Sequence = next[len(next)-1].Sequence + 1
+ c.WriteFlight([]DTLSMessage{extra})
+
+ // Now send the flight, including a KeyUpdate. The shim
+ // should now notice the extra message and reject.
+ c.WriteFlight(next)
+ }),
+ },
+ },
+ sendKeyUpdates: 1,
+ keyUpdateRequest: keyUpdateNotRequested,
+ shouldFail: true,
+ expectedError: ":EXCESS_HANDSHAKE_DATA:",
+ expectedLocalError: "remote error: unexpected message",
+ })
+
+ // 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 message read number overflowing, once
+ // we fix the lack of checking for it.
+ //
+ // 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.
+ writeFlightKeyUpdate := func(c *DTLSController, prev, received, next []DTLSMessage, records []DTLSRecordNumberInfo) {
+ if next[0].Type == typeKeyUpdate {
+ // Exchange some data to avoid tripping KeyUpdate DoS limits.
+ msg := []byte("test")
+ c.WriteAppData(c.OutEpoch()-1, msg)
+ c.ReadAppData(c.InEpoch(), expectedReply(msg))
+ }
+ 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,
+ name: "KeyUpdate-MaxReadEpoch-DTLS",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ Bugs: ProtocolBugs{
+ AllowEpochOverflow: true,
+ WriteFlightDTLS: writeFlightKeyUpdate,
+ },
+ },
+ // 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,
+ keyUpdateRequest: keyUpdateNotRequested,
+ })
+ testCases = append(testCases, testCase{
+ testType: serverTest,
+ protocol: dtls,
+ name: "KeyUpdate-ReadEpochOverflow-DTLS",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ Bugs: ProtocolBugs{
+ AllowEpochOverflow: true,
+ WriteFlightDTLS: writeFlightKeyUpdate,
+ },
+ },
+ // 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,
+ keyUpdateRequest: keyUpdateNotRequested,
+ shouldFail: true,
+ expectedError: ":TOO_MANY_KEY_UPDATES:",
+ expectedLocalError: "remote error: unexpected message",
+ })
+}
+
func worker(dispatcher *shimDispatcher, statusChan chan statusMsg, c chan *testCase, shimPath string, wg *sync.WaitGroup) {
defer wg.Done()
@@ -22338,6 +22580,7 @@
addHintMismatchTests()
addCompliancePolicyTests()
addCertificateSelectionTests()
+ addKeyUpdateTests()
toAppend, err := convertToSplitHandshakeTests(testCases)
if err != nil {
diff --git a/ssl/tls13_both.cc b/ssl/tls13_both.cc
index 197baaa..697d274 100644
--- a/ssl/tls13_both.cc
+++ b/ssl/tls13_both.cc
@@ -654,7 +654,9 @@
}
// Acknowledge the KeyUpdate
- if (key_update_request == SSL_KEY_UPDATE_REQUESTED &&
+ // 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 &&
!tls13_add_key_update(ssl, SSL_KEY_UPDATE_NOT_REQUESTED)) {
return false;
@@ -669,10 +671,6 @@
}
if (msg.type == SSL3_MT_KEY_UPDATE) {
- if (SSL_is_dtls(ssl)) {
- // TODO(crbug.com/42290594): Process post-handshake messages in DTLS 1.3.
- return true;
- }
ssl->s3->key_update_count++;
if (ssl->quic_method != nullptr ||
ssl->s3->key_update_count > kMaxKeyUpdates) {
