From 5a22cf96a0a1c34968c0664f99b7ebb7ccf6ed75 Mon Sep 17 00:00:00 2001 From: Emilia Kasper Date: Wed, 6 Apr 2016 16:03:06 +0200 Subject: [PATCH] Replace cipherlist test The old cipherlist test in ssltest.c only tests the internal order of the cipher table, which is pretty useless. Replace this test with a test that catches inadvertent changes to the default cipherlist. Fix run_tests.pl to correctly filter tests that have "list" in their name. (Also includes a small drive-by fix in .gitignore.) Reviewed-by: Rich Salz --- .gitignore | 1 + test/build.info | 6 +- test/cipherlist_test.c | 212 ++++++++++++++++++++++++++++++ test/recipes/80-test_cipherlist.t | 18 +++ test/recipes/80-test_ssl_old.t | 19 +-- test/run_tests.pl | 2 +- test/ssltest_old.c | 56 +------- 7 files changed, 244 insertions(+), 70 deletions(-) create mode 100644 test/cipherlist_test.c create mode 100644 test/recipes/80-test_cipherlist.t diff --git a/.gitignore b/.gitignore index 06549caef7..6ad374d959 100644 --- a/.gitignore +++ b/.gitignore @@ -79,6 +79,7 @@ Makefile /test/fips_ecdsavs /test/fips_rngvs /test/fips_test_suite +/test/ssltest_old *.so* *.dylib* *.dll* diff --git a/test/build.info b/test/build.info index 8bd7f62d37..0f41a7317b 100644 --- a/test/build.info +++ b/test/build.info @@ -16,7 +16,7 @@ IF[{- !$disabled{tests} -}] constant_time_test verify_extra_test clienthellotest \ packettest asynctest secmemtest srptest memleaktest \ dtlsv1listentest ct_test threadstest afalgtest d2i_test \ - ssl_test_ctx_test ssl_test x509aux + ssl_test_ctx_test ssl_test x509aux cipherlist_test SOURCE[aborttest]=aborttest.c INCLUDE[aborttest]={- rel2abs(catdir($builddir,"../include")) -} ../include @@ -234,6 +234,10 @@ IF[{- !$disabled{tests} -}] INCLUDE[ssl_test]={- rel2abs(catdir($builddir,"../include")) -} .. ../include DEPEND[ssl_test]=../libcrypto ../libssl + SOURCE[cipherlist_test]=cipherlist_test.c testutil.c + INCLUDE[cipherlist_test]={- rel2abs(catdir($builddir,"../include")) -} .. ../include + DEPEND[cipherlist_test]=../libcrypto ../libssl + INCLUDE[testutil.o]=.. INCLUDE[ssl_test_ctx.o]={- rel2abs(catdir($builddir,"../include")) -} ../include INCLUDE[handshake_helper.o]={- rel2abs(catdir($builddir,"../include")) -} ../include diff --git a/test/cipherlist_test.c b/test/cipherlist_test.c new file mode 100644 index 0000000000..e892f9d5a3 --- /dev/null +++ b/test/cipherlist_test.c @@ -0,0 +1,212 @@ +/* + * Copyright 2016 The OpenSSL Project Authors. All Rights Reserved. + * + * Licensed under the OpenSSL licenses, (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * https://www.openssl.org/source/license.html + * or in the file LICENSE in the source distribution. + */ + +#include + +#include +#include +#include +#include +#include +#include + +#include "e_os.h" +#include "testutil.h" + +typedef struct cipherlist_test_fixture { + const char *test_case_name; + SSL_CTX *server; + SSL_CTX *client; +} CIPHERLIST_TEST_FIXTURE; + + +static CIPHERLIST_TEST_FIXTURE set_up(const char *const test_case_name) +{ + CIPHERLIST_TEST_FIXTURE fixture; + fixture.test_case_name = test_case_name; + fixture.server = SSL_CTX_new(TLS_server_method()); + fixture.client = SSL_CTX_new(TLS_client_method()); + OPENSSL_assert(fixture.client != NULL && fixture.server != NULL); + return fixture; +} + +/* + * All ciphers in the DEFAULT cipherlist meet the default security level. + * However, default supported ciphers exclude SRP and PSK ciphersuites + * for which no callbacks have been set up. + * + * Supported ciphers also exclude TLSv1.2 ciphers if TLSv1.2 is disabled, + * and individual disabled algorithms. However, NO_RSA, NO_AES and NO_SHA + * are currently broken and should be considered mission impossible in libssl. + */ +static const uint32_t default_ciphers_in_order[] = { +#ifndef OPENSSL_NO_TLS1_2 +# ifndef OPENSSL_NO_EC + TLS1_CK_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, + TLS1_CK_ECDHE_RSA_WITH_AES_256_GCM_SHA384, +# endif +# ifndef OPENSSL_NO_DH + TLS1_CK_DHE_RSA_WITH_AES_256_GCM_SHA384, +# endif + +# if !defined OPENSSL_NO_CHACHA && !defined OPENSSL_NO_POLY1305 +# ifndef OPENSSL_NO_EC + TLS1_CK_ECDHE_ECDSA_WITH_CHACHA20_POLY1305, + TLS1_CK_ECDHE_RSA_WITH_CHACHA20_POLY1305, +# endif +# ifndef OPENSSL_NO_DH + TLS1_CK_DHE_RSA_WITH_CHACHA20_POLY1305, +# endif +# endif /* !OPENSSL_NO_CHACHA && !OPENSSL_NO_POLY1305 */ + +# ifndef OPENSSL_NO_EC + TLS1_CK_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, + TLS1_CK_ECDHE_RSA_WITH_AES_128_GCM_SHA256, +# endif +# ifndef OPENSSL_NO_DH + TLS1_CK_DHE_RSA_WITH_AES_128_GCM_SHA256, +# endif +# ifndef OPENSSL_NO_EC + TLS1_CK_ECDHE_ECDSA_WITH_AES_256_SHA384, + TLS1_CK_ECDHE_RSA_WITH_AES_256_SHA384, +# endif +# ifndef OPENSSL_NO_DH + TLS1_CK_DHE_RSA_WITH_AES_256_SHA256, +# endif +# ifndef OPENSSL_NO_EC + TLS1_CK_ECDHE_ECDSA_WITH_AES_128_SHA256, + TLS1_CK_ECDHE_RSA_WITH_AES_128_SHA256, +# endif +# ifndef OPENSSL_NO_DH + TLS1_CK_DHE_RSA_WITH_AES_128_SHA256, +# endif +#endif /* !OPENSSL_NO_TLS1_2 */ + +#ifndef OPENSSL_NO_EC + TLS1_CK_ECDHE_ECDSA_WITH_AES_256_CBC_SHA, + TLS1_CK_ECDHE_RSA_WITH_AES_256_CBC_SHA, +#endif +#ifndef OPENSSL_NO_DH + TLS1_CK_DHE_RSA_WITH_AES_256_SHA, +#endif +#ifndef OPENSSL_NO_EC + TLS1_CK_ECDHE_ECDSA_WITH_AES_128_CBC_SHA, + TLS1_CK_ECDHE_RSA_WITH_AES_128_CBC_SHA, +#endif +#ifndef OPENSSL_NO_DH + TLS1_CK_DHE_RSA_WITH_AES_128_SHA, +#endif + +#ifndef OPENSSL_NO_DES +# ifndef OPENSSL_NO_EC + TLS1_CK_ECDHE_ECDSA_WITH_DES_192_CBC3_SHA, + TLS1_CK_ECDHE_RSA_WITH_DES_192_CBC3_SHA, +# endif +# ifndef OPENSSL_NO_DH + SSL3_CK_DHE_RSA_DES_192_CBC3_SHA, +# endif +#endif /* !OPENSSL_NO_DES */ + +#ifndef OPENSSL_NO_TLS1_2 + TLS1_CK_RSA_WITH_AES_256_GCM_SHA384, + TLS1_CK_RSA_WITH_AES_128_GCM_SHA256, + TLS1_CK_RSA_WITH_AES_256_SHA256, + TLS1_CK_RSA_WITH_AES_128_SHA256, +#endif + + TLS1_CK_RSA_WITH_AES_256_SHA, + TLS1_CK_RSA_WITH_AES_128_SHA, +#ifndef OPENSSL_NO_DES + SSL3_CK_RSA_DES_192_CBC3_SHA, +#endif +}; + +static int test_default_cipherlist(SSL_CTX *ctx) +{ + STACK_OF(SSL_CIPHER) *ciphers; + SSL *ssl; + int i, ret = 0, num_expected_ciphers, num_ciphers; + uint32_t expected_cipher_id, cipher_id; + + ssl = SSL_new(ctx); + OPENSSL_assert(ssl != NULL); + + ciphers = SSL_get1_supported_ciphers(ssl); + OPENSSL_assert(ciphers != NULL); + num_expected_ciphers = OSSL_NELEM(default_ciphers_in_order); + num_ciphers = sk_SSL_CIPHER_num(ciphers); + if (num_ciphers != num_expected_ciphers) { + fprintf(stderr, "Expected %d supported ciphers, got %d.\n", + num_expected_ciphers, num_ciphers); + goto err; + } + + for (i = 0; i < num_ciphers; i++) { + expected_cipher_id = default_ciphers_in_order[i]; + cipher_id = SSL_CIPHER_get_id(sk_SSL_CIPHER_value(ciphers, i)); + if (cipher_id != expected_cipher_id) { + fprintf(stderr, "Wrong cipher at position %d: expected %x, " + "got %x\n", i, expected_cipher_id, cipher_id); + goto err; + } + } + + ret = 1; + + err: + sk_SSL_CIPHER_free(ciphers); + SSL_free(ssl); + return ret; +} + +static int execute_test(CIPHERLIST_TEST_FIXTURE fixture) +{ + return test_default_cipherlist(fixture.server) + && test_default_cipherlist(fixture.client); +} + +static void tear_down(CIPHERLIST_TEST_FIXTURE fixture) +{ + SSL_CTX_free(fixture.server); + SSL_CTX_free(fixture.client); + ERR_print_errors_fp(stderr); +} + +#define SETUP_CIPHERLIST_TEST_FIXTURE() \ + SETUP_TEST_FIXTURE(CIPHERLIST_TEST_FIXTURE, set_up) + +#define EXECUTE_CIPHERLIST_TEST() \ + EXECUTE_TEST(execute_test, tear_down) + +static int test_default_cipherlist_implicit() +{ + SETUP_CIPHERLIST_TEST_FIXTURE(); + EXECUTE_CIPHERLIST_TEST(); +} + +static int test_default_cipherlist_explicit() +{ + SETUP_CIPHERLIST_TEST_FIXTURE(); + OPENSSL_assert(SSL_CTX_set_cipher_list(fixture.server, "DEFAULT")); + OPENSSL_assert(SSL_CTX_set_cipher_list(fixture.client, "DEFAULT")); + EXECUTE_CIPHERLIST_TEST(); +} + +int main(int argc, char **argv) +{ + int result = 0; + + ADD_TEST(test_default_cipherlist_implicit); + ADD_TEST(test_default_cipherlist_explicit); + + result = run_tests(argv[0]); + + return result; +} diff --git a/test/recipes/80-test_cipherlist.t b/test/recipes/80-test_cipherlist.t new file mode 100644 index 0000000000..af9ac33733 --- /dev/null +++ b/test/recipes/80-test_cipherlist.t @@ -0,0 +1,18 @@ +#! /usr/bin/perl + +use strict; +use warnings; + +use OpenSSL::Test::Simple; +use OpenSSL::Test; +use OpenSSL::Test::Utils qw(alldisabled available_protocols); + +setup("test_cipherlist"); + +my $no_anytls = alldisabled(available_protocols("tls")); + +# If we have no protocols, then we also have no supported ciphers. +plan skip_all => "No SSL/TLS protocol is supported by this OpenSSL build." + if $no_anytls; + +simple_test("test_cipherlist", "cipherlist_test", "cipherlist"); diff --git a/test/recipes/80-test_ssl_old.t b/test/recipes/80-test_ssl_old.t index bf7fb39a42..61fc423424 100644 --- a/test/recipes/80-test_ssl_old.t +++ b/test/recipes/80-test_ssl_old.t @@ -78,7 +78,6 @@ my $client_sess="client.ss"; # new format in ssl_test.c and add recipes to 80-test_ssl_new.t instead. plan tests => 1 # For testss - + 1 # For ssltest_old -test_cipherlist + 14 # For the first testssl + 16 # For the first testsslproxy + 16 # For the second testsslproxy @@ -96,21 +95,15 @@ subtest 'test_ss' => sub { } }; -my $check = ok(run(test(["ssltest_old","-test_cipherlist"])), "running ssltest_old"); +note('test_ssl -- key U'); +testssl("keyU.ss", $Ucert, $CAcert); - SKIP: { - skip "ssltest_old ended with error, skipping the rest", 3 - if !$check; - - note('test_ssl -- key U'); - testssl("keyU.ss", $Ucert, $CAcert); +note('test_ssl -- key P1'); +testsslproxy("keyP1.ss", "certP1.ss", "intP1.ss", "AB"); - note('test_ssl -- key P1'); - testsslproxy("keyP1.ss", "certP1.ss", "intP1.ss", "AB"); +note('test_ssl -- key P2'); +testsslproxy("keyP2.ss", "certP2.ss", "intP2.ss", "BC"); - note('test_ssl -- key P2'); - testsslproxy("keyP2.ss", "certP2.ss", "intP2.ss", "BC"); - } # ----------- # subtest functions diff --git a/test/run_tests.pl b/test/run_tests.pl index b8281acceb..158eaf9bab 100644 --- a/test/run_tests.pl +++ b/test/run_tests.pl @@ -39,7 +39,7 @@ if (@ARGV) { @tests = @ARGV; } my $list_mode = scalar(grep /^list$/, @tests) != 0; -if (grep /^alltests|list$/, @tests) { +if (grep /^(alltests|list)$/, @tests) { @tests = grep { basename($_) =~ /^[0-9][0-9]-[^\.]*\.t$/ } glob(catfile($recipesdir,"*.t")); diff --git a/test/ssltest_old.c b/test/ssltest_old.c index 2fd7da824a..c7f3e1872d 100644 --- a/test/ssltest_old.c +++ b/test/ssltest_old.c @@ -799,7 +799,6 @@ int doit_localhost(SSL *s_ssl, SSL *c_ssl, int family, int doit_biopair(SSL *s_ssl, SSL *c_ssl, long bytes, clock_t *s_time, clock_t *c_time); int doit(SSL *s_ssl, SSL *c_ssl, long bytes); -static int do_test_cipherlist(void); static void sv_usage(void) { @@ -870,10 +869,6 @@ static void sv_usage(void) fprintf(stderr, " -time - measure processor time used by client and server\n"); fprintf(stderr, " -zlib - use zlib compression\n"); - fprintf(stderr, - " -test_cipherlist - Verifies the order of the ssl cipher lists.\n" - " When this option is requested, the cipherlist\n" - " tests are run instead of handshake tests.\n"); #ifndef OPENSSL_NO_NEXTPROTONEG fprintf(stderr, " -npn_client - have client side offer NPN\n"); fprintf(stderr, " -npn_server - have server side offer NPN\n"); @@ -1102,7 +1097,6 @@ int main(int argc, char *argv[]) COMP_METHOD *cm = NULL; STACK_OF(SSL_COMP) *ssl_comp_methods = NULL; #endif - int test_cipherlist = 0; #ifdef OPENSSL_FIPS int fips_mode = 0; #endif @@ -1315,11 +1309,9 @@ int main(int argc, char *argv[]) app_verify_arg.app_verify = 1; } else if (strcmp(*argv, "-proxy") == 0) { app_verify_arg.allow_proxy_certs = 1; - } else if (strcmp(*argv, "-test_cipherlist") == 0) { - test_cipherlist = 1; } #ifndef OPENSSL_NO_NEXTPROTONEG - else if (strcmp(*argv, "-npn_client") == 0) { + else if (strcmp(*argv, "-npn_client") == 0) { npn_client = 1; } else if (strcmp(*argv, "-npn_server") == 0) { npn_server = 1; @@ -1454,22 +1446,6 @@ int main(int argc, char *argv[]) goto end; } - /* - * test_cipherlist prevails over protocol switch: we test the cipherlist - * for all enabled protocols. - */ - if (test_cipherlist == 1) { - /* - * ensure that the cipher list are correctly sorted and exit - */ - fprintf(stdout, "Testing cipherlist order only. Ignoring all " - "other options.\n"); - if (do_test_cipherlist() == 0) - EXIT(1); - ret = 0; - goto end; - } - if (ssl3 + tls1 + dtls + dtls1 + dtls12 > 1) { fprintf(stderr, "At most one of -ssl3, -tls1, -dtls, -dtls1 or -dtls12 should " "be requested.\n"); @@ -3726,33 +3702,3 @@ static unsigned int psk_server_callback(SSL *ssl, const char *identity, return psk_len; } #endif - -static int do_test_cipherlist(void) -{ -#ifndef OPENSSL_NO_TLS - int i = 0; - const SSL_METHOD *meth; - const SSL_CIPHER *ci, *tci = NULL; - - /* - * This is required because ssltest "cheats" and uses internal headers to - * call functions, thus avoiding auto-init - */ - OPENSSL_init_crypto(0, NULL); - OPENSSL_init_ssl(0, NULL); - - meth = TLS_method(); - tci = NULL; - while ((ci = meth->get_cipher(i++)) != NULL) { - if (tci != NULL) - if (ci->id >= tci->id) { - fprintf(stderr, "testing SSLv3 cipher list order: "); - fprintf(stderr, "failed %x vs. %x\n", ci->id, tci->id); - return 0; - } - tci = ci; - } -#endif - - return 1; -} -- 2.25.1