From 837fef78b16a5a83657c7cfa9a79d8a0f926bc3c Mon Sep 17 00:00:00 2001 From: Artem Boldariev Date: Fri, 9 Dec 2022 18:44:01 +0200 Subject: [PATCH 1/4] Fix TLS session resumption via IDs when Mutual TLS is used This commit fixes TLS session resumption via session IDs when client certificates are used. To do so it makes sure that session ID contexts are set within server TLS contexts. See OpenSSL documentation for 'SSL_CTX_set_session_id_context()', the "Warnings" section. --- lib/isc/include/isc/tls.h | 20 ++++++++++++++++++++ lib/isc/tls.c | 13 +++++++++++++ lib/ns/listenlist.c | 11 +++++++++++ 3 files changed, 44 insertions(+) diff --git a/lib/isc/include/isc/tls.h b/lib/isc/include/isc/tls.h index 26c684d6a2..24577ec13d 100644 --- a/lib/isc/include/isc/tls.h +++ b/lib/isc/include/isc/tls.h @@ -563,6 +563,26 @@ isc_tlsctx_cache_find( * relation between stores and contexts. */ +void +isc_tlsctx_set_random_session_id_context(isc_tlsctx_t *ctx); +/*%< + * Set context within which session can be reused to a randomly + * generated value. This one is used for TLS session resumption using + * session IDs. See OpenSSL documentation for + * 'SSL_CTX_set_session_id_context()'. + * + * It might be worth noting that usually session ID contexts are kept + * static for an application and particular certificate + * combination. However, for the cases when exporting server side TLS + * session cache to/loading from external memory is not required, we + * might use random IDs just fine. See, + * e.g. 'ngx_ssl_session_id_context()' in NGINX for an example of how + * a session ID might be obtained. + * + * Requires: + *\li 'ctx' - a valid non-NULL pointer; + */ + void isc__tls_initialize(void); diff --git a/lib/isc/tls.c b/lib/isc/tls.c index c027ed91e0..a7d9a93332 100644 --- a/lib/isc/tls.c +++ b/lib/isc/tls.c @@ -1728,3 +1728,16 @@ isc_tlsctx_client_session_cache_getctx( REQUIRE(VALID_TLSCTX_CLIENT_SESSION_CACHE(cache)); return (cache->ctx); } + +void +isc_tlsctx_set_random_session_id_context(isc_tlsctx_t *ctx) { + uint8_t session_id_ctx[SSL_MAX_SID_CTX_LENGTH] = { 0 }; + const size_t len = ISC_MIN(20, sizeof(session_id_ctx)); + + REQUIRE(ctx != NULL); + + RUNTIME_CHECK(RAND_bytes(session_id_ctx, len) == 1); + + RUNTIME_CHECK( + SSL_CTX_set_session_id_context(ctx, session_id_ctx, len) == 1); +} diff --git a/lib/ns/listenlist.c b/lib/ns/listenlist.c index df9719f5e1..d864ca97f1 100644 --- a/lib/ns/listenlist.c +++ b/lib/ns/listenlist.c @@ -64,6 +64,17 @@ listenelt_create(isc_mem_t *mctx, in_port_t port, isc_dscp_t dscp, goto tls_error; } + /* + * We need to initialise session ID context to make TLS + * session resumption work correctly - in particular in + * the case when client certificates are used (Mutual + * TLS) - otherwise resumption attempts will lead to + * handshake failures. See OpenSSL documentation for + * 'SSL_CTX_set_session_id_context()', the "Warnings" + * section. + */ + isc_tlsctx_set_random_session_id_context(sslctx); + /* * If CA-bundle file is specified - enable client * certificates validation. From d5d31c6ba15b42f797a8dab341a97f8f0f6aca1b Mon Sep 17 00:00:00 2001 From: Artem Boldariev Date: Fri, 9 Dec 2022 18:47:07 +0200 Subject: [PATCH 2/4] Extend the 'doth' system test with a Mutual TLS resumption check This commit adds a simple check to the 'doth' system test which ensures that session resumption when Mutual TLS is used works as expected. --- bin/tests/system/doth/tests.sh | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/bin/tests/system/doth/tests.sh b/bin/tests/system/doth/tests.sh index 920d6af214..d22f0ef649 100644 --- a/bin/tests/system/doth/tests.sh +++ b/bin/tests/system/doth/tests.sh @@ -776,6 +776,16 @@ grep "status: NOERROR" dig.out.test$n > /dev/null || ret=1 if [ $ret != 0 ]; then echo_i "failed"; fi status=$((status + ret)) +# send two requests one after another so that session resumption will happen +n=$((n + 1)) +echo_i "checking DoH query (client certificate used - session resumption when using Mutual TLS) ($n)" +ret=0 +# shellcheck disable=SC2086 +dig_with_https_opts +https +tls-ca="$ca_file" +tls-certfile="./CA/certs/srv01.client01.example.com.pem" +tls-keyfile="./CA/certs/srv01.client01.example.com.key" -p "${EXTRAPORT6}" +comm @10.53.0.1 . SOA . SOA > dig.out.test$n +grep "TLS error" dig.out.test$n > /dev/null && ret=1 +if [ $ret != 0 ]; then echo_i "failed"; fi +status=$((status + ret)) + test_opcodes() { EXPECT_STATUS="$1" shift From d8e04cdbc743bf45c8b82ccc3d72dee24842c9be Mon Sep 17 00:00:00 2001 From: Artem Boldariev Date: Tue, 13 Dec 2022 14:14:43 +0200 Subject: [PATCH 3/4] Update CHANGES [GL #3725] Mention that TLS session resumption for Mutual TLS has been fixed. --- CHANGES | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGES b/CHANGES index 4063fa1a4d..e949c73042 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,8 @@ +6046. [bug] TLS session resumption might lead to handshake + failures when client certificates are used for + authentication (Mutual TLS). This has been fixed. + [GL #3725] + 6045. [cleanup] The list of supported DNSSEC algorithms changed log level from "warning" to "notice" to match named's other startup messages. [GL !7217] From 67d74e228f43382206e47b52836e7b47421e1576 Mon Sep 17 00:00:00 2001 From: Artem Boldariev Date: Wed, 14 Dec 2022 18:07:40 +0200 Subject: [PATCH 4/4] Update Release notes [GL #3725] Mention that TLS session resumption for Mutual TLS has been fixed. --- doc/notes/notes-current.rst | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index 39afcc2de6..d0a633c1b7 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -35,7 +35,9 @@ Feature Changes Bug Fixes ~~~~~~~~~ -- None. +- TLS session resumption might lead to handshake failures when client + certificates are used for authentication (Mutual TLS). This has + been fixed. :gl:`#3725` Known Issues ~~~~~~~~~~~~