From 7e25c14eab5a6cb289de59044771ff2924cf72cb Mon Sep 17 00:00:00 2001 From: Daiki Ueno Date: Tue, 5 Jun 2018 14:08:26 +0200 Subject: ext/psk_ke_modes: always send extension unless disabled in config With the psk_key_exchange_modes extension, clients can restrict the key exchange modes for use with resumption and in that case the server shouldn't send NewSessionTicket. This patch makes use of it to avoid receiving useless tickets, by sending the psk_key_exchange_modes extension unless PSK is completely disabled. A couple of tests need to be adjusted: tls13/prf to take into account of the psk_key_exchange_modes extension sent, and tls13/no-psk-exts to not treat the presence of the extension as error. Signed-off-by: Daiki Ueno --- lib/ext/psk_ke_modes.c | 25 ++++--------------------- tests/tls13/no-psk-exts.c | 4 ++-- tests/tls13/prf.c | 8 ++++---- 3 files changed, 10 insertions(+), 27 deletions(-) diff --git a/lib/ext/psk_ke_modes.c b/lib/ext/psk_ke_modes.c index 9c41d9e94a..281ca0a1a6 100644 --- a/lib/ext/psk_ke_modes.c +++ b/lib/ext/psk_ke_modes.c @@ -28,26 +28,6 @@ #define PSK_KE 0 #define PSK_DHE_KE 1 -/* Relevant to client only */ -static bool -psk_ke_modes_is_required(gnutls_session_t session) -{ - gnutls_psk_client_credentials_t cred; - - if (!(session->internals.flags & GNUTLS_NO_TICKETS) && - session->internals.tls13_ticket.ticket.data != NULL) - return 1; - - if (session->internals.priorities->have_psk) { - cred = (gnutls_psk_client_credentials_t) - _gnutls_get_cred(session, GNUTLS_CRD_PSK); - if (cred && _gnutls_have_psk_credentials(cred, session)) - return 1; - } - - return 0; -} - static int psk_ke_modes_send_params(gnutls_session_t session, gnutls_buffer_t extdata) @@ -63,7 +43,10 @@ psk_ke_modes_send_params(gnutls_session_t session, if (session->security_parameters.entity == GNUTLS_SERVER) return 0; - if (!psk_ke_modes_is_required(session)) + /* If session ticket is disabled and no PSK key exchange is + * enabled, don't send the extension */ + if ((session->internals.flags & GNUTLS_NO_TICKETS) && + !session->internals.priorities->have_psk) return 0; vers = _gnutls_version_max(session); diff --git a/tests/tls13/no-psk-exts.c b/tests/tls13/no-psk-exts.c index b70c667822..e8f1e2e84f 100644 --- a/tests/tls13/no-psk-exts.c +++ b/tests/tls13/no-psk-exts.c @@ -84,7 +84,7 @@ static void client(int fd) /* Initialize TLS session */ - gnutls_init(&session, GNUTLS_CLIENT); + gnutls_init(&session, GNUTLS_CLIENT|GNUTLS_NO_TICKETS); gnutls_handshake_set_timeout(session, 20 * 1000); @@ -148,7 +148,7 @@ static int hellos_callback(gnutls_session_t session, unsigned int htype, fail("PSK extension seen in client hello with no PSK!\n"); if (find_client_extension(msg, TLS_EXT_PSK_KE, NULL, NULL)) - fail("PSK extension seen in client hello with no PSK!\n"); + fail("PSK KE extension seen in client hello with no PSK!\n"); return 0; } diff --git a/tests/tls13/prf.c b/tests/tls13/prf.c index f221896124..22e7f0e08f 100644 --- a/tests/tls13/prf.c +++ b/tests/tls13/prf.c @@ -126,8 +126,8 @@ static void dump(const char *name, const uint8_t *data, unsigned data_size) } \ } -#define KEY_EXP_VALUE "\x1f\x17\xb5\xe2\xef\xfc\x1c\x27\x1e\x1a\x2c\x9b\x36\xc7\x43\x70\x5c\x80\x93\x7a\xce\x7b\x52\x18\xe0\x22\xca\x0d\xf8\x01\xa5\x7f\xef\x8b" -#define HELLO_VALUE "\x5c\x6d\x0a\xa0\xc7\x1e\x52\xb5\xb4\x2c\x4b\xfa\x24\xc8\x3f\x3f\xba\xfc\x43\xc2\x05\xe5\x04\xd9\xfd\x86\x84\x00\x1c\xfb\xf3" +#define KEY_EXP_VALUE "\x81\x7a\x37\xc6\xa3\x2b\x83\x47\x3b\xad\x03\xa6\xd4\x6d\xac\xe3\x1c\x9d\xa4\xdc\x8e\x0c\x77\xf8\x1c\x5e\x5d\xe5\xca\x1c\xbc\x89\x4c\x37" +#define HELLO_VALUE "\x10\x3e\xef\x5e\x62\x1d\x03\x95\xfc\x8f\x59\xc7\x7d\xdc\x14\x7e\xcf\x46\x86\x2f\xfb\x1c\x5a\x16\x6a\xf5\x38\x69\xa8\x3c\x85" static void check_prfs(gnutls_session_t session) { unsigned char key_material[512]; @@ -138,8 +138,8 @@ static void check_prfs(gnutls_session_t session) TRY(13, "key expansion", 0, NULL, 34, (uint8_t*)KEY_EXP_VALUE); TRY(6, "hello", 0, NULL, 31, (uint8_t*)HELLO_VALUE); - TRY(7, "context", 5, "abcd\xfa", 31, (uint8_t*)"\xa5\xc0\x3e\x31\x5b\x70\x57\x48\x1e\xfe\x11\x2b\x13\x13\x8f\x97\x14\x2d\x4d\x35\xac\x0a\x20\x4e\x9c\x84\xcf\x48\x8c\xa2\x0b"); - TRY(12, "null-context", 0, "", 31, (uint8_t*)"\x7b\xb9\x00\x8a\x2c\x97\xa0\x73\x28\x91\xbf\x73\xda\xa5\x78\x08\x45\xac\xa7\x29\xa8\xc4\x30\x30\xc2\x76\x94\x1a\xaf\x74\x4b"); + TRY(7, "context", 5, "abcd\xfa", 31, (uint8_t*)"\xbc\x23\xe3\xf4\x29\xdb\x48\x20\x48\x8c\x37\xd9\xd4\xe0\xcf\x88\xc3\x3d\x7b\x12\x59\xfb\xad\x8e\x4d\x8c\x53\x58\xf4\xe6\xef"); + TRY(12, "null-context", 0, "", 31, (uint8_t*)"\x89\x89\x1f\x2f\x6c\x35\x26\x0b\xe9\x1c\x7b\xb7\x27\x5e\x7c\x41\xfb\xa0\x11\x9c\xd7\xe6\xd5\xdc\x2a\xcc\x54\x23\x3f\x52\x9f"); /* Try whether calling gnutls_prf() with non-null context or server-first * param, will fail */ -- cgit v1.2.1 From 6e7c50781d0c3d6cf831b48038013a6c7a4bda89 Mon Sep 17 00:00:00 2001 From: Daiki Ueno Date: Fri, 1 Jun 2018 15:04:49 +0200 Subject: tls13/session_ticket: don't send ticket when no common KE modes When the server had received psk_key_exchange_modes extension which doesn't have any overlap with the server configuration, omit to send NewSessionTicket. Signed-off-by: Daiki Ueno --- lib/ext/psk_ke_modes.c | 2 +- lib/tls13/session_ticket.c | 6 ++++++ tests/session-tickets-missing.c | 32 ++++++++++++++++++++------------ 3 files changed, 27 insertions(+), 13 deletions(-) diff --git a/lib/ext/psk_ke_modes.c b/lib/ext/psk_ke_modes.c index 281ca0a1a6..dacfba7ef6 100644 --- a/lib/ext/psk_ke_modes.c +++ b/lib/ext/psk_ke_modes.c @@ -118,7 +118,7 @@ psk_ke_modes_recv_params(gnutls_session_t session, int cli_dhpsk_pos = MAX_POS; unsigned i; - /* Server doesn't send psk_key_exchange_modes */ + /* Client doesn't receive psk_key_exchange_modes */ if (session->security_parameters.entity == GNUTLS_CLIENT) return gnutls_assert_val(GNUTLS_E_RECEIVED_ILLEGAL_EXTENSION); diff --git a/lib/tls13/session_ticket.c b/lib/tls13/session_ticket.c index d98475094a..8515b9cb19 100644 --- a/lib/tls13/session_ticket.c +++ b/lib/tls13/session_ticket.c @@ -230,6 +230,12 @@ int _gnutls13_send_session_ticket(gnutls_session_t session, unsigned again) if (session->internals.flags & GNUTLS_NO_TICKETS) return gnutls_assert_val(0); + /* If we received the psk_key_exchange_modes extension which + * does not have overlap with the server configuration, don't + * send a session ticket */ + if (session->internals.hsk_flags & HSK_PSK_KE_MODE_INVALID) + return gnutls_assert_val(0); + if (again == 0) { memset(&ticket, 0, sizeof(tls13_ticket_t)); diff --git a/tests/session-tickets-missing.c b/tests/session-tickets-missing.c index a767cbfd37..9db194904f 100644 --- a/tests/session-tickets-missing.c +++ b/tests/session-tickets-missing.c @@ -94,13 +94,15 @@ static int handshake_callback(gnutls_session_t session, unsigned int htype, #define MAX_BUF 1024 -static void client(int fd, const char *prio) +static void client(int fd, const char *prio, unsigned int flags) { int ret; gnutls_certificate_credentials_t x509_cred; gnutls_session_t session; /* Need to enable anonymous KX specifically. */ + flags |= GNUTLS_CLIENT; + gnutls_global_set_time_function(mytime); global_init(); @@ -113,7 +115,7 @@ static void client(int fd, const char *prio) /* Initialize TLS session */ - gnutls_init(&session, GNUTLS_CLIENT|GNUTLS_NO_TICKETS); + gnutls_init(&session, flags); assert(gnutls_priority_set_direct(session, prio, NULL)>=0); @@ -171,17 +173,15 @@ static void terminate(void) exit(1); } -static void server(int fd, const char *prio, unsigned server_no_tickets) +static void server(int fd, const char *prio, unsigned int flags) { int ret; char buffer[MAX_BUF + 1]; gnutls_session_t session; gnutls_certificate_credentials_t x509_cred; gnutls_datum_t skey = {NULL, 0}; - unsigned int flags = GNUTLS_SERVER; - if (server_no_tickets) - flags |= GNUTLS_NO_TICKETS; + flags |= GNUTLS_SERVER; /* this must be called once in the program */ @@ -200,7 +200,7 @@ static void server(int fd, const char *prio, unsigned server_no_tickets) assert(gnutls_init(&session, flags)>=0); - if (!server_no_tickets) { + if (!(flags & GNUTLS_NO_TICKETS)) { assert(gnutls_session_ticket_key_generate(&skey)>=0); assert(gnutls_session_ticket_enable_server(session, &skey) >= 0); } @@ -263,7 +263,7 @@ static void ch_handler(int sig) } static -void start(const char *prio, unsigned server_no_tickets) +void start2(const char *prio, const char *sprio, unsigned int flags, unsigned int sflags) { int fd[2]; int ret, status = 0; @@ -290,24 +290,32 @@ void start(const char *prio, unsigned server_no_tickets) if (child) { /* parent */ close(fd[1]); - server(fd[0], prio, server_no_tickets); + server(fd[0], sprio, sflags); waitpid(child, &status, 0); check_wait_status(status); } else { close(fd[0]); - client(fd[1], prio); + client(fd[1], prio, flags); exit(0); } return; } +static +void start(const char *prio, unsigned int flags) +{ + start2(prio, prio, GNUTLS_NO_TICKETS, flags); +} + void doit(void) { start("NORMAL:-VERS-ALL:+VERS-TLS1.2", 0); /* Under TLS 1.3 session tickets are not negotiated; they are - * "always sent unless server sets GNUTLS_NO_TICKETS */ - start("NORMAL:-VERS-ALL:+VERS-TLS1.3", 1); + * always sent unless server sets GNUTLS_NO_TICKETS... */ + start("NORMAL:-VERS-ALL:+VERS-TLS1.3", GNUTLS_NO_TICKETS); + /* ...or there is no overlap between PSK key exchange modes */ + start2("NORMAL:-VERS-ALL:+VERS-TLS1.3:+PSK:-DHE-PSK", "NORMAL:-VERS-ALL:+VERS-TLS1.3", 0, 0); start("NORMAL", 0); } -- cgit v1.2.1