From aaf286293050a4a2dbcd98d9eb2d69eca99c502a Mon Sep 17 00:00:00 2001 From: Nikos Mavrogiannopoulos Date: Sat, 2 Feb 2019 09:13:40 +0100 Subject: Fallback to TLS 1.2 when incompatible with signature certs are provided This only takes into account certificates in the credentials structure. If certificates are provided in a callback, these must be checked by the provider. For that we assume that the credentials structure is filled when associated with a session; if not then the fallback mechanism will not work and the handshake will fail. Signed-off-by: Nikos Mavrogiannopoulos --- NEWS | 4 ++- lib/algorithms/ciphersuites.c | 2 +- lib/auth.c | 23 ++++++++++++++++ lib/auth/cert.h | 3 ++- lib/handshake.c | 8 +++++- tests/common-cert-key-exchange.c | 57 ++++++++++++++++++++-------------------- tests/tls13-cert-key-exchange.c | 7 ++++- 7 files changed, 70 insertions(+), 34 deletions(-) diff --git a/NEWS b/NEWS index 80d5399630..af6aee6872 100644 --- a/NEWS +++ b/NEWS @@ -9,7 +9,9 @@ See the end for copying conditions. ** libgnutls: enforce key usage limitations on certificates more actively. Previously we would enforce it for TLS1.2 protocol, now we enforce it - even when TLS1.3 is negotiated, or on client certificates as well (#690). + even when TLS1.3 is negotiated, or on client certificates as well. When + an inappropriate for TLS1.3 certificate is seen on the credentials structure + GnuTLS will disable TLS1.3 support for that session (#690). ** API and ABI modifications: No changes since last version. diff --git a/lib/algorithms/ciphersuites.c b/lib/algorithms/ciphersuites.c index b97bbc82db..7269861ffe 100644 --- a/lib/algorithms/ciphersuites.c +++ b/lib/algorithms/ciphersuites.c @@ -1578,7 +1578,7 @@ _gnutls_figure_common_ciphersuite(gnutls_session_t session, /* RFC7919 requires that we reply with insufficient security if we have * negotiated an FFDHE group, but cannot find a common ciphersuite. However, * we must also distinguish between not matching a ciphersuite due to an - * incompatible certificate which we traditionally return GNUTLS_E_INSUFFICIENT_SECURITY. + * incompatible certificate which we traditionally return GNUTLS_E_NO_CIPHER_SUITES. */ if (!no_cert_found && (session->internals.hsk_flags & HSK_HAVE_FFDHE) && session->internals.priorities->groups.have_ffdhe && !version->tls13_sem) diff --git a/lib/auth.c b/lib/auth.c index 91a67c9afa..dd3fc861fb 100644 --- a/lib/auth.c +++ b/lib/auth.c @@ -138,6 +138,29 @@ gnutls_credentials_set(gnutls_session_t session, } } + /* sanity tests */ + if (type == GNUTLS_CRD_CERTIFICATE) { + gnutls_certificate_credentials_t c = cred; + unsigned i; + bool allow_tls13 = 0; + unsigned key_usage; + + if (c != NULL && c->ncerts != 0) { + for (i = 0; i < c->ncerts; i++) { + key_usage = get_key_usage(session, c->certs[i].cert_list[0].pubkey); + if (key_usage == 0 || (key_usage & GNUTLS_KEY_DIGITAL_SIGNATURE)) { + allow_tls13 = 1; + break; + } + } + + if (!allow_tls13) { + /* to prevent the server random indicate TLS1.3 support */ + session->internals.flags |= INT_FLAG_NO_TLS13; + } + } + } + return 0; } diff --git a/lib/auth/cert.h b/lib/auth/cert.h index 3d653cb81c..fd66628820 100644 --- a/lib/auth/cert.h +++ b/lib/auth/cert.h @@ -174,7 +174,8 @@ int _gnutls_proc_rawpk_crt(gnutls_session_t session, inline static unsigned get_key_usage(gnutls_session_t session, gnutls_pubkey_t pubkey) { - if (unlikely(session->internals.priorities->allow_server_key_usage_violation)) + if (unlikely(session->internals.priorities && + session->internals.priorities->allow_server_key_usage_violation)) return 0; else return pubkey->key_usage; diff --git a/lib/handshake.c b/lib/handshake.c index 70b4486266..481210ebc0 100644 --- a/lib/handshake.c +++ b/lib/handshake.c @@ -444,6 +444,9 @@ _gnutls_negotiate_version(gnutls_session_t session, if (aversion && aversion->id == GNUTLS_TLS1_2) { vers = _gnutls_version_max(session); + if (unlikely(vers == NULL)) + return gnutls_assert_val(GNUTLS_E_NO_CIPHER_SUITES); + if (vers->id >= GNUTLS_TLS1_2) { session->security_parameters.pversion = aversion; return 0; @@ -2138,7 +2141,10 @@ static int send_client_hello(gnutls_session_t session, int again) if (hver == NULL) { gnutls_assert(); - ret = GNUTLS_E_NO_PRIORITIES_WERE_SET; + if (session->internals.flags & INT_FLAG_NO_TLS13) + ret = GNUTLS_E_INSUFFICIENT_CREDENTIALS; + else + ret = GNUTLS_E_NO_PRIORITIES_WERE_SET; goto cleanup; } diff --git a/tests/common-cert-key-exchange.c b/tests/common-cert-key-exchange.c index 468475f846..c0c27a4064 100644 --- a/tests/common-cert-key-exchange.c +++ b/tests/common-cert-key-exchange.c @@ -74,7 +74,7 @@ void try_with_key_fail(const char *name, const char *client_prio, reset_buffers(); /* Init server */ - gnutls_certificate_allocate_credentials(&serverx509cred); + assert(gnutls_certificate_allocate_credentials(&serverx509cred)>=0); ret = gnutls_certificate_set_x509_key_mem(serverx509cred, serv_cert, serv_key, @@ -82,16 +82,15 @@ void try_with_key_fail(const char *name, const char *client_prio, if (ret < 0) fail("Could not set key/cert: %s\n", gnutls_strerror(ret)); - gnutls_init(&server, GNUTLS_SERVER); - gnutls_credentials_set(server, GNUTLS_CRD_CERTIFICATE, - serverx509cred); - - + assert(gnutls_init(&server, GNUTLS_SERVER)>=0); if (server_priority) assert(gnutls_priority_set_direct(server, server_priority, NULL) >= 0); else assert(gnutls_priority_set_direct(server, client_prio, NULL) >= 0); + assert(gnutls_credentials_set(server, GNUTLS_CRD_CERTIFICATE, + serverx509cred)>=0); + gnutls_transport_set_push_function(server, server_push); gnutls_transport_set_pull_function(server, server_pull); gnutls_transport_set_ptr(server, server); @@ -112,11 +111,6 @@ void try_with_key_fail(const char *name, const char *client_prio, if (ret < 0) exit(1); - ret = gnutls_credentials_set(client, GNUTLS_CRD_CERTIFICATE, - clientx509cred); - if (ret < 0) - exit(1); - gnutls_transport_set_push_function(client, client_push); gnutls_transport_set_pull_function(client, client_pull); gnutls_transport_set_ptr(client, client); @@ -127,6 +121,12 @@ void try_with_key_fail(const char *name, const char *client_prio, fprintf(stderr, "Error in %s\n", err); exit(1); } + + ret = gnutls_credentials_set(client, GNUTLS_CRD_CERTIFICATE, + clientx509cred); + if (ret < 0) + exit(1); + success("negotiating %s\n", name); HANDSHAKE_EXPECT(client, server, client_err, server_err); @@ -173,8 +173,8 @@ void try_with_key_ks(const char *name, const char *client_prio, gnutls_kx_algori reset_buffers(); /* Init server */ - gnutls_anon_allocate_server_credentials(&s_anoncred); - gnutls_certificate_allocate_credentials(&server_cred); + assert(gnutls_anon_allocate_server_credentials(&s_anoncred)>=0); + assert(gnutls_certificate_allocate_credentials(&server_cred)>=0); // Set server crt creds based on ctype switch (server_ctype) { @@ -201,11 +201,10 @@ void try_with_key_ks(const char *name, const char *client_prio, gnutls_kx_algori gnutls_certificate_set_dh_params(server_cred, dh_params); gnutls_anon_set_server_dh_params(s_anoncred, dh_params); - gnutls_init(&server, GNUTLS_SERVER | GNUTLS_ENABLE_RAWPK); - gnutls_credentials_set(server, GNUTLS_CRD_CERTIFICATE, - server_cred); - gnutls_credentials_set(server, GNUTLS_CRD_ANON, s_anoncred); - + assert(gnutls_init(&server, GNUTLS_SERVER | GNUTLS_ENABLE_RAWPK)>=0); + assert(gnutls_credentials_set(server, GNUTLS_CRD_CERTIFICATE, + server_cred)>=0); + assert(gnutls_credentials_set(server, GNUTLS_CRD_ANON, s_anoncred)>=0); if (server_priority) assert(gnutls_priority_set_direct(server, server_priority, NULL) >= 0); @@ -254,8 +253,8 @@ void try_with_key_ks(const char *name, const char *client_prio, gnutls_kx_algori exit(1); - gnutls_anon_allocate_client_credentials(&c_anoncred); - gnutls_credentials_set(client, GNUTLS_CRD_ANON, c_anoncred); + assert(gnutls_anon_allocate_client_credentials(&c_anoncred)>=0); + assert(gnutls_credentials_set(client, GNUTLS_CRD_ANON, c_anoncred)>=0); ret = gnutls_credentials_set(client, GNUTLS_CRD_CERTIFICATE, client_cred); if (ret < 0) @@ -397,14 +396,14 @@ void dtls_try_with_key_mtu(const char *name, const char *client_prio, gnutls_kx_ gnutls_certificate_set_dh_params(serverx509cred, dh_params); gnutls_anon_set_server_dh_params(s_anoncred, dh_params); - gnutls_init(&server, GNUTLS_SERVER|GNUTLS_DATAGRAM|GNUTLS_NONBLOCK); - gnutls_credentials_set(server, GNUTLS_CRD_CERTIFICATE, - serverx509cred); - gnutls_credentials_set(server, GNUTLS_CRD_ANON, s_anoncred); + assert(gnutls_init(&server, GNUTLS_SERVER|GNUTLS_DATAGRAM|GNUTLS_NONBLOCK)>=0); + assert(gnutls_credentials_set(server, GNUTLS_CRD_CERTIFICATE, + serverx509cred)>=0); + assert(gnutls_credentials_set(server, GNUTLS_CRD_ANON, s_anoncred)>=0); - gnutls_priority_set_direct(server, - "NORMAL:+ANON-ECDH:+ANON-DH:+ECDHE-RSA:+DHE-RSA:+RSA:+ECDHE-ECDSA:+CURVE-X25519", - NULL); + assert(gnutls_priority_set_direct(server, + "NORMAL:+ANON-ECDH:+ANON-DH:+ECDHE-RSA:+DHE-RSA:+RSA:+ECDHE-ECDSA:+CURVE-X25519", + NULL)>=0); gnutls_transport_set_push_function(server, server_push); gnutls_transport_set_pull_function(server, server_pull); gnutls_transport_set_pull_timeout_function(server, server_pull_timeout_func); @@ -440,8 +439,8 @@ void dtls_try_with_key_mtu(const char *name, const char *client_prio, gnutls_kx_ if (ret < 0) exit(1); - gnutls_anon_allocate_client_credentials(&c_anoncred); - gnutls_credentials_set(client, GNUTLS_CRD_ANON, c_anoncred); + assert(gnutls_anon_allocate_client_credentials(&c_anoncred)>=0); + assert(gnutls_credentials_set(client, GNUTLS_CRD_ANON, c_anoncred)>=0); ret = gnutls_credentials_set(client, GNUTLS_CRD_CERTIFICATE, clientx509cred); if (ret < 0) diff --git a/tests/tls13-cert-key-exchange.c b/tests/tls13-cert-key-exchange.c index 066c7d2fb0..3a214f9ad1 100644 --- a/tests/tls13-cert-key-exchange.c +++ b/tests/tls13-cert-key-exchange.c @@ -143,6 +143,11 @@ void doit(void) GNUTLS_E_NO_CIPHER_SUITES, GNUTLS_E_AGAIN, &server_ca3_localhost_rsa_decrypt_cert, &server_ca3_key, NULL, NULL); + try_with_key_fail("TLS 1.3 and TLS 1.2 with rsa encryption cert", + "NORMAL:-VERS-ALL:+VERS-TLS1.3:+VERS-TLS1.2", + GNUTLS_E_SUCCESS, GNUTLS_E_SUCCESS, + &server_ca3_localhost_rsa_decrypt_cert, &server_ca3_key, NULL, NULL); + try_with_key_fail("TLS 1.3 with (forced) rsa encryption cert - client should detect", "NORMAL:-VERS-ALL:+VERS-TLS1.3:%DEBUG_ALLOW_KEY_USAGE_VIOLATIONS", GNUTLS_E_AGAIN, GNUTLS_E_KEY_USAGE_VIOLATION, @@ -150,7 +155,7 @@ void doit(void) try_with_key_fail("TLS 1.3 with client rsa encryption cert", "NORMAL:-VERS-ALL:+VERS-TLS1.3", - GNUTLS_E_AGAIN, GNUTLS_E_KEY_USAGE_VIOLATION, + GNUTLS_E_AGAIN, GNUTLS_E_INSUFFICIENT_CREDENTIALS, &server_ca3_rsa_pss_cert, &server_ca3_rsa_pss_key, &server_ca3_localhost_rsa_decrypt_cert, &server_ca3_key); try_with_key_fail("TLS 1.3 with (forced) client rsa encryption cert - server should detect", -- cgit v1.2.1