From 5d62fd7cb2d7e1abc8c9a09cbc05744a7d346775 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Wed, 26 Apr 2017 10:38:32 +0100 Subject: [PATCH] Add a ciphersuite config sanity check for clients Ensure that there are ciphersuites enabled for the maximum supported version we are claiming in the ClientHello. Reviewed-by: Rich Salz (Merged from https://github.com/openssl/openssl/pull/3334) --- ssl/statem/statem_clnt.c | 22 ++++++++++++++-- test/ssl-tests/14-curves.conf | 29 ++++++++++++++++++++++ test/ssl-tests/14-curves.conf.in | 5 ++-- test/ssl-tests/17-renegotiate.conf | 8 +++--- test/ssl-tests/17-renegotiate.conf.in | 8 +++--- test/ssl-tests/19-mac-then-encrypt.conf | 2 +- test/ssl-tests/19-mac-then-encrypt.conf.in | 2 +- test/ssl-tests/20-cert-select.conf | 3 +++ test/ssl-tests/20-cert-select.conf.in | 5 +++- 9 files changed, 69 insertions(+), 15 deletions(-) diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c index 9f4a719fa1..f6b6429391 100644 --- a/ssl/statem/statem_clnt.c +++ b/ssl/statem/statem_clnt.c @@ -3470,7 +3470,7 @@ int ssl_do_client_cert_cb(SSL *s, X509 **px509, EVP_PKEY **ppkey) int ssl_cipher_list_to_bytes(SSL *s, STACK_OF(SSL_CIPHER) *sk, WPACKET *pkt) { int i; - size_t totlen = 0, len, maxlen; + size_t totlen = 0, len, maxlen, maxverok = 0; int empty_reneg_info_scsv = !s->renegotiate; /* Set disabled masks for this session */ ssl_set_client_disabled(s); @@ -3512,11 +3512,29 @@ int ssl_cipher_list_to_bytes(SSL *s, STACK_OF(SSL_CIPHER) *sk, WPACKET *pkt) return 0; } + /* Sanity check that the maximum version we offer has ciphers enabled */ + if (!maxverok) { + if (SSL_IS_DTLS(s)) { + if (DTLS_VERSION_GE(c->max_dtls, s->s3->tmp.max_ver) + && DTLS_VERSION_LE(c->min_dtls, s->s3->tmp.max_ver)) + maxverok = 1; + } else { + if (c->max_tls >= s->s3->tmp.max_ver + && c->min_tls <= s->s3->tmp.max_ver) + maxverok = 1; + } + } + totlen += len; } - if (totlen == 0) { + if (totlen == 0 || !maxverok) { SSLerr(SSL_F_SSL_CIPHER_LIST_TO_BYTES, SSL_R_NO_CIPHERS_AVAILABLE); + + if (!maxverok) + ERR_add_error_data(1, "No ciphers enabled for max supported " + "SSL/TLS version"); + return 0; } diff --git a/test/ssl-tests/14-curves.conf b/test/ssl-tests/14-curves.conf index 83911b0aef..ab04c2ef39 100644 --- a/test/ssl-tests/14-curves.conf +++ b/test/ssl-tests/14-curves.conf @@ -50,6 +50,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem [0-curve-sect163k1-client] CipherString = ECDHE Curves = sect163k1 +MaxProtocol = TLSv1.2 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem VerifyMode = Peer @@ -77,6 +78,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem [1-curve-sect163r1-client] CipherString = ECDHE Curves = sect163r1 +MaxProtocol = TLSv1.2 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem VerifyMode = Peer @@ -104,6 +106,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem [2-curve-sect163r2-client] CipherString = ECDHE Curves = sect163r2 +MaxProtocol = TLSv1.2 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem VerifyMode = Peer @@ -131,6 +134,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem [3-curve-sect193r1-client] CipherString = ECDHE Curves = sect193r1 +MaxProtocol = TLSv1.2 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem VerifyMode = Peer @@ -158,6 +162,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem [4-curve-sect193r2-client] CipherString = ECDHE Curves = sect193r2 +MaxProtocol = TLSv1.2 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem VerifyMode = Peer @@ -185,6 +190,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem [5-curve-sect233k1-client] CipherString = ECDHE Curves = sect233k1 +MaxProtocol = TLSv1.2 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem VerifyMode = Peer @@ -212,6 +218,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem [6-curve-sect233r1-client] CipherString = ECDHE Curves = sect233r1 +MaxProtocol = TLSv1.2 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem VerifyMode = Peer @@ -239,6 +246,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem [7-curve-sect239k1-client] CipherString = ECDHE Curves = sect239k1 +MaxProtocol = TLSv1.2 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem VerifyMode = Peer @@ -266,6 +274,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem [8-curve-sect283k1-client] CipherString = ECDHE Curves = sect283k1 +MaxProtocol = TLSv1.2 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem VerifyMode = Peer @@ -293,6 +302,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem [9-curve-sect283r1-client] CipherString = ECDHE Curves = sect283r1 +MaxProtocol = TLSv1.2 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem VerifyMode = Peer @@ -320,6 +330,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem [10-curve-sect409k1-client] CipherString = ECDHE Curves = sect409k1 +MaxProtocol = TLSv1.2 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem VerifyMode = Peer @@ -347,6 +358,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem [11-curve-sect409r1-client] CipherString = ECDHE Curves = sect409r1 +MaxProtocol = TLSv1.2 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem VerifyMode = Peer @@ -374,6 +386,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem [12-curve-sect571k1-client] CipherString = ECDHE Curves = sect571k1 +MaxProtocol = TLSv1.2 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem VerifyMode = Peer @@ -401,6 +414,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem [13-curve-sect571r1-client] CipherString = ECDHE Curves = sect571r1 +MaxProtocol = TLSv1.2 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem VerifyMode = Peer @@ -428,6 +442,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem [14-curve-secp160k1-client] CipherString = ECDHE Curves = secp160k1 +MaxProtocol = TLSv1.2 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem VerifyMode = Peer @@ -455,6 +470,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem [15-curve-secp160r1-client] CipherString = ECDHE Curves = secp160r1 +MaxProtocol = TLSv1.2 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem VerifyMode = Peer @@ -482,6 +498,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem [16-curve-secp160r2-client] CipherString = ECDHE Curves = secp160r2 +MaxProtocol = TLSv1.2 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem VerifyMode = Peer @@ -509,6 +526,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem [17-curve-secp192k1-client] CipherString = ECDHE Curves = secp192k1 +MaxProtocol = TLSv1.2 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem VerifyMode = Peer @@ -536,6 +554,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem [18-curve-prime192v1-client] CipherString = ECDHE Curves = prime192v1 +MaxProtocol = TLSv1.2 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem VerifyMode = Peer @@ -563,6 +582,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem [19-curve-secp224k1-client] CipherString = ECDHE Curves = secp224k1 +MaxProtocol = TLSv1.2 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem VerifyMode = Peer @@ -590,6 +610,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem [20-curve-secp224r1-client] CipherString = ECDHE Curves = secp224r1 +MaxProtocol = TLSv1.2 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem VerifyMode = Peer @@ -617,6 +638,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem [21-curve-secp256k1-client] CipherString = ECDHE Curves = secp256k1 +MaxProtocol = TLSv1.2 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem VerifyMode = Peer @@ -644,6 +666,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem [22-curve-prime256v1-client] CipherString = ECDHE Curves = prime256v1 +MaxProtocol = TLSv1.2 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem VerifyMode = Peer @@ -671,6 +694,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem [23-curve-secp384r1-client] CipherString = ECDHE Curves = secp384r1 +MaxProtocol = TLSv1.2 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem VerifyMode = Peer @@ -698,6 +722,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem [24-curve-secp521r1-client] CipherString = ECDHE Curves = secp521r1 +MaxProtocol = TLSv1.2 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem VerifyMode = Peer @@ -725,6 +750,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem [25-curve-brainpoolP256r1-client] CipherString = ECDHE Curves = brainpoolP256r1 +MaxProtocol = TLSv1.2 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem VerifyMode = Peer @@ -752,6 +778,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem [26-curve-brainpoolP384r1-client] CipherString = ECDHE Curves = brainpoolP384r1 +MaxProtocol = TLSv1.2 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem VerifyMode = Peer @@ -779,6 +806,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem [27-curve-brainpoolP512r1-client] CipherString = ECDHE Curves = brainpoolP512r1 +MaxProtocol = TLSv1.2 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem VerifyMode = Peer @@ -806,6 +834,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem [28-curve-X25519-client] CipherString = ECDHE Curves = X25519 +MaxProtocol = TLSv1.2 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem VerifyMode = Peer diff --git a/test/ssl-tests/14-curves.conf.in b/test/ssl-tests/14-curves.conf.in index 6e98b5a69a..9f6e4339a4 100644 --- a/test/ssl-tests/14-curves.conf.in +++ b/test/ssl-tests/14-curves.conf.in @@ -25,14 +25,15 @@ sub generate_tests() { foreach (0..$#curves) { my $curve = $curves[$_]; push @tests, { - name => "curve-${curve}", + name => "curve-${curve}", server => { "Curves" => $curve, # TODO(TLS1.3): Can we get this to work for TLSv1.3? "MaxProtocol" => "TLSv1.2" }, client => { - "CipherString" => "ECDHE", + "CipherString" => "ECDHE", + "MaxProtocol" => "TLSv1.2", "Curves" => $curve }, test => { diff --git a/test/ssl-tests/17-renegotiate.conf b/test/ssl-tests/17-renegotiate.conf index 8376eeaf89..3f3769ff02 100644 --- a/test/ssl-tests/17-renegotiate.conf +++ b/test/ssl-tests/17-renegotiate.conf @@ -198,12 +198,12 @@ client = 6-renegotiate-aead-to-non-aead-client [6-renegotiate-aead-to-non-aead-server] Certificate = ${ENV::TEST_CERTS_DIR}/servercert.pem CipherString = DEFAULT -MaxProtocol = TLSv1.2 Options = NoResumptionOnRenegotiation PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem [6-renegotiate-aead-to-non-aead-client] CipherString = AES128-GCM-SHA256 +MaxProtocol = TLSv1.2 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem VerifyMode = Peer @@ -230,12 +230,12 @@ client = 7-renegotiate-non-aead-to-aead-client [7-renegotiate-non-aead-to-aead-server] Certificate = ${ENV::TEST_CERTS_DIR}/servercert.pem CipherString = DEFAULT -MaxProtocol = TLSv1.2 Options = NoResumptionOnRenegotiation PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem [7-renegotiate-non-aead-to-aead-client] CipherString = AES128-SHA +MaxProtocol = TLSv1.2 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem VerifyMode = Peer @@ -262,12 +262,12 @@ client = 8-renegotiate-non-aead-to-non-aead-client [8-renegotiate-non-aead-to-non-aead-server] Certificate = ${ENV::TEST_CERTS_DIR}/servercert.pem CipherString = DEFAULT -MaxProtocol = TLSv1.2 Options = NoResumptionOnRenegotiation PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem [8-renegotiate-non-aead-to-non-aead-client] CipherString = AES128-SHA +MaxProtocol = TLSv1.2 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem VerifyMode = Peer @@ -294,12 +294,12 @@ client = 9-renegotiate-aead-to-aead-client [9-renegotiate-aead-to-aead-server] Certificate = ${ENV::TEST_CERTS_DIR}/servercert.pem CipherString = DEFAULT -MaxProtocol = TLSv1.2 Options = NoResumptionOnRenegotiation PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem [9-renegotiate-aead-to-aead-client] CipherString = AES128-GCM-SHA256 +MaxProtocol = TLSv1.2 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem VerifyMode = Peer diff --git a/test/ssl-tests/17-renegotiate.conf.in b/test/ssl-tests/17-renegotiate.conf.in index 867a4f2220..b5d07b0705 100644 --- a/test/ssl-tests/17-renegotiate.conf.in +++ b/test/ssl-tests/17-renegotiate.conf.in @@ -114,10 +114,10 @@ our @tests_tls1_2 = ( name => "renegotiate-aead-to-non-aead", server => { "Options" => "NoResumptionOnRenegotiation", - "MaxProtocol" => "TLSv1.2" }, client => { "CipherString" => "AES128-GCM-SHA256", + "MaxProtocol" => "TLSv1.2", extra => { "RenegotiateCiphers" => "AES128-SHA" } @@ -133,10 +133,10 @@ our @tests_tls1_2 = ( name => "renegotiate-non-aead-to-aead", server => { "Options" => "NoResumptionOnRenegotiation", - "MaxProtocol" => "TLSv1.2" }, client => { "CipherString" => "AES128-SHA", + "MaxProtocol" => "TLSv1.2", extra => { "RenegotiateCiphers" => "AES128-GCM-SHA256" } @@ -152,10 +152,10 @@ our @tests_tls1_2 = ( name => "renegotiate-non-aead-to-non-aead", server => { "Options" => "NoResumptionOnRenegotiation", - "MaxProtocol" => "TLSv1.2" }, client => { "CipherString" => "AES128-SHA", + "MaxProtocol" => "TLSv1.2", extra => { "RenegotiateCiphers" => "AES256-SHA" } @@ -171,10 +171,10 @@ our @tests_tls1_2 = ( name => "renegotiate-aead-to-aead", server => { "Options" => "NoResumptionOnRenegotiation", - "MaxProtocol" => "TLSv1.2" }, client => { "CipherString" => "AES128-GCM-SHA256", + "MaxProtocol" => "TLSv1.2", extra => { "RenegotiateCiphers" => "AES256-GCM-SHA384" } diff --git a/test/ssl-tests/19-mac-then-encrypt.conf b/test/ssl-tests/19-mac-then-encrypt.conf index bba44d1703..0dd384ea6c 100644 --- a/test/ssl-tests/19-mac-then-encrypt.conf +++ b/test/ssl-tests/19-mac-then-encrypt.conf @@ -96,12 +96,12 @@ client = 3-disable-encrypt-then-mac-server-sha2-client [3-disable-encrypt-then-mac-server-sha2-server] Certificate = ${ENV::TEST_CERTS_DIR}/servercert.pem CipherString = DEFAULT -MaxProtocol = TLSv1.2 Options = -EncryptThenMac PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem [3-disable-encrypt-then-mac-server-sha2-client] CipherString = AES128-SHA256 +MaxProtocol = TLSv1.2 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem VerifyMode = Peer diff --git a/test/ssl-tests/19-mac-then-encrypt.conf.in b/test/ssl-tests/19-mac-then-encrypt.conf.in index d51cfa3ea7..dfe529c9cd 100644 --- a/test/ssl-tests/19-mac-then-encrypt.conf.in +++ b/test/ssl-tests/19-mac-then-encrypt.conf.in @@ -61,10 +61,10 @@ my @tests_tls1_2 = ( name => "disable-encrypt-then-mac-server-sha2", server => { "Options" => "-EncryptThenMac", - "MaxProtocol" => "TLSv1.2" }, client => { "CipherString" => "AES128-SHA256", + "MaxProtocol" => "TLSv1.2" }, test => { "ExpectedResult" => "Success", diff --git a/test/ssl-tests/20-cert-select.conf b/test/ssl-tests/20-cert-select.conf index e787efc5f0..d07a989c98 100644 --- a/test/ssl-tests/20-cert-select.conf +++ b/test/ssl-tests/20-cert-select.conf @@ -34,6 +34,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem [0-ECDSA CipherString Selection-client] CipherString = aECDSA +MaxProtocol = TLSv1.2 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem VerifyMode = Peer @@ -62,6 +63,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem [1-RSA CipherString Selection-client] CipherString = aRSA +MaxProtocol = TLSv1.2 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem VerifyMode = Peer @@ -88,6 +90,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem [2-ECDSA CipherString Selection, no ECDSA certificate-client] CipherString = aECDSA +MaxProtocol = TLSv1.2 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem VerifyMode = Peer diff --git a/test/ssl-tests/20-cert-select.conf.in b/test/ssl-tests/20-cert-select.conf.in index 3d50f0220d..d333d5de63 100644 --- a/test/ssl-tests/20-cert-select.conf.in +++ b/test/ssl-tests/20-cert-select.conf.in @@ -21,6 +21,7 @@ our @tests = ( server => $server, client => { "CipherString" => "aECDSA", + "MaxProtocol" => "TLSv1.2", }, test => { "ExpectedServerCertType" =>, "P-256", @@ -33,6 +34,7 @@ our @tests = ( server => $server, client => { "CipherString" => "aRSA", + "MaxProtocol" => "TLSv1.2", }, test => { "ExpectedServerCertType" =>, "RSA", @@ -46,7 +48,8 @@ our @tests = ( "MaxProtocol" => "TLSv1.2" }, client => { - "CipherString" => "aECDSA" + "CipherString" => "aECDSA", + "MaxProtocol" => "TLSv1.2" }, test => { "ExpectedResult" => "ServerFail" -- 2.25.1