cron2 has uploaded a new patch set (#8) to the change originally created by plaisthos. ( http://gerrit.openvpn.net/c/openvpn/+/1827?usp=email )
The following approvals got outdated and were removed: Code-Review+2 by flichtenheld Change subject: Replace SHA256 with SIPHASH24 in HMAC cookie approach ...................................................................... Replace SHA256 with SIPHASH24 in HMAC cookie approach Using SHA256 for this is overkill since we only need a 64bit hash value that is not predictable. Siphash24 also fulfils these requirements while being much faster. Change-Id: I3b6bb178ffb2bb49981bc23eabf04fe8e06d6fc3 Signed-off-by: Arne Schwabe <[email protected]> Acked-by: Frank Lichtenheld <[email protected]> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1827 Message-Id: <[email protected]> URL: https://www.mail-archive.com/[email protected]/msg38316.html Signed-off-by: Gert Doering <[email protected]> --- M CMakeLists.txt M src/openvpn/init.c M src/openvpn/mudp.c M src/openvpn/openvpn.h M src/openvpn/siphash.h M src/openvpn/ssl_pkt.c M src/openvpn/ssl_pkt.h M tests/unit_tests/openvpn/Makefile.am M tests/unit_tests/openvpn/test_pkt.c 9 files changed, 79 insertions(+), 102 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/27/1827/8 diff --git a/CMakeLists.txt b/CMakeLists.txt index 643dec6..ad7de6e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -878,6 +878,7 @@ src/openvpn/packet_id.c src/openvpn/reliable.c src/openvpn/run_command.c + src/openvpn/siphash_reference.c src/openvpn/session_id.c src/openvpn/ssl_pkt.c src/openvpn/tls_crypt.c diff --git a/src/openvpn/init.c b/src/openvpn/init.c index 08278fc2..b8732dd 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -3464,7 +3464,7 @@ if (flags & CF_INIT_TLS_AUTH_STANDALONE) { c->c2.tls_auth_standalone = tls_auth_standalone_init(&to, &c->c2.gc); - c->c2.session_id_hmac = session_id_hmac_init(); + siphash_key_init(c->c2.session_id_key); } } diff --git a/src/openvpn/mudp.c b/src/openvpn/mudp.c index 0979002..08d79c9 100644 --- a/src/openvpn/mudp.c +++ b/src/openvpn/mudp.c @@ -101,7 +101,7 @@ verdict = tls_pre_decrypt_lite(tas, state, &m->top.c2.from, &m->top.c2.buf); - hmac_ctx_t *hmac = m->top.c2.session_id_hmac; + uint8_t *hmac_key = m->top.c2.session_id_key; struct openvpn_sockaddr *from = &m->top.c2.from.dest; int handwindow = m->top.options.handshake_window; @@ -133,7 +133,7 @@ { /* Calculate the session ID HMAC for our reply and create reset packet */ struct session_id sid = - calculate_session_id_hmac(state->peer_session_id, from, hmac, handwindow, 0); + calculate_session_id_hmac(state->peer_session_id, from, hmac_key, handwindow, 0); send_hmac_reset_packet(m, state, tas, &sid, true, sock); return false; @@ -165,7 +165,7 @@ { /* Calculate the session ID HMAC for our reply and create reset packet */ struct session_id sid = - calculate_session_id_hmac(state->peer_session_id, from, hmac, handwindow, 0); + calculate_session_id_hmac(state->peer_session_id, from, hmac_key, handwindow, 0); send_hmac_reset_packet(m, state, tas, &sid, false, sock); @@ -180,7 +180,7 @@ struct gc_arena gc = gc_new(); bool pkt_is_ack = (verdict == VERDICT_VALID_ACK_V1); - bool ret = check_session_hmac_and_pkt_id(state, from, hmac, handwindow, pkt_is_ack); + bool ret = check_session_hmac_and_pkt_id(state, from, hmac_key, handwindow, pkt_is_ack); const char *peer = print_link_socket_actual(&m->top.c2.from, &gc); uint8_t pkt_firstbyte = *BPTR(&m->top.c2.buf); diff --git a/src/openvpn/openvpn.h b/src/openvpn/openvpn.h index fa00822..e9e18bf 100644 --- a/src/openvpn/openvpn.h +++ b/src/openvpn/openvpn.h @@ -45,6 +45,7 @@ #include "plugin.h" #include "manage.h" #include "dns.h" +#include "siphash.h" /* * Our global key schedules, packaged thusly @@ -335,10 +336,9 @@ * \c --tls-auth commandline option. */ - hmac_ctx_t *session_id_hmac; - /**< the HMAC we use to generate and verify our syn cookie like - * session ids from the server. - */ + uint8_t session_id_key[SIPHASH_KEY_SIZE]; + /**< the siphash secret we use to generate and verify our syn cookie like + * session ids from the server. */ /* used to optimize calls to tls_multi_process */ struct interval tmp_int; diff --git a/src/openvpn/siphash.h b/src/openvpn/siphash.h index ade7762..462175c 100644 --- a/src/openvpn/siphash.h +++ b/src/openvpn/siphash.h @@ -24,7 +24,7 @@ #include <stdint.h> #include <stdio.h> -#include <stdbool.h> +#include "crypto.h" /* We need to include this to check for the OPENSSL_IS_AWSLC macro */ #ifdef ENABLE_CRYPTO_OPENSSL @@ -77,4 +77,14 @@ #endif } +/** + * Initialises a SIPHASH key with a random value + * @param key the key to be initialised + */ +static inline void +siphash_key_init(uint8_t *key) +{ + prng_bytes(key, SIPHASH_KEY_SIZE); +} + #endif /* ifndef SIPHASH_H */ diff --git a/src/openvpn/ssl_pkt.c b/src/openvpn/ssl_pkt.c index f8444451..6a6d9f9 100644 --- a/src/openvpn/ssl_pkt.c +++ b/src/openvpn/ssl_pkt.c @@ -31,6 +31,7 @@ #include "crypto.h" #include "session_id.h" #include "reliable.h" +#include "siphash.h" #include "tls_crypt.h" /* @@ -442,64 +443,53 @@ return buf; } -hmac_ctx_t * -session_id_hmac_init(void) -{ - /* We assume that SHA256 is always available */ - ASSERT(md_valid("SHA256")); - hmac_ctx_t *hmac_ctx = hmac_ctx_new(); - - uint8_t key[SHA256_DIGEST_LENGTH]; - ASSERT(rand_bytes(key, sizeof(key))); - - hmac_ctx_init(hmac_ctx, key, "SHA256"); - return hmac_ctx; -} - struct session_id calculate_session_id_hmac(struct session_id client_sid, const struct openvpn_sockaddr *from, - hmac_ctx_t *hmac, int handwindow, int offset) + const uint8_t *key, int handwindow, int offset) { - union - { - uint8_t hmac_result[SHA256_DIGEST_LENGTH]; - struct session_id sid; - } result; - /* Get the valid time quantisation for our hmac, * we divide time by handwindow/2 and allow the previous * and future session time if specified by offset */ uint32_t session_id_time = ntohl((uint32_t)(now / ((handwindow + 1) / 2) + offset)); - hmac_ctx_reset(hmac); + uint8_t input[64]; + + /* ensure input array is large enough */ + static_assert(sizeof(input) >= sizeof(struct sockaddr_in6) + sizeof(session_id_time) + sizeof(client_sid.id), "input buffer not sized correctly"); + static_assert(sizeof(input) >= sizeof(struct sockaddr_in) + sizeof(session_id_time) + sizeof(client_sid.id), "input buffer not sized correctly"); + + struct buffer in = { 0 }; + buf_set_write(&in, input, sizeof(input)); + /* We do not care about endian here since it does not need to be * portable */ - hmac_ctx_update(hmac, (const uint8_t *)&session_id_time, sizeof(session_id_time)); + buf_write(&in, (const uint8_t *)&session_id_time, sizeof(session_id_time)); /* add client IP and port */ switch (from->addr.sa.sa_family) { case AF_INET: - hmac_ctx_update(hmac, (const uint8_t *)&from->addr.in4, sizeof(struct sockaddr_in)); + buf_write(&in, (const uint8_t *)&from->addr.in4, sizeof(struct sockaddr_in)); break; case AF_INET6: - hmac_ctx_update(hmac, (const uint8_t *)&from->addr.in6, sizeof(struct sockaddr_in6)); + buf_write(&in, (const uint8_t *)&from->addr.in6, sizeof(struct sockaddr_in6)); break; } /* add session id of client */ - hmac_ctx_update(hmac, client_sid.id, SID_SIZE); + buf_write(&in, client_sid.id, SID_SIZE); - hmac_ctx_final(hmac, result.hmac_result); + struct session_id sid; + siphash(buf_bptr(&in), buf_len(&in), key, sid.id, sizeof(sid.id)); - return result.sid; + return sid; } bool check_session_hmac_and_pkt_id(struct tls_pre_decrypt_state *state, const struct openvpn_sockaddr *from, - hmac_ctx_t *hmac, + uint8_t *key, int handwindow, bool pkt_is_ack) { @@ -551,7 +541,7 @@ for (int offset = -2; offset <= 0; offset++) { struct session_id expected_id = - calculate_session_id_hmac(state->peer_session_id, from, hmac, handwindow, offset); + calculate_session_id_hmac(state->peer_session_id, from, key, handwindow, offset); if (memcmp_constant_time(&expected_id, &state->server_session_id, SID_SIZE) == 0) { diff --git a/src/openvpn/ssl_pkt.h b/src/openvpn/ssl_pkt.h index 82cb5b1..03e8930 100644 --- a/src/openvpn/ssl_pkt.h +++ b/src/openvpn/ssl_pkt.h @@ -151,28 +151,20 @@ const struct link_socket_actual *from, const struct buffer *buf); -/* Creates an SHA256 HMAC context with a random key that is used for the - * session id. - * - * We do not support loading this from a config file since continuing session - * between restarts of OpenVPN has never been supported and that includes - * early session setup. - */ -hmac_ctx_t *session_id_hmac_init(void); - /** * Calculates the HMAC based server session id based on a client session id * and socket addr. * * @param client_sid session id of the client * @param from link_socket from the client - * @param hmac the hmac context to use for the calculation + * @param key the siphash key to use for the calculation * @param handwindow the quantisation of the current time * @param offset offset to 'now' to use * @return the expected server session id */ struct session_id calculate_session_id_hmac(struct session_id client_sid, - const struct openvpn_sockaddr *from, hmac_ctx_t *hmac, + const struct openvpn_sockaddr *from, + const uint8_t *key, int handwindow, int offset); /** @@ -185,13 +177,13 @@ * * @param state session information * @param from link_socket from the client - * @param hmac the hmac context to use for the calculation + * @param key the siphash key to use for the calculation * @param handwindow the quantisation of the current time * @param pkt_is_ack the packet being checked is a P_ACK_V1 * @return the expected server session id */ bool check_session_hmac_and_pkt_id(struct tls_pre_decrypt_state *state, const struct openvpn_sockaddr *from, - hmac_ctx_t *hmac, int handwindow, bool pkt_is_ack); + uint8_t *key, int handwindow, bool pkt_is_ack); /* * Write a control channel authentication record. diff --git a/tests/unit_tests/openvpn/Makefile.am b/tests/unit_tests/openvpn/Makefile.am index 38adfd7..f2c7a0d 100644 --- a/tests/unit_tests/openvpn/Makefile.am +++ b/tests/unit_tests/openvpn/Makefile.am @@ -172,6 +172,7 @@ $(top_srcdir)/src/openvpn/reliable.c \ $(top_srcdir)/src/openvpn/run_command.c \ $(top_srcdir)/src/openvpn/session_id.c \ + $(top_srcdir)/src/openvpn/siphash_reference.c \ $(top_srcdir)/src/openvpn/ssl_pkt.c \ $(top_srcdir)/src/openvpn/win32-util.c \ $(top_srcdir)/src/openvpn/tls_crypt.c diff --git a/tests/unit_tests/openvpn/test_pkt.c b/tests/unit_tests/openvpn/test_pkt.c index cad2ce0..5ec6781 100644 --- a/tests/unit_tests/openvpn/test_pkt.c +++ b/tests/unit_tests/openvpn/test_pkt.c @@ -42,6 +42,7 @@ #include "mss.h" #include "reliable.h" +#include "siphash.h" int parse_line(const char *line, char **p, const int n, const char *file, const int line_num, @@ -403,7 +404,8 @@ static void test_verify_hmac_tls_auth(void **ut_state) { - hmac_ctx_t *hmac = session_id_hmac_init(); + uint8_t key[SIPHASH_KEY_SIZE] = { 0 }; + siphash_key_init(key); struct link_socket_actual from = { 0 }; from.dest.addr.sa.sa_family = AF_INET; @@ -422,21 +424,20 @@ assert_int_equal(verdict, VERDICT_VALID_CONTROL_V1); /* This is a valid packet but containing a random id instead of an HMAC id*/ - bool valid = check_session_hmac_and_pkt_id(&state, &from.dest, hmac, 30, false); + bool valid = check_session_hmac_and_pkt_id(&state, &from.dest, key, 30, false); assert_false(valid); free_tls_pre_decrypt_state(&state); free_buf(&buf); free_tas(&tas); - hmac_ctx_cleanup(hmac); - hmac_ctx_free(hmac); } static void test_verify_hmac_none(void **ut_state) { now = 1000; - hmac_ctx_t *hmac = session_id_hmac_init(); + uint8_t key[SIPHASH_KEY_SIZE] = { 0 }; + siphash_key_init(key); struct link_socket_actual from = { 0 }; from.dest.addr.sa.sa_family = AF_INET; @@ -456,13 +457,13 @@ assert_int_equal(verdict, VERDICT_VALID_ACK_V1); /* This packet has a random hmac, so it should fail to validate */ - bool valid = check_session_hmac_and_pkt_id(&state, &from.dest, hmac, 30, true); + bool valid = check_session_hmac_and_pkt_id(&state, &from.dest, key, 30, true); assert_false(valid); struct session_id client_id = { { 0xae, 0xb9, 0xaf, 0xe1, 0xf0, 0x1d, 0x79, 0xc8 } }; assert_memory_equal(&client_id, &state.peer_session_id, sizeof(struct session_id)); - struct session_id expected_id = calculate_session_id_hmac(client_id, &from.dest, hmac, 30, 0); + struct session_id expected_id = calculate_session_id_hmac(client_id, &from.dest, key, 30, 0); free_tls_pre_decrypt_state(&state); buf_reset_len(&buf); @@ -474,7 +475,7 @@ verdict = tls_pre_decrypt_lite(&tas, &state, &from, &buf); assert_int_equal(verdict, VERDICT_VALID_ACK_V1); - valid = check_session_hmac_and_pkt_id(&state, &from.dest, hmac, 30, true); + valid = check_session_hmac_and_pkt_id(&state, &from.dest, key, 30, true); assert_true(valid); @@ -483,23 +484,23 @@ * So setting time to the two future ones should work */ now = 980; - assert_false(check_session_hmac_and_pkt_id(&state, &from.dest, hmac, 30, true)); + assert_false(check_session_hmac_and_pkt_id(&state, &from.dest, key, 30, true)); now = 1040; - assert_false(check_session_hmac_and_pkt_id(&state, &from.dest, hmac, 30, true)); + assert_false(check_session_hmac_and_pkt_id(&state, &from.dest, key, 30, true)); now = 1002; - assert_true(check_session_hmac_and_pkt_id(&state, &from.dest, hmac, 30, true)); + assert_true(check_session_hmac_and_pkt_id(&state, &from.dest, key, 30, true)); now = 1022; - assert_true(check_session_hmac_and_pkt_id(&state, &from.dest, hmac, 30, true)); + assert_true(check_session_hmac_and_pkt_id(&state, &from.dest, key, 30, true)); now = 1010; - assert_true(check_session_hmac_and_pkt_id(&state, &from.dest, hmac, 30, true)); + assert_true(check_session_hmac_and_pkt_id(&state, &from.dest, key, 30, true)); /* Changing the IP address should make this invalid */ from.dest.addr.in4.sin_addr.s_addr = ntohl(0x01020305); - assert_false(check_session_hmac_and_pkt_id(&state, &from.dest, hmac, 30, true)); + assert_false(check_session_hmac_and_pkt_id(&state, &from.dest, key, 30, true)); /* Change to the correct one again */ from.dest.addr.in4.sin_addr.s_addr = ntohl(0x01020304); - assert_true(check_session_hmac_and_pkt_id(&state, &from.dest, hmac, 30, true)); + assert_true(check_session_hmac_and_pkt_id(&state, &from.dest, key, 30, true)); /* Modify the peer id, should now fail hmac verification */ buf_inc_len(&buf, -4); @@ -508,18 +509,17 @@ free_tls_pre_decrypt_state(&state); verdict = tls_pre_decrypt_lite(&tas, &state, &from, &buf); assert_int_equal(verdict, VERDICT_VALID_ACK_V1); - assert_false(check_session_hmac_and_pkt_id(&state, &from.dest, hmac, 30, true)); + assert_false(check_session_hmac_and_pkt_id(&state, &from.dest, key, 30, true)); free_tls_pre_decrypt_state(&state); free_buf(&buf); - hmac_ctx_cleanup(hmac); - hmac_ctx_free(hmac); } static void test_verify_hmac_none_out_of_range_ack(void **ut_state) { - hmac_ctx_t *hmac = session_id_hmac_init(); + uint8_t key[SIPHASH_KEY_SIZE] = { 0 }; + siphash_key_init(key); struct link_socket_actual from = { 0 }; from.dest.addr.sa.sa_family = AF_INET; @@ -540,7 +540,7 @@ assert_int_equal(verdict, VERDICT_VALID_ACK_V1); /* should fail because it acks 2 */ - bool valid = check_session_hmac_and_pkt_id(&state, &from.dest, hmac, 30, true); + bool valid = check_session_hmac_and_pkt_id(&state, &from.dest, key, 30, true); assert_false(valid); free_tls_pre_decrypt_state(&state); @@ -552,31 +552,17 @@ assert_int_equal(verdict, VERDICT_VALID_CONTROL_V1); /* should fail because it has message id 2 */ - valid = check_session_hmac_and_pkt_id(&state, &from.dest, hmac, 30, true); + valid = check_session_hmac_and_pkt_id(&state, &from.dest, key, 30, true); assert_false(valid); free_tls_pre_decrypt_state(&state); free_buf(&buf); - hmac_ctx_cleanup(hmac); - hmac_ctx_free(hmac); -} - -static hmac_ctx_t * -init_static_hmac(void) -{ - ASSERT(md_valid("SHA256")); - hmac_ctx_t *hmac_ctx = hmac_ctx_new(); - - uint8_t key[SHA256_DIGEST_LENGTH] = { 1, 2, 3, 0 }; - - hmac_ctx_init(hmac_ctx, key, "SHA256"); - return hmac_ctx; } static void test_calc_session_id_hmac_static(void **ut_state) { - hmac_ctx_t *hmac = init_static_hmac(); + uint8_t key[SIPHASH_KEY_SIZE] = { 1, 2, 3, 0 }; static const int handwindow = 100; struct openvpn_sockaddr addr = { 0 }; @@ -588,27 +574,27 @@ struct session_id client_id = { { 0, 1, 2, 3, 4, 5, 6, 7 } }; now = 1005; - struct session_id server_id = calculate_session_id_hmac(client_id, &addr, hmac, handwindow, 0); + struct session_id server_id = calculate_session_id_hmac(client_id, &addr, key, handwindow, 0); - struct session_id expected_server_id = { { 0x84, 0x73, 0x52, 0x2b, 0x5b, 0xa9, 0x2a, 0x70 } }; + struct session_id expected_server_id = { { 0xec, 0xa3, 0xd5, 0xcc, 0xb4, 0x7c, 0xa1, 0xee } }; /* We have to deal with different structs here annoyingly */ /* Linux has an unsigned short int as family_t and this is field is always * stored in host endianness even though the rest of the struct isn't..., * so Linux little endian differs from all BSD and Linux big endian */ if (sizeof(addr.addr.in4.sin_family) == sizeof(unsigned short int) && ntohs(AF_INET) != AF_INET) { - struct session_id linuxle = { { 0x8b, 0xeb, 0x3d, 0x20, 0x14, 0x53, 0xbe, 0x0a } }; + struct session_id linuxle = { { 0x70, 0x04, 0x8c, 0x0f, 0xfe, 0x30, 0x85, 0x12 } }; expected_server_id = linuxle; } assert_memory_equal(expected_server_id.id, server_id.id, SID_SIZE); struct session_id server_id_m1 = - calculate_session_id_hmac(client_id, &addr, hmac, handwindow, -1); + calculate_session_id_hmac(client_id, &addr, key, handwindow, -1); struct session_id server_id_p1 = - calculate_session_id_hmac(client_id, &addr, hmac, handwindow, 1); + calculate_session_id_hmac(client_id, &addr, key, handwindow, 1); struct session_id server_id_p2 = - calculate_session_id_hmac(client_id, &addr, hmac, handwindow, 2); + calculate_session_id_hmac(client_id, &addr, key, handwindow, 2); assert_memory_not_equal(expected_server_id.id, server_id_m1.id, SID_SIZE); assert_memory_not_equal(expected_server_id.id, server_id_p1.id, SID_SIZE); @@ -618,20 +604,17 @@ now = 1062; struct session_id server_id2_m2 = - calculate_session_id_hmac(client_id, &addr, hmac, handwindow, -2); + calculate_session_id_hmac(client_id, &addr, key, handwindow, -2); struct session_id server_id2_m1 = - calculate_session_id_hmac(client_id, &addr, hmac, handwindow, -1); - struct session_id server_id2 = calculate_session_id_hmac(client_id, &addr, hmac, handwindow, 0); + calculate_session_id_hmac(client_id, &addr, key, handwindow, -1); + struct session_id server_id2 = calculate_session_id_hmac(client_id, &addr, key, handwindow, 0); struct session_id server_id2_p1 = - calculate_session_id_hmac(client_id, &addr, hmac, handwindow, 1); + calculate_session_id_hmac(client_id, &addr, key, handwindow, 1); assert_memory_equal(server_id2_m2.id, server_id_m1.id, SID_SIZE); assert_memory_equal(server_id2_m1.id, expected_server_id.id, SID_SIZE); assert_memory_equal(server_id2.id, server_id_p1.id, SID_SIZE); assert_memory_equal(server_id2_p1.id, server_id_p2.id, SID_SIZE); - - hmac_ctx_cleanup(hmac); - hmac_ctx_free(hmac); } static void -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1827?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: I3b6bb178ffb2bb49981bc23eabf04fe8e06d6fc3 Gerrit-Change-Number: 1827 Gerrit-PatchSet: 8 Gerrit-Owner: plaisthos <[email protected]> Gerrit-Reviewer: flichtenheld <[email protected]> Gerrit-CC: openvpn-devel <[email protected]>
_______________________________________________ Openvpn-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/openvpn-devel
