The branch master has been updated via 767ccc3b77cde82c46ab4af541663f6c80e538d3 (commit) via f046afb0663fc4514f7fc5d1724439caa6858932 (commit) from ee4cdb7fdbdc46f931cb6e2eca109cc92832eb33 (commit)
- Log ----------------------------------------------------------------- commit 767ccc3b77cde82c46ab4af541663f6c80e538d3 Author: Matt Caswell <m...@openssl.org> Date: Tue Aug 30 14:20:18 2016 +0100 Add some CertStatus tests The previous commit revealed a long standing problem where CertStatus processing was broken in DTLS. This would have been revealed by better testing - so add some! Reviewed-by: Rich Salz <rs...@openssl.org> commit f046afb0663fc4514f7fc5d1724439caa6858932 Author: Matt Caswell <m...@openssl.org> Date: Tue Aug 30 11:32:49 2016 +0100 Ensure the CertStatus message adds a DTLS message header where needed The function tls_construct_cert_status() is called by both TLS and DTLS code. However it only ever constructed a TLS message header for the message which obviously failed in DTLS. Reviewed-by: Rich Salz <rs...@openssl.org> ----------------------------------------------------------------------- Summary of changes: ssl/statem/statem_srvr.c | 27 +++++----- test/handshake_helper.c | 42 +++++++++++++++ test/recipes/80-test_ssl_new.t | 5 +- test/ssl-tests/15-certstatus.conf | 62 ++++++++++++++++++++++ test/ssl-tests/15-certstatus.conf.in | 45 ++++++++++++++++ .../ssl-tests/16-certstatus.conf | 0 test/ssl-tests/16-dtls-certstatus.conf | 62 ++++++++++++++++++++++ test/ssl-tests/16-dtls-certstatus.conf.in | 45 ++++++++++++++++ test/ssl_test_ctx.c | 29 ++++++++++ test/ssl_test_ctx.h | 9 ++++ test/ssl_test_ctx_test.c | 6 +++ 11 files changed, 317 insertions(+), 15 deletions(-) create mode 100644 test/ssl-tests/15-certstatus.conf create mode 100644 test/ssl-tests/15-certstatus.conf.in copy fuzz/corpora/bignum/da39a3ee5e6b4b0d3255bfef95601890afd80709 => test/ssl-tests/16-certstatus.conf (100%) create mode 100644 test/ssl-tests/16-dtls-certstatus.conf create mode 100644 test/ssl-tests/16-dtls-certstatus.conf.in diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c index 95dcc9b..a6b8a87 100644 --- a/ssl/statem/statem_srvr.c +++ b/ssl/statem/statem_srvr.c @@ -3150,34 +3150,35 @@ int tls_construct_new_session_ticket(SSL *s) int tls_construct_cert_status(SSL *s) { unsigned char *p; + size_t msglen; + /*- * Grow buffer if need be: the length calculation is as - * follows 1 (message type) + 3 (message length) + + * follows handshake_header_length + * 1 (ocsp response type) + 3 (ocsp response length) * + (ocsp response) */ - if (!BUF_MEM_grow(s->init_buf, 8 + s->tlsext_ocsp_resplen)) { - ossl_statem_set_error(s); - return 0; - } + msglen = 4 + s->tlsext_ocsp_resplen; + if (!BUF_MEM_grow(s->init_buf, SSL_HM_HEADER_LENGTH(s) + msglen)) + goto err; - p = (unsigned char *)s->init_buf->data; + p = ssl_handshake_start(s); - /* do the header */ - *(p++) = SSL3_MT_CERTIFICATE_STATUS; - /* message length */ - l2n3(s->tlsext_ocsp_resplen + 4, p); /* status type */ *(p++) = s->tlsext_status_type; /* length of OCSP response */ l2n3(s->tlsext_ocsp_resplen, p); /* actual response */ memcpy(p, s->tlsext_ocsp_resp, s->tlsext_ocsp_resplen); - /* number of bytes to write */ - s->init_num = 8 + s->tlsext_ocsp_resplen; - s->init_off = 0; + + if (!ssl_set_handshake_header(s, SSL3_MT_CERTIFICATE_STATUS, msglen)) + goto err; return 1; + + err: + ossl_statem_set_error(s); + return 0; } #ifndef OPENSSL_NO_NEXTPROTONEG diff --git a/test/handshake_helper.c b/test/handshake_helper.c index 409f16c..90e18fc 100644 --- a/test/handshake_helper.c +++ b/test/handshake_helper.c @@ -144,6 +144,38 @@ static int servername_reject_cb(SSL *s, int *ad, void *arg) return select_server_ctx(s, arg, 0); } +static unsigned char dummy_ocsp_resp_good_val = 0xff; +static unsigned char dummy_ocsp_resp_bad_val = 0xfe; + +static int server_ocsp_cb(SSL *s, void *arg) +{ + unsigned char *resp; + + resp = OPENSSL_malloc(1); + if (resp == NULL) + return SSL_TLSEXT_ERR_ALERT_FATAL; + /* + * For the purposes of testing we just send back a dummy OCSP response + */ + *resp = *(unsigned char *)arg; + if (!SSL_set_tlsext_status_ocsp_resp(s, resp, 1)) + return SSL_TLSEXT_ERR_ALERT_FATAL; + + return SSL_TLSEXT_ERR_OK; +} + +static int client_ocsp_cb(SSL *s, void *arg) +{ + const unsigned char *resp; + int len; + + len = SSL_get_tlsext_status_ocsp_resp(s, &resp); + if (len != 1 || *resp != dummy_ocsp_resp_good_val) + return 0; + + return 1; +} + static int verify_reject_cb(X509_STORE_CTX *ctx, void *arg) { X509_STORE_CTX_set_error(ctx, X509_V_ERR_APPLICATION_VERIFICATION); return 0; @@ -319,6 +351,16 @@ static void configure_handshake_ctx(SSL_CTX *server_ctx, SSL_CTX *server2_ctx, break; } + if (extra->server.cert_status != SSL_TEST_CERT_STATUS_NONE) { + SSL_CTX_set_tlsext_status_type(client_ctx, TLSEXT_STATUSTYPE_ocsp); + SSL_CTX_set_tlsext_status_cb(client_ctx, client_ocsp_cb); + SSL_CTX_set_tlsext_status_arg(client_ctx, NULL); + SSL_CTX_set_tlsext_status_cb(server_ctx, server_ocsp_cb); + SSL_CTX_set_tlsext_status_arg(server_ctx, + ((extra->server.cert_status == SSL_TEST_CERT_STATUS_GOOD_RESPONSE) + ? &dummy_ocsp_resp_good_val : &dummy_ocsp_resp_bad_val)); + } + /* * The initial_ctx/session_ctx always handles the encrypt/decrypt of the * session ticket. This ticket_key callback is assigned to the second diff --git a/test/recipes/80-test_ssl_new.t b/test/recipes/80-test_ssl_new.t index 175b3b2..46c2f42 100644 --- a/test/recipes/80-test_ssl_new.t +++ b/test/recipes/80-test_ssl_new.t @@ -29,7 +29,7 @@ map { s/\.in// } @conf_files; # We hard-code the number of tests to double-check that the globbing above # finds all files as expected. -plan tests => 14; # = scalar @conf_srcs +plan tests => 16; # = scalar @conf_srcs # Some test results depend on the configuration of enabled protocols. We only # verify generated sources in the default configuration. @@ -69,7 +69,8 @@ my %skip = ( # special-casing for. # We should review this once we have TLS 1.3. "13-fragmentation.conf" => disabled("tls1_2"), - "14-curves.conf" => disabled("tls1_2") || $no_ec || $no_ec2m + "14-curves.conf" => disabled("tls1_2") || $no_ec || $no_ec2m, + "16-dtls-certstatus.conf" => $no_dtls ); foreach my $conf (@conf_files) { diff --git a/test/ssl-tests/15-certstatus.conf b/test/ssl-tests/15-certstatus.conf new file mode 100644 index 0000000..bf6c41c --- /dev/null +++ b/test/ssl-tests/15-certstatus.conf @@ -0,0 +1,62 @@ +# Generated with generate_ssl_tests.pl + +num_tests = 2 + +test-0 = 0-certstatus-good +test-1 = 1-certstatus-bad +# =========================================================== + +[0-certstatus-good] +ssl_conf = 0-certstatus-good-ssl + +[0-certstatus-good-ssl] +server = 0-certstatus-good-server +client = 0-certstatus-good-client + +[0-certstatus-good-server] +Certificate = ${ENV::TEST_CERTS_DIR}/servercert.pem +CipherString = DEFAULT +PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem + +[0-certstatus-good-client] +CipherString = DEFAULT +VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem +VerifyMode = Peer + +[test-0] +ExpectedResult = Success +Method = TLS +server = 0-certstatus-good-server-extra + +[0-certstatus-good-server-extra] +CertStatus = GoodResponse + + +# =========================================================== + +[1-certstatus-bad] +ssl_conf = 1-certstatus-bad-ssl + +[1-certstatus-bad-ssl] +server = 1-certstatus-bad-server +client = 1-certstatus-bad-client + +[1-certstatus-bad-server] +Certificate = ${ENV::TEST_CERTS_DIR}/servercert.pem +CipherString = DEFAULT +PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem + +[1-certstatus-bad-client] +CipherString = DEFAULT +VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem +VerifyMode = Peer + +[test-1] +ExpectedResult = ClientFail +Method = TLS +server = 1-certstatus-bad-server-extra + +[1-certstatus-bad-server-extra] +CertStatus = BadResponse + + diff --git a/test/ssl-tests/15-certstatus.conf.in b/test/ssl-tests/15-certstatus.conf.in new file mode 100644 index 0000000..074602d --- /dev/null +++ b/test/ssl-tests/15-certstatus.conf.in @@ -0,0 +1,45 @@ +# -*- mode: perl; -*- +# Copyright 2016-2016 The OpenSSL Project Authors. All Rights Reserved. +# +# Licensed under the OpenSSL license (the "License"). You may not use +# this file except in compliance with the License. You can obtain a copy +# in the file LICENSE in the source distribution or at +# https://www.openssl.org/source/license.html + + +## Test CertStatus messages + +use strict; +use warnings; + +package ssltests; + + +our @tests = ( + { + name => "certstatus-good", + server => { + extra => { + "CertStatus" => "GoodResponse", + }, + }, + client => {}, + test => { + "Method" => "TLS", + "ExpectedResult" => "Success" + } + }, + { + name => "certstatus-bad", + server => { + extra => { + "CertStatus" => "BadResponse", + }, + }, + client => {}, + test => { + "Method" => "TLS", + "ExpectedResult" => "ClientFail" + } + }, +); diff --git a/fuzz/corpora/bignum/da39a3ee5e6b4b0d3255bfef95601890afd80709 b/test/ssl-tests/16-certstatus.conf similarity index 100% copy from fuzz/corpora/bignum/da39a3ee5e6b4b0d3255bfef95601890afd80709 copy to test/ssl-tests/16-certstatus.conf diff --git a/test/ssl-tests/16-dtls-certstatus.conf b/test/ssl-tests/16-dtls-certstatus.conf new file mode 100644 index 0000000..a561803 --- /dev/null +++ b/test/ssl-tests/16-dtls-certstatus.conf @@ -0,0 +1,62 @@ +# Generated with generate_ssl_tests.pl + +num_tests = 2 + +test-0 = 0-certstatus-good +test-1 = 1-certstatus-bad +# =========================================================== + +[0-certstatus-good] +ssl_conf = 0-certstatus-good-ssl + +[0-certstatus-good-ssl] +server = 0-certstatus-good-server +client = 0-certstatus-good-client + +[0-certstatus-good-server] +Certificate = ${ENV::TEST_CERTS_DIR}/servercert.pem +CipherString = DEFAULT +PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem + +[0-certstatus-good-client] +CipherString = DEFAULT +VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem +VerifyMode = Peer + +[test-0] +ExpectedResult = Success +Method = DTLS +server = 0-certstatus-good-server-extra + +[0-certstatus-good-server-extra] +CertStatus = GoodResponse + + +# =========================================================== + +[1-certstatus-bad] +ssl_conf = 1-certstatus-bad-ssl + +[1-certstatus-bad-ssl] +server = 1-certstatus-bad-server +client = 1-certstatus-bad-client + +[1-certstatus-bad-server] +Certificate = ${ENV::TEST_CERTS_DIR}/servercert.pem +CipherString = DEFAULT +PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem + +[1-certstatus-bad-client] +CipherString = DEFAULT +VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem +VerifyMode = Peer + +[test-1] +ExpectedResult = ClientFail +Method = DTLS +server = 1-certstatus-bad-server-extra + +[1-certstatus-bad-server-extra] +CertStatus = BadResponse + + diff --git a/test/ssl-tests/16-dtls-certstatus.conf.in b/test/ssl-tests/16-dtls-certstatus.conf.in new file mode 100644 index 0000000..7280029 --- /dev/null +++ b/test/ssl-tests/16-dtls-certstatus.conf.in @@ -0,0 +1,45 @@ +# -*- mode: perl; -*- +# Copyright 2016-2016 The OpenSSL Project Authors. All Rights Reserved. +# +# Licensed under the OpenSSL license (the "License"). You may not use +# this file except in compliance with the License. You can obtain a copy +# in the file LICENSE in the source distribution or at +# https://www.openssl.org/source/license.html + + +## Test DTLS CertStatus messages + +use strict; +use warnings; + +package ssltests; + + +our @tests = ( + { + name => "certstatus-good", + server => { + extra => { + "CertStatus" => "GoodResponse", + }, + }, + client => {}, + test => { + "Method" => "DTLS", + "ExpectedResult" => "Success" + } + }, + { + name => "certstatus-bad", + server => { + extra => { + "CertStatus" => "BadResponse", + }, + }, + client => {}, + test => { + "Method" => "DTLS", + "ExpectedResult" => "ClientFail" + } + }, +); diff --git a/test/ssl_test_ctx.c b/test/ssl_test_ctx.c index ac90f19..ee2e8a1 100644 --- a/test/ssl_test_ctx.c +++ b/test/ssl_test_ctx.c @@ -390,6 +390,34 @@ const char *ssl_ct_validation_name(ssl_ct_validation_t mode) IMPLEMENT_SSL_TEST_BOOL_OPTION(SSL_TEST_CTX, test, resumption_expected) IMPLEMENT_SSL_TEST_BOOL_OPTION(SSL_TEST_SERVER_CONF, server, broken_session_ticket) +/**************/ +/* CertStatus */ +/**************/ + +static const test_enum ssl_certstatus[] = { + {"None", SSL_TEST_CERT_STATUS_NONE}, + {"GoodResponse", SSL_TEST_CERT_STATUS_GOOD_RESPONSE}, + {"BadResponse", SSL_TEST_CERT_STATUS_BAD_RESPONSE} +}; + +__owur static int parse_certstatus(SSL_TEST_SERVER_CONF *server_conf, + const char *value) +{ + int ret_value; + if (!parse_enum(ssl_certstatus, OSSL_NELEM(ssl_certstatus), &ret_value, + value)) { + return 0; + } + server_conf->cert_status = ret_value; + return 1; +} + +const char *ssl_certstatus_name(ssl_cert_status_t cert_status) +{ + return enum_name(ssl_certstatus, + OSSL_NELEM(ssl_certstatus), cert_status); +} + /***********************/ /* ApplicationData */ /***********************/ @@ -453,6 +481,7 @@ static const ssl_test_server_option ssl_test_server_options[] = { { "NPNProtocols", &parse_server_npn_protocols }, { "ALPNProtocols", &parse_server_alpn_protocols }, { "BrokenSessionTicket", &parse_server_broken_session_ticket }, + { "CertStatus", &parse_certstatus }, }; /* diff --git a/test/ssl_test_ctx.h b/test/ssl_test_ctx.h index 11b6c33..c8c66d6 100644 --- a/test/ssl_test_ctx.h +++ b/test/ssl_test_ctx.h @@ -65,6 +65,12 @@ typedef enum { SSL_TEST_CT_VALIDATION_PERMISSIVE, SSL_TEST_CT_VALIDATION_STRICT } ssl_ct_validation_t; + +typedef enum { + SSL_TEST_CERT_STATUS_NONE = 0, /* Default */ + SSL_TEST_CERT_STATUS_GOOD_RESPONSE, + SSL_TEST_CERT_STATUS_BAD_RESPONSE +} ssl_cert_status_t; /* * Server/client settings that aren't supported by the SSL CONF library, * such as callbacks. @@ -88,6 +94,8 @@ typedef struct { char *alpn_protocols; /* Whether to set a broken session ticket callback. */ int broken_session_ticket; + /* Should we send a CertStatus message? */ + ssl_cert_status_t cert_status; } SSL_TEST_SERVER_CONF; typedef struct { @@ -164,6 +172,7 @@ const char *ssl_session_ticket_name(ssl_session_ticket_t server); const char *ssl_test_method_name(ssl_test_method_t method); const char *ssl_handshake_mode_name(ssl_handshake_mode_t mode); const char *ssl_ct_validation_name(ssl_ct_validation_t mode); +const char *ssl_certstatus_name(ssl_cert_status_t cert_status); /* * Load the test case context from |conf|. diff --git a/test/ssl_test_ctx_test.c b/test/ssl_test_ctx_test.c index b54d7ea..0f321c6 100644 --- a/test/ssl_test_ctx_test.c +++ b/test/ssl_test_ctx_test.c @@ -83,6 +83,12 @@ static int SSL_TEST_SERVER_CONF_equal(SSL_TEST_SERVER_CONF *server, server->broken_session_ticket, server2->broken_session_ticket); return 0; } + if (server->cert_status != server2->cert_status) { + fprintf(stderr, "CertStatus mismatch: %s vs %s.\n", + ssl_certstatus_name(server->cert_status), + ssl_certstatus_name(server2->cert_status)); + return 0; + } return 1; } _____ openssl-commits mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-commits