Fix DTLS_ANY_VERSION and add tests.
This fixes bugs that kept the tests from working:
- Resolve DTLS version and cookie before the session.
- In DTLS_ANY_VERSION, ServerHello should be read with first_packet = 1. This
is a regression from f2fedefdcaf62f10b566f55858c25f35112072ea. We'll want to
do the same for TLS, but first let's change this to a boolean has_version in a
follow-up.
Things not yet fixed:
- DTLS code is not EVP_AEAD-aware. Those ciphers are disabled for now.
- On the client, DTLS_ANY_VERSION creates SSL_SESSIONs with the wrong
ssl_version. The tests pass because we no longer enforce the match as of
e37216f56009fbf48c3a1e733b7a546ca6dfc2af. (In fact, we've gone from the server
ignoring ssl_version and client enforcing to the client mostly ignoring
ssl_version and the server enforcing.)
- ssl3_send_client_hello's ssl_version check checks for equality against
s->version rather than >.
Change-Id: I5a0dde221b2009413df9b9443882b9bf3b29519c
Reviewed-on: https://boringssl-review.googlesource.com/2403
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc
index a4dd06d..0202407 100644
--- a/ssl/test/bssl_shim.cc
+++ b/ssl/test/bssl_shim.cc
@@ -229,14 +229,10 @@
const SSL_METHOD *method;
if (config->is_dtls) {
- // TODO(davidben): Get DTLS 1.2 working and test the version negotiation
- // codepath. This doesn't currently work because
- // - Session resumption is broken: https://crbug.com/403378
- // - DTLS hasn't been updated for EVP_AEAD.
if (config->is_server) {
- method = DTLSv1_server_method();
+ method = DTLS_server_method();
} else {
- method = DTLSv1_client_method();
+ method = DTLS_client_method();
}
} else {
if (config->is_server) {
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index f106e60..79f8ee0 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -889,11 +889,12 @@
name string
version uint16
flag string
+ hasDTLS bool
}{
- {"SSL3", VersionSSL30, "-no-ssl3"},
- {"TLS1", VersionTLS10, "-no-tls1"},
- {"TLS11", VersionTLS11, "-no-tls11"},
- {"TLS12", VersionTLS12, "-no-tls12"},
+ {"SSL3", VersionSSL30, "-no-ssl3", false},
+ {"TLS1", VersionTLS10, "-no-tls1", true},
+ {"TLS11", VersionTLS11, "-no-tls11", false},
+ {"TLS12", VersionTLS12, "-no-tls12", true},
}
var testCipherSuites = []struct {
@@ -935,10 +936,21 @@
{"RC4-SHA", TLS_RSA_WITH_RC4_128_SHA},
}
+func hasComponent(suiteName, component string) bool {
+ return strings.Contains("-"+suiteName+"-", "-"+component+"-")
+}
+
func isTLS12Only(suiteName string) bool {
- return strings.HasSuffix(suiteName, "-GCM") ||
- strings.HasSuffix(suiteName, "-SHA256") ||
- strings.HasSuffix(suiteName, "-SHA384")
+ return hasComponent(suiteName, "GCM") ||
+ hasComponent(suiteName, "SHA256") ||
+ hasComponent(suiteName, "SHA384")
+}
+
+func isDTLSCipher(suiteName string) bool {
+ // TODO(davidben): AES-GCM exists in DTLS 1.2 but is currently
+ // broken because DTLS is not EVP_AEAD-aware.
+ return !hasComponent(suiteName, "RC4") &&
+ !hasComponent(suiteName, "GCM")
}
func addCipherSuiteTests() {
@@ -949,7 +961,7 @@
var cert Certificate
var certFile string
var keyFile string
- if strings.Contains(suite.name, "ECDSA") {
+ if hasComponent(suite.name, "ECDSA") {
cert = getECDSACertificate()
certFile = ecdsaCertificateFile
keyFile = ecdsaKeyFile
@@ -960,7 +972,7 @@
}
var flags []string
- if strings.HasPrefix(suite.name, "PSK-") || strings.Contains(suite.name, "-PSK-") {
+ if hasComponent(suite.name, "PSK") {
flags = append(flags,
"-psk", psk,
"-psk-identity", pskIdentity)
@@ -1003,8 +1015,7 @@
resumeSession: true,
})
- // TODO(davidben): Fix DTLS 1.2 support and test that.
- if ver.version == VersionTLS10 && strings.Index(suite.name, "RC4") == -1 {
+ if ver.hasDTLS && isDTLSCipher(suite.name) {
testCases = append(testCases, testCase{
testType: clientTest,
protocol: dtls,
@@ -1593,31 +1604,43 @@
}
for _, runnerVers := range tlsVersions {
- expectedVersion := shimVers.version
- if runnerVers.version < shimVers.version {
- expectedVersion = runnerVers.version
+ protocols := []protocol{tls}
+ if runnerVers.hasDTLS && shimVers.hasDTLS {
+ protocols = append(protocols, dtls)
}
- suffix := shimVers.name + "-" + runnerVers.name
+ for _, protocol := range protocols {
+ expectedVersion := shimVers.version
+ if runnerVers.version < shimVers.version {
+ expectedVersion = runnerVers.version
+ }
- testCases = append(testCases, testCase{
- testType: clientTest,
- name: "VersionNegotiation-Client-" + suffix,
- config: Config{
- MaxVersion: runnerVers.version,
- },
- flags: flags,
- expectedVersion: expectedVersion,
- })
+ suffix := shimVers.name + "-" + runnerVers.name
+ if protocol == dtls {
+ suffix += "-DTLS"
+ }
- testCases = append(testCases, testCase{
- testType: serverTest,
- name: "VersionNegotiation-Server-" + suffix,
- config: Config{
- MaxVersion: runnerVers.version,
- },
- flags: flags,
- expectedVersion: expectedVersion,
- })
+ testCases = append(testCases, testCase{
+ protocol: protocol,
+ testType: clientTest,
+ name: "VersionNegotiation-Client-" + suffix,
+ config: Config{
+ MaxVersion: runnerVers.version,
+ },
+ flags: flags,
+ expectedVersion: expectedVersion,
+ })
+
+ testCases = append(testCases, testCase{
+ protocol: protocol,
+ testType: serverTest,
+ name: "VersionNegotiation-Server-" + suffix,
+ config: Config{
+ MaxVersion: runnerVers.version,
+ },
+ flags: flags,
+ expectedVersion: expectedVersion,
+ })
+ }
}
}
}
@@ -1904,69 +1927,80 @@
}
func addResumptionVersionTests() {
- // TODO(davidben): Once DTLS 1.2 is working, test that as well.
for _, sessionVers := range tlsVersions {
for _, resumeVers := range tlsVersions {
- suffix := "-" + sessionVers.name + "-" + resumeVers.name
-
- testCases = append(testCases, testCase{
- name: "Resume-Client" + suffix,
- resumeSession: true,
- config: Config{
- MaxVersion: sessionVers.version,
- CipherSuites: []uint16{TLS_RSA_WITH_RC4_128_SHA},
- Bugs: ProtocolBugs{
- AllowSessionVersionMismatch: true,
- },
- },
- expectedVersion: sessionVers.version,
- resumeConfig: &Config{
- MaxVersion: resumeVers.version,
- CipherSuites: []uint16{TLS_RSA_WITH_RC4_128_SHA},
- Bugs: ProtocolBugs{
- AllowSessionVersionMismatch: true,
- },
- },
- expectedResumeVersion: resumeVers.version,
- })
-
- testCases = append(testCases, testCase{
- name: "Resume-Client-NoResume" + suffix,
- flags: []string{"-expect-session-miss"},
- resumeSession: true,
- config: Config{
- MaxVersion: sessionVers.version,
- CipherSuites: []uint16{TLS_RSA_WITH_RC4_128_SHA},
- },
- expectedVersion: sessionVers.version,
- resumeConfig: &Config{
- MaxVersion: resumeVers.version,
- CipherSuites: []uint16{TLS_RSA_WITH_RC4_128_SHA},
- },
- newSessionsOnResume: true,
- expectedResumeVersion: resumeVers.version,
- })
-
- var flags []string
- if sessionVers.version != resumeVers.version {
- flags = append(flags, "-expect-session-miss")
+ protocols := []protocol{tls}
+ if sessionVers.hasDTLS && resumeVers.hasDTLS {
+ protocols = append(protocols, dtls)
}
- testCases = append(testCases, testCase{
- testType: serverTest,
- name: "Resume-Server" + suffix,
- flags: flags,
- resumeSession: true,
- config: Config{
- MaxVersion: sessionVers.version,
- CipherSuites: []uint16{TLS_RSA_WITH_RC4_128_SHA},
- },
- expectedVersion: sessionVers.version,
- resumeConfig: &Config{
- MaxVersion: resumeVers.version,
- CipherSuites: []uint16{TLS_RSA_WITH_RC4_128_SHA},
- },
- expectedResumeVersion: resumeVers.version,
- })
+ for _, protocol := range protocols {
+ suffix := "-" + sessionVers.name + "-" + resumeVers.name
+ if protocol == dtls {
+ suffix += "-DTLS"
+ }
+
+ testCases = append(testCases, testCase{
+ protocol: protocol,
+ name: "Resume-Client" + suffix,
+ resumeSession: true,
+ config: Config{
+ MaxVersion: sessionVers.version,
+ CipherSuites: []uint16{TLS_RSA_WITH_AES_128_CBC_SHA},
+ Bugs: ProtocolBugs{
+ AllowSessionVersionMismatch: true,
+ },
+ },
+ expectedVersion: sessionVers.version,
+ resumeConfig: &Config{
+ MaxVersion: resumeVers.version,
+ CipherSuites: []uint16{TLS_RSA_WITH_AES_128_CBC_SHA},
+ Bugs: ProtocolBugs{
+ AllowSessionVersionMismatch: true,
+ },
+ },
+ expectedResumeVersion: resumeVers.version,
+ })
+
+ testCases = append(testCases, testCase{
+ protocol: protocol,
+ name: "Resume-Client-NoResume" + suffix,
+ flags: []string{"-expect-session-miss"},
+ resumeSession: true,
+ config: Config{
+ MaxVersion: sessionVers.version,
+ CipherSuites: []uint16{TLS_RSA_WITH_AES_128_CBC_SHA},
+ },
+ expectedVersion: sessionVers.version,
+ resumeConfig: &Config{
+ MaxVersion: resumeVers.version,
+ CipherSuites: []uint16{TLS_RSA_WITH_AES_128_CBC_SHA},
+ },
+ newSessionsOnResume: true,
+ expectedResumeVersion: resumeVers.version,
+ })
+
+ var flags []string
+ if sessionVers.version != resumeVers.version {
+ flags = append(flags, "-expect-session-miss")
+ }
+ testCases = append(testCases, testCase{
+ protocol: protocol,
+ testType: serverTest,
+ name: "Resume-Server" + suffix,
+ flags: flags,
+ resumeSession: true,
+ config: Config{
+ MaxVersion: sessionVers.version,
+ CipherSuites: []uint16{TLS_RSA_WITH_AES_128_CBC_SHA},
+ },
+ expectedVersion: sessionVers.version,
+ resumeConfig: &Config{
+ MaxVersion: resumeVers.version,
+ CipherSuites: []uint16{TLS_RSA_WITH_AES_128_CBC_SHA},
+ },
+ expectedResumeVersion: resumeVers.version,
+ })
+ }
}
}
}
