From 911cc0a0aa66b8d51defd4e9875075f895a07649 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Tue, 15 May 2018 14:22:17 -0400 Subject: [PATCH] The legacy client OCSP callback should run without server OCSP. It's conditioned in OpenSSL on client offer, not server accept. Change-Id: Iae5483a33d9365258446ce0ae34132aeb4a92c66 Reviewed-on: https://boringssl-review.googlesource.com/28545 Reviewed-by: Adam Langley --- ssl/handshake.cc | 2 +- ssl/test/runner/runner.go | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/ssl/handshake.cc b/ssl/handshake.cc index 9cad9710..bd304eb3 100644 --- a/ssl/handshake.cc +++ b/ssl/handshake.cc @@ -367,7 +367,7 @@ enum ssl_verify_result_t ssl_verify_peer_cert(SSL_HANDSHAKE *hs) { // Emulate OpenSSL's client OCSP callback. OpenSSL verifies certificates // before it receives the OCSP, so it needs a second callback for OCSP. if (ret == ssl_verify_ok && !ssl->server && - hs->new_session->ocsp_response != nullptr && + hs->config->ocsp_stapling_enabled && ssl->ctx->legacy_ocsp_callback != nullptr) { int cb_ret = ssl->ctx->legacy_ocsp_callback(ssl, ssl->ctx->legacy_ocsp_callback_arg); diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go index 7e9a6efb..3d8a6b0e 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go @@ -4841,8 +4841,8 @@ func addStateMachineCoverageTests(config stateMachineTestConfig) { expectedLocalError: expectedLocalError, expectedError: ":OCSP_CB_ERROR:", }) - // The callback does not run if the server does not send an - // OCSP response. + // The callback still runs if the server does not send an OCSP + // response. certNoStaple := rsaCertificate certNoStaple.OCSPStaple = nil tests = append(tests, testCase{ @@ -4858,6 +4858,9 @@ func addStateMachineCoverageTests(config stateMachineTestConfig) { "-use-ocsp-callback", "-fail-ocsp-callback", }, + shouldFail: true, + expectedLocalError: expectedLocalError, + expectedError: ":OCSP_CB_ERROR:", }) // The server OCSP callback is a legacy mechanism for