From b0031e5dc2c8c99a6c04bc7625aa00d3d20a59a5 Mon Sep 17 00:00:00 2001 From: Kurt Roeckx Date: Thu, 2 Jan 2020 22:53:32 +0100 Subject: [PATCH] Check that the default signature type is allowed TLS < 1.2 has fixed signature algorithms: MD5+SHA1 for RSA and SHA1 for the others. TLS 1.2 sends a list of supported ciphers, but allows not sending it in which case SHA1 is used. TLS 1.3 makes sending the list mandatory. When we didn't receive a list from the client, we always used the defaults without checking that they are allowed by the configuration. Reviewed-by: Paul Dale GH: #10784 --- ssl/ssl_local.h | 2 +- ssl/t1_lib.c | 16 ++++++--- test/recipes/70-test_sslsigalgs.t | 57 +++++++++++++++++++++---------- 3 files changed, 52 insertions(+), 23 deletions(-) diff --git a/ssl/ssl_local.h b/ssl/ssl_local.h index 14515cadfe336..43b0623a0b9d5 100644 --- a/ssl/ssl_local.h +++ b/ssl/ssl_local.h @@ -2606,7 +2606,7 @@ __owur int tls_check_sigalg_curve(const SSL *s, int curve); # endif __owur int tls12_check_peer_sigalg(SSL *s, uint16_t, EVP_PKEY *pkey); __owur int ssl_set_client_disabled(SSL *s); -__owur int ssl_cipher_disabled(SSL *s, const SSL_CIPHER *c, int op, int echde); +__owur int ssl_cipher_disabled(const SSL *s, const SSL_CIPHER *c, int op, int echde); __owur int ssl_handshake_hash(SSL *s, unsigned char *out, size_t outlen, size_t *hashlen); diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index afb72857e5483..0504f6bba1dde 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -22,6 +22,7 @@ #include static const SIGALG_LOOKUP *find_sig_alg(SSL *s, X509 *x, EVP_PKEY *pkey); +static int tls12_sigalg_allowed(const SSL *s, int op, const SIGALG_LOOKUP *lu); SSL3_ENC_METHOD const TLSv1_enc_data = { tls1_enc, @@ -928,8 +929,11 @@ static int rsa_pss_check_min_key_size(const RSA *rsa, const SIGALG_LOOKUP *lu) } /* - * Return a signature algorithm for TLS < 1.2 where the signature type - * is fixed by the certificate type. + * Returns a signature algorithm when the peer did not send a list of supported + * signature algorithms. The signature algorithm is fixed for the certificate + * type. |idx| is a certificate type index (SSL_PKEY_*). When |idx| is -1 the + * certificate type from |s| will be used. + * Returns the signature algorithm to use, or NULL on error. */ static const SIGALG_LOOKUP *tls1_get_legacy_sigalg(const SSL *s, int idx) { @@ -972,8 +976,12 @@ static const SIGALG_LOOKUP *tls1_get_legacy_sigalg(const SSL *s, int idx) if (!tls1_lookup_md(lu, NULL)) return NULL; + if (!tls12_sigalg_allowed(s, SSL_SECOP_SIGALG_SUPPORTED, lu)) + return NULL; return lu; } + if (!tls12_sigalg_allowed(s, SSL_SECOP_SIGALG_SUPPORTED, &legacy_rsa_sigalg)) + return NULL; return &legacy_rsa_sigalg; } /* Set peer sigalg based key type */ @@ -1255,7 +1263,7 @@ int ssl_set_client_disabled(SSL *s) * * Returns 1 when it's disabled, 0 when enabled. */ -int ssl_cipher_disabled(SSL *s, const SSL_CIPHER *c, int op, int ecdhe) +int ssl_cipher_disabled(const SSL *s, const SSL_CIPHER *c, int op, int ecdhe) { if (c->algorithm_mkey & s->s3.tmp.mask_k || c->algorithm_auth & s->s3.tmp.mask_a) @@ -1635,7 +1643,7 @@ SSL_TICKET_STATUS tls_decrypt_ticket(SSL *s, const unsigned char *etick, } /* Check to see if a signature algorithm is allowed */ -static int tls12_sigalg_allowed(SSL *s, int op, const SIGALG_LOOKUP *lu) +static int tls12_sigalg_allowed(const SSL *s, int op, const SIGALG_LOOKUP *lu) { unsigned char sigalgstr[2]; int secbits; diff --git a/test/recipes/70-test_sslsigalgs.t b/test/recipes/70-test_sslsigalgs.t index 43cad5f4d771d..98482079b32cd 100644 --- a/test/recipes/70-test_sslsigalgs.t +++ b/test/recipes/70-test_sslsigalgs.t @@ -53,7 +53,7 @@ use constant { #Test 1: Default sig algs should succeed $proxy->start() or plan skip_all => "Unable to start up Proxy for tests"; -plan tests => 22; +plan tests => 24; ok(TLSProxy::Message->success, "Default sigalgs"); my $testtype; @@ -132,19 +132,40 @@ SKIP: { } SKIP: { - skip "EC or TLSv1.2 disabled", 8 if disabled("tls1_2") || disabled("ec"); + skip "EC or TLSv1.2 disabled", 10 if disabled("tls1_2") || disabled("ec"); $proxy->filter(\&sigalgs_filter); - #Test 10: Sending no sig algs extension in TLSv1.2 should succeed + #Test 10: Sending no sig algs extension in TLSv1.2 should succeed at + # security level 1 $proxy->clear(); $testtype = NO_SIG_ALGS_EXT; - $proxy->clientflags("-no_tls1_3"); - $proxy->ciphers("ECDHE-RSA-AES128-SHA"); + $proxy->clientflags("-no_tls1_3 -cipher DEFAULT\@SECLEVEL=1"); + $proxy->ciphers("ECDHE-RSA-AES128-SHA\@SECLEVEL=1"); + $proxy->start(); + ok(TLSProxy::Message->success, "No TLSv1.2 sigalgs seclevel 1"); + + #Test 11: Sending no sig algs extension in TLSv1.2 should fail at security + # level 2 since it will try to use SHA1. Testing client at level 1, + # server level 2. + $proxy->clear(); + $testtype = NO_SIG_ALGS_EXT; + $proxy->clientflags("-tls1_2 -cipher DEFAULT\@SECLEVEL=1"); + $proxy->ciphers("DEFAULT\@SECLEVEL=2"); + $proxy->start(); + ok(TLSProxy::Message->fail, "No TLSv1.2 sigalgs server seclevel 2"); + + #Test 12: Sending no sig algs extension in TLSv1.2 should fail at security + # level 2 since it will try to use SHA1. Testing client at level 2, + # server level 1. + $proxy->clear(); + $testtype = NO_SIG_ALGS_EXT; + $proxy->clientflags("-tls1_2 -cipher DEFAULT\@SECLEVEL=2"); + $proxy->ciphers("DEFAULT\@SECLEVEL=1"); $proxy->start(); - ok(TLSProxy::Message->success, "No TLSv1.2 sigalgs"); + ok(TLSProxy::Message->fail, "No TLSv1.2 sigalgs client seclevel 2"); - #Test 11: Sending an empty sig algs extension in TLSv1.2 should fail + #Test 13: Sending an empty sig algs extension in TLSv1.2 should fail $proxy->clear(); $testtype = EMPTY_SIG_ALGS_EXT; $proxy->clientflags("-no_tls1_3"); @@ -152,7 +173,7 @@ SKIP: { $proxy->start(); ok(TLSProxy::Message->fail, "Empty TLSv1.2 sigalgs"); - #Test 12: Sending a list with no recognised sig algs in TLSv1.2 should fail + #Test 14: Sending a list with no recognised sig algs in TLSv1.2 should fail $proxy->clear(); $testtype = NO_KNOWN_SIG_ALGS; $proxy->clientflags("-no_tls1_3"); @@ -160,7 +181,7 @@ SKIP: { $proxy->start(); ok(TLSProxy::Message->fail, "No known TLSv1.3 sigalgs"); - #Test 13: Sending a sig algs list without pss for an RSA cert in TLSv1.2 + #Test 15: Sending a sig algs list without pss for an RSA cert in TLSv1.2 # should succeed $proxy->clear(); $testtype = NO_PSS_SIG_ALGS; @@ -169,7 +190,7 @@ SKIP: { $proxy->start(); ok(TLSProxy::Message->success, "No PSS TLSv1.2 sigalgs"); - #Test 14: Sending only TLSv1.3 PSS sig algs in TLSv1.2 should succeed + #Test 16: Sending only TLSv1.3 PSS sig algs in TLSv1.2 should succeed $proxy->clear(); $testtype = PSS_ONLY_SIG_ALGS; $proxy->serverflags("-no_tls1_3"); @@ -177,7 +198,7 @@ SKIP: { $proxy->start(); ok(TLSProxy::Message->success, "PSS only sigalgs in TLSv1.2"); - #Test 15: Responding with a sig alg we did not send in TLSv1.2 should fail + #Test 17: Responding with a sig alg we did not send in TLSv1.2 should fail # We send rsa_pkcs1_sha256 and respond with rsa_pss_rsae_sha256 # TODO(TLS1.3): Add a similar test to the TLSv1.3 section above # when we have an API capable of configuring the TLSv1.3 sig algs @@ -188,7 +209,7 @@ SKIP: { $proxy->start(); ok(TLSProxy::Message->fail, "Sigalg we did not send in TLSv1.2"); - #Test 16: Sending a valid sig algs list but not including a sig type that + #Test 18: Sending a valid sig algs list but not including a sig type that # matches the certificate should fail in TLSv1.2 $proxy->clear(); $proxy->clientflags("-no_tls1_3 -sigalgs ECDSA+SHA256"); @@ -198,7 +219,7 @@ SKIP: { ok(TLSProxy::Message->fail, "No matching TLSv1.2 sigalgs"); $proxy->filter(\&sigalgs_filter); - #Test 17: No sig algs extension, ECDSA cert, TLSv1.2 should succeed + #Test 19: No sig algs extension, ECDSA cert, TLSv1.2 should succeed $proxy->clear(); $testtype = NO_SIG_ALGS_EXT; $proxy->clientflags("-no_tls1_3"); @@ -214,7 +235,7 @@ SKIP: { my ($dsa_status, $sha1_status, $sha224_status); SKIP: { skip "TLSv1.3 disabled", 2 if disabled("tls1_3") || disabled("dsa"); - #Test 18: signature_algorithms with 1.3-only ClientHello + #Test 20: signature_algorithms with 1.3-only ClientHello $testtype = PURE_SIGALGS; $dsa_status = $sha1_status = $sha224_status = 0; $proxy->clear(); @@ -224,7 +245,7 @@ SKIP: { ok($dsa_status && $sha1_status && $sha224_status, "DSA/SHA2 sigalg sent for 1.3-only ClientHello"); - #Test 19: signature_algorithms with backwards compatible ClientHello + #Test 21: signature_algorithms with backwards compatible ClientHello SKIP: { skip "TLSv1.2 disabled", 1 if disabled("tls1_2"); $testtype = COMPAT_SIGALGS; @@ -239,21 +260,21 @@ SKIP: { SKIP: { skip "TLSv1.3 disabled", 3 if disabled("tls1_3"); - #Test 20: Insert signature_algorithms_cert that match normal sigalgs + #Test 22: Insert signature_algorithms_cert that match normal sigalgs $testtype = SIGALGS_CERT_ALL; $proxy->clear(); $proxy->filter(\&modify_sigalgs_cert_filter); $proxy->start(); ok(TLSProxy::Message->success, "sigalgs_cert in TLSv1.3"); - #Test 21: Insert signature_algorithms_cert that forces PKCS#1 cert + #Test 23: Insert signature_algorithms_cert that forces PKCS#1 cert $testtype = SIGALGS_CERT_PKCS; $proxy->clear(); $proxy->filter(\&modify_sigalgs_cert_filter); $proxy->start(); ok(TLSProxy::Message->success, "sigalgs_cert in TLSv1.3 with PKCS#1 cert"); - #Test 22: Insert signature_algorithms_cert that fails + #Test 24: Insert signature_algorithms_cert that fails $testtype = SIGALGS_CERT_INVALID; $proxy->clear(); $proxy->filter(\&modify_sigalgs_cert_filter);