Skip to content

Commit

Permalink
Check that the default signature type is allowed
Browse files Browse the repository at this point in the history 
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 <paul.dale@oracle.com>
GH: #10784
  • Loading branch information
@kroeckx
kroeckx committed Jan 25, 2020
1 parent 5fd72d9 commit b0031e5
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 23 deletions.
2 changes: 1 addition & 1 deletion ssl/ssl_local.h
View file
Expand Up @@ -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);
Expand Down
16 changes: 12 additions & 4 deletions ssl/t1_lib.c
View file
Expand Up @@ -22,6 +22,7 @@
#include <openssl/ct.h>

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,
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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 */
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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;
Expand Down
57 changes: 39 additions & 18 deletions test/recipes/70-test_sslsigalgs.t
View file
Expand Up @@ -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;

Expand Down Expand Up @@ -132,35 +132,56 @@ 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");
$proxy->ciphers("ECDHE-RSA-AES128-SHA");
$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");
$proxy->ciphers("ECDHE-RSA-AES128-SHA");
$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;
Expand All @@ -169,15 +190,15 @@ 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");
$proxy->ciphers("ECDHE-RSA-AES128-SHA");
$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
Expand All @@ -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");
Expand All @@ -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");
Expand All @@ -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();
Expand All @@ -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;
Expand All @@ -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);
Expand Down

0 comments on commit b0031e5

Please sign in to comment.