Have |SSL_get_verify_result| return |X509_V_OK| when no client certificate is given.
9498e74 changed the default value of verify_result to an error. This
tripped up NGINX, which depends on a bug[1] in OpenSSL. netty-tcnative
also uses this behavior, though it currently isn't tripped up by 9498e74
because it calls |SSL_set_verify_result|. However, we would like to
remove |SSL_set_verify_result| and with two data points, it seems this
is behavior we must preserve.
This change sets |verify_result| to |X509_V_OK| when a) no client
certificate is requested or b) none is given and it's optional.
[1] See BUGS in https://www.openssl.org/docs/manmaster/ssl/SSL_get_verify_result.html
Change-Id: Ibd33660ae409bfe272963a8c39b7e9aa83c3d635
Reviewed-on: https://boringssl-review.googlesource.com/9067
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 4c7d2e7..8221286 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -2730,91 +2730,84 @@
},
})
}
+
+ testCases = append(testCases, testCase{
+ name: "NoClientCertificate-" + ver.name,
+ config: Config{
+ MinVersion: ver.version,
+ MaxVersion: ver.version,
+ ClientAuth: RequireAnyClientCert,
+ },
+ shouldFail: true,
+ expectedLocalError: "client didn't provide a certificate",
+ })
+
+ testCases = append(testCases, testCase{
+ // Even if not configured to expect a certificate, OpenSSL will
+ // return X509_V_OK as the verify_result.
+ testType: serverTest,
+ name: "NoClientCertificateRequested-Server-" + ver.name,
+ config: Config{
+ MinVersion: ver.version,
+ MaxVersion: ver.version,
+ },
+ flags: []string{
+ "-expect-verify-result",
+ },
+ // TODO(davidben): Switch this to true when TLS 1.3
+ // supports session resumption.
+ resumeSession: ver.version < VersionTLS13,
+ })
+
+ testCases = append(testCases, testCase{
+ // If a client certificate is not provided, OpenSSL will still
+ // return X509_V_OK as the verify_result.
+ testType: serverTest,
+ name: "NoClientCertificate-Server-" + ver.name,
+ config: Config{
+ MinVersion: ver.version,
+ MaxVersion: ver.version,
+ },
+ flags: []string{
+ "-expect-verify-result",
+ "-verify-peer",
+ },
+ // TODO(davidben): Switch this to true when TLS 1.3
+ // supports session resumption.
+ resumeSession: ver.version < VersionTLS13,
+ })
+
+ testCases = append(testCases, testCase{
+ testType: serverTest,
+ name: "RequireAnyClientCertificate-" + ver.name,
+ config: Config{
+ MinVersion: ver.version,
+ MaxVersion: ver.version,
+ },
+ flags: []string{"-require-any-client-certificate"},
+ shouldFail: true,
+ expectedError: ":PEER_DID_NOT_RETURN_A_CERTIFICATE:",
+ })
+
+ if ver.version != VersionSSL30 {
+ testCases = append(testCases, testCase{
+ testType: serverTest,
+ name: "SkipClientCertificate-" + ver.name,
+ config: Config{
+ MinVersion: ver.version,
+ MaxVersion: ver.version,
+ Bugs: ProtocolBugs{
+ SkipClientCertificate: true,
+ },
+ },
+ // Setting SSL_VERIFY_PEER allows anonymous clients.
+ flags: []string{"-verify-peer"},
+ shouldFail: true,
+ expectedError: ":UNEXPECTED_MESSAGE:",
+ })
+ }
}
- testCases = append(testCases, testCase{
- name: "NoClientCertificate",
- config: Config{
- MaxVersion: VersionTLS12,
- ClientAuth: RequireAnyClientCert,
- },
- shouldFail: true,
- expectedLocalError: "client didn't provide a certificate",
- })
-
- testCases = append(testCases, testCase{
- name: "NoClientCertificate-TLS13",
- config: Config{
- MaxVersion: VersionTLS13,
- ClientAuth: RequireAnyClientCert,
- },
- shouldFail: true,
- expectedLocalError: "client didn't provide a certificate",
- })
-
- testCases = append(testCases, testCase{
- testType: serverTest,
- name: "RequireAnyClientCertificate",
- config: Config{
- MaxVersion: VersionTLS12,
- },
- flags: []string{"-require-any-client-certificate"},
- shouldFail: true,
- expectedError: ":PEER_DID_NOT_RETURN_A_CERTIFICATE:",
- })
-
- testCases = append(testCases, testCase{
- testType: serverTest,
- name: "RequireAnyClientCertificate-TLS13",
- config: Config{
- MaxVersion: VersionTLS13,
- },
- flags: []string{"-require-any-client-certificate"},
- shouldFail: true,
- expectedError: ":PEER_DID_NOT_RETURN_A_CERTIFICATE:",
- })
-
- testCases = append(testCases, testCase{
- testType: serverTest,
- name: "RequireAnyClientCertificate-SSL3",
- config: Config{
- MaxVersion: VersionSSL30,
- },
- flags: []string{"-require-any-client-certificate"},
- shouldFail: true,
- expectedError: ":PEER_DID_NOT_RETURN_A_CERTIFICATE:",
- })
-
- testCases = append(testCases, testCase{
- testType: serverTest,
- name: "SkipClientCertificate",
- config: Config{
- MaxVersion: VersionTLS12,
- Bugs: ProtocolBugs{
- SkipClientCertificate: true,
- },
- },
- // Setting SSL_VERIFY_PEER allows anonymous clients.
- flags: []string{"-verify-peer"},
- shouldFail: true,
- expectedError: ":UNEXPECTED_MESSAGE:",
- })
-
- testCases = append(testCases, testCase{
- testType: serverTest,
- name: "SkipClientCertificate-TLS13",
- config: Config{
- MaxVersion: VersionTLS13,
- Bugs: ProtocolBugs{
- SkipClientCertificate: true,
- },
- },
- // Setting SSL_VERIFY_PEER allows anonymous clients.
- flags: []string{"-verify-peer"},
- shouldFail: true,
- expectedError: ":UNEXPECTED_MESSAGE:",
- })
-
// Client auth is only legal in certificate-based ciphers.
testCases = append(testCases, testCase{
testType: clientTest,