Test that DTLS retransmit can react to MTU changes
This works, but we never tested it. It will make DTLS 1.3 tests more
interesting, because ACKing older vs newer records will result in
different byte boundaries.
Bug: 42290594
Change-Id: Id7d5b98620373362ca9e500cda73cbb120cccc6c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/72787
Reviewed-by: Nick Harper <nharper@chromium.org>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc
index 94d529b..40e6230 100644
--- a/ssl/test/bssl_shim.cc
+++ b/ssl/test/bssl_shim.cc
@@ -844,7 +844,10 @@
}
if (config->is_dtls) {
- bssl::UniquePtr<BIO> packeted = PacketedBioCreate(GetClock());
+ bssl::UniquePtr<BIO> packeted = PacketedBioCreate(
+ GetClock(), [ssl_raw = ssl.get()](uint32_t mtu) -> bool {
+ return SSL_set_mtu(ssl_raw, mtu);
+ });
if (!packeted) {
return false;
}
diff --git a/ssl/test/packeted_bio.cc b/ssl/test/packeted_bio.cc
index 35dd8a8..c7cccd7 100644
--- a/ssl/test/packeted_bio.cc
+++ b/ssl/test/packeted_bio.cc
@@ -19,6 +19,10 @@
#include <stdio.h>
#include <string.h>
+#include <functional>
+#include <utility>
+#include <vector>
+
#include <openssl/mem.h>
#include "../../crypto/internal.h"
@@ -28,13 +32,14 @@
extern const BIO_METHOD g_packeted_bio_method;
-const uint8_t kOpcodePacket = 'P';
-const uint8_t kOpcodeTimeout = 'T';
-const uint8_t kOpcodeTimeoutAck = 't';
+constexpr uint8_t kOpcodePacket = 'P';
+constexpr uint8_t kOpcodeTimeout = 'T';
+constexpr uint8_t kOpcodeTimeoutAck = 't';
+constexpr uint8_t kOpcodeMTU = 'M';
struct PacketedBio {
- explicit PacketedBio(timeval *clock_arg)
- : clock(clock_arg) {
+ PacketedBio(timeval *clock_arg, std::function<bool(uint32_t)> set_mtu_arg)
+ : clock(clock_arg), set_mtu(std::move(set_mtu_arg)) {
OPENSSL_memset(&timeout, 0, sizeof(timeout));
}
@@ -44,6 +49,7 @@
timeval timeout;
timeval *clock;
+ std::function<bool(uint32_t)> set_mtu;
};
PacketedBio *GetData(BIO *bio) {
@@ -109,84 +115,90 @@
BIO_clear_retry_flags(bio);
- // Read the opcode.
- uint8_t opcode;
- int ret = ReadAll(bio->next_bio, &opcode, sizeof(opcode));
- if (ret <= 0) {
- BIO_copy_next_retry(bio);
- return ret;
- }
-
- if (opcode == kOpcodeTimeout) {
- // The caller is required to advance any pending timeouts before continuing.
- if (data->HasTimeout()) {
- fprintf(stderr, "Unprocessed timeout!\n");
- return -1;
- }
-
- // Process the timeout.
- uint8_t buf[8];
- ret = ReadAll(bio->next_bio, buf, sizeof(buf));
+ for (;;) {
+ // Read the opcode.
+ uint8_t opcode;
+ int ret = ReadAll(bio->next_bio, &opcode, sizeof(opcode));
if (ret <= 0) {
BIO_copy_next_retry(bio);
return ret;
}
- uint64_t timeout = (static_cast<uint64_t>(buf[0]) << 56) |
- (static_cast<uint64_t>(buf[1]) << 48) |
- (static_cast<uint64_t>(buf[2]) << 40) |
- (static_cast<uint64_t>(buf[3]) << 32) |
- (static_cast<uint64_t>(buf[4]) << 24) |
- (static_cast<uint64_t>(buf[5]) << 16) |
- (static_cast<uint64_t>(buf[6]) << 8) |
- static_cast<uint64_t>(buf[7]);
- timeout /= 1000; // Convert nanoseconds to microseconds.
- data->timeout.tv_usec = timeout % 1000000;
- data->timeout.tv_sec = timeout / 1000000;
+ if (opcode == kOpcodeTimeout) {
+ // The caller is required to advance any pending timeouts before
+ // continuing.
+ if (data->HasTimeout()) {
+ fprintf(stderr, "Unprocessed timeout!\n");
+ return -1;
+ }
- // Send an ACK to the peer.
- ret = BIO_write(bio->next_bio, &kOpcodeTimeoutAck, 1);
+ // Process the timeout.
+ uint8_t buf[8];
+ ret = ReadAll(bio->next_bio, buf, sizeof(buf));
+ if (ret <= 0) {
+ BIO_copy_next_retry(bio);
+ return ret;
+ }
+ uint64_t timeout = CRYPTO_load_u64_be(buf);
+ timeout /= 1000; // Convert nanoseconds to microseconds.
+
+ data->timeout.tv_usec = timeout % 1000000;
+ data->timeout.tv_sec = timeout / 1000000;
+
+ // Send an ACK to the peer.
+ ret = BIO_write(bio->next_bio, &kOpcodeTimeoutAck, 1);
+ if (ret <= 0) {
+ return ret;
+ }
+ assert(ret == 1);
+
+ // Signal to the caller to retry the read, after advancing the clock.
+ BIO_set_retry_read(bio);
+ return -1;
+ }
+
+ if (opcode == kOpcodeMTU) {
+ uint8_t buf[4];
+ ret = ReadAll(bio->next_bio, buf, sizeof(buf));
+ if (ret <= 0) {
+ BIO_copy_next_retry(bio);
+ return ret;
+ }
+ uint32_t mtu = CRYPTO_load_u32_be(buf);
+ if (!data->set_mtu(mtu)) {
+ fprintf(stderr, "Error setting MTU\n");
+ return -1;
+ }
+ // Continue reading.
+ continue;
+ }
+
+ if (opcode != kOpcodePacket) {
+ fprintf(stderr, "Unknown opcode, %u\n", opcode);
+ return -1;
+ }
+
+ // Read the length prefix.
+ uint8_t len_bytes[4];
+ ret = ReadAll(bio->next_bio, len_bytes, sizeof(len_bytes));
if (ret <= 0) {
+ BIO_copy_next_retry(bio);
return ret;
}
- assert(ret == 1);
- // Signal to the caller to retry the read, after advancing the clock.
- BIO_set_retry_read(bio);
- return -1;
- }
+ std::vector<uint8_t> buf(CRYPTO_load_u32_be(len_bytes), 0);
+ ret = ReadAll(bio->next_bio, buf.data(), buf.size());
+ if (ret <= 0) {
+ fprintf(stderr, "Packeted BIO was truncated\n");
+ return -1;
+ }
- if (opcode != kOpcodePacket) {
- fprintf(stderr, "Unknown opcode, %u\n", opcode);
- return -1;
+ if (static_cast<size_t>(outl) > buf.size()) {
+ outl = static_cast<int>(buf.size());
+ }
+ OPENSSL_memcpy(out, buf.data(), outl);
+ return outl;
}
-
- // Read the length prefix.
- uint8_t len_bytes[4];
- ret = ReadAll(bio->next_bio, len_bytes, sizeof(len_bytes));
- if (ret <= 0) {
- BIO_copy_next_retry(bio);
- return ret;
- }
-
- uint32_t len = (len_bytes[0] << 24) | (len_bytes[1] << 16) |
- (len_bytes[2] << 8) | len_bytes[3];
- uint8_t *buf = (uint8_t *)OPENSSL_malloc(len);
- if (buf == NULL) {
- return -1;
- }
- ret = ReadAll(bio->next_bio, buf, len);
- if (ret <= 0) {
- fprintf(stderr, "Packeted BIO was truncated\n");
- return -1;
- }
-
- if (outl > (int)len) {
- outl = len;
- }
- OPENSSL_memcpy(out, buf, outl);
- OPENSSL_free(buf);
- return outl;
}
static long PacketedCtrl(BIO *bio, int cmd, long num, void *ptr) {
@@ -237,12 +249,13 @@
} // namespace
-bssl::UniquePtr<BIO> PacketedBioCreate(timeval *clock) {
+bssl::UniquePtr<BIO> PacketedBioCreate(timeval *clock,
+ std::function<bool(uint32_t)> set_mtu) {
bssl::UniquePtr<BIO> bio(BIO_new(&g_packeted_bio_method));
if (!bio) {
return nullptr;
}
- bio->ptr = new PacketedBio(clock);
+ bio->ptr = new PacketedBio(clock, std::move(set_mtu));
return bio;
}
diff --git a/ssl/test/packeted_bio.h b/ssl/test/packeted_bio.h
index 7f07fcb..a064b10 100644
--- a/ssl/test/packeted_bio.h
+++ b/ssl/test/packeted_bio.h
@@ -15,6 +15,8 @@
#ifndef HEADER_PACKETED_BIO
#define HEADER_PACKETED_BIO
+#include <functional>
+
#include <openssl/base.h>
#include <openssl/bio.h>
@@ -29,11 +31,13 @@
// PacketedBioCreate creates a filter BIO which implements a reliable in-order
// blocking datagram socket. It uses the value of |*clock| as the clock.
+// |set_mtu| will be called when the runner asks to change the MTU.
//
// During a |BIO_read|, the peer may signal the filter BIO to simulate a
// timeout. The operation will fail immediately. The caller must then call
// |PacketedBioAdvanceClock| before retrying |BIO_read|.
-bssl::UniquePtr<BIO> PacketedBioCreate(timeval *clock);
+bssl::UniquePtr<BIO> PacketedBioCreate(timeval *clock,
+ std::function<bool(uint32_t)> set_mtu);
// PacketedBioAdvanceClock advances |bio|'s clock and returns true if there is a
// pending timeout. Otherwise, it returns false.
diff --git a/ssl/test/runner/conn.go b/ssl/test/runner/conn.go
index 23d53f5..d70061b 100644
--- a/ssl/test/runner/conn.go
+++ b/ssl/test/runner/conn.go
@@ -119,6 +119,7 @@
handMsg []byte // pending assembled handshake message
handMsgLen int // handshake message length, not including the header
pendingPacket []byte // pending outgoing packet.
+ maxPacketLen int
previousFlight []DTLSMessage
receivedFlight []DTLSMessage
@@ -141,8 +142,7 @@
lastRecordInFlight *dtlsRecordInfo
// bytesAvailableInPacket is the number of bytes that were still available
- // in the current DTLS packet, up to a budget of
- // config.Bugs.MaxPacketLength.
+ // in the current DTLS packet, up to a budget of maxPacketLen.
bytesAvailableInPacket int
// skipRecordVersionCheck, if true, causes the DTLS record layer to skip the
@@ -163,6 +163,7 @@
c.out.config = c.config
c.in.conn = c
c.out.conn = c
+ c.maxPacketLen = c.config.Bugs.MaxPacketLength
}
// Access to net.Conn methods.
diff --git a/ssl/test/runner/dtls.go b/ssl/test/runner/dtls.go
index 18abd91..d718f9a 100644
--- a/ssl/test/runner/dtls.go
+++ b/ssl/test/runner/dtls.go
@@ -234,11 +234,11 @@
if err != nil {
return 0, nil, err
}
- if maxPacket := c.config.Bugs.MaxPacketLength; maxPacket != 0 {
- if n > maxPacket {
+ if c.maxPacketLen != 0 {
+ if n > c.maxPacketLen {
return 0, nil, fmt.Errorf("dtls: exceeded maximum packet length")
}
- c.bytesAvailableInPacket = maxPacket - n
+ c.bytesAvailableInPacket = c.maxPacketLen - n
} else {
c.bytesAvailableInPacket = 0
}
@@ -750,9 +750,6 @@
// makes all methods do nothing. The Err method may be used to query if it is in
// this state, if it would otherwise cause an infinite loop.
//
-// TODO(crbug.com/42290594): Add a way to change the MTU and test that the shim
-// re-packs packets accordingly.
-//
// TODO(crbug.com/42290594): Add a way to send and expect application data, to
// test that final flight retransmissions and post-handshake messages can
// interleave with application data.
@@ -806,6 +803,21 @@
}
}
+// SetMTU updates the shim's MTU to mtu.
+func (c *DTLSController) SetMTU(mtu int) {
+ if c.err != nil {
+ return
+ }
+
+ adaptor := c.conn.config.Bugs.PacketAdaptor
+ if adaptor == nil {
+ panic("tls: no PacketAdapter set")
+ }
+
+ c.conn.maxPacketLen = mtu
+ c.err = adaptor.SetPeerMTU(mtu)
+}
+
// WriteFlight writes msgs to the shim, using the default fragmenting logic.
// This may be used when the test is not concerned with fragmentation.
func (c *DTLSController) WriteFlight(msgs []DTLSMessage) {
diff --git a/ssl/test/runner/packet_adapter.go b/ssl/test/runner/packet_adapter.go
index 42684fb..bf1a0bb 100644
--- a/ssl/test/runner/packet_adapter.go
+++ b/ssl/test/runner/packet_adapter.go
@@ -27,6 +27,9 @@
// at the timeout, to bracket one flight of messages from C.
const opcodeTimeoutAck = byte('t')
+// opcodeMTU updates the shim's MTU, encoded as a 32-bit number of bytes.
+const opcodeMTU = byte('M')
+
type packetAdaptor struct {
net.Conn
debug *recordingConn
@@ -131,6 +134,17 @@
}
}
+// SetPeerMTU instructs the peer to set the MTU to the specified value.
+func (p *packetAdaptor) SetPeerMTU(mtu int) error {
+ p.log(fmt.Sprintf("Setting MTU to %d", mtu), nil)
+
+ payload := make([]byte, 1+4)
+ payload[0] = opcodeMTU
+ binary.BigEndian.PutUint32(payload[1:], uint32(mtu))
+ _, err := p.Conn.Write(payload)
+ return err
+}
+
type replayAdaptor struct {
net.Conn
prevWrite []byte
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 69269f5..604bb1a 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -11617,6 +11617,29 @@
flags: []string{"-async"},
})
+ // Test that the shim can retransmit at different MTUs.
+ testCases = append(testCases, testCase{
+ protocol: dtls,
+ name: "DTLS-Retransmit-ChangeMTU",
+ config: Config{
+ MaxVersion: VersionTLS12,
+ // Request a client certificate, so the shim has more to send.
+ ClientAuth: RequireAnyClientCert,
+ Bugs: ProtocolBugs{
+ WriteFlightDTLS: func(c *DTLSController, prev, received, next []DTLSMessage) {
+ for i, mtu := range []int{300, 301, 302, 303, 299, 298, 297} {
+ c.SetMTU(mtu)
+ c.AdvanceClock(timeouts[i])
+ c.ReadRetransmit()
+ }
+ c.WriteFlight(next)
+ },
+ },
+ },
+ shimCertificate: &rsaChainCertificate,
+ flags: []string{"-async"},
+ })
+
// If the shim sends the last Finished (server full or client resume
// handshakes), it must retransmit that Finished when it sees a
// post-handshake penultimate Finished from the runner. The above tests
