From c2646aeee94e71cb15c90a3147cf3b5b0ca158ca Mon Sep 17 00:00:00 2001 From: Daiki Ueno Date: Tue, 2 Jun 2020 20:53:11 +0200 Subject: stek: differentiate initial state from valid time window of TOTP There was a confusion in the TOTP implementation in stek.c. When the mechanism is initialized at the first time, it records the timestamp but doesn't initialize the key. This removes the timestamp recording at the initialization phase, so the key is properly set later. Signed-off-by: Daiki Ueno --- lib/stek.c | 17 +++++------------ tests/resume-with-previous-stek.c | 4 ++-- tests/tls13/prf-early.c | 8 ++++---- 3 files changed, 11 insertions(+), 18 deletions(-) diff --git a/lib/stek.c b/lib/stek.c index 2f885cee37..5ab9e7d2d1 100644 --- a/lib/stek.c +++ b/lib/stek.c @@ -323,20 +323,13 @@ int _gnutls_initialize_session_ticket_key_rotation(gnutls_session_t session, con if (unlikely(session == NULL || key == NULL)) return gnutls_assert_val(GNUTLS_E_INTERNAL_ERROR); - if (session->key.totp.last_result == 0) { - int64_t t; - memcpy(session->key.initial_stek, key->data, key->size); - t = totp_next(session); - if (t < 0) - return gnutls_assert_val(t); + if (unlikely(session->key.totp.last_result != 0)) + return GNUTLS_E_INVALID_REQUEST; - session->key.totp.last_result = t; - session->key.totp.was_rotated = 0; - - return GNUTLS_E_SUCCESS; - } + memcpy(session->key.initial_stek, key->data, key->size); - return GNUTLS_E_INVALID_REQUEST; + session->key.totp.was_rotated = 0; + return 0; } /* diff --git a/tests/resume-with-previous-stek.c b/tests/resume-with-previous-stek.c index f212b188b9..05c1c90868 100644 --- a/tests/resume-with-previous-stek.c +++ b/tests/resume-with-previous-stek.c @@ -196,8 +196,8 @@ static void server(int fd, unsigned rounds, const char *prio) serverx509cred = NULL; } - if (num_stek_rotations != 2) - fail("STEK should be rotated exactly twice (%d)!\n", num_stek_rotations); + if (num_stek_rotations != 3) + fail("STEK should be rotated exactly three times (%d)!\n", num_stek_rotations); if (serverx509cred) gnutls_certificate_free_credentials(serverx509cred); diff --git a/tests/tls13/prf-early.c b/tests/tls13/prf-early.c index 414b1db5ea..bc3196248f 100644 --- a/tests/tls13/prf-early.c +++ b/tests/tls13/prf-early.c @@ -123,10 +123,10 @@ static void dump(const char *name, const uint8_t *data, unsigned data_size) } \ } -#define KEY_EXP_VALUE "\xc0\x1e\xc2\xa4\xb7\xb4\x04\xaa\x91\x5d\xaf\xe8\xf7\x4d\x19\xdf\xd0\xe6\x08\xd6\xb4\x3b\xcf\xca\xc9\x32\x75\x3b\xe3\x11\x19\xb1\xac\x68" -#define HELLO_VALUE "\x77\xdb\x10\x0b\xe8\xd0\xb9\x38\xbc\x49\xe6\xbe\xf2\x47\x2a\xcc\x6b\xea\xce\x85\x04\xd3\x9e\xd8\x06\x16\xad\xff\xcd\xbf\x4b" -#define CONTEXT_VALUE "\xf2\x17\x9f\xf2\x66\x56\x87\x66\xf9\x5c\x8a\xd7\x4e\x1d\x46\xee\x0e\x44\x41\x4c\xcd\xac\xcb\xc0\x31\x41\x2a\xb6\xd7\x01\x62" -#define NULL_CONTEXT_VALUE "\xcd\x79\x07\x93\xeb\x96\x07\x3e\xec\x78\x90\x89\xf7\x16\x42\x6d\x27\x87\x56\x7c\x7b\x60\x2b\x20\x44\xd1\xea\x0c\x89\xfb\x8b" +#define KEY_EXP_VALUE "\xc1\x6b\x6c\xb9\x88\x33\xd5\x28\x80\xec\x27\x87\xa2\x6f\x4b\xd0\x01\x5e\x7f\xca\xd7\xd4\x8a\x3f\xe2\x48\x92\xef\x02\x14\xfb\x81\x90\x04" +#define HELLO_VALUE "\x2a\x73\xd9\x74\x04\x4e\x0a\x5f\x41\x8a\x09\xcb\x45\x33\x1a\xec\xd3\xfc\xdc\x1b\x2c\x67\x26\xe4\x9c\xfe\x1f\xa5\x74\xf1\x4f" +#define CONTEXT_VALUE "\x87\xf6\x88\xe3\xd7\xf2\x05\xbc\xa4\x10\xa3\x48\x9f\xf5\xcf\x97\x06\x22\x4e\xfd\x18\x32\x52\x1d\xbd\x26\xf5\x5b\x21\x20\xec" +#define NULL_CONTEXT_VALUE "\xf9\xca\xfe\x45\x44\x96\xdb\xc5\x41\x8f\x7e\x8e\xd7\xb0\x7d\x19\x45\xaf\x09\xbc\x1e\x82\x94\xac\x55\xe5\xb9\xb4\x3b\xe8\xc0" static int handshake_callback_called; -- cgit v1.2.1 From 3d7fae761e65e9d0f16d7247ee8a464d4fe002da Mon Sep 17 00:00:00 2001 From: Daiki Ueno Date: Tue, 2 Jun 2020 21:45:17 +0200 Subject: valgrind: check if session ticket key is used without initialization This adds a valgrind client request for session->key.session_ticket_key to make sure that it is not used without initialization. Signed-off-by: Daiki Ueno --- lib/state.c | 5 ++++- lib/stek.c | 8 ++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/lib/state.c b/lib/state.c index 8ba2cc4a32..7d0a77dc95 100644 --- a/lib/state.c +++ b/lib/state.c @@ -578,9 +578,12 @@ int gnutls_init(gnutls_session_t * session, unsigned int flags) if (flags & GNUTLS_CLIENT) VALGRIND_MAKE_MEM_UNDEFINED((*session)->security_parameters.client_random, GNUTLS_RANDOM_SIZE); - if (flags & GNUTLS_SERVER) + if (flags & GNUTLS_SERVER) { VALGRIND_MAKE_MEM_UNDEFINED((*session)->security_parameters.server_random, GNUTLS_RANDOM_SIZE); + VALGRIND_MAKE_MEM_UNDEFINED((*session)->key.session_ticket_key, + TICKET_MASTER_KEY_SIZE); + } } #endif handshake_internal_state_clear1(*session); diff --git a/lib/stek.c b/lib/stek.c index 5ab9e7d2d1..316555b49a 100644 --- a/lib/stek.c +++ b/lib/stek.c @@ -21,6 +21,9 @@ */ #include "gnutls_int.h" #include "stek.h" +#ifdef HAVE_VALGRIND_MEMCHECK_H +#include +#endif #define NAME_POS (0) #define KEY_POS (TICKET_KEY_NAME_SIZE) @@ -143,6 +146,11 @@ static int rotate(gnutls_session_t session) call_rotation_callback(session, key, t); session->key.totp.last_result = t; memcpy(session->key.session_ticket_key, key, sizeof(key)); +#ifdef HAVE_VALGRIND_MEMCHECK_H + if (RUNNING_ON_VALGRIND) + VALGRIND_MAKE_MEM_DEFINED(session->key.session_ticket_key, + TICKET_MASTER_KEY_SIZE); +#endif session->key.totp.was_rotated = 1; } else if (t < 0) { -- cgit v1.2.1