From 1b40d99d6a90d0039e9021adde5ad4de743cf0ad Mon Sep 17 00:00:00 2001 From: Nick Harper Date: Tue, 18 Jun 2024 18:57:40 +0000 Subject: [PATCH] Add runner test support for DTLS 1.25 Change-Id: Ic4095814ee814b4274b1e90b8a937a21244b4c55 Bug: 715 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/69427 Reviewed-by: Bob Beck Commit-Queue: Bob Beck --- ssl/test/bssl_shim.cc | 3 +- ssl/test/runner/common.go | 19 ++-- ssl/test/runner/conn.go | 57 ++++++++---- ssl/test/runner/dtls.go | 63 ++++++++++---- ssl/test/runner/handshake_messages.go | 8 +- ssl/test/runner/handshake_server.go | 5 +- ssl/test/runner/runner.go | 121 ++++++++++++++++---------- ssl/test/test_config.cc | 4 + 8 files changed, 189 insertions(+), 91 deletions(-) diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc index 72793f27e..2c70c55b2 100644 --- a/ssl/test/bssl_shim.cc +++ b/ssl/test/bssl_shim.cc @@ -1264,7 +1264,8 @@ static bool DoExchange(bssl::UniquePtr *out_session, return false; } - if (GetProtocolVersion(ssl) >= TLS1_3_VERSION && !config->is_server) { + if (GetProtocolVersion(ssl) >= TLS1_3_VERSION && !SSL_is_dtls(ssl) && + !config->is_server) { bool expect_new_session = !config->expect_no_session && !config->shim_shuts_down; if (expect_new_session != test_state->got_new_session) { diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go index 6bdd664dc..ab36e563b 100644 --- a/ssl/test/runner/common.go +++ b/ssl/test/runner/common.go @@ -31,8 +31,9 @@ const ( ) const ( - VersionDTLS10 = 0xfeff - VersionDTLS12 = 0xfefd + VersionDTLS10 = 0xfeff + VersionDTLS12 = 0xfefd + VersionDTLS125Experimental = 0xfc25 ) var allTLSWireVersions = []uint16{ @@ -44,14 +45,16 @@ var allTLSWireVersions = []uint16{ } var allDTLSWireVersions = []uint16{ + VersionDTLS125Experimental, VersionDTLS12, VersionDTLS10, } const ( - maxPlaintext = 16384 // maximum plaintext payload length - maxCiphertext = 16384 + 2048 // maximum ciphertext payload length - tlsRecordHeaderLen = 5 // record header length + maxPlaintext = 16384 // maximum plaintext payload length + maxCiphertext = 16384 + 2048 // maximum ciphertext payload length + tlsRecordHeaderLen = 5 // record header length + // TODO(nharper): check whether this value needs to be changed for DTLS 1.3 dtlsRecordHeaderLen = 13 maxHandshake = 65536 // maximum handshake we support (protocol max is 16 MB) @@ -2035,10 +2038,6 @@ func (c *Config) maxVersion(isDTLS bool) uint16 { ret = c.MaxVersion } if isDTLS { - // We only implement up to DTLS 1.2. - if ret > VersionTLS12 { - return VersionTLS12 - } // There is no such thing as DTLS 1.1. if ret == VersionTLS11 { return VersionTLS10 @@ -2084,6 +2083,8 @@ func (c *Config) echCipherSuitePreferences() []HPKECipherSuite { func wireToVersion(vers uint16, isDTLS bool) (uint16, bool) { if isDTLS { switch vers { + case VersionDTLS125Experimental: + return VersionTLS13, true case VersionDTLS12: return VersionTLS12, true case VersionDTLS10: diff --git a/ssl/test/runner/conn.go b/ssl/test/runner/conn.go index ca1a037ac..c94f112bf 100644 --- a/ssl/test/runner/conn.go +++ b/ssl/test/runner/conn.go @@ -242,7 +242,7 @@ func (hc *halfConn) changeCipherSpec(config *Config) error { } // useTrafficSecret sets the current cipher state for TLS 1.3. -func (hc *halfConn) useTrafficSecret(version uint16, suite *cipherSuite, secret []byte, side trafficDirection) { +func (hc *halfConn) useTrafficSecret(version uint16, suite *cipherSuite, secret []byte, side trafficDirection, level encryptionLevel) { hc.wireVersion = version protocolVersion, ok := wireToVersion(version, hc.isDTLS) if !ok { @@ -254,7 +254,11 @@ func (hc *halfConn) useTrafficSecret(version uint16, suite *cipherSuite, secret hc.cipher = nullCipher{} } hc.trafficSecret = secret - hc.incEpoch() + if hc.isDTLS && protocolVersion == VersionTLS13 { + hc.setEpoch(uint16(level)) + } else { + hc.incEpoch() + } } // resetCipher changes the cipher state back to no encryption to be able @@ -326,6 +330,19 @@ func (hc *halfConn) incEpoch() { hc.updateOutSeq() } +func (hc *halfConn) setEpoch(epoch uint16) { + if !hc.isDTLS { + panic("Internal error: called setEpoch on non-DTLS connection") + } + hc.seq[0] = byte(epoch >> 8) + hc.seq[1] = byte(epoch) + copy(hc.seq[2:], hc.nextSeq[:]) + for i := range hc.nextSeq { + hc.nextSeq[i] = 0 + } + hc.updateOutSeq() +} + func (hc *halfConn) updateOutSeq() { if hc.config.Bugs.SequenceNumberMapping != nil { seqU64 := binary.BigEndian.Uint64(hc.seq[:]) @@ -486,7 +503,7 @@ func (hc *halfConn) decrypt(b *block) (ok bool, prefixLen int, contentType recor panic("unknown cipher type") } - if hc.version >= VersionTLS13 { + if hc.version >= VersionTLS13 && !hc.isDTLS { i := len(payload) for i > 0 && payload[i-1] == 0 { i-- @@ -734,7 +751,7 @@ func (c *Conn) useInTrafficSecret(level encryptionLevel, version uint16, suite * c.config.Bugs.MockQUICTransport.readSecret = secret c.config.Bugs.MockQUICTransport.readCipherSuite = suite.id } - c.in.useTrafficSecret(version, suite, secret, side) + c.in.useTrafficSecret(version, suite, secret, side, level) c.seenHandshakePackEnd = false return nil } @@ -749,7 +766,7 @@ func (c *Conn) useOutTrafficSecret(level encryptionLevel, version uint16, suite c.config.Bugs.MockQUICTransport.writeSecret = secret c.config.Bugs.MockQUICTransport.writeCipherSuite = suite.id } - c.out.useTrafficSecret(version, suite, secret, side) + c.out.useTrafficSecret(version, suite, secret, side, level) } func (c *Conn) setSkipEarlyData() { @@ -896,6 +913,10 @@ func (c *Conn) readTLS13ChangeCipherSpec() error { if c.config.Bugs.MockQUICTransport != nil { return nil } + if c.isDTLS { + // ChangeCipherSpec in DTLS 1.3 is handled within dtlsDoReadRecord. + return nil + } if !c.expectTLS13ChangeCipherSpec { panic("c.expectTLS13ChangeCipherSpec not set") } @@ -1173,17 +1194,21 @@ func (c *Conn) addTLS13Padding(b *block, recordHeaderLen, recordLen int, typ rec if c.out.version < VersionTLS13 || c.out.cipher == nil { return recordLen } - paddingLen := c.config.Bugs.RecordPadding - if c.config.Bugs.OmitRecordContents { - recordLen = paddingLen - b.resize(recordHeaderLen + paddingLen) - } else { - recordLen += 1 + paddingLen - b.resize(len(b.data) + 1 + paddingLen) - b.data[len(b.data)-paddingLen-1] = byte(typ) - } - for i := 0; i < paddingLen; i++ { - b.data[len(b.data)-paddingLen+i] = 0 + // TODO(nharper): DTLS 1.3 should be adding padding, but the currently + // implemented DTLS 1.25 doesn't include padding. + if !c.isDTLS { + paddingLen := c.config.Bugs.RecordPadding + if c.config.Bugs.OmitRecordContents { + recordLen = paddingLen + b.resize(recordHeaderLen + paddingLen) + } else { + recordLen += 1 + paddingLen + b.resize(len(b.data) + 1 + paddingLen) + b.data[len(b.data)-paddingLen-1] = byte(typ) + } + for i := 0; i < paddingLen; i++ { + b.data[len(b.data)-paddingLen+i] = 0 + } } if c, ok := c.out.cipher.(*tlsAead); ok { recordLen += c.Overhead() diff --git a/ssl/test/runner/dtls.go b/ssl/test/runner/dtls.go index b97f82946..b55f3472a 100644 --- a/ssl/test/runner/dtls.go +++ b/ssl/test/runner/dtls.go @@ -62,7 +62,11 @@ func (c *Conn) dtlsDoReadRecord(want recordType) (recordType, *block, error) { // version is irrelevant.) if typ != recordTypeAlert { if c.haveVers { - if vers != c.wireVersion { + wireVersion := c.wireVersion + if c.vers >= VersionTLS13 { + wireVersion = VersionDTLS12 + } + if vers != wireVersion { c.sendAlert(alertProtocolVersion) return 0, nil, c.in.setErrorLocked(fmt.Errorf("dtls: received record with version %x when expecting version %x", vers, c.wireVersion)) } @@ -76,17 +80,25 @@ func (c *Conn) dtlsDoReadRecord(want recordType) (recordType, *block, error) { } epoch := b.data[3:5] seq := b.data[5:11] - // For test purposes, require the sequence number be monotonically - // increasing, so c.in includes the minimum next sequence number. Gaps - // may occur if packets failed to be sent out. A real implementation - // would maintain a replay window and such. - if !bytes.Equal(epoch, c.in.seq[:2]) { - c.sendAlert(alertIllegalParameter) - return 0, nil, c.in.setErrorLocked(fmt.Errorf("dtls: bad epoch")) - } - if bytes.Compare(seq, c.in.seq[2:]) < 0 { - c.sendAlert(alertIllegalParameter) - return 0, nil, c.in.setErrorLocked(fmt.Errorf("dtls: bad sequence number")) + if c.expectTLS13ChangeCipherSpec && typ == recordTypeChangeCipherSpec { + // CCS should only be at epoch 0 + if !bytes.Equal(epoch, []byte{0, 0}) { + c.sendAlert(alertIllegalParameter) + return 0, nil, c.in.setErrorLocked(fmt.Errorf("dtls: bad epoch, expected CCS at epoch 0")) + } + } else { + // For test purposes, require the sequence number be monotonically + // increasing, so c.in includes the minimum next sequence number. Gaps + // may occur if packets failed to be sent out. A real implementation + // would maintain a replay window and such. + if !bytes.Equal(epoch, c.in.seq[:2]) { + c.sendAlert(alertIllegalParameter) + return 0, nil, c.in.setErrorLocked(fmt.Errorf("dtls: bad epoch, want %x, got %x", c.in.seq[:2], epoch)) + } + if bytes.Compare(seq, c.in.seq[2:]) < 0 { + c.sendAlert(alertIllegalParameter) + return 0, nil, c.in.setErrorLocked(fmt.Errorf("dtls: bad sequence number")) + } } copy(c.in.seq[2:], seq) n := int(b.data[11])<<8 | int(b.data[12]) @@ -94,9 +106,24 @@ func (c *Conn) dtlsDoReadRecord(want recordType) (recordType, *block, error) { c.sendAlert(alertRecordOverflow) return 0, nil, c.in.setErrorLocked(fmt.Errorf("dtls: oversized record received with length %d", n)) } + b, c.rawInput = c.in.splitBlock(b, recordHeaderLen+n) + + // Process CCS + if c.expectTLS13ChangeCipherSpec { + c.expectTLS13ChangeCipherSpec = false + ccsData := b.data[recordHeaderLen:] + if typ == recordTypeChangeCipherSpec { + if !bytes.Equal(ccsData, []byte{1}) { + return 0, nil, c.in.setErrorLocked(fmt.Errorf("dtls: ChangeCipherSpec of length %d did not contain single byte value 0x01", len(ccsData))) + } + return c.dtlsDoReadRecord(want) + } + if typ != recordTypeAlert { + return 0, nil, c.in.setErrorLocked(fmt.Errorf("dtls: Expected ChangeCipherSpec but got record type %d", typ)) + } + } // Process message. - b, c.rawInput = c.in.splitBlock(b, recordHeaderLen+n) ok, off, _, alertValue := c.in.decrypt(b) if !ok { // A real DTLS implementation would silently ignore bad records, @@ -165,7 +192,7 @@ func (c *Conn) dtlsWriteRecord(typ recordType, data []byte) (n int, err error) { } } - if typ == recordTypeChangeCipherSpec { + if typ == recordTypeChangeCipherSpec && c.vers < VersionTLS13 { err = c.out.changeCipherSpec(c.config) if err != nil { return n, c.sendAlertLocked(alertLevelError, err.(alert)) @@ -371,12 +398,13 @@ func (c *Conn) dtlsPackRecord(typ recordType, data []byte, mustPack bool) (n int vers = VersionTLS10 } } + if c.vers >= VersionTLS13 || c.out.version >= VersionTLS13 { + vers = VersionDTLS12 + } b.data[1] = byte(vers >> 8) b.data[2] = byte(vers) // DTLS records include an explicit sequence number. copy(b.data[3:11], c.out.outSeq[0:]) - b.data[11] = byte(len(data) >> 8) - b.data[12] = byte(len(data)) if explicitIVLen > 0 { explicitIV := b.data[recordHeaderLen : recordHeaderLen+explicitIVLen] if explicitIVIsSeq { @@ -388,6 +416,9 @@ func (c *Conn) dtlsPackRecord(typ recordType, data []byte, mustPack bool) (n int } } copy(b.data[recordHeaderLen+explicitIVLen:], data) + recordLen := c.addTLS13Padding(b, recordHeaderLen, len(data), typ) + b.data[11] = byte(recordLen >> 8) + b.data[12] = byte(recordLen) c.out.encrypt(b, explicitIVLen, typ) // Flush the current pending packet if necessary. diff --git a/ssl/test/runner/handshake_messages.go b/ssl/test/runner/handshake_messages.go index 6d2f902f1..38dbc6d76 100644 --- a/ssl/test/runner/handshake_messages.go +++ b/ssl/test/runner/handshake_messages.go @@ -1219,7 +1219,11 @@ func (m *serverHelloMsg) marshal() []byte { if m.versOverride != 0 { hello.AddUint16(m.versOverride) } else if vers >= VersionTLS13 { - hello.AddUint16(VersionTLS12) + legacyVersion := uint16(VersionTLS12) + if m.isDTLS { + legacyVersion = VersionDTLS12 + } + hello.AddUint16(legacyVersion) } else { hello.AddUint16(m.vers) } @@ -1316,7 +1320,7 @@ func (m *serverHelloMsg) unmarshal(data []byte) bool { } // Parse out the version from supported_versions if available. - if m.vers == VersionTLS12 { + if vers == VersionTLS12 { extensionsCopy := extensions for len(extensionsCopy) > 0 { var extension uint16 diff --git a/ssl/test/runner/handshake_server.go b/ssl/test/runner/handshake_server.go index 2e87d54fb..deb9ca52d 100644 --- a/ssl/test/runner/handshake_server.go +++ b/ssl/test/runner/handshake_server.go @@ -1040,7 +1040,7 @@ ResendHelloRetryRequest: } c.flushHandshake() - if !c.config.Bugs.SkipChangeCipherSpec && !sendHelloRetryRequest { + if !c.config.Bugs.SkipChangeCipherSpec && !sendHelloRetryRequest && !c.isDTLS { c.writeRecord(recordTypeChangeCipherSpec, []byte{1}) } @@ -1420,7 +1420,8 @@ ResendHelloRetryRequest: // TODO(davidben): Allow configuring the number of tickets sent for // testing. - if !c.config.SessionTicketsDisabled && foundKEMode { + // TODO(nharper): Add support for post-handshake messages in DTLS 1.3. + if !c.config.SessionTicketsDisabled && foundKEMode && !c.isDTLS { ticketCount := 2 for i := 0; i < ticketCount; i++ { c.SendNewSessionTicket([]byte{byte(i)}) diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go index 97eac0277..2cd15494f 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go @@ -1835,6 +1835,8 @@ var tlsVersions = []tlsVersion{ version: VersionTLS13, excludeFlag: "-no-tls13", hasQUIC: true, + hasDTLS: true, + versionDTLS: VersionDTLS125Experimental, versionWire: VersionTLS13, }, } @@ -2353,7 +2355,7 @@ read alert 1 0 { protocol: dtls, name: "DisableEverything-DTLS", - flags: []string{"-no-tls12", "-no-tls1"}, + flags: []string{"-no-tls13", "-no-tls12", "-no-tls1"}, shouldFail: true, expectedError: ":NO_SUPPORTED_VERSIONS_ENABLED:", }, @@ -3219,10 +3221,13 @@ read alert 1 0 }, resumeSession: true, }, + // TODO(crbug.com/boringssl/715): This test and the next shouldn't be + // restricted to a max version of TLS 1.2, but they're broken in DTLS 1.3. { protocol: dtls, name: "DTLS-SendExtraFinished", config: Config{ + MaxVersion: VersionTLS12, Bugs: ProtocolBugs{ SendExtraFinished: true, }, @@ -3234,6 +3239,7 @@ read alert 1 0 protocol: dtls, name: "DTLS-SendExtraFinished-Reordered", config: Config{ + MaxVersion: VersionTLS12, Bugs: ProtocolBugs{ MaxHandshakeRecordLength: 2, ReorderHandshakeFragments: true, @@ -3767,51 +3773,53 @@ func addTestForCipherSuite(suite testCipherSuite, ver tlsVersion, protocol proto // Verify exporters interoperate. exportKeyingMaterial := 1024 - testCases = append(testCases, testCase{ - testType: serverTest, - protocol: protocol, - name: prefix + ver.name + "-" + suite.name + "-server", - config: Config{ - MinVersion: ver.version, - MaxVersion: ver.version, - CipherSuites: []uint16{suite.id}, - Credential: cert, - PreSharedKey: []byte(psk), - PreSharedKeyIdentity: pskIdentity, - Bugs: ProtocolBugs{ - AdvertiseAllConfiguredCiphers: true, + if ver.version != VersionTLS13 || !ver.hasDTLS { + testCases = append(testCases, testCase{ + testType: serverTest, + protocol: protocol, + name: prefix + ver.name + "-" + suite.name + "-server", + config: Config{ + MinVersion: ver.version, + MaxVersion: ver.version, + CipherSuites: []uint16{suite.id}, + Credential: cert, + PreSharedKey: []byte(psk), + PreSharedKeyIdentity: pskIdentity, + Bugs: ProtocolBugs{ + AdvertiseAllConfiguredCiphers: true, + }, }, - }, - shimCertificate: cert, - flags: flags, - resumeSession: true, - shouldFail: shouldFail, - expectedError: expectedServerError, - exportKeyingMaterial: exportKeyingMaterial, - }) + shimCertificate: cert, + flags: flags, + resumeSession: true, + shouldFail: shouldFail, + expectedError: expectedServerError, + exportKeyingMaterial: exportKeyingMaterial, + }) - testCases = append(testCases, testCase{ - testType: clientTest, - protocol: protocol, - name: prefix + ver.name + "-" + suite.name + "-client", - config: Config{ - MinVersion: ver.version, - MaxVersion: ver.version, - CipherSuites: serverCipherSuites, - Credential: cert, - PreSharedKey: []byte(psk), - PreSharedKeyIdentity: pskIdentity, - Bugs: ProtocolBugs{ - IgnorePeerCipherPreferences: shouldFail, - SendCipherSuite: sendCipherSuite, + testCases = append(testCases, testCase{ + testType: clientTest, + protocol: protocol, + name: prefix + ver.name + "-" + suite.name + "-client", + config: Config{ + MinVersion: ver.version, + MaxVersion: ver.version, + CipherSuites: serverCipherSuites, + Credential: cert, + PreSharedKey: []byte(psk), + PreSharedKeyIdentity: pskIdentity, + Bugs: ProtocolBugs{ + IgnorePeerCipherPreferences: shouldFail, + SendCipherSuite: sendCipherSuite, + }, }, - }, - flags: flags, - resumeSession: true, - shouldFail: shouldFail, - expectedError: expectedClientError, - exportKeyingMaterial: exportKeyingMaterial, - }) + flags: flags, + resumeSession: true, + shouldFail: shouldFail, + expectedError: expectedClientError, + exportKeyingMaterial: exportKeyingMaterial, + }) + } if shouldFail { return @@ -6579,7 +6587,8 @@ func addVersionNegotiationTests() { name: "VersionTooLow-DTLS", config: Config{ Bugs: ProtocolBugs{ - SendClientVersion: 0xffff, + SendClientVersion: 0xffff, + OmitSupportedVersions: true, }, }, shouldFail: true, @@ -8201,7 +8210,7 @@ func addExtensionTests() { }, flags: []string{ "-max-version", - strconv.Itoa(int(ver.versionWire)), + ver.shimFlag(protocol), "-quic-transport-params", base64FlagValue([]byte{3, 4}), "-quic-use-legacy-codepoint", useCodepointFlag, @@ -20661,6 +20670,27 @@ func checkTests() { } } +// TODO(crbug.com/boringssl/715): Once our DTLS 1.3 implementation supports +// resumption, remove this filter. +func filterTests() { + tests := make([]testCase, 0, len(testCases)) + isDTLS13ResumptionTest := func(test testCase) bool { + if !test.resumeSession { + return false + } + if test.protocol != dtls { + return false + } + return test.config.MaxVersion == VersionTLS13 || test.config.MinVersion == VersionTLS13 || test.expectations.version == VersionTLS13 + } + for _, test := range testCases { + if !isDTLS13ResumptionTest(test) { + tests = append(tests, test) + } + } + testCases = tests +} + func main() { flag.Parse() var err error @@ -20746,6 +20776,7 @@ func main() { testCases = append(testCases, toAppend...) checkTests() + filterTests() dispatcher, err := newShimDispatcher() if err != nil { diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc index 9ecbea478..ae4f87b41 100644 --- a/ssl/test/test_config.cc +++ b/ssl/test/test_config.cc @@ -2205,6 +2205,10 @@ bssl::UniquePtr TestConfig::NewSSL( !SSL_set_min_proto_version(ssl.get(), min_version)) { return nullptr; } + if (is_dtls && max_version == 0 && + !SSL_set_max_proto_version(ssl.get(), DTLS1_3_EXPERIMENTAL_VERSION)) { + return nullptr; + } if (max_version != 0 && !SSL_set_max_proto_version(ssl.get(), max_version)) { return nullptr;