Skip to content

Commit

Permalink
Add a ciphersuite config sanity check for clients
Browse files Browse the repository at this point in the history 
Ensure that there are ciphersuites enabled for the maximum supported
version we are claiming in the ClientHello.

Reviewed-by: Rich Salz <[email protected]>
(Merged from openssl#3334)
  • Loading branch information
@mattcaswell
mattcaswell committed May 4, 2017
1 parent 05a2feb commit 5d62fd7
Show file tree
Hide file tree
Showing 9 changed files with 69 additions and 15 deletions.
22 changes: 20 additions & 2 deletions ssl/statem/statem_clnt.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}

Expand Down
29 changes: 29 additions & 0 deletions test/ssl-tests/14-curves.conf
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down
5 changes: 3 additions & 2 deletions test/ssl-tests/14-curves.conf.in
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand Down
8 changes: 4 additions & 4 deletions test/ssl-tests/17-renegotiate.conf
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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

Expand All @@ -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

Expand All @@ -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

Expand Down
8 changes: 4 additions & 4 deletions test/ssl-tests/17-renegotiate.conf.in
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
Expand All @@ -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"
}
Expand All @@ -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"
}
Expand All @@ -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"
}
Expand Down
Loading

0 comments on commit 5d62fd7

Please sign in to comment.