From 9498e74a92ce6289af2a9dddb14b75445de87d9c Mon Sep 17 00:00:00 2001 From: Adam Langley Date: Mon, 18 Jul 2016 10:17:16 -0700 Subject: [PATCH] Don't have the default value of |verify_result| be X509_V_OK. It seems much safer for the default value of |verify_result| to be an error value. Change-Id: I372ec19c41d77516ed12d0169969994f7d23ed70 Reviewed-on: https://boringssl-review.googlesource.com/9063 Reviewed-by: Adam Langley Commit-Queue: Adam Langley CQ-Verified: CQ bot account: commit-bot@chromium.org --- ssl/handshake_client.c | 11 +++++------ ssl/ssl_lib.c | 2 +- ssl/ssl_session.c | 4 ++-- ssl/test/runner/runner.go | 2 ++ 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/ssl/handshake_client.c b/ssl/handshake_client.c index d78bc270..d4344874 100644 --- a/ssl/handshake_client.c +++ b/ssl/handshake_client.c @@ -1041,8 +1041,6 @@ static int ssl3_get_server_certificate(SSL *ssl) { X509_free(ssl->s3->new_session->peer); ssl->s3->new_session->peer = X509_up_ref(leaf); - ssl->s3->new_session->verify_result = ssl->verify_result; - return 1; err: @@ -1097,12 +1095,13 @@ static int ssl3_verify_server_cert(SSL *ssl) { int al = ssl_verify_alarm_type(ssl->verify_result); ssl3_send_alert(ssl, SSL3_AL_FATAL, al); OPENSSL_PUT_ERROR(SSL, SSL_R_CERTIFICATE_VERIFY_FAILED); - } else { - ret = 1; - ERR_clear_error(); /* but we keep ssl->verify_result */ + return ret; } - return ret; + /* Otherwise the error is non-fatal, but we keep verify_result. */ + ERR_clear_error(); + ssl->s3->new_session->verify_result = ssl->verify_result; + return 1; } static int ssl3_get_server_key_exchange(SSL *ssl) { diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index f06c0ded..21699c4f 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -433,7 +433,7 @@ SSL *SSL_new(SSL_CTX *ctx) { ssl->alpn_client_proto_list_len = ssl->ctx->alpn_client_proto_list_len; } - ssl->verify_result = X509_V_OK; + ssl->verify_result = X509_V_ERR_INVALID_CALL; ssl->method = ctx->method; if (!ssl->method->ssl_new(ssl)) { diff --git a/ssl/ssl_session.c b/ssl/ssl_session.c index 0a9ccba5..006e6350 100644 --- a/ssl/ssl_session.c +++ b/ssl/ssl_session.c @@ -167,7 +167,7 @@ SSL_SESSION *SSL_SESSION_new(void) { } memset(session, 0, sizeof(SSL_SESSION)); - session->verify_result = 1; /* avoid 0 (= X509_V_OK) just in case */ + session->verify_result = X509_V_ERR_INVALID_CALL; session->references = 1; session->timeout = SSL_DEFAULT_SESSION_TIMEOUT; session->time = (unsigned long)time(NULL); @@ -457,7 +457,7 @@ int ssl_get_new_session(SSL *ssl, int is_server) { /* The session is marked not resumable until it is completely filled in. */ session->not_resumable = 1; - session->verify_result = X509_V_OK; + session->verify_result = X509_V_ERR_INVALID_CALL; SSL_SESSION_free(ssl->s3->new_session); ssl->s3->new_session = session; diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go index 23e9cb4e..b45e42ae 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go @@ -3400,6 +3400,7 @@ func addStateMachineCoverageTests(config stateMachineTestConfig) { flags: []string{ "-verify-peer", }, + resumeSession: vers.version != VersionTLS13, }) tests = append(tests, testCase{ testType: clientTest, @@ -3424,6 +3425,7 @@ func addStateMachineCoverageTests(config stateMachineTestConfig) { "-verify-fail", "-expect-verify-result", }, + resumeSession: vers.version != VersionTLS13, }) }