From c423ecaa7f6d0cc77c4f121c6de7d585439bca8f Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Fri, 9 Feb 2018 10:19:14 +0000 Subject: [PATCH] Fixes for no-tls1_2 and no-tls1_2-method The no-tls1_2 option does not work properly in conjunction with TLSv1.3 being enabled (which is now the default). This commit fixes the issues. Reviewed-by: Richard Levitte (Merged from https://github.com/openssl/openssl/pull/5301) --- test/cipherlist_test.c | 22 +++++++++----- test/clienthellotest.c | 5 ++++ test/recipes/70-test_key_share.t | 44 +++++++++++++++------------- test/recipes/70-test_sslcertstatus.t | 3 +- test/recipes/70-test_sslextension.t | 41 ++++++++++++++------------ test/recipes/70-test_sslmessages.t | 3 +- test/recipes/70-test_sslsigalgs.t | 17 ++++++----- test/recipes/80-test_ssl_new.t | 3 ++ test/sslapitest.c | 24 +++++++++++---- 9 files changed, 101 insertions(+), 61 deletions(-) diff --git a/test/cipherlist_test.c b/test/cipherlist_test.c index 336bdb0318..b4e6ea26d6 100644 --- a/test/cipherlist_test.c +++ b/test/cipherlist_test.c @@ -105,20 +105,23 @@ static const uint32_t default_ciphers_in_order[] = { # endif #endif /* !OPENSSL_NO_TLS1_2 */ -#ifndef OPENSSL_NO_EC +#if !defined(OPENSSL_NO_TLS1_2) || defined(OPENSSL_NO_TLS1_3) + /* These won't be usable if TLSv1.3 is available but TLSv1.2 isn't */ +# 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 +# endif + #ifndef OPENSSL_NO_DH TLS1_CK_DHE_RSA_WITH_AES_256_SHA, -#endif -#ifndef OPENSSL_NO_EC +# 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 +# endif +# ifndef OPENSSL_NO_DH TLS1_CK_DHE_RSA_WITH_AES_128_SHA, -#endif +# endif +#endif /* !defined(OPENSSL_NO_TLS1_2) || defined(OPENSSL_NO_TLS1_3) */ #ifndef OPENSSL_NO_TLS1_2 TLS1_CK_RSA_WITH_AES_256_GCM_SHA384, @@ -135,8 +138,11 @@ static const uint32_t default_ciphers_in_order[] = { TLS1_CK_RSA_WITH_AES_256_SHA256, TLS1_CK_RSA_WITH_AES_128_SHA256, #endif +#if !defined(OPENSSL_NO_TLS1_2) || defined(OPENSSL_NO_TLS1_3) + /* These won't be usable if TLSv1.3 is available but TLSv1.2 isn't */ TLS1_CK_RSA_WITH_AES_256_SHA, TLS1_CK_RSA_WITH_AES_128_SHA, +#endif }; static int test_default_cipherlist(SSL_CTX *ctx) diff --git a/test/clienthellotest.c b/test/clienthellotest.c index f3e9588e51..32103d0c35 100644 --- a/test/clienthellotest.c +++ b/test/clienthellotest.c @@ -81,9 +81,14 @@ static int test_client_hello(int currtest) switch(currtest) { case TEST_SET_SESSION_TICK_DATA_VER_NEG: +#if !defined(OPENSSL_NO_TLS1_3) && defined(OPENSSL_NO_TLS1_2) + /* TLSv1.3 is enabled and TLSv1.2 is disabled so can't do this test */ + return 1; +#else /* Testing for session tickets <= TLS1.2; not relevant for 1.3 */ if (!TEST_true(SSL_CTX_set_max_proto_version(ctx, TLS1_2_VERSION))) goto end; +#endif break; case TEST_ADD_PADDING_AND_PSK: diff --git a/test/recipes/70-test_key_share.t b/test/recipes/70-test_key_share.t index f4cc764be2..512a3bc807 100644 --- a/test/recipes/70-test_key_share.t +++ b/test/recipes/70-test_key_share.t @@ -199,26 +199,30 @@ $testtype = TRAILING_DATA; $proxy->start(); ok(TLSProxy::Message->fail(), "key_share trailing data in ServerHello"); -#Test 20: key_share should not be sent if the client is not capable of -# negotiating TLSv1.3 -$proxy->clear(); -$proxy->filter(undef); -$proxy->clientflags("-no_tls1_3"); -$proxy->start(); -my $clienthello = $proxy->message_list->[0]; -ok(TLSProxy::Message->success() - && !defined $clienthello->extension_data->{TLSProxy::Message::EXT_KEY_SHARE}, - "No key_share for TLS<=1.2 client"); -$proxy->filter(\&modify_key_shares_filter); - -#Test 21: A server not capable of negotiating TLSv1.3 should not attempt to -# process a key_share -$proxy->clear(); -$direction = CLIENT_TO_SERVER; -$testtype = NO_ACCEPTABLE_KEY_SHARES; -$proxy->serverflags("-no_tls1_3"); -$proxy->start(); -ok(TLSProxy::Message->success(), "Ignore key_share for TLS<=1.2 server"); +SKIP: { + skip "No TLSv1.2 support in this OpenSSL build", 2 if disabled("tls1_2"); + + #Test 20: key_share should not be sent if the client is not capable of + # negotiating TLSv1.3 + $proxy->clear(); + $proxy->filter(undef); + $proxy->clientflags("-no_tls1_3"); + $proxy->start(); + my $clienthello = $proxy->message_list->[0]; + ok(TLSProxy::Message->success() + && !defined $clienthello->extension_data->{TLSProxy::Message::EXT_KEY_SHARE}, + "No key_share for TLS<=1.2 client"); + $proxy->filter(\&modify_key_shares_filter); + + #Test 21: A server not capable of negotiating TLSv1.3 should not attempt to + # process a key_share + $proxy->clear(); + $direction = CLIENT_TO_SERVER; + $testtype = NO_ACCEPTABLE_KEY_SHARES; + $proxy->serverflags("-no_tls1_3"); + $proxy->start(); + ok(TLSProxy::Message->success(), "Ignore key_share for TLS<=1.2 server"); +} #Test 22: The server sending an HRR but not requesting a new key_share should # fail diff --git a/test/recipes/70-test_sslcertstatus.t b/test/recipes/70-test_sslcertstatus.t index da8a3e51e5..96a46ce5f4 100644 --- a/test/recipes/70-test_sslcertstatus.t +++ b/test/recipes/70-test_sslcertstatus.t @@ -27,7 +27,8 @@ plan skip_all => "$test_name needs the ocsp feature enabled" if disabled("ocsp"); plan skip_all => "$test_name needs TLS enabled" - if alldisabled(available_protocols("tls")); + if alldisabled(available_protocols("tls")) + || (!disabled("tls1_3") && disabled("tls1_2")); $ENV{OPENSSL_ia32cap} = '~0x200000200000000'; my $proxy = TLSProxy::Proxy->new( diff --git a/test/recipes/70-test_sslextension.t b/test/recipes/70-test_sslextension.t index d185bda78b..142ce0e64d 100644 --- a/test/recipes/70-test_sslextension.t +++ b/test/recipes/70-test_sslextension.t @@ -26,6 +26,9 @@ plan skip_all => "$test_name needs the sock feature enabled" plan skip_all => "$test_name needs TLS enabled" if alldisabled(available_protocols("tls")); +my $no_below_tls13 = alldisabled(("tls1", "tls1_1", "tls1_2")) + || (!disabled("tls1_3") && disabled("tls1_2")); + use constant { UNSOLICITED_SERVER_NAME => 0, UNSOLICITED_SERVER_NAME_TLS13 => 1, @@ -37,16 +40,12 @@ my $testtype; $ENV{OPENSSL_ia32cap} = '~0x200000200000000'; my $proxy = TLSProxy::Proxy->new( - \&extension_filter, + \&inject_duplicate_extension_clienthello, cmdstr(app(["openssl"]), display => 1), srctop_file("apps", "server.pem"), (!$ENV{HARNESS_ACTIVE} || $ENV{HARNESS_VERBOSE}) ); -# Test 1: Sending a zero length extension block should pass -$proxy->start() or plan skip_all => "Unable to start up Proxy for tests"; -plan tests => 7; -ok(TLSProxy::Message->success, "Zero extension length test"); sub extension_filter { @@ -79,7 +78,6 @@ sub extension_filter } } -# Test 2-3: Sending a duplicate extension should fail. sub inject_duplicate_extension { my ($proxy, $message_type) = @_; @@ -119,16 +117,6 @@ sub inject_duplicate_extension_serverhello inject_duplicate_extension($proxy, TLSProxy::Message::MT_SERVER_HELLO); } -$proxy->clear(); -$proxy->filter(\&inject_duplicate_extension_clienthello); -$proxy->start(); -ok(TLSProxy::Message->fail(), "Duplicate ClientHello extension"); - -$proxy->clear(); -$proxy->filter(\&inject_duplicate_extension_serverhello); -$proxy->start(); -ok(TLSProxy::Message->fail(), "Duplicate ServerHello extension"); - sub inject_unsolicited_extension { my $proxy = shift; @@ -162,8 +150,25 @@ sub inject_unsolicited_extension $message->repack(); } +# Test 1-2: Sending a duplicate extension should fail. +$proxy->start() or plan skip_all => "Unable to start up Proxy for tests"; +plan tests => 7; +ok(TLSProxy::Message->fail(), "Duplicate ClientHello extension"); + +$proxy->clear(); +$proxy->filter(\&inject_duplicate_extension_serverhello); +$proxy->start(); +ok(TLSProxy::Message->fail(), "Duplicate ServerHello extension"); + SKIP: { - skip "TLS <= 1.2 disabled", 2 if alldisabled(("tls1", "tls1_1", "tls1_2")); + skip "TLS <= 1.2 disabled", 3 if $no_below_tls13; + + #Test 3: Sending a zero length extension block should pass + $proxy->clear(); + $proxy->filter(\&extension_filter); + $proxy->start(); + ok(TLSProxy::Message->success, "Zero extension length test"); + #Test 4: Inject an unsolicited extension (<= TLSv1.2) $proxy->clear(); $proxy->filter(\&inject_unsolicited_extension); @@ -183,7 +188,7 @@ SKIP: { SKIP: { skip "TLS <= 1.2 or CT disabled", 1 - if alldisabled(("tls1", "tls1_1", "tls1_2")) || disabled("ct"); + if $no_below_tls13 || disabled("ct"); #Test 6: Same as above for the SCT extension which has special handling $proxy->clear(); $testtype = UNSOLICITED_SCT; diff --git a/test/recipes/70-test_sslmessages.t b/test/recipes/70-test_sslmessages.t index abd6cc7d87..78d9737e54 100644 --- a/test/recipes/70-test_sslmessages.t +++ b/test/recipes/70-test_sslmessages.t @@ -26,7 +26,8 @@ plan skip_all => "$test_name needs the sock feature enabled" if disabled("sock"); plan skip_all => "$test_name needs TLS enabled" - if alldisabled(available_protocols("tls")); + if alldisabled(available_protocols("tls")) + || (!disabled("tls1_3") && disabled("tls1_2")); $ENV{OPENSSL_ia32cap} = '~0x200000200000000'; $ENV{CTLOG_FILE} = srctop_file("test", "ct", "log_list.conf"); diff --git a/test/recipes/70-test_sslsigalgs.t b/test/recipes/70-test_sslsigalgs.t index 35531c634b..fca53aca25 100644 --- a/test/recipes/70-test_sslsigalgs.t +++ b/test/recipes/70-test_sslsigalgs.t @@ -216,13 +216,16 @@ SKIP: { "DSA/SHA2 sigalg sent for 1.3-only ClientHello"); #Test 18: signature_algorithms with backwards compatible ClientHello - $testtype = COMPAT_SIGALGS; - $dsa_status = $sha1_status = $sha224_status = 0; - $proxy->clear(); - $proxy->filter(\&modify_sigalgs_filter); - $proxy->start(); - ok($dsa_status && $sha1_status && $sha224_status, - "DSA sigalg not sent for compat ClientHello"); + SKIP: { + skip "TLSv1.2 disabled", 1 if disabled("tls1_2"); + $testtype = COMPAT_SIGALGS; + $dsa_status = $sha1_status = $sha224_status = 0; + $proxy->clear(); + $proxy->filter(\&modify_sigalgs_filter); + $proxy->start(); + ok($dsa_status && $sha1_status && $sha224_status, + "DSA sigalg not sent for compat ClientHello"); + } } SKIP: { diff --git a/test/recipes/80-test_ssl_new.t b/test/recipes/80-test_ssl_new.t index 26bcb39c7b..bb60168993 100644 --- a/test/recipes/80-test_ssl_new.t +++ b/test/recipes/80-test_ssl_new.t @@ -40,6 +40,7 @@ my $is_default_dtls = (!disabled("dtls1") && !disabled("dtls1_2")); my @all_pre_tls1_3 = ("ssl3", "tls1", "tls1_1", "tls1_2"); my $no_tls = alldisabled(available_protocols("tls")); +my $no_tls_below1_3 = $no_tls || (disabled("tls1_2") && !disabled("tls1_3")); my $no_pre_tls1_3 = alldisabled(@all_pre_tls1_3); my $no_dtls = alldisabled(available_protocols("dtls")); my $no_npn = disabled("nextprotoneg"); @@ -73,6 +74,7 @@ my %conf_dependent_tests = ( # configurations. Default is $no_tls but some tests have different skip # conditions. my %skip = ( + "06-sni-ticket.conf" => $no_tls_below1_3, "07-dtls-protocol-version.conf" => $no_dtls, "08-npn.conf" => (disabled("tls1") && disabled("tls1_1") && disabled("tls1_2")) || $no_npn, @@ -87,6 +89,7 @@ my %skip = ( "14-curves.conf" => disabled("tls1_2") || $no_ec || $no_ec2m, "15-certstatus.conf" => $no_tls || $no_ocsp, "16-dtls-certstatus.conf" => $no_dtls || $no_ocsp, + "17-renegotiate.conf" => $no_tls_below1_3, "18-dtls-renegotiate.conf" => $no_dtls, "19-mac-then-encrypt.conf" => $no_pre_tls1_3, "20-cert-select.conf" => disabled("tls1_2") || $no_ec, diff --git a/test/sslapitest.c b/test/sslapitest.c index 483941ac5c..c7b06e0076 100644 --- a/test/sslapitest.c +++ b/test/sslapitest.c @@ -254,6 +254,7 @@ static int test_keylog_output(char *buffer, const SSL *ssl, return 1; } +#if !defined(OPENSSL_NO_TLS1_2) || defined(OPENSSL_NO_TLS1_3) static int test_keylog(void) { SSL_CTX *cctx = NULL, *sctx = NULL; @@ -330,6 +331,7 @@ end: return testresult; } +#endif #ifndef OPENSSL_NO_TLS1_3 static int test_keylog_no_master_key(void) @@ -1273,6 +1275,7 @@ static int test_ssl_bio_change_wbio(void) return execute_test_ssl_bio(0, CHANGE_WBIO); } +#if !defined(OPENSSL_NO_TLS1_2) || defined(OPENSSL_NO_TLS1_3) typedef struct { /* The list of sig algs */ const int *list; @@ -1287,25 +1290,25 @@ typedef struct { } sigalgs_list; static const int validlist1[] = {NID_sha256, EVP_PKEY_RSA}; -#ifndef OPENSSL_NO_EC +# ifndef OPENSSL_NO_EC static const int validlist2[] = {NID_sha256, EVP_PKEY_RSA, NID_sha512, EVP_PKEY_EC}; static const int validlist3[] = {NID_sha512, EVP_PKEY_EC}; -#endif +# endif static const int invalidlist1[] = {NID_undef, EVP_PKEY_RSA}; static const int invalidlist2[] = {NID_sha256, NID_undef}; static const int invalidlist3[] = {NID_sha256, EVP_PKEY_RSA, NID_sha256}; static const int invalidlist4[] = {NID_sha256}; static const sigalgs_list testsigalgs[] = { {validlist1, OSSL_NELEM(validlist1), NULL, 1, 1}, -#ifndef OPENSSL_NO_EC +# ifndef OPENSSL_NO_EC {validlist2, OSSL_NELEM(validlist2), NULL, 1, 1}, {validlist3, OSSL_NELEM(validlist3), NULL, 1, 0}, -#endif +# endif {NULL, 0, "RSA+SHA256", 1, 1}, -#ifndef OPENSSL_NO_EC +# ifndef OPENSSL_NO_EC {NULL, 0, "RSA+SHA256:ECDSA+SHA512", 1, 1}, {NULL, 0, "ECDSA+SHA512", 1, 0}, -#endif +# endif {invalidlist1, OSSL_NELEM(invalidlist1), NULL, 0, 0}, {invalidlist2, OSSL_NELEM(invalidlist2), NULL, 0, 0}, {invalidlist3, OSSL_NELEM(invalidlist3), NULL, 0, 0}, @@ -1401,6 +1404,7 @@ static int test_set_sigalgs(int idx) return testresult; } +#endif #ifndef OPENSSL_NO_TLS1_3 @@ -2778,6 +2782,12 @@ static int test_custom_exts(int tst) SSL_SESSION *sess = NULL; unsigned int context; +#if defined(OPENSSL_NO_TLS1_2) && !defined(OPENSSL_NO_TLS1_3) + /* Skip tests for TLSv1.2 and below in this case */ + if (tst < 3) + return 1; +#endif + /* Reset callback counters */ clntaddoldcb = clntparseoldcb = srvaddoldcb = srvparseoldcb = 0; clntaddnewcb = clntparsenewcb = srvaddnewcb = srvparsenewcb = 0; @@ -3389,8 +3399,10 @@ int setup_tests(void) ADD_TEST(test_ssl_bio_pop_ssl_bio); ADD_TEST(test_ssl_bio_change_rbio); ADD_TEST(test_ssl_bio_change_wbio); +#if !defined(OPENSSL_NO_TLS1_2) || defined(OPENSSL_NO_TLS1_3) ADD_ALL_TESTS(test_set_sigalgs, OSSL_NELEM(testsigalgs) * 2); ADD_TEST(test_keylog); +#endif #ifndef OPENSSL_NO_TLS1_3 ADD_TEST(test_keylog_no_master_key); #endif -- GitLab