Process (and ignore) DTLS 1.3 ACK messages.
Bug: 42290594
Change-Id: Icbff180fbfecfc704b2ea487b9de345f3c214d71
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/72147
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/include/openssl/ssl3.h b/include/openssl/ssl3.h
index 221adf0..4a391cb 100644
--- a/include/openssl/ssl3.h
+++ b/include/openssl/ssl3.h
@@ -266,6 +266,7 @@
#define SSL3_RT_ALERT 21
#define SSL3_RT_HANDSHAKE 22
#define SSL3_RT_APPLICATION_DATA 23
+#define SSL3_RT_ACK 26
// Pseudo content type for SSL/TLS header info
#define SSL3_RT_HEADER 0x100
diff --git a/ssl/d1_both.cc b/ssl/d1_both.cc
index f9f3a90..f8ac34a 100644
--- a/ssl/d1_both.cc
+++ b/ssl/d1_both.cc
@@ -404,6 +404,9 @@
record);
return ssl_open_record_success;
+ case SSL3_RT_ACK:
+ return dtls1_process_ack(ssl, out_alert);
+
case SSL3_RT_HANDSHAKE:
// Break out to main processing.
break;
diff --git a/ssl/d1_pkt.cc b/ssl/d1_pkt.cc
index 1dc5775..e768e03 100644
--- a/ssl/d1_pkt.cc
+++ b/ssl/d1_pkt.cc
@@ -127,6 +127,22 @@
BSSL_NAMESPACE_BEGIN
+ssl_open_record_t dtls1_process_ack(SSL *ssl, uint8_t *out_alert) {
+ // ACKs are only allowed in DTLS 1.3. Reject them if we've negotiated a
+ // version and it's not 1.3. (It's theoretically possible to receive an ACK
+ // before version negotiation, e.g. due to packet loss or a server ACKing a
+ // ClientHello prior to sending the ServerHello, so if we don't have a version
+ // we'll accept the ACK.)
+ if (ssl->s3->version != 0 && ssl_protocol_version(ssl) < TLS1_3_VERSION) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_UNEXPECTED_RECORD);
+ *out_alert = SSL_AD_UNEXPECTED_MESSAGE;
+ return ssl_open_record_error;
+ }
+ // TODO(crbug.com/42290594): Implement proper support for ACKs. Currently,
+ // this just drops the ACK on the floor.
+ return ssl_open_record_discard;
+}
+
ssl_open_record_t dtls1_open_app_data(SSL *ssl, Span<uint8_t> *out,
size_t *out_consumed, uint8_t *out_alert,
Span<uint8_t> in) {
@@ -180,6 +196,10 @@
// renegotiation attempt. Fall through to the error path.
}
+ if (type == SSL3_RT_ACK) {
+ return dtls1_process_ack(ssl, out_alert);
+ }
+
if (type != SSL3_RT_APPLICATION_DATA) {
OPENSSL_PUT_ERROR(SSL, SSL_R_UNEXPECTED_RECORD);
*out_alert = SSL_AD_UNEXPECTED_MESSAGE;
diff --git a/ssl/internal.h b/ssl/internal.h
index 65ed013..c4a36ac 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -3688,6 +3688,7 @@
// on success and false on allocation failure.
bool ssl_hash_message(SSL_HANDSHAKE *hs, const SSLMessage &msg);
+ssl_open_record_t dtls1_process_ack(SSL *ssl, uint8_t *out_alert);
ssl_open_record_t dtls1_open_app_data(SSL *ssl, Span<uint8_t> *out,
size_t *out_consumed, uint8_t *out_alert,
Span<uint8_t> in);
diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go
index 2074335..d308db9 100644
--- a/ssl/test/runner/common.go
+++ b/ssl/test/runner/common.go
@@ -70,6 +70,7 @@
recordTypeHandshake recordType = 22
recordTypeApplicationData recordType = 23
recordTypePlaintextHandshake recordType = 24
+ recordTypeACK recordType = 26
)
// TLS handshake message types.
@@ -2011,6 +2012,10 @@
// the DTLS 1.3 record header.
DTLS13RecordHeaderSetCIDBit bool
+ // ACKEveryRecord sends an ACK record immediately on response to each
+ // handshake record received.
+ ACKEveryRecord bool
+
// EncryptSessionTicketKey, if non-nil, is the ticket key to use when
// encrypting tickets.
EncryptSessionTicketKey *[32]byte
diff --git a/ssl/test/runner/dtls.go b/ssl/test/runner/dtls.go
index 63bc864..e0b8604 100644
--- a/ssl/test/runner/dtls.go
+++ b/ssl/test/runner/dtls.go
@@ -22,6 +22,8 @@
"math/rand"
"net"
"slices"
+
+ "golang.org/x/crypto/cryptobyte"
)
func (c *Conn) readDTLS13RecordHeader(b []byte) (headerLen int, recordLen int, recTyp recordType, seq []byte, err error) {
@@ -147,6 +149,18 @@
return recordHeaderLen, recordLen, typ, b[3:11], nil
}
+func (c *Conn) writeACKs(seqnums []uint64) {
+ recordNumbers := new(cryptobyte.Builder)
+ epoch := binary.BigEndian.Uint16(c.in.seq[:2])
+ recordNumbers.AddUint16LengthPrefixed(func(b *cryptobyte.Builder) {
+ for _, seq := range seqnums {
+ b.AddUint64(uint64(epoch))
+ b.AddUint64(seq)
+ }
+ })
+ c.writeRecord(recordTypeACK, recordNumbers.BytesOrPanic())
+}
+
func (c *Conn) dtlsDoReadRecord(want recordType) (recordType, []byte, error) {
// Read a new packet only if the current one is empty.
var newPacket bool
@@ -184,6 +198,9 @@
// test.
return 0, nil, c.in.setErrorLocked(c.sendAlert(alertValue))
}
+ if c.config.Bugs.ACKEveryRecord {
+ c.writeACKs([]uint64{binary.BigEndian.Uint64(seq)})
+ }
if typ == 0 {
// readDTLSRecordHeader sets typ=0 when decoding the DTLS 1.3
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index ecfc61e..c0d4276 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -11515,6 +11515,30 @@
},
resumeSession: true,
})
+
+ // DTLS 1.3 ACK/retransmit tests
+ testCases = append(testCases, testCase{
+ protocol: dtls,
+ name: "DTLS13-ImmediateACKs",
+ config: Config{
+ MinVersion: VersionTLS13,
+ Bugs: ProtocolBugs{
+ ACKEveryRecord: true,
+ },
+ },
+ })
+ testCases = append(testCases, testCase{
+ protocol: dtls,
+ name: "DTLS12-RejectACKs",
+ config: Config{
+ MaxVersion: VersionTLS12,
+ Bugs: ProtocolBugs{
+ ACKEveryRecord: true,
+ },
+ },
+ shouldFail: true,
+ expectedError: ":UNEXPECTED_RECORD:",
+ })
}
func addExportKeyingMaterialTests() {
