From 57a3af94a7ccff2efa99c26b2e842f520e4a731c Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Tue, 23 Jul 2019 17:10:05 +0100 Subject: [PATCH] Extend tests of SSL_check_chain() Actually supply a chain and then test: 1) A successful check of both the ee and chain certs 2) A failure to check the ee cert 3) A failure to check a chain cert Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/9443) --- test/ct_test.c | 23 +------- test/recipes/90-test_sslapi.t | 5 +- test/sslapitest.c | 98 +++++++++++++++++++++++++++++------ test/testutil.h | 3 ++ test/testutil/driver.c | 18 +++++++ 5 files changed, 106 insertions(+), 41 deletions(-) diff --git a/test/ct_test.c b/test/ct_test.c index f881d5f6a9..78d11ca98c 100644 --- a/test/ct_test.c +++ b/test/ct_test.c @@ -87,29 +87,10 @@ static void tear_down(CT_TEST_FIXTURE *fixture) OPENSSL_free(fixture); } -static char *mk_file_path(const char *dir, const char *file) -{ -# ifndef OPENSSL_SYS_VMS - const char *sep = "/"; -# else - const char *sep = ""; -# endif - size_t len = strlen(dir) + strlen(sep) + strlen(file) + 1; - char *full_file = OPENSSL_zalloc(len); - - if (full_file != NULL) { - OPENSSL_strlcpy(full_file, dir, len); - OPENSSL_strlcat(full_file, sep, len); - OPENSSL_strlcat(full_file, file, len); - } - - return full_file; -} - static X509 *load_pem_cert(const char *dir, const char *file) { X509 *cert = NULL; - char *file_path = mk_file_path(dir, file); + char *file_path = test_mk_file_path(dir, file); if (file_path != NULL) { BIO *cert_io = BIO_new_file(file_path, "r"); @@ -127,7 +108,7 @@ static int read_text_file(const char *dir, const char *file, char *buffer, int buffer_length) { int len = -1; - char *file_path = mk_file_path(dir, file); + char *file_path = test_mk_file_path(dir, file); if (file_path != NULL) { BIO *file_io = BIO_new_file(file_path, "r"); diff --git a/test/recipes/90-test_sslapi.t b/test/recipes/90-test_sslapi.t index 633df47736..4dd70bf18d 100644 --- a/test/recipes/90-test_sslapi.t +++ b/test/recipes/90-test_sslapi.t @@ -8,7 +8,7 @@ use OpenSSL::Test::Utils; -use OpenSSL::Test qw/:DEFAULT srctop_file/; +use OpenSSL::Test qw/:DEFAULT srctop_file srctop_dir/; use File::Temp qw(tempfile); setup("test_sslapi"); @@ -20,8 +20,7 @@ plan tests => 1; (undef, my $tmpfilename) = tempfile(); -ok(run(test(["sslapitest", srctop_file("apps", "server.pem"), - srctop_file("apps", "server.pem"), +ok(run(test(["sslapitest", srctop_dir("test", "certs"), srctop_file("test", "recipes", "90-test_sslapi_data", "passwd.txt"), $tmpfilename])), "running sslapitest"); diff --git a/test/sslapitest.c b/test/sslapitest.c index 8c6e16c2fc..c29bb64f6f 100644 --- a/test/sslapitest.c +++ b/test/sslapitest.c @@ -42,6 +42,7 @@ static int find_session_cb_cnt = 0; static SSL_SESSION *create_a_psk(SSL *ssl); #endif +static char *certsdir = NULL; static char *cert = NULL; static char *privkey = NULL; static char *srpvfile = NULL; @@ -5664,7 +5665,10 @@ static int cert_cb(SSL *s, void *arg) SSL_CTX *ctx = (SSL_CTX *)arg; BIO *in = NULL; EVP_PKEY *pkey = NULL; - X509 *x509 = NULL; + X509 *x509 = NULL, *rootx = NULL; + STACK_OF(X509) *chain = NULL; + char *rootfile = NULL, *ecdsacert = NULL, *ecdsakey = NULL; + int ret = 0; if (cert_cb_cnt == 0) { /* Suspend the handshake */ @@ -5687,38 +5691,58 @@ static int cert_cb(SSL *s, void *arg) return 1; } else if (cert_cb_cnt == 3) { int rv; + + rootfile = test_mk_file_path(certsdir, "rootcert.pem"); + ecdsacert = test_mk_file_path(certsdir, "server-ecdsa-cert.pem"); + ecdsakey = test_mk_file_path(certsdir, "server-ecdsa-key.pem"); + if (!TEST_ptr(rootfile) || !TEST_ptr(ecdsacert) || !TEST_ptr(ecdsakey)) + goto out; + chain = sk_X509_new_null(); + if (!TEST_ptr(chain)) + goto out; if (!TEST_ptr(in = BIO_new(BIO_s_file())) - || !TEST_int_ge(BIO_read_filename(in, cert), 0) + || !TEST_int_ge(BIO_read_filename(in, rootfile), 0) + || !TEST_ptr(rootx = PEM_read_bio_X509(in, NULL, NULL, NULL)) + || !TEST_true(sk_X509_push(chain, rootx))) + goto out; + rootx = NULL; + BIO_free(in); + if (!TEST_ptr(in = BIO_new(BIO_s_file())) + || !TEST_int_ge(BIO_read_filename(in, ecdsacert), 0) || !TEST_ptr(x509 = PEM_read_bio_X509(in, NULL, NULL, NULL))) goto out; BIO_free(in); if (!TEST_ptr(in = BIO_new(BIO_s_file())) - || !TEST_int_ge(BIO_read_filename(in, privkey), 0) + || !TEST_int_ge(BIO_read_filename(in, ecdsakey), 0) || !TEST_ptr(pkey = PEM_read_bio_PrivateKey(in, NULL, NULL, NULL))) goto out; - rv = SSL_check_chain(s, x509, pkey, NULL); + rv = SSL_check_chain(s, x509, pkey, chain); /* * If the cert doesn't show as valid here (e.g., because we don't * have any shared sigalgs), then we will not set it, and there will * be no certificate at all on the SSL or SSL_CTX. This, in turn, * will cause tls_choose_sigalgs() to fail the connection. */ - if ((rv & CERT_PKEY_VALID)) { + if ((rv & (CERT_PKEY_VALID | CERT_PKEY_CA_SIGNATURE)) + == (CERT_PKEY_VALID | CERT_PKEY_CA_SIGNATURE)) { if (!SSL_use_cert_and_key(s, x509, pkey, NULL, 1)) goto out; } - BIO_free(in); - EVP_PKEY_free(pkey); - X509_free(x509); - return 1; + + ret = 1; } /* Abort the handshake */ out: + OPENSSL_free(ecdsacert); + OPENSSL_free(ecdsakey); + OPENSSL_free(rootfile); BIO_free(in); EVP_PKEY_free(pkey); X509_free(x509); - return 0; + X509_free(rootx); + sk_X509_pop_free(chain, X509_free); + return ret; } /* @@ -5726,6 +5750,10 @@ static int cert_cb(SSL *s, void *arg) * Test 0: Callback fails * Test 1: Success - no SSL_set_SSL_CTX() in the callback * Test 2: Success - SSL_set_SSL_CTX() in the callback + * Test 3: Success - Call SSL_check_chain from the callback + * Test 4: Failure - SSL_check_chain fails from callback due to bad cert in the + * chain + * Test 5: Failure - SSL_check_chain fails from callback due to bad ee cert */ static int test_cert_cb_int(int prot, int tst) { @@ -5733,6 +5761,12 @@ static int test_cert_cb_int(int prot, int tst) SSL *clientssl = NULL, *serverssl = NULL; int testresult = 0, ret; +#ifdef OPENSSL_NO_EC + /* We use an EC cert in these tests, so we skip in a no-ec build */ + if (tst >= 3) + return 1; +#endif + if (!TEST_true(create_ssl_ctx_pair(TLS_server_method(), TLS_client_method(), TLS1_VERSION, @@ -5742,10 +5776,11 @@ static int test_cert_cb_int(int prot, int tst) if (tst == 0) cert_cb_cnt = -1; - else if (tst == 3) + else if (tst >= 3) cert_cb_cnt = 3; else cert_cb_cnt = 0; + if (tst == 2) snictx = SSL_CTX_new(TLS_server_method()); SSL_CTX_set_cert_cb(sctx, cert_cb, snictx); @@ -5754,8 +5789,26 @@ static int test_cert_cb_int(int prot, int tst) NULL, NULL))) goto end; + if (tst == 4) { + /* + * We cause SSL_check_chain() to fail by specifying sig_algs that + * the chain doesn't meet (the root uses an RSA cert) + */ + if (!TEST_true(SSL_set1_sigalgs_list(clientssl, + "ecdsa_secp256r1_sha256"))) + goto end; + } else if (tst == 5) { + /* + * We cause SSL_check_chain() to fail by specifying sig_algs that + * the ee cert doesn't meet (the ee uses an ECDSA cert) + */ + if (!TEST_true(SSL_set1_sigalgs_list(clientssl, + "rsa_pss_rsae_sha256:rsa_pkcs1_sha256"))) + goto end; + } + ret = create_ssl_connection(serverssl, clientssl, SSL_ERROR_NONE); - if (!TEST_true(tst == 0 ? !ret : ret) + if (!TEST_true(tst == 0 || tst == 4 || tst == 5 ? !ret : ret) || (tst > 0 && !TEST_int_eq((cert_cb_cnt - 2) * (cert_cb_cnt - 3), 0))) { goto end; @@ -6018,10 +6071,9 @@ static int test_ca_names(int tst) int setup_tests(void) { - if (!TEST_ptr(cert = test_get_argument(0)) - || !TEST_ptr(privkey = test_get_argument(1)) - || !TEST_ptr(srpvfile = test_get_argument(2)) - || !TEST_ptr(tmpfilename = test_get_argument(3))) + if (!TEST_ptr(certsdir = test_get_argument(0)) + || !TEST_ptr(srpvfile = test_get_argument(1)) + || !TEST_ptr(tmpfilename = test_get_argument(2))) return 0; if (getenv("OPENSSL_TEST_GETCOUNTS") != NULL) { @@ -6040,6 +6092,16 @@ int setup_tests(void) #endif } + cert = test_mk_file_path(certsdir, "servercert.pem"); + if (cert == NULL) + return 0; + + privkey = test_mk_file_path(certsdir, "serverkey.pem"); + if (privkey == NULL) { + OPENSSL_free(cert); + return 0; + } + ADD_TEST(test_large_message_tls); ADD_TEST(test_large_message_tls_read_ahead); #ifndef OPENSSL_NO_DTLS @@ -6120,7 +6182,7 @@ int setup_tests(void) ADD_ALL_TESTS(test_ssl_get_shared_ciphers, OSSL_NELEM(shared_ciphers_data)); ADD_ALL_TESTS(test_ticket_callbacks, 12); ADD_ALL_TESTS(test_shutdown, 7); - ADD_ALL_TESTS(test_cert_cb, 4); + ADD_ALL_TESTS(test_cert_cb, 6); ADD_ALL_TESTS(test_client_cert_cb, 2); ADD_ALL_TESTS(test_ca_names, 3); return 1; @@ -6128,6 +6190,8 @@ int setup_tests(void) void cleanup_tests(void) { + OPENSSL_free(cert); + OPENSSL_free(privkey); bio_s_mempacket_test_free(); bio_s_always_retry_free(); } diff --git a/test/testutil.h b/test/testutil.h index db0c74ef88..0e9e3d5c9e 100644 --- a/test/testutil.h +++ b/test/testutil.h @@ -462,4 +462,7 @@ char *glue_strings(const char *list[], size_t *out_len); uint32_t test_random(void); void test_random_seed(uint32_t sd); +/* Create a file path from a directory and a filename */ +char *test_mk_file_path(const char *dir, const char *file); + #endif /* HEADER_TESTUTIL_H */ diff --git a/test/testutil/driver.c b/test/testutil/driver.c index 48f94aea1e..89a3a0ba43 100644 --- a/test/testutil/driver.c +++ b/test/testutil/driver.c @@ -297,3 +297,21 @@ char *glue_strings(const char *list[], size_t *out_len) return ret; } +char *test_mk_file_path(const char *dir, const char *file) +{ +# ifndef OPENSSL_SYS_VMS + const char *sep = "/"; +# else + const char *sep = ""; +# endif + size_t len = strlen(dir) + strlen(sep) + strlen(file) + 1; + char *full_file = OPENSSL_zalloc(len); + + if (full_file != NULL) { + OPENSSL_strlcpy(full_file, dir, len); + OPENSSL_strlcat(full_file, sep, len); + OPENSSL_strlcat(full_file, file, len); + } + + return full_file; +} -- 2.25.1