From dc1defac66c2bc654a1aa49222ce268aff74cb75 Mon Sep 17 00:00:00 2001 From: Daiki Ueno Date: Fri, 16 Apr 2021 13:56:40 +0200 Subject: [PATCH] priority: add option to disable TLS 1.3 middlebox compatibility mode This adds a new option %DISABLE_TLS13_COMPAT_MODE to disable TLS 1.3 compatibility mode at run-time. Signed-off-by: Daiki Ueno --- .gitignore | 1 + NEWS | 5 ++ doc/cha-gtls-app.texi | 4 ++ lib/gnutls_int.h | 1 + lib/handshake-tls13.c | 23 +++--- lib/handshake.c | 4 +- lib/priority.c | 9 +++ lib/priority_options.gperf | 1 + tests/Makefile.am | 2 +- tests/tls13-compat-mode.c | 140 +++++++++++++++++++++++++++++++++++++ 10 files changed, 179 insertions(+), 11 deletions(-) create mode 100644 tests/tls13-compat-mode.c diff --git a/NEWS b/NEWS index ba6827358d..b3590a316a 100644 --- a/NEWS +++ b/NEWS @@ -5,6 +5,11 @@ Copyright (C) 2000-2016 Free Software Foundation, Inc. Copyright (C) 2013-2019 Nikos Mavrogiannopoulos See the end for copying conditions. +* Version 3.7.2 (unreleased) + +** libgnutls: The priority string option %DISABLE_TLS13_COMPAT_MODE was added + to disable TLS 1.3 middlebox compatibility mode + * Version 3.7.1 (released 2021-03-10) ** libgnutls: Fixed potential use-after-free in sending "key_share" diff --git a/doc/cha-gtls-app.texi b/doc/cha-gtls-app.texi index 36ba55e3ab..2399bf82eb 100644 --- a/doc/cha-gtls-app.texi +++ b/doc/cha-gtls-app.texi @@ -1610,6 +1610,10 @@ client hello. Note that this should be set only by applications that try to reconnect with a downgraded protocol version. See RFC7507 for details. +@item %DISABLE_TLS13_COMPAT_MODE @tab +will disable TLS 1.3 middlebox compatibility mode (RFC8446, Appendix +D.4) for non-compliant middleboxes. + @item %VERIFY_ALLOW_BROKEN @tab will allow signatures with known to be broken algorithms (such as MD5 or SHA1) in certificate chains. diff --git a/lib/gnutls_int.h b/lib/gnutls_int.h index 2611b5af54..62a061e67a 100644 --- a/lib/gnutls_int.h +++ b/lib/gnutls_int.h @@ -950,6 +950,7 @@ struct gnutls_priority_st { bool have_psk; bool force_etm; unsigned int additional_verify_flags; + bool tls13_compat_mode; /* TLS_FALLBACK_SCSV */ bool fallback; diff --git a/lib/handshake-tls13.c b/lib/handshake-tls13.c index 7dd42becf1..9687707a32 100644 --- a/lib/handshake-tls13.c +++ b/lib/handshake-tls13.c @@ -84,11 +84,13 @@ int _gnutls13_handshake_client(gnutls_session_t session) case STATE99: case STATE100: #ifdef TLS13_APPENDIX_D4 - /* We send it before keys are generated. That works because CCS - * is always being cached and queued and not being sent directly */ - ret = _gnutls_send_change_cipher_spec(session, AGAIN(STATE100)); - STATE = STATE100; - IMED_RET("send change cipher spec", ret, 0); + if (session->internals.priorities->tls13_compat_mode) { + /* We send it before keys are generated. That works because CCS + * is always being cached and queued and not being sent directly */ + ret = _gnutls_send_change_cipher_spec(session, AGAIN(STATE100)); + STATE = STATE100; + IMED_RET("send change cipher spec", ret, 0); + } #endif FALLTHROUGH; case STATE101: @@ -385,9 +387,11 @@ int _gnutls13_handshake_server(gnutls_session_t session) FALLTHROUGH; case STATE92: #ifdef TLS13_APPENDIX_D4 - ret = _gnutls_send_change_cipher_spec(session, AGAIN(STATE92)); - STATE = STATE92; - IMED_RET("send change cipher spec", ret, 0); + if (session->internals.priorities->tls13_compat_mode) { + ret = _gnutls_send_change_cipher_spec(session, AGAIN(STATE92)); + STATE = STATE92; + IMED_RET("send change cipher spec", ret, 0); + } #endif FALLTHROUGH; case STATE93: @@ -416,7 +420,8 @@ int _gnutls13_handshake_server(gnutls_session_t session) #ifdef TLS13_APPENDIX_D4 /* don't send CCS twice: when HRR has already been * sent, CCS should have followed it (see above) */ - if (!(session->internals.hsk_flags & HSK_HRR_SENT)) { + if (session->internals.priorities->tls13_compat_mode && + !(session->internals.hsk_flags & HSK_HRR_SENT)) { ret = _gnutls_send_change_cipher_spec(session, AGAIN(STATE100)); STATE = STATE100; IMED_RET("send change cipher spec", ret, 0); diff --git a/lib/handshake.c b/lib/handshake.c index 6c894eb68a..33bc7f7be6 100644 --- a/lib/handshake.c +++ b/lib/handshake.c @@ -2213,7 +2213,9 @@ static int send_client_hello(gnutls_session_t session, int again) } #ifdef TLS13_APPENDIX_D4 - if (max_ver->tls13_sem && !resuming) { + if (max_ver->tls13_sem && + session->internals.priorities->tls13_compat_mode && + !resuming) { /* Under TLS1.3 we generate a random session ID to make * the TLS1.3 session look like a resumed TLS1.2 session */ ret = _gnutls_generate_session_id(session->security_parameters. diff --git a/lib/priority.c b/lib/priority.c index 7686c7530a..8cd8a1b260 100644 --- a/lib/priority.c +++ b/lib/priority.c @@ -989,6 +989,10 @@ static void enable_server_precedence(gnutls_priority_t c) { c->server_precedence = 1; } +static void disable_tls13_compat_mode(gnutls_priority_t c) +{ + c->tls13_compat_mode = false; +} static void dummy_func(gnutls_priority_t c) { } @@ -2005,6 +2009,11 @@ gnutls_priority_init(gnutls_priority_t * priority_cache, * when we make it the default. */ (*priority_cache)->sr = SR_PARTIAL; + /* For now TLS 1.3 middlebox compatibility mode is enabled by default. + * This will eventually be disabled by default and moved to the %COMPAT + * setting. + */ + (*priority_cache)->tls13_compat_mode = true; (*priority_cache)->min_record_version = 1; gnutls_atomic_init(&(*priority_cache)->usage_cnt); diff --git a/lib/priority_options.gperf b/lib/priority_options.gperf index a81001dda1..6fb9ae603c 100644 --- a/lib/priority_options.gperf +++ b/lib/priority_options.gperf @@ -39,3 +39,4 @@ PROFILE_SUITEB192, enable_profile_suiteb192 NEW_PADDING, dummy_func DEBUG_ALLOW_KEY_USAGE_VIOLATIONS, enable_server_key_usage_violations ALLOW_SMALL_RECORDS, enable_allow_small_records +DISABLE_TLS13_COMPAT_MODE, disable_tls13_compat_mode diff --git a/tests/Makefile.am b/tests/Makefile.am index c6d994f877..ed32dadcc5 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -223,7 +223,7 @@ ctests += mini-record-2 simple gnutls_hmac_fast set_pkcs12_cred cert certuniquei sign-verify-newapi sign-verify-deterministic iov aead-cipher-vec \ tls13-without-timeout-func buffer status-request-revoked \ set_x509_ocsp_multi_cli kdf-api keylog-func handshake-write \ - x509cert-dntypes id-on-xmppAddr + x509cert-dntypes id-on-xmppAddr tls13-compat-mode if HAVE_SECCOMP_TESTS ctests += dtls-with-seccomp tls-with-seccomp dtls-client-with-seccomp tls-client-with-seccomp diff --git a/tests/tls13-compat-mode.c b/tests/tls13-compat-mode.c new file mode 100644 index 0000000000..e8f99802df --- /dev/null +++ b/tests/tls13-compat-mode.c @@ -0,0 +1,140 @@ +/* + * Copyright (C) 2021 Free Software Foundation, Inc. + * + * Author: Daiki Ueno + * + * This file is part of GnuTLS. + * + * GnuTLS is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * GnuTLS is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with GnuTLS. If not, see . + */ + +#ifdef HAVE_CONFIG_H +#include +#endif + +#include +#include +#include +#include +#include +#include +#include "cert-common.h" +#include "eagain-common.h" +#include "utils.h" + +/* This tests TLS 1.3 middlebox compatibility mode. */ + +#define COMPAT_PRIO "NORMAL:-VERS-ALL:+VERS-TLS1.3" +#define NO_COMPAT_PRIO COMPAT_PRIO ":%DISABLE_TLS13_COMPAT_MODE" + +#define HANDSHAKE_SESSION_ID_POS 34 + +struct data { + bool compat; +}; + +static int +handshake_callback(gnutls_session_t session, unsigned int htype, + unsigned post, unsigned int incoming, + const gnutls_datum_t *msg) +{ + unsigned pos; + struct data *data; + uint8_t s; + + assert(htype == GNUTLS_HANDSHAKE_CLIENT_HELLO); + assert(msg->size >= HANDSHAKE_SESSION_ID_POS); + + data = gnutls_session_get_ptr(session); + + pos = HANDSHAKE_SESSION_ID_POS; + if (pos + 1 > msg->size) + fail("error\n"); + s = msg->data[pos]; + + if (data->compat && s == 0) { + fail("empty session ID while compat mode is enabled\n"); + } else if (!data->compat && s > 0) { + fail("non-empty session ID while compat mode is disabled\n"); + } + + return 0; +} + +static void +test(const char *name, bool client_compat, bool server_compat) +{ + /* Server stuff. */ + gnutls_certificate_credentials_t serverx509cred; + gnutls_session_t server; + int sret = GNUTLS_E_AGAIN; + /* Client stuff. */ + gnutls_certificate_credentials_t clientx509cred; + gnutls_session_t client; + struct data data; + int cret = GNUTLS_E_AGAIN; + + success("%s\n", name); + + /* Init server */ + assert(gnutls_certificate_allocate_credentials(&serverx509cred) >= 0); + gnutls_certificate_set_x509_key_mem(serverx509cred, + &server_cert, &server_key, + GNUTLS_X509_FMT_PEM); + + assert(gnutls_init(&server, GNUTLS_SERVER) >= 0); + assert(gnutls_credentials_set(server, GNUTLS_CRD_CERTIFICATE, serverx509cred) >= 0); + assert(gnutls_priority_set_direct(server, + server_compat ? + COMPAT_PRIO : NO_COMPAT_PRIO, + NULL) >= 0); + gnutls_transport_set_push_function(server, server_push); + gnutls_transport_set_pull_function(server, server_pull); + gnutls_transport_set_ptr(server, server); + + /* Init client */ + assert(gnutls_certificate_allocate_credentials(&clientx509cred) >= 0); + assert(gnutls_init(&client, GNUTLS_CLIENT) >= 0); + assert(gnutls_credentials_set(client, GNUTLS_CRD_CERTIFICATE, + clientx509cred) >= 0); + + assert(gnutls_priority_set_direct(client, + client_compat ? + COMPAT_PRIO : NO_COMPAT_PRIO, + NULL) >= 0); + gnutls_transport_set_push_function(client, client_push); + gnutls_transport_set_pull_function(client, client_pull); + gnutls_transport_set_ptr(client, client); + data.compat = client_compat; + gnutls_session_set_ptr(client, &data); + gnutls_handshake_set_hook_function(client, GNUTLS_HANDSHAKE_CLIENT_HELLO, + GNUTLS_HOOK_POST, + handshake_callback); + + HANDSHAKE(client, server); + + gnutls_deinit(client); + gnutls_deinit(server); + + gnutls_certificate_free_credentials(serverx509cred); + gnutls_certificate_free_credentials(clientx509cred); +} + +void doit(void) +{ + test("client compat, server compat", true, true); + test("client compat, server non-compat", true, false); + test("client non-compat, server compat", false, true); + test("client non-compat, server non-compat", false, false); +} -- 2.30.2