From patchwork Mon Aug 10 14:36:51 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [Openvpn-devel,01/17] Refactor/Reformat tls_pre_decrypt X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 1360 Message-Id: <20200810143707.5834-2-arne@rfc2549.org> To: openvpn-devel@lists.sourceforge.net Date: Mon, 10 Aug 2020 16:36:51 +0200 From: Arne Schwabe List-Id: - Extract data packet handling to its own function - Replace two instances of if (x) { code } with if (!x) return; code - Remove extra curly braces that were used for pre C99 code style to be able to declare variables in the middle of a block This patch is easier to review with "ignore white space" as the Tested-By: Vladislav Grishenko diff is then a lot smaller in that case and the changes more obvious. Signed-off-by: Arne Schwabe --- src/openvpn/ssl.c | 791 ++++++++++++++++++++++++---------------------- 1 file changed, 410 insertions(+), 381 deletions(-) diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 06dc9f8f..6d146a63 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -3166,6 +3166,102 @@ nohard: * to implement a multiplexed TLS channel over the TCP/UDP port. */ +static inline void +handle_data_channel_paket(struct tls_multi *multi, + const struct link_socket_actual *from, + struct buffer *buf, + struct crypto_options **opt, + bool floated, + const uint8_t **ad_start) +{ + struct gc_arena gc = gc_new(); + + uint8_t c = *BPTR(buf); + int op = c >> P_OPCODE_SHIFT; + int key_id = c & P_KEY_ID_MASK; + + /* data channel packet */ + for (int i = 0; i < KEY_SCAN_SIZE; ++i) + { + struct key_state *ks = multi->key_scan[i]; + + /* + * This is the basic test of TLS state compatibility between a local OpenVPN + * instance and its remote peer. + * + * If the test fails, it tells us that we are getting a packet from a source + * which claims reference to a prior negotiated TLS session, but the local + * OpenVPN instance has no memory of such a negotiation. + * + * It almost always occurs on UDP sessions when the passive side of the + * connection is restarted without the active side restarting as well (the + * passive side is the server which only listens for the connections, the + * active side is the client which initiates connections). + */ + if (DECRYPT_KEY_ENABLED(multi, ks) + && key_id == ks->key_id + && (ks->authenticated == KS_AUTH_TRUE) + && (floated || link_socket_actual_match(from, &ks->remote_addr))) + { + if (!ks->crypto_options.key_ctx_bi.initialized) + { + msg(D_MULTI_DROPPED, + "Key %s [%d] not initialized (yet), dropping packet.", + print_link_socket_actual(from, &gc), key_id); + goto error_lite; + } + + /* return appropriate data channel decrypt key in opt */ + *opt = &ks->crypto_options; + if (op == P_DATA_V2) + { + *ad_start = BPTR(buf); + } + ASSERT(buf_advance(buf, 1)); + if (op == P_DATA_V1) + { + *ad_start = BPTR(buf); + } + else if (op == P_DATA_V2) + { + if (buf->len < 4) + { + msg(D_TLS_ERRORS, "Protocol error: received P_DATA_V2 from %s but length is < 4", + print_link_socket_actual(from, &gc)); + goto error; + } + ASSERT(buf_advance(buf, 3)); + } + + ++ks->n_packets; + ks->n_bytes += buf->len; + dmsg(D_TLS_KEYSELECT, + "TLS: tls_pre_decrypt, key_id=%d, IP=%s", + key_id, print_link_socket_actual(from, &gc)); + gc_free(&gc); + return; + } + } + + msg(D_TLS_ERRORS, + "TLS Error: local/remote TLS keys are out of sync: %s [%d]", + print_link_socket_actual(from, &gc), key_id); + goto error_lite; + + +done: + buf->len = 0; + *opt = NULL; + gc_free(&gc); + return; + +error: + ++multi->n_soft_errors; +error_lite: + tls_clear_error(); + goto done; +} + /* * * When we are in TLS mode, this is the first routine which sees @@ -3199,440 +3295,374 @@ tls_pre_decrypt(struct tls_multi *multi, bool floated, const uint8_t **ad_start) { + + if (buf->len <= 0) + { + buf->len = 0; + *opt = NULL; + return false; + } + struct gc_arena gc = gc_new(); bool ret = false; - if (buf->len > 0) + /* get opcode */ + uint8_t pkt_firstbyte = *BPTR(buf); + int op = pkt_firstbyte >> P_OPCODE_SHIFT; + + if ((op == P_DATA_V1) || (op == P_DATA_V2)) { - int i; - int op; - int key_id; + handle_data_channel_paket(multi, from, buf, opt, floated, ad_start); + return false; + } - /* get opcode and key ID */ + /* get key_id */ + int key_id = pkt_firstbyte & P_KEY_ID_MASK; + + /* control channel packet */ + bool do_burst = false; + bool new_link = false; + struct session_id sid; /* remote session ID */ + + /* verify legal opcode */ + if (op < P_FIRST_OPCODE || op > P_LAST_OPCODE) + { + if (op == P_CONTROL_HARD_RESET_CLIENT_V1 + || op == P_CONTROL_HARD_RESET_SERVER_V1) { - uint8_t c = *BPTR(buf); - op = c >> P_OPCODE_SHIFT; - key_id = c & P_KEY_ID_MASK; + msg(D_TLS_ERRORS, "Peer tried unsupported key-method 1"); } + msg(D_TLS_ERRORS, + "TLS Error: unknown opcode received from %s op=%d", + print_link_socket_actual(from, &gc), op); + goto error; + } - if ((op == P_DATA_V1) || (op == P_DATA_V2)) + /* hard reset ? */ + if (is_hard_reset_method2(op)) + { + /* verify client -> server or server -> client connection */ + if (((op == P_CONTROL_HARD_RESET_CLIENT_V2 + || op == P_CONTROL_HARD_RESET_CLIENT_V3) && !multi->opt.server) + || ((op == P_CONTROL_HARD_RESET_SERVER_V2) && multi->opt.server)) { - /* data channel packet */ - for (i = 0; i < KEY_SCAN_SIZE; ++i) - { - struct key_state *ks = multi->key_scan[i]; - - /* - * This is the basic test of TLS state compatibility between a local OpenVPN - * instance and its remote peer. - * - * If the test fails, it tells us that we are getting a packet from a source - * which claims reference to a prior negotiated TLS session, but the local - * OpenVPN instance has no memory of such a negotiation. - * - * It almost always occurs on UDP sessions when the passive side of the - * connection is restarted without the active side restarting as well (the - * passive side is the server which only listens for the connections, the - * active side is the client which initiates connections). - */ - if (DECRYPT_KEY_ENABLED(multi, ks) - && key_id == ks->key_id - && (ks->authenticated == KS_AUTH_TRUE) - && (floated || link_socket_actual_match(from, &ks->remote_addr))) - { - if (!ks->crypto_options.key_ctx_bi.initialized) - { - msg(D_MULTI_DROPPED, - "Key %s [%d] not initialized (yet), dropping packet.", - print_link_socket_actual(from, &gc), key_id); - goto error_lite; - } - - /* return appropriate data channel decrypt key in opt */ - *opt = &ks->crypto_options; - if (op == P_DATA_V2) - { - *ad_start = BPTR(buf); - } - ASSERT(buf_advance(buf, 1)); - if (op == P_DATA_V1) - { - *ad_start = BPTR(buf); - } - else if (op == P_DATA_V2) - { - if (buf->len < 4) - { - msg(D_TLS_ERRORS, "Protocol error: received P_DATA_V2 from %s but length is < 4", - print_link_socket_actual(from, &gc)); - goto error; - } - ASSERT(buf_advance(buf, 3)); - } + msg(D_TLS_ERRORS, + "TLS Error: client->client or server->server connection attempted from %s", + print_link_socket_actual(from, &gc)); + goto error; + } + } - ++ks->n_packets; - ks->n_bytes += buf->len; - dmsg(D_TLS_KEYSELECT, - "TLS: tls_pre_decrypt, key_id=%d, IP=%s", - key_id, print_link_socket_actual(from, &gc)); - gc_free(&gc); - return ret; - } - } + /* + * Authenticate Packet + */ + dmsg(D_TLS_DEBUG, "TLS: control channel, op=%s, IP=%s", + packet_opcode_name(op), print_link_socket_actual(from, &gc)); + /* get remote session-id */ + { + struct buffer tmp = *buf; + buf_advance(&tmp, 1); + if (!session_id_read(&sid, &tmp) || !session_id_defined(&sid)) + { msg(D_TLS_ERRORS, - "TLS Error: local/remote TLS keys are out of sync: %s [%d]", - print_link_socket_actual(from, &gc), key_id); - goto error_lite; + "TLS Error: session-id not found in packet from %s", + print_link_socket_actual(from, &gc)); + goto error; } - else /* control channel packet */ - { - bool do_burst = false; - bool new_link = false; - struct session_id sid; /* remote session ID */ + } + + int i; + /* use session ID to match up packet with appropriate tls_session object */ + for (i = 0; i < TM_SIZE; ++i) + { + struct tls_session *session = &multi->session[i]; + struct key_state *ks = &session->key[KS_PRIMARY]; + + dmsg(D_TLS_DEBUG, + "TLS: initial packet test, i=%d state=%s, mysid=%s, rec-sid=%s, rec-ip=%s, stored-sid=%s, stored-ip=%s", + i, + state_name(ks->state), + session_id_print(&session->session_id, &gc), + session_id_print(&sid, &gc), + print_link_socket_actual(from, &gc), + session_id_print(&ks->session_id_remote, &gc), + print_link_socket_actual(&ks->remote_addr, &gc)); - /* verify legal opcode */ - if (op < P_FIRST_OPCODE || op > P_LAST_OPCODE) + if (session_id_equal(&ks->session_id_remote, &sid)) + /* found a match */ + { + if (i == TM_LAME_DUCK) { - if (op == P_CONTROL_HARD_RESET_CLIENT_V1 - || op == P_CONTROL_HARD_RESET_SERVER_V1) - { - msg(D_TLS_ERRORS, "Peer tried unsupported key-method 1"); - } msg(D_TLS_ERRORS, - "TLS Error: unknown opcode received from %s op=%d", - print_link_socket_actual(from, &gc), op); + "TLS ERROR: received control packet with stale session-id=%s", + session_id_print(&sid, &gc)); goto error; } + dmsg(D_TLS_DEBUG, + "TLS: found match, session[%d], sid=%s", + i, session_id_print(&sid, &gc)); + break; + } + } - /* hard reset ? */ - if (is_hard_reset_method2(op)) - { - /* verify client -> server or server -> client connection */ - if (((op == P_CONTROL_HARD_RESET_CLIENT_V2 - || op == P_CONTROL_HARD_RESET_CLIENT_V3) && !multi->opt.server) - || ((op == P_CONTROL_HARD_RESET_SERVER_V2) && multi->opt.server)) - { - msg(D_TLS_ERRORS, - "TLS Error: client->client or server->server connection attempted from %s", - print_link_socket_actual(from, &gc)); - goto error; - } - } - - /* - * Authenticate Packet - */ - dmsg(D_TLS_DEBUG, "TLS: control channel, op=%s, IP=%s", - packet_opcode_name(op), print_link_socket_actual(from, &gc)); + /* + * Hard reset and session id does not match any session in + * multi->session: Possible initial packet + */ + if (i == TM_SIZE && is_hard_reset_method2(op)) + { + struct tls_session *session = &multi->session[TM_ACTIVE]; + struct key_state *ks = &session->key[KS_PRIMARY]; - /* get remote session-id */ + /* + * If we have no session currently in progress, the initial packet will + * open a new session in TM_ACTIVE rather than TM_UNTRUSTED. + */ + if (!session_id_defined(&ks->session_id_remote)) + { + if (multi->opt.single_session && multi->n_sessions) { - struct buffer tmp = *buf; - buf_advance(&tmp, 1); - if (!session_id_read(&sid, &tmp) || !session_id_defined(&sid)) - { - msg(D_TLS_ERRORS, - "TLS Error: session-id not found in packet from %s", - print_link_socket_actual(from, &gc)); - goto error; - } + msg(D_TLS_ERRORS, + "TLS Error: Cannot accept new session request from %s due to session context expire or --single-session [1]", + print_link_socket_actual(from, &gc)); + goto error; } - /* use session ID to match up packet with appropriate tls_session object */ - for (i = 0; i < TM_SIZE; ++i) +#ifdef ENABLE_MANAGEMENT + if (management) { - struct tls_session *session = &multi->session[i]; - struct key_state *ks = &session->key[KS_PRIMARY]; - - dmsg(D_TLS_DEBUG, - "TLS: initial packet test, i=%d state=%s, mysid=%s, rec-sid=%s, rec-ip=%s, stored-sid=%s, stored-ip=%s", - i, - state_name(ks->state), - session_id_print(&session->session_id, &gc), - session_id_print(&sid, &gc), - print_link_socket_actual(from, &gc), - session_id_print(&ks->session_id_remote, &gc), - print_link_socket_actual(&ks->remote_addr, &gc)); - - if (session_id_equal(&ks->session_id_remote, &sid)) - /* found a match */ - { - if (i == TM_LAME_DUCK) - { - msg(D_TLS_ERRORS, - "TLS ERROR: received control packet with stale session-id=%s", - session_id_print(&sid, &gc)); - goto error; - } - dmsg(D_TLS_DEBUG, - "TLS: found match, session[%d], sid=%s", - i, session_id_print(&sid, &gc)); - break; - } + management_set_state(management, + OPENVPN_STATE_AUTH, + NULL, + NULL, + NULL, + NULL, + NULL); } +#endif - /* - * Hard reset and session id does not match any session in - * multi->session: Possible initial packet - */ - if (i == TM_SIZE && is_hard_reset_method2(op)) - { - struct tls_session *session = &multi->session[TM_ACTIVE]; - struct key_state *ks = &session->key[KS_PRIMARY]; - - /* - * If we have no session currently in progress, the initial packet will - * open a new session in TM_ACTIVE rather than TM_UNTRUSTED. - */ - if (!session_id_defined(&ks->session_id_remote)) - { - if (multi->opt.single_session && multi->n_sessions) - { - msg(D_TLS_ERRORS, - "TLS Error: Cannot accept new session request from %s due to session context expire or --single-session [1]", - print_link_socket_actual(from, &gc)); - goto error; - } + msg(D_TLS_DEBUG_LOW, + "TLS: Initial packet from %s, sid=%s", + print_link_socket_actual(from, &gc), + session_id_print(&sid, &gc)); -#ifdef ENABLE_MANAGEMENT - if (management) - { - management_set_state(management, - OPENVPN_STATE_AUTH, - NULL, - NULL, - NULL, - NULL, - NULL); - } -#endif + do_burst = true; + new_link = true; + i = TM_ACTIVE; + session->untrusted_addr = *from; + } + } - msg(D_TLS_DEBUG_LOW, - "TLS: Initial packet from %s, sid=%s", - print_link_socket_actual(from, &gc), - session_id_print(&sid, &gc)); + /* + * If we detected new session in the last if block, i has + * changed to TM_ACTIVE, so check the condition again. + */ + if (i == TM_SIZE && is_hard_reset_method2(op)) + { + /* + * No match with existing sessions, + * probably a new session. + */ + struct tls_session *session = &multi->session[TM_UNTRUSTED]; - do_burst = true; - new_link = true; - i = TM_ACTIVE; - session->untrusted_addr = *from; - } - } + /* + * If --single-session, don't allow any hard-reset connection request + * unless it the the first packet of the session. + */ + if (multi->opt.single_session) + { + msg(D_TLS_ERRORS, + "TLS Error: Cannot accept new session request from %s due to session context expire or --single-session [2]", + print_link_socket_actual(from, &gc)); + goto error; + } - /* - * If we detected new session in the last if block, i has - * changed to TM_ACTIVE, so check the condition again. - */ - if (i == TM_SIZE && is_hard_reset_method2(op)) - { - /* - * No match with existing sessions, - * probably a new session. - */ - struct tls_session *session = &multi->session[TM_UNTRUSTED]; - - /* - * If --single-session, don't allow any hard-reset connection request - * unless it the the first packet of the session. - */ - if (multi->opt.single_session) - { - msg(D_TLS_ERRORS, - "TLS Error: Cannot accept new session request from %s due to session context expire or --single-session [2]", - print_link_socket_actual(from, &gc)); - goto error; - } + if (!read_control_auth(buf, &session->tls_wrap, from, + session->opt)) + { + goto error; + } - if (!read_control_auth(buf, &session->tls_wrap, from, - session->opt)) - { - goto error; - } + /* + * New session-initiating control packet is authenticated at this point, + * assuming that the --tls-auth command line option was used. + * + * Without --tls-auth, we leave authentication entirely up to TLS. + */ + msg(D_TLS_DEBUG_LOW, + "TLS: new session incoming connection from %s", + print_link_socket_actual(from, &gc)); - /* - * New session-initiating control packet is authenticated at this point, - * assuming that the --tls-auth command line option was used. - * - * Without --tls-auth, we leave authentication entirely up to TLS. - */ - msg(D_TLS_DEBUG_LOW, - "TLS: new session incoming connection from %s", - print_link_socket_actual(from, &gc)); + new_link = true; + i = TM_UNTRUSTED; + session->untrusted_addr = *from; + } + else + { + struct tls_session *session = &multi->session[i]; + struct key_state *ks = &session->key[KS_PRIMARY]; - new_link = true; - i = TM_UNTRUSTED; - session->untrusted_addr = *from; - } - else + /* + * Packet must belong to an existing session. + */ + if (i != TM_ACTIVE && i != TM_UNTRUSTED) + { + msg(D_TLS_ERRORS, + "TLS Error: Unroutable control packet received from %s (si=%d op=%s)", + print_link_socket_actual(from, &gc), + i, + packet_opcode_name(op)); + goto error; + } + + /* + * Verify remote IP address + */ + if (!new_link && !link_socket_actual_match(&ks->remote_addr, from)) + { + msg(D_TLS_ERRORS, "TLS Error: Received control packet from unexpected IP addr: %s", + print_link_socket_actual(from, &gc)); + goto error; + } + + /* + * Remote is requesting a key renegotiation + */ + if (op == P_CONTROL_SOFT_RESET_V1 + && DECRYPT_KEY_ENABLED(multi, ks)) + { + if (!read_control_auth(buf, &session->tls_wrap, from, + session->opt)) { - struct tls_session *session = &multi->session[i]; - struct key_state *ks = &session->key[KS_PRIMARY]; + goto error; + } - /* - * Packet must belong to an existing session. - */ - if (i != TM_ACTIVE && i != TM_UNTRUSTED) - { - msg(D_TLS_ERRORS, - "TLS Error: Unroutable control packet received from %s (si=%d op=%s)", - print_link_socket_actual(from, &gc), - i, - packet_opcode_name(op)); - goto error; - } + key_state_soft_reset(session); - /* - * Verify remote IP address - */ - if (!new_link && !link_socket_actual_match(&ks->remote_addr, from)) - { - msg(D_TLS_ERRORS, "TLS Error: Received control packet from unexpected IP addr: %s", - print_link_socket_actual(from, &gc)); - goto error; - } + dmsg(D_TLS_DEBUG, + "TLS: received P_CONTROL_SOFT_RESET_V1 s=%d sid=%s", + i, session_id_print(&sid, &gc)); + } + else + { + /* + * Remote responding to our key renegotiation request? + */ + if (op == P_CONTROL_SOFT_RESET_V1) + { + do_burst = true; + } - /* - * Remote is requesting a key renegotiation - */ - if (op == P_CONTROL_SOFT_RESET_V1 - && DECRYPT_KEY_ENABLED(multi, ks)) - { - if (!read_control_auth(buf, &session->tls_wrap, from, - session->opt)) - { - goto error; - } + if (!read_control_auth(buf, &session->tls_wrap, from, + session->opt)) + { + goto error; + } - key_state_soft_reset(session); + dmsg(D_TLS_DEBUG, + "TLS: received control channel packet s#=%d sid=%s", + i, session_id_print(&sid, &gc)); + } + } - dmsg(D_TLS_DEBUG, - "TLS: received P_CONTROL_SOFT_RESET_V1 s=%d sid=%s", - i, session_id_print(&sid, &gc)); - } - else - { - /* - * Remote responding to our key renegotiation request? - */ - if (op == P_CONTROL_SOFT_RESET_V1) - { - do_burst = true; - } + /* + * We have an authenticated control channel packet (if --tls-auth was set). + * Now pass to our reliability layer which deals with + * packet acknowledgements, retransmits, sequencing, etc. + */ + struct tls_session *session = &multi->session[i]; + struct key_state *ks = &session->key[KS_PRIMARY]; - if (!read_control_auth(buf, &session->tls_wrap, from, - session->opt)) - { - goto error; - } + /* Make sure we were initialized and that we're not in an error state */ + ASSERT(ks->state != S_UNDEF); + ASSERT(ks->state != S_ERROR); + ASSERT(session_id_defined(&session->session_id)); - dmsg(D_TLS_DEBUG, - "TLS: received control channel packet s#=%d sid=%s", - i, session_id_print(&sid, &gc)); - } - } + /* Let our caller know we processed a control channel packet */ + ret = true; - /* - * We have an authenticated control channel packet (if --tls-auth was set). - * Now pass to our reliability layer which deals with - * packet acknowledgements, retransmits, sequencing, etc. - */ - { - struct tls_session *session = &multi->session[i]; - struct key_state *ks = &session->key[KS_PRIMARY]; + /* + * Set our remote address and remote session_id + */ + if (new_link) + { + ks->session_id_remote = sid; + ks->remote_addr = *from; + ++multi->n_sessions; + } + else if (!link_socket_actual_match(&ks->remote_addr, from)) + { + msg(D_TLS_ERRORS, + "TLS Error: Existing session control channel packet from unknown IP address: %s", + print_link_socket_actual(from, &gc)); + goto error; + } - /* Make sure we were initialized and that we're not in an error state */ - ASSERT(ks->state != S_UNDEF); - ASSERT(ks->state != S_ERROR); - ASSERT(session_id_defined(&session->session_id)); + /* + * Should we do a retransmit of all unacknowledged packets in + * the send buffer? This improves the start-up efficiency of the + * initial key negotiation after the 2nd peer comes online. + */ + if (do_burst && !session->burst) + { + reliable_schedule_now(ks->send_reliable); + session->burst = true; + } - /* Let our caller know we processed a control channel packet */ - ret = true; + /* Check key_id */ + if (ks->key_id != key_id) + { + msg(D_TLS_ERRORS, + "TLS ERROR: local/remote key IDs out of sync (%d/%d) ID: %s", + ks->key_id, key_id, print_key_id(multi, &gc)); + goto error; + } - /* - * Set our remote address and remote session_id - */ - if (new_link) - { - ks->session_id_remote = sid; - ks->remote_addr = *from; - ++multi->n_sessions; - } - else if (!link_socket_actual_match(&ks->remote_addr, from)) - { - msg(D_TLS_ERRORS, - "TLS Error: Existing session control channel packet from unknown IP address: %s", - print_link_socket_actual(from, &gc)); - goto error; - } + /* + * Process incoming ACKs for packets we can now + * delete from reliable send buffer + */ + { + /* buffers all packet IDs to delete from send_reliable */ + struct reliable_ack send_ack; - /* - * Should we do a retransmit of all unacknowledged packets in - * the send buffer? This improves the start-up efficiency of the - * initial key negotiation after the 2nd peer comes online. - */ - if (do_burst && !session->burst) - { - reliable_schedule_now(ks->send_reliable); - session->burst = true; - } + send_ack.len = 0; + if (!reliable_ack_read(&send_ack, buf, &session->session_id)) + { + msg(D_TLS_ERRORS, + "TLS Error: reading acknowledgement record from packet"); + goto error; + } + reliable_send_purge(ks->send_reliable, &send_ack); + } - /* Check key_id */ - if (ks->key_id != key_id) - { - msg(D_TLS_ERRORS, - "TLS ERROR: local/remote key IDs out of sync (%d/%d) ID: %s", - ks->key_id, key_id, print_key_id(multi, &gc)); - goto error; - } + if (op != P_ACK_V1 && reliable_can_get(ks->rec_reliable)) + { + packet_id_type id; - /* - * Process incoming ACKs for packets we can now - * delete from reliable send buffer - */ + /* Extract the packet ID from the packet */ + if (reliable_ack_read_packet_id(buf, &id)) + { + /* Avoid deadlock by rejecting packet that would de-sequentialize receive buffer */ + if (reliable_wont_break_sequentiality(ks->rec_reliable, id)) + { + if (reliable_not_replay(ks->rec_reliable, id)) { - /* buffers all packet IDs to delete from send_reliable */ - struct reliable_ack send_ack; - - send_ack.len = 0; - if (!reliable_ack_read(&send_ack, buf, &session->session_id)) + /* Save incoming ciphertext packet to reliable buffer */ + struct buffer *in = reliable_get_buf(ks->rec_reliable); + ASSERT(in); + if (!buf_copy(in, buf)) { - msg(D_TLS_ERRORS, - "TLS Error: reading acknowledgement record from packet"); + msg(D_MULTI_DROPPED, + "Incoming control channel packet too big, dropping."); goto error; } - reliable_send_purge(ks->send_reliable, &send_ack); + reliable_mark_active_incoming(ks->rec_reliable, in, id, op); } - if (op != P_ACK_V1 && reliable_can_get(ks->rec_reliable)) - { - packet_id_type id; - - /* Extract the packet ID from the packet */ - if (reliable_ack_read_packet_id(buf, &id)) - { - /* Avoid deadlock by rejecting packet that would de-sequentialize receive buffer */ - if (reliable_wont_break_sequentiality(ks->rec_reliable, id)) - { - if (reliable_not_replay(ks->rec_reliable, id)) - { - /* Save incoming ciphertext packet to reliable buffer */ - struct buffer *in = reliable_get_buf(ks->rec_reliable); - ASSERT(in); - if (!buf_copy(in, buf)) - { - msg(D_MULTI_DROPPED, - "Incoming control channel packet too big, dropping."); - goto error; - } - reliable_mark_active_incoming(ks->rec_reliable, in, id, op); - } - - /* Process outgoing acknowledgment for packet just received, even if it's a replay */ - reliable_ack_acknowledge_packet_id(ks->rec_ack, id); - } - } - } + /* Process outgoing acknowledgment for packet just received, even if it's a replay */ + reliable_ack_acknowledge_packet_id(ks->rec_ack, id); } } } @@ -3645,7 +3675,6 @@ done: error: ++multi->n_soft_errors; -error_lite: tls_clear_error(); goto done; } From patchwork Mon Aug 10 14:36:52 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [Openvpn-devel,02/17] Cleanup tls_pre_decrypt_lite and tls_pre_encrypt X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 1367 Message-Id: <20200810143707.5834-3-arne@rfc2549.org> To: openvpn-devel@lists.sourceforge.net Date: Mon, 10 Aug 2020 16:36:52 +0200 From: Arne Schwabe List-Id: Mostly C90 -> C99 cleanups and again immediately instead wrapping function body into if. (Review with ignore whitespace) Signed-off-by: Arne Schwabe Tested-By: Vladislav Grishenko Acked-by: Gert Doering --- src/openvpn/ssl.c | 224 ++++++++++++++++++++++------------------------ 1 file changed, 109 insertions(+), 115 deletions(-) diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 6d146a63..2354a017 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -3455,7 +3455,7 @@ tls_pre_decrypt(struct tls_multi *multi, } /* - * If we detected new session in the last if block, i has + * If we detected new session in the last if block, variable i has * changed to TM_ACTIVE, so check the condition again. */ if (i == TM_SIZE && is_hard_reset_method2(op)) @@ -3468,7 +3468,7 @@ tls_pre_decrypt(struct tls_multi *multi, /* * If --single-session, don't allow any hard-reset connection request - * unless it the the first packet of the session. + * unless it the first packet of the session. */ if (multi->opt.single_session) { @@ -3696,100 +3696,91 @@ tls_pre_decrypt_lite(const struct tls_auth_standalone *tas, const struct buffer *buf) { - struct gc_arena gc = gc_new(); - bool ret = false; - - if (buf->len > 0) + if (buf->len <= 0) { - int op; - int key_id; + return false; + } + struct gc_arena gc = gc_new(); - /* get opcode and key ID */ - { - uint8_t c = *BPTR(buf); - op = c >> P_OPCODE_SHIFT; - key_id = c & P_KEY_ID_MASK; - } + /* get opcode and key ID */ + uint8_t pkt_firstbyte = *BPTR(buf); + int op = pkt_firstbyte >> P_OPCODE_SHIFT; + int key_id = pkt_firstbyte & P_KEY_ID_MASK; - /* this packet is from an as-yet untrusted source, so - * scrutinize carefully */ + /* this packet is from an as-yet untrusted source, so + * scrutinize carefully */ - if (op != P_CONTROL_HARD_RESET_CLIENT_V2 - && op != P_CONTROL_HARD_RESET_CLIENT_V3) - { - /* - * This can occur due to bogus data or DoS packets. - */ - dmsg(D_TLS_STATE_ERRORS, - "TLS State Error: No TLS state for client %s, opcode=%d", - print_link_socket_actual(from, &gc), - op); - goto error; - } + if (op != P_CONTROL_HARD_RESET_CLIENT_V2 + && op != P_CONTROL_HARD_RESET_CLIENT_V3) + { + /* + * This can occur due to bogus data or DoS packets. + */ + dmsg(D_TLS_STATE_ERRORS, + "TLS State Error: No TLS state for client %s, opcode=%d", + print_link_socket_actual(from, &gc), + op); + goto error; + } - if (key_id != 0) - { - dmsg(D_TLS_STATE_ERRORS, - "TLS State Error: Unknown key ID (%d) received from %s -- 0 was expected", - key_id, - print_link_socket_actual(from, &gc)); - goto error; - } + if (key_id != 0) + { + dmsg(D_TLS_STATE_ERRORS, + "TLS State Error: Unknown key ID (%d) received from %s -- 0 was expected", + key_id, + print_link_socket_actual(from, &gc)); + goto error; + } - if (buf->len > EXPANDED_SIZE_DYNAMIC(&tas->frame)) - { - dmsg(D_TLS_STATE_ERRORS, - "TLS State Error: Large packet (size %d) received from %s -- a packet no larger than %d bytes was expected", - buf->len, - print_link_socket_actual(from, &gc), - EXPANDED_SIZE_DYNAMIC(&tas->frame)); - goto error; - } + if (buf->len > EXPANDED_SIZE_DYNAMIC(&tas->frame)) + { + dmsg(D_TLS_STATE_ERRORS, + "TLS State Error: Large packet (size %d) received from %s -- a packet no larger than %d bytes was expected", + buf->len, + print_link_socket_actual(from, &gc), + EXPANDED_SIZE_DYNAMIC(&tas->frame)); + goto error; + } - { - struct buffer newbuf = clone_buf(buf); - struct tls_wrap_ctx tls_wrap_tmp = tas->tls_wrap; - bool status; - - /* HMAC test, if --tls-auth was specified */ - status = read_control_auth(&newbuf, &tls_wrap_tmp, from, NULL); - free_buf(&newbuf); - free_buf(&tls_wrap_tmp.tls_crypt_v2_metadata); - if (tls_wrap_tmp.cleanup_key_ctx) - { - free_key_ctx_bi(&tls_wrap_tmp.opt.key_ctx_bi); - } - if (!status) - { - goto error; - } - /* - * At this point, if --tls-auth is being used, we know that - * the packet has passed the HMAC test, but we don't know if - * it is a replay yet. We will attempt to defeat replays - * by not advancing to the S_START state until we - * receive an ACK from our first reply to the client - * that includes an HMAC of our randomly generated 64 bit - * session ID. - * - * On the other hand if --tls-auth is not being used, we - * will proceed to begin the TLS authentication - * handshake with only cursory integrity checks having - * been performed, since we will be leaving the task - * of authentication solely up to TLS. - */ + struct buffer newbuf = clone_buf(buf); + struct tls_wrap_ctx tls_wrap_tmp = tas->tls_wrap; - ret = true; - } + /* HMAC test, if --tls-auth was specified */ + bool status = read_control_auth(&newbuf, &tls_wrap_tmp, from, NULL); + free_buf(&newbuf); + free_buf(&tls_wrap_tmp.tls_crypt_v2_metadata); + if (tls_wrap_tmp.cleanup_key_ctx) + { + free_key_ctx_bi(&tls_wrap_tmp.opt.key_ctx_bi); + } + if (!status) + { + goto error; } + + /* + * At this point, if --tls-auth is being used, we know that + * the packet has passed the HMAC test, but we don't know if + * it is a replay yet. We will attempt to defeat replays + * by not advancing to the S_START state until we + * receive an ACK from our first reply to the client + * that includes an HMAC of our randomly generated 64 bit + * session ID. + * + * On the other hand if --tls-auth is not being used, we + * will proceed to begin the TLS authentication + * handshake with only cursory integrity checks having + * been performed, since we will be leaving the task + * of authentication solely up to TLS. + */ gc_free(&gc); - return ret; + return true; error: tls_clear_error(); gc_free(&gc); - return ret; + return false; } /* Choose the key with which to encrypt a data packet */ @@ -3798,48 +3789,51 @@ tls_pre_encrypt(struct tls_multi *multi, struct buffer *buf, struct crypto_options **opt) { multi->save_ks = NULL; - if (buf->len > 0) + if (buf->len <= 0) + { + buf->len = 0; + *opt = NULL; + return; + } + + struct key_state *ks_select = NULL; + for (int i = 0; i < KEY_SCAN_SIZE; ++i) { - int i; - struct key_state *ks_select = NULL; - for (i = 0; i < KEY_SCAN_SIZE; ++i) + struct key_state *ks = multi->key_scan[i]; + if (ks->state >= S_ACTIVE + && (ks->authenticated == KS_AUTH_TRUE) + && ks->crypto_options.key_ctx_bi.initialized + ) { - struct key_state *ks = multi->key_scan[i]; - if (ks->state >= S_ACTIVE - && (ks->authenticated == KS_AUTH_TRUE) - && ks->crypto_options.key_ctx_bi.initialized - ) + if (!ks_select) { - if (!ks_select) - { - ks_select = ks; - } - if (now >= ks->auth_deferred_expire) - { - ks_select = ks; - break; - } + ks_select = ks; + } + if (now >= ks->auth_deferred_expire) + { + ks_select = ks; + break; } } + } - if (ks_select) - { - *opt = &ks_select->crypto_options; - multi->save_ks = ks_select; - dmsg(D_TLS_KEYSELECT, "TLS: tls_pre_encrypt: key_id=%d", ks_select->key_id); - return; - } - else - { - struct gc_arena gc = gc_new(); - dmsg(D_TLS_KEYSELECT, "TLS Warning: no data channel send key available: %s", - print_key_id(multi, &gc)); - gc_free(&gc); - } + if (ks_select) + { + *opt = &ks_select->crypto_options; + multi->save_ks = ks_select; + dmsg(D_TLS_KEYSELECT, "TLS: tls_pre_encrypt: key_id=%d", ks_select->key_id); + return; } + else + { + struct gc_arena gc = gc_new(); + dmsg(D_TLS_KEYSELECT, "TLS Warning: no data channel send key available: %s", + print_key_id(multi, &gc)); + gc_free(&gc); - buf->len = 0; - *opt = NULL; + *opt = NULL; + buf->len = 0; + } } void From patchwork Mon Aug 10 14:36:53 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [Openvpn-devel,03/17] Clean up a number of leftover C89 initialisations in ssl.c X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 1361 Message-Id: <20200810143707.5834-4-arne@rfc2549.org> To: openvpn-devel@lists.sourceforge.net Date: Mon, 10 Aug 2020 16:36:53 +0200 From: Arne Schwabe List-Id: Signed-off-by: Arne Schwabe Acked-by: Gert Doering --- src/openvpn/ssl.c | 56 +++++++++++++++++------------------------------ 1 file changed, 20 insertions(+), 36 deletions(-) diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 2354a017..3bf0dcf8 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -831,10 +831,9 @@ session_index_name(int index) static const char * print_key_id(struct tls_multi *multi, struct gc_arena *gc) { - int i; struct buffer out = alloc_buf_gc(256, gc); - for (i = 0; i < KEY_SCAN_SIZE; ++i) + for (int i = 0; i < KEY_SCAN_SIZE; ++i) { struct key_state *ks = multi->key_scan[i]; buf_printf(&out, " [key#%d state=%s id=%d sid=%s]", i, @@ -1315,8 +1314,6 @@ tls_multi_init_set_options(struct tls_multi *multi, void tls_multi_free(struct tls_multi *multi, bool clear) { - int i; - ASSERT(multi); auth_set_client_reason(multi, NULL); @@ -1339,7 +1336,7 @@ tls_multi_free(struct tls_multi *multi, bool clear) free(multi->remote_ciphername); - for (i = 0; i < TM_SIZE; ++i) + for (int i = 0; i < TM_SIZE; ++i) { tls_session_free(&multi->session[i], false); } @@ -1367,11 +1364,10 @@ tls_multi_free(struct tls_multi *multi, bool clear) static bool swap_hmac(struct buffer *buf, const struct crypto_options *co, bool incoming) { - const struct key_ctx *ctx; - ASSERT(co); - ctx = (incoming ? &co->key_ctx_bi.decrypt : &co->key_ctx_bi.encrypt); + const struct key_ctx *ctx = (incoming ? &co->key_ctx_bi.decrypt : + &co->key_ctx_bi.encrypt); ASSERT(ctx->hmac); { @@ -1623,25 +1619,21 @@ tls1_P_hash(const md_kt_t *md_kt, int olen) { struct gc_arena gc = gc_new(); - int chunk; - hmac_ctx_t *ctx; - hmac_ctx_t *ctx_tmp; uint8_t A1[MAX_HMAC_KEY_LENGTH]; - unsigned int A1_len; #ifdef ENABLE_DEBUG const int olen_orig = olen; const uint8_t *out_orig = out; #endif - ctx = hmac_ctx_new(); - ctx_tmp = hmac_ctx_new(); + hmac_ctx_t *ctx = hmac_ctx_new(); + hmac_ctx_t *ctx_tmp = hmac_ctx_new(); dmsg(D_SHOW_KEY_SOURCE, "tls1_P_hash sec: %s", format_hex(sec, sec_len, 0, &gc)); dmsg(D_SHOW_KEY_SOURCE, "tls1_P_hash seed: %s", format_hex(seed, seed_len, 0, &gc)); - chunk = md_kt_size(md_kt); - A1_len = md_kt_size(md_kt); + int chunk = md_kt_size(md_kt); + unsigned int A1_len = md_kt_size(md_kt); hmac_ctx_init(ctx, sec, sec_len, md_kt); hmac_ctx_init(ctx_tmp, sec, sec_len, md_kt); @@ -1711,21 +1703,18 @@ tls1_PRF(const uint8_t *label, struct gc_arena gc = gc_new(); const md_kt_t *md5 = md_kt_get("MD5"); const md_kt_t *sha1 = md_kt_get("SHA1"); - int len,i; - const uint8_t *S1,*S2; - uint8_t *out2; - out2 = (uint8_t *) gc_malloc(olen, false, &gc); + uint8_t *out2 = (uint8_t *) gc_malloc(olen, false, &gc); - len = slen/2; - S1 = sec; - S2 = &(sec[len]); + int len = slen/2; + const uint8_t *S1 = sec; + const uint8_t *S2 = &(sec[len]); len += (slen&1); /* add for odd, make longer */ tls1_P_hash(md5,S1,len,label,label_len,out1,olen); tls1_P_hash(sha1,S2,len,label,label_len,out2,olen); - for (i = 0; iopt->push_peer_info_detail > 0) { struct env_set *es = session->opt->es; - struct env_item *e; struct buffer out = alloc_buf_gc(512*3, &gc); /* push version */ @@ -2271,7 +2259,7 @@ push_peer_info(struct buffer *buf, struct tls_session *session) } /* push env vars that begin with UV_, IV_PLAT_VER and IV_GUI_VER */ - for (e = es->list; e != NULL; e = e->next) + for (struct env_item *e = es->list; e != NULL; e = e->next) { if (e->string) { @@ -3004,10 +2992,8 @@ tls_multi_process(struct tls_multi *multi, interval_t *wakeup) { struct gc_arena gc = gc_new(); - int i; int active = TLSMP_INACTIVE; bool error = false; - int tas; perf_push(PERF_TLS_MULTI_PROCESS); @@ -3018,7 +3004,7 @@ tls_multi_process(struct tls_multi *multi, * and which has a defined remote IP addr. */ - for (i = 0; i < TM_SIZE; ++i) + for (int i = 0; i < TM_SIZE; ++i) { struct tls_session *session = &multi->session[i]; struct key_state *ks = &session->key[KS_PRIMARY]; @@ -3093,7 +3079,7 @@ tls_multi_process(struct tls_multi *multi, update_time(); - tas = tls_authentication_status(multi, TLS_MULTI_AUTH_STATUS_INTERVAL); + int tas = tls_authentication_status(multi, TLS_MULTI_AUTH_STATUS_INTERVAL); /* * If lame duck session expires, kill it. @@ -3126,7 +3112,7 @@ tls_multi_process(struct tls_multi *multi, */ if (error) { - for (i = 0; i < (int) SIZE(multi->key_scan); ++i) + for (int i = 0; i < (int) SIZE(multi->key_scan); ++i) { if (multi->key_scan[i]->state >= S_ACTIVE) { @@ -3143,7 +3129,7 @@ nohard: const int throw_level = GREMLIN_CONNECTION_FLOOD_LEVEL(multi->opt.gremlin); if (throw_level) { - for (i = 0; i < (int) SIZE(multi->key_scan); ++i) + for (int i = 0; i < (int) SIZE(multi->key_scan); ++i) { if (multi->key_scan[i]->state >= throw_level) { @@ -3957,13 +3943,11 @@ void tls_update_remote_addr(struct tls_multi *multi, const struct link_socket_actual *addr) { struct gc_arena gc = gc_new(); - int i, j; - - for (i = 0; i < TM_SIZE; ++i) + for (int i = 0; i < TM_SIZE; ++i) { struct tls_session *session = &multi->session[i]; - for (j = 0; j < KS_SIZE; ++j) + for (int j = 0; j < KS_SIZE; ++j) { struct key_state *ks = &session->key[j]; From patchwork Mon Aug 10 14:36:54 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [Openvpn-devel,04/17] Minor cleanup in push.c X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 1368 Message-Id: <20200810143707.5834-5-arne@rfc2549.org> To: openvpn-devel@lists.sourceforge.net Date: Mon, 10 Aug 2020 16:36:54 +0200 From: Arne Schwabe List-Id: Signed-off-by: Arne Schwabe Acked-by: Gert Doering --- src/openvpn/push.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/openvpn/push.c b/src/openvpn/push.c index f10021f8..d20b345d 100644 --- a/src/openvpn/push.c +++ b/src/openvpn/push.c @@ -330,13 +330,10 @@ incoming_push_message(struct context *c, const struct buffer *buffer) { struct gc_arena gc = gc_new(); unsigned int option_types_found = 0; - int status; msg(D_PUSH, "PUSH: Received control message: '%s'", sanitize_control_message(BSTR(buffer), &gc)); - status = process_incoming_push_msg(c, - buffer, - c->options.pull, + int status = process_incoming_push_msg(c, buffer, c->options.pull, pull_permission_mask(c), &option_types_found); @@ -866,7 +863,7 @@ process_incoming_push_msg(struct context *c, return process_incoming_push_request(c); } else if (honor_received_options - && buf_string_compare_advance(&buf, "PUSH_REPLY")) + && buf_string_compare_advance(&buf, push_reply_cmd)) { return process_incoming_push_reply(c, permission_mask, option_types_found, &buf); From patchwork Mon Aug 10 14:36:55 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [Openvpn-devel,05/17] Remove buf argument from link_socket_set_outgoing_addr X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 1356 Message-Id: <20200810143707.5834-6-arne@rfc2549.org> To: openvpn-devel@lists.sourceforge.net Date: Mon, 10 Aug 2020 16:36:55 +0200 From: Arne Schwabe List-Id: This was only used in a check that is better suited in the calling functions. This also removes passing the buf argument to link_socket_connection_initiated that also does not use that parameter at all. Signed-off-by: Arne Schwabe Acked-by: Gert Doering --- src/openvpn/forward.c | 4 ++-- src/openvpn/socket.c | 3 +-- src/openvpn/socket.h | 35 +++++++++++++++-------------------- src/openvpn/ssl.c | 2 +- 4 files changed, 19 insertions(+), 25 deletions(-) diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c index 79c07e46..8a0d63f7 100644 --- a/src/openvpn/forward.c +++ b/src/openvpn/forward.c @@ -1177,9 +1177,9 @@ process_incoming_link_part2(struct context *c, struct link_socket_info *lsi, con * * Also, update the persisted version of our packet-id. */ - if (!TLS_MODE(c)) + if (!TLS_MODE(c) && c->c2.buf.len > 0) { - link_socket_set_outgoing_addr(&c->c2.buf, lsi, &c->c2.from, NULL, c->c2.es); + link_socket_set_outgoing_addr(lsi, &c->c2.from, NULL, c->c2.es); } /* reset packet received timer */ diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c index 61463bcd..c486327b 100644 --- a/src/openvpn/socket.c +++ b/src/openvpn/socket.c @@ -2450,8 +2450,7 @@ ipchange_fmt(const bool include_cmd, struct argv *argv, const struct link_socket } void -link_socket_connection_initiated(const struct buffer *buf, - struct link_socket_info *info, +link_socket_connection_initiated(struct link_socket_info *info, const struct link_socket_actual *act, const char *common_name, struct env_set *es) diff --git a/src/openvpn/socket.h b/src/openvpn/socket.h index 8db9e8ba..7aeae527 100644 --- a/src/openvpn/socket.h +++ b/src/openvpn/socket.h @@ -435,8 +435,7 @@ in_addr_t link_socket_current_remote(const struct link_socket_info *info); const struct in6_addr *link_socket_current_remote_ipv6 (const struct link_socket_info *info); -void link_socket_connection_initiated(const struct buffer *buf, - struct link_socket_info *info, +void link_socket_connection_initiated(struct link_socket_info *info, const struct link_socket_actual *addr, const char *common_name, struct env_set *es); @@ -984,29 +983,25 @@ link_socket_get_outgoing_addr(struct buffer *buf, } static inline void -link_socket_set_outgoing_addr(const struct buffer *buf, - struct link_socket_info *info, +link_socket_set_outgoing_addr(struct link_socket_info *info, const struct link_socket_actual *act, const char *common_name, struct env_set *es) { - if (!buf || buf->len > 0) + struct link_socket_addr *lsa = info->lsa; + if ( + /* new or changed address? */ + (!info->connection_established + || !addr_match_proto(&act->dest, &lsa->actual.dest, info->proto) + ) + && + /* address undef or address == remote or --float */ + (info->remote_float + || (!lsa->remote_list || addrlist_match_proto(&act->dest, lsa->remote_list, info->proto)) + ) + ) { - struct link_socket_addr *lsa = info->lsa; - if ( - /* new or changed address? */ - (!info->connection_established - || !addr_match_proto(&act->dest, &lsa->actual.dest, info->proto) - ) - && - /* address undef or address == remote or --float */ - (info->remote_float - || (!lsa->remote_list || addrlist_match_proto(&act->dest, lsa->remote_list, info->proto)) - ) - ) - { - link_socket_connection_initiated(buf, info, act, common_name, es); - } + link_socket_connection_initiated(info, act, common_name, es); } } diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 3bf0dcf8..a43ee985 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -2762,7 +2762,7 @@ tls_process(struct tls_multi *multi, INCR_SUCCESS; /* Set outgoing address for data channel packets */ - link_socket_set_outgoing_addr(NULL, to_link_socket_info, &ks->remote_addr, session->common_name, session->opt->es); + link_socket_set_outgoing_addr(to_link_socket_info, &ks->remote_addr, session->common_name, session->opt->es); /* Flush any payload packets that were buffered before our state transitioned to S_ACTIVE */ flush_payload_buffer(ks); From patchwork Mon Aug 10 14:36:56 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [Openvpn-devel,06/17] Remove a number of check/do_work wrapper calls from coarse_timers X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 1370 Message-Id: <20200810143707.5834-7-arne@rfc2549.org> To: openvpn-devel@lists.sourceforge.net Date: Mon, 10 Aug 2020 16:36:56 +0200 From: Arne Schwabe List-Id: This indirection is not very helpful in understanding the code flow. Moving the check to process_coarse_timers and remove the check function and rename the do_work function to the drop the do_work as it does no longer serve a purpose Signed-off-by: Arne Schwabe Acked-by: Gert Doering --- src/openvpn/forward.c | 166 +++++++++++------------------------------- src/openvpn/forward.h | 12 +-- src/openvpn/status.c | 13 ---- src/openvpn/status.h | 2 - 4 files changed, 47 insertions(+), 146 deletions(-) diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c index 8a0d63f7..7ac878f9 100644 --- a/src/openvpn/forward.c +++ b/src/openvpn/forward.c @@ -138,84 +138,6 @@ check_incoming_control_channel(struct context *c) #endif } -/* - * Should we add routes? - */ -static inline void -check_add_routes(struct context *c) -{ - void check_add_routes_dowork(struct context *c); - - if (event_timeout_trigger(&c->c2.route_wakeup, &c->c2.timeval, ETT_DEFAULT)) - { - check_add_routes_dowork(c); - } -} - -/* - * Should we exit due to inactivity timeout? - */ -static inline void -check_inactivity_timeout(struct context *c) -{ - void check_inactivity_timeout_dowork(struct context *c); - - if (c->options.inactivity_timeout - && event_timeout_trigger(&c->c2.inactivity_interval, &c->c2.timeval, ETT_DEFAULT)) - { - check_inactivity_timeout_dowork(c); - } -} - -#if P2MP - -static inline void -check_server_poll_timeout(struct context *c) -{ - void check_server_poll_timeout_dowork(struct context *c); - - if (c->options.ce.connect_timeout - && event_timeout_trigger(&c->c2.server_poll_interval, &c->c2.timeval, ETT_DEFAULT)) - { - check_server_poll_timeout_dowork(c); - } -} - -/* - * Scheduled exit? - */ -static inline void -check_scheduled_exit(struct context *c) -{ - void check_scheduled_exit_dowork(struct context *c); - - if (event_timeout_defined(&c->c2.scheduled_exit)) - { - if (event_timeout_trigger(&c->c2.scheduled_exit, &c->c2.timeval, ETT_DEFAULT)) - { - check_scheduled_exit_dowork(c); - } - } -} -#endif /* if P2MP */ - -/* - * Should we write timer-triggered status file. - */ -static inline void -check_status_file(struct context *c) -{ - void check_status_file_dowork(struct context *c); - - if (c->c1.status_output) - { - if (status_trigger_tv(c->c1.status_output, &c->c2.timeval)) - { - check_status_file_dowork(c); - } - } -} - #ifdef ENABLE_FRAGMENT /* * Should we deliver a datagram fragment to remote? @@ -232,37 +154,6 @@ check_fragment(struct context *c) } #endif -#if P2MP - -/* - * see if we should send a push_request in response to --pull - */ -static inline void -check_push_request(struct context *c) -{ - void check_push_request_dowork(struct context *c); - - if (event_timeout_trigger(&c->c2.push_request_interval, &c->c2.timeval, ETT_DEFAULT)) - { - check_push_request_dowork(c); - } -} - -#endif - -/* - * Should we persist our anti-replay packet ID state to disk? - */ -static inline void -check_packet_id_persist_flush(struct context *c) -{ - if (packet_id_persist_enabled(&c->c1.pid_persist) - && event_timeout_trigger(&c->c2.packet_id_persist_interval, &c->c2.timeval, ETT_DEFAULT)) - { - packet_id_persist_save(&c->c1.pid_persist); - } -} - /* * Set our wakeup to 0 seconds, so we will be rescheduled * immediately. @@ -410,7 +301,7 @@ check_incoming_control_channel_dowork(struct context *c) * Periodically resend PUSH_REQUEST until PUSH message received */ void -check_push_request_dowork(struct context *c) +check_push_request(struct context *c) { send_push_request(c); @@ -521,7 +412,7 @@ check_add_routes_action(struct context *c, const bool errors) } void -check_add_routes_dowork(struct context *c) +check_add_routes(struct context *c) { if (test_routes(c->c1.route_list, c->c1.tuntap)) { @@ -559,7 +450,7 @@ check_add_routes_dowork(struct context *c) * Should we exit due to inactivity timeout? */ void -check_inactivity_timeout_dowork(struct context *c) +check_inactivity_timeout(struct context *c) { msg(M_INFO, "Inactivity timeout (--inactive), exiting"); register_signal(c, SIGTERM, "inactive"); @@ -575,7 +466,7 @@ get_server_poll_remaining_time(struct event_timeout *server_poll_timeout) #if P2MP void -check_server_poll_timeout_dowork(struct context *c) +check_server_poll_timeout(struct context *c) { event_timeout_reset(&c->c2.server_poll_interval); ASSERT(c->c2.tls_multi); @@ -605,7 +496,7 @@ schedule_exit(struct context *c, const int n_seconds, const int signal) * Scheduled exit? */ void -check_scheduled_exit_dowork(struct context *c) +check_scheduled_exit(struct context *c) { register_signal(c, c->c2.scheduled_exit_signal, "delayed-exit"); } @@ -616,7 +507,7 @@ check_scheduled_exit_dowork(struct context *c) * Should we write timer-triggered status file. */ void -check_status_file_dowork(struct context *c) +check_status_file(struct context *c) { if (c->c1.status_output) { @@ -761,10 +652,18 @@ process_coarse_timers(struct context *c) { /* flush current packet-id to file once per 60 * seconds if --replay-persist was specified */ - check_packet_id_persist_flush(c); + if (packet_id_persist_enabled(&c->c1.pid_persist) + && event_timeout_trigger(&c->c2.packet_id_persist_interval, &c->c2.timeval, ETT_DEFAULT)) + { + packet_id_persist_save(&c->c1.pid_persist); + } - /* should we update status file? */ - check_status_file(c); + /* Should we write timer-triggered status file */ + if (c->c1.status_output + && event_timeout_trigger(&c->c1.status_output->et, &c->c2.timeval, ETT_DEFAULT)) + { + check_status_file(c); + } /* process connection establishment items */ if (event_timeout_trigger(&c->c2.wait_for_connect, &c->c2.timeval, ETT_DEFAULT)) @@ -772,8 +671,11 @@ process_coarse_timers(struct context *c) check_connection_established(c); } #if P2MP - /* see if we should send a push_request in response to --pull */ - check_push_request(c); + /* see if we should send a push_request (option --pull) */ + if (event_timeout_trigger(&c->c2.push_request_interval, &c->c2.timeval, ETT_DEFAULT)) + { + check_push_request(c); + } #endif #ifdef PLUGIN_PF @@ -781,10 +683,18 @@ process_coarse_timers(struct context *c) #endif /* process --route options */ - check_add_routes(c); + if (event_timeout_trigger(&c->c2.route_wakeup, &c->c2.timeval, ETT_DEFAULT)) + { + check_add_routes(c); + } /* possibly exit due to --inactive */ - check_inactivity_timeout(c); + if (c->options.inactivity_timeout + && event_timeout_trigger(&c->c2.inactivity_interval, &c->c2.timeval, ETT_DEFAULT)) + { + check_inactivity_timeout(c); + } + if (c->sig->signal_received) { return; @@ -800,13 +710,19 @@ process_coarse_timers(struct context *c) #if P2MP if (c->c2.tls_multi) { - check_server_poll_timeout(c); + if (c->options.ce.connect_timeout + && event_timeout_trigger(&c->c2.server_poll_interval, &c->c2.timeval, ETT_DEFAULT)) + { + check_server_poll_timeout(c); + } if (c->sig->signal_received) { return; } - - check_scheduled_exit(c); + if (event_timeout_trigger(&c->c2.scheduled_exit, &c->c2.timeval, ETT_DEFAULT)) + { + check_scheduled_exit(c); + } if (c->sig->signal_received) { return; diff --git a/src/openvpn/forward.h b/src/openvpn/forward.h index 635e84ae..114a24e7 100644 --- a/src/openvpn/forward.h +++ b/src/openvpn/forward.h @@ -77,9 +77,9 @@ void check_tls_errors_nco(struct context *c); #if P2MP void check_incoming_control_channel_dowork(struct context *c); -void check_scheduled_exit_dowork(struct context *c); +void check_scheduled_exit(struct context *c); -void check_push_request_dowork(struct context *c); +void check_push_request(struct context *c); #endif /* P2MP */ @@ -90,13 +90,13 @@ void check_fragment_dowork(struct context *c); void check_connection_established(struct context *c); -void check_add_routes_dowork(struct context *c); +void check_add_routes(struct context *c); -void check_inactivity_timeout_dowork(struct context *c); +void check_inactivity_timeout(struct context *c); -void check_server_poll_timeout_dowork(struct context *c); +void check_server_poll_timeout(struct context *c); -void check_status_file_dowork(struct context *c); +void check_status_file(struct context *c); void io_wait_dowork(struct context *c, const unsigned int flags); diff --git a/src/openvpn/status.c b/src/openvpn/status.c index 91391d1a..e8dcf7cd 100644 --- a/src/openvpn/status.c +++ b/src/openvpn/status.c @@ -146,19 +146,6 @@ status_trigger(struct status_output *so) } } -bool -status_trigger_tv(struct status_output *so, struct timeval *tv) -{ - if (so) - { - return event_timeout_trigger(&so->et, tv, ETT_DEFAULT); - } - else - { - return false; - } -} - void status_reset(struct status_output *so) { diff --git a/src/openvpn/status.h b/src/openvpn/status.h index 2a399d7d..66e5bc53 100644 --- a/src/openvpn/status.h +++ b/src/openvpn/status.h @@ -69,8 +69,6 @@ struct status_output *status_open(const char *filename, const struct virtual_output *vout, const unsigned int flags); -bool status_trigger_tv(struct status_output *so, struct timeval *tv); - bool status_trigger(struct status_output *so); void status_reset(struct status_output *so); From patchwork Mon Aug 10 14:36:57 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [Openvpn-devel,07/17] Split pf_check_reload check and check timer in process_coarse_timers X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 1364 Message-Id: <20200810143707.5834-8-arne@rfc2549.org> To: openvpn-devel@lists.sourceforge.net Date: Mon, 10 Aug 2020 16:36:57 +0200 From: Arne Schwabe List-Id: This move the timer check into process_coarse_timers and makes in line with the other functions. The the pf.enabled check is also moved process_coarse_timers to make it more clear this only is used if pf is enabled at all. Signed-off-by: Arne Schwabe Acked-by: Gert Doering --- src/openvpn/forward.c | 6 +++++- src/openvpn/pf.c | 4 +--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c index 7ac878f9..27a40b0c 100644 --- a/src/openvpn/forward.c +++ b/src/openvpn/forward.c @@ -679,7 +679,11 @@ process_coarse_timers(struct context *c) #endif #ifdef PLUGIN_PF - pf_check_reload(c); + if (c->c2.pf.enabled + && event_timeout_trigger(&c->c2.pf.reload, &c->c2.timeval, ETT_DEFAULT)) + { + pf_check_reload(c); + } #endif /* process --route options */ diff --git a/src/openvpn/pf.c b/src/openvpn/pf.c index b8da26e4..f9bbfb50 100644 --- a/src/openvpn/pf.c +++ b/src/openvpn/pf.c @@ -547,9 +547,7 @@ pf_check_reload(struct context *c) const int wakeup_transition = 60; bool reloaded = false; - if (c->c2.pf.enabled - && c->c2.pf.filename - && event_timeout_trigger(&c->c2.pf.reload, &c->c2.timeval, ETT_DEFAULT)) + if (c->c2.pf.filename) { platform_stat_t s; if (!platform_stat(c->c2.pf.filename, &s)) From patchwork Mon Aug 10 14:36:58 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [Openvpn-devel,08/17] Rename check_ping_restart_dowork to trigger_ping_timeout_signal X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 1358 Message-Id: <20200810143707.5834-9-arne@rfc2549.org> To: openvpn-devel@lists.sourceforge.net Date: Mon, 10 Aug 2020 16:36:58 +0200 From: Arne Schwabe List-Id: Rename the function to better capture its actual function. Signed-off-by: Arne Schwabe Acked-by: Gert Doering --- src/openvpn/ping.c | 6 +----- src/openvpn/ping.h | 13 +++++++------ 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/src/openvpn/ping.c b/src/openvpn/ping.c index 358d54b0..aa176fdb 100644 --- a/src/openvpn/ping.c +++ b/src/openvpn/ping.c @@ -46,12 +46,8 @@ const uint8_t ping_string[] = { 0x07, 0xed, 0x2d, 0x0a, 0x98, 0x1f, 0xc7, 0x48 }; -/* - * Should we exit or restart due to ping (or other authenticated packet) - * not received in n seconds? - */ void -check_ping_restart_dowork(struct context *c) +trigger_ping_timeout_signal(struct context *c) { struct gc_arena gc = gc_new(); switch (c->options.ping_rec_timeout_action) diff --git a/src/openvpn/ping.h b/src/openvpn/ping.h index b51f082a..6feaa878 100644 --- a/src/openvpn/ping.h +++ b/src/openvpn/ping.h @@ -43,7 +43,12 @@ is_ping_msg(const struct buffer *buf) return buf_string_match(buf, ping_string, PING_STRING_SIZE); } -void check_ping_restart_dowork(struct context *c); +/** + * Trigger the correct signal on a --ping timeout + * depending if --ping-exit is set (SIGTERM) or not + * (SIGUSR1) + */ +void trigger_ping_timeout_signal(struct context *c); void check_ping_send_dowork(struct context *c); @@ -54,8 +59,6 @@ void check_ping_send_dowork(struct context *c); static inline void check_ping_restart(struct context *c) { - void check_ping_restart_dowork(struct context *c); - if (c->options.ping_rec_timeout && event_timeout_trigger(&c->c2.ping_rec_interval, &c->c2.timeval, @@ -63,7 +66,7 @@ check_ping_restart(struct context *c) || link_socket_actual_defined(&c->c1.link_socket_addr.actual)) ? ETT_DEFAULT : 15)) { - check_ping_restart_dowork(c); + trigger_ping_timeout_signal(c); } } @@ -73,8 +76,6 @@ check_ping_restart(struct context *c) static inline void check_ping_send(struct context *c) { - void check_ping_send_dowork(struct context *c); - if (c->options.ping_send_timeout && event_timeout_trigger(&c->c2.ping_send_interval, &c->c2.timeval, From patchwork Mon Aug 10 14:36:59 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [Openvpn-devel,09/17] Eliminate check_fragment function X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 1366 Message-Id: <20200810143707.5834-10-arne@rfc2549.org> To: openvpn-devel@lists.sourceforge.net Date: Mon, 10 Aug 2020 16:36:59 +0200 From: Arne Schwabe List-Id: This another of the small wrapper function where the check is better move into the calling function. Signed-off-by: Arne Schwabe Acked-by: Gert Doering --- src/openvpn/forward.c | 25 +++++-------------------- src/openvpn/forward.h | 2 +- 2 files changed, 6 insertions(+), 21 deletions(-) diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c index 27a40b0c..866dd138 100644 --- a/src/openvpn/forward.c +++ b/src/openvpn/forward.c @@ -129,8 +129,6 @@ static inline void check_incoming_control_channel(struct context *c) { #if P2MP - void check_incoming_control_channel_dowork(struct context *c); - if (tls_test_payload_len(c->c2.tls_multi) > 0) { check_incoming_control_channel_dowork(c); @@ -138,22 +136,6 @@ check_incoming_control_channel(struct context *c) #endif } -#ifdef ENABLE_FRAGMENT -/* - * Should we deliver a datagram fragment to remote? - */ -static inline void -check_fragment(struct context *c) -{ - void check_fragment_dowork(struct context *c); - - if (c->c2.fragment) - { - check_fragment_dowork(c); - } -} -#endif - /* * Set our wakeup to 0 seconds, so we will be rescheduled * immediately. @@ -520,7 +502,7 @@ check_status_file(struct context *c) * Should we deliver a datagram fragment to remote? */ void -check_fragment_dowork(struct context *c) +check_fragment(struct context *c) { struct link_socket_info *lsi = get_link_socket_info(c); @@ -1903,7 +1885,10 @@ pre_select(struct context *c) #ifdef ENABLE_FRAGMENT /* Should we deliver a datagram fragment to remote? */ - check_fragment(c); + if (c->c2.fragment) + { + check_fragment(c); + } #endif /* Update random component of timeout */ diff --git a/src/openvpn/forward.h b/src/openvpn/forward.h index 114a24e7..e8b8900a 100644 --- a/src/openvpn/forward.h +++ b/src/openvpn/forward.h @@ -84,7 +84,7 @@ void check_push_request(struct context *c); #endif /* P2MP */ #ifdef ENABLE_FRAGMENT -void check_fragment_dowork(struct context *c); +void check_fragment(struct context *c); #endif /* ENABLE_FRAGMENT */ From patchwork Mon Aug 10 14:37:00 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [Openvpn-devel,10/17] Eliminate check_incoming_control_channel wrapper function X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 1369 Message-Id: <20200810143707.5834-11-arne@rfc2549.org> To: openvpn-devel@lists.sourceforge.net Date: Mon, 10 Aug 2020 16:37:00 +0200 From: Arne Schwabe List-Id: Move the check that calls this function into the calling function. Also eliminate the if (len) check in the check_incoming_control_channel_dowork function as it is only called if len is > 0 anyway and replace it with a ASSERT. Signed-off-by: Arne Schwabe Acked-by: Gert Doering --- src/openvpn/forward.c | 117 +++++++++++++++++++----------------------- src/openvpn/forward.h | 2 +- 2 files changed, 55 insertions(+), 64 deletions(-) diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c index 866dd138..0e05b08b 100644 --- a/src/openvpn/forward.c +++ b/src/openvpn/forward.c @@ -121,21 +121,6 @@ check_tls_errors(struct context *c) } } -/* - * Check for possible incoming configuration - * messages on the control channel. - */ -static inline void -check_incoming_control_channel(struct context *c) -{ -#if P2MP - if (tls_test_payload_len(c->c2.tls_multi) > 0) - { - check_incoming_control_channel_dowork(c); - } -#endif -} - /* * Set our wakeup to 0 seconds, so we will be rescheduled * immediately. @@ -222,61 +207,61 @@ check_tls_errors_nco(struct context *c) * messages on the control channel. */ void -check_incoming_control_channel_dowork(struct context *c) +check_incoming_control_channel(struct context *c) { - const int len = tls_test_payload_len(c->c2.tls_multi); - if (len) + int len = tls_test_payload_len(c->c2.tls_multi); + /* We should only be called with len >0 */ + ASSERT(len > 0); + + struct gc_arena gc = gc_new(); + struct buffer buf = alloc_buf_gc(len, &gc); + if (tls_rec_payload(c->c2.tls_multi, &buf)) { - struct gc_arena gc = gc_new(); - struct buffer buf = alloc_buf_gc(len, &gc); - if (tls_rec_payload(c->c2.tls_multi, &buf)) - { - /* force null termination of message */ - buf_null_terminate(&buf); + /* force null termination of message */ + buf_null_terminate(&buf); - /* enforce character class restrictions */ - string_mod(BSTR(&buf), CC_PRINT, CC_CRLF, 0); + /* enforce character class restrictions */ + string_mod(BSTR(&buf), CC_PRINT, CC_CRLF, 0); - if (buf_string_match_head_str(&buf, "AUTH_FAILED")) - { - receive_auth_failed(c, &buf); - } - else if (buf_string_match_head_str(&buf, "PUSH_")) - { - incoming_push_message(c, &buf); - } - else if (buf_string_match_head_str(&buf, "RESTART")) - { - server_pushed_signal(c, &buf, true, 7); - } - else if (buf_string_match_head_str(&buf, "HALT")) - { - server_pushed_signal(c, &buf, false, 4); - } - else if (buf_string_match_head_str(&buf, "INFO_PRE")) - { - server_pushed_info(c, &buf, 8); - } - else if (buf_string_match_head_str(&buf, "INFO")) - { - server_pushed_info(c, &buf, 4); - } - else if (buf_string_match_head_str(&buf, "CR_RESPONSE")) - { - receive_cr_response(c, &buf); - } - else - { - msg(D_PUSH_ERRORS, "WARNING: Received unknown control message: %s", BSTR(&buf)); - } + if (buf_string_match_head_str(&buf, "AUTH_FAILED")) + { + receive_auth_failed(c, &buf); + } + else if (buf_string_match_head_str(&buf, "PUSH_")) + { + incoming_push_message(c, &buf); + } + else if (buf_string_match_head_str(&buf, "RESTART")) + { + server_pushed_signal(c, &buf, true, 7); + } + else if (buf_string_match_head_str(&buf, "HALT")) + { + server_pushed_signal(c, &buf, false, 4); + } + else if (buf_string_match_head_str(&buf, "INFO_PRE")) + { + server_pushed_info(c, &buf, 8); + } + else if (buf_string_match_head_str(&buf, "INFO")) + { + server_pushed_info(c, &buf, 4); + } + else if (buf_string_match_head_str(&buf, "CR_RESPONSE")) + { + receive_cr_response(c, &buf); } else { - msg(D_PUSH_ERRORS, "WARNING: Receive control message failed"); + msg(D_PUSH_ERRORS, "WARNING: Received unknown control message: %s", BSTR(&buf)); } - - gc_free(&gc); } + else + { + msg(D_PUSH_ERRORS, "WARNING: Receive control message failed"); + } + + gc_free(&gc); } /* @@ -1877,8 +1862,14 @@ pre_select(struct context *c) return; } - /* check for incoming configuration info on the control channel */ - check_incoming_control_channel(c); +#if P2MP + /* check for incoming control messages on the control channel like + * push request/reply, or authentication failure and 2FA messages */ + if (tls_test_payload_len(c->c2.tls_multi) > 0) + { + check_incoming_control_channel(c); + } +#endif /* Should we send an OCC message? */ check_send_occ_msg(c); diff --git a/src/openvpn/forward.h b/src/openvpn/forward.h index e8b8900a..27e7fde7 100644 --- a/src/openvpn/forward.h +++ b/src/openvpn/forward.h @@ -75,7 +75,7 @@ void check_tls_errors_co(struct context *c); void check_tls_errors_nco(struct context *c); #if P2MP -void check_incoming_control_channel_dowork(struct context *c); +void check_incoming_control_channel(struct context *c); void check_scheduled_exit(struct context *c); From patchwork Mon Aug 10 14:37:01 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [Openvpn-devel,11/17] Eliminate check_tls wrapper function X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 1357 Message-Id: <20200810143707.5834-12-arne@rfc2549.org> To: openvpn-devel@lists.sourceforge.net Date: Mon, 10 Aug 2020 16:37:01 +0200 From: Arne Schwabe List-Id: Move check into caller. Remove two in function forward declarations that are not needed from check_tls_errors. Signed-off-by: Arne Schwabe Acked-by: Gert Doering --- src/openvpn/forward.c | 27 ++++++--------------------- src/openvpn/forward.h | 2 +- 2 files changed, 7 insertions(+), 22 deletions(-) diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c index 0e05b08b..36e5c175 100644 --- a/src/openvpn/forward.c +++ b/src/openvpn/forward.c @@ -77,20 +77,6 @@ show_wait_status(struct context *c) #endif /* ifdef ENABLE_DEBUG */ -/* - * Does TLS session need service? - */ -static inline void -check_tls(struct context *c) -{ - void check_tls_dowork(struct context *c); - - if (c->c2.tls_multi) - { - check_tls_dowork(c); - } -} - /* * TLS errors are fatal in TCP mode. * Also check for --tls-exit trigger. @@ -98,10 +84,6 @@ check_tls(struct context *c) static inline void check_tls_errors(struct context *c) { - void check_tls_errors_co(struct context *c); - - void check_tls_errors_nco(struct context *c); - if (c->c2.tls_multi && c->c2.tls_exit_signal) { if (link_socket_connection_oriented(c->c2.link_socket)) @@ -157,7 +139,7 @@ context_reschedule_sec(struct context *c, int sec) * */ void -check_tls_dowork(struct context *c) +check_tls(struct context *c) { interval_t wakeup = BIG_TIMEOUT; @@ -1852,8 +1834,11 @@ pre_select(struct context *c) return; } - /* Does TLS need service? */ - check_tls(c); + /* If tls is enabled, do tls control channel packet processing. */ + if (c->c2.tls_multi) + { + check_tls(c); + } /* In certain cases, TLS errors will require a restart */ check_tls_errors(c); diff --git a/src/openvpn/forward.h b/src/openvpn/forward.h index 27e7fde7..a8b19f69 100644 --- a/src/openvpn/forward.h +++ b/src/openvpn/forward.h @@ -68,7 +68,7 @@ extern counter_type link_read_bytes_global; extern counter_type link_write_bytes_global; -void check_tls_dowork(struct context *c); +void check_tls(struct context *c); void check_tls_errors_co(struct context *c); From patchwork Mon Aug 10 14:37:02 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [Openvpn-devel,12/17] Merge check_coarse_timers and check_coarse_timers_dowork X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 1371 Message-Id: <20200810143707.5834-13-arne@rfc2549.org> To: openvpn-devel@lists.sourceforge.net Date: Mon, 10 Aug 2020 16:37:02 +0200 From: Arne Schwabe List-Id: This simplifies the code a bit and makes the code flow clearer as it only adds three curly brackets in check_coarse_timers. Merging the resulting check_coarse_timers_dowork function into the caller and called function as with the other function does not make sense here since it does more than similar function. Signed-off-by: Arne Schwabe Acked-by: Gert Doering --- src/openvpn/forward.c | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c index 36e5c175..7ed8d0d7 100644 --- a/src/openvpn/forward.c +++ b/src/openvpn/forward.c @@ -700,8 +700,14 @@ process_coarse_timers(struct context *c) } static void -check_coarse_timers_dowork(struct context *c) +check_coarse_timers(struct context *c) { + if (now < c->c2.coarse_timer_wakeup) + { + context_reschedule_sec(c, c->c2.coarse_timer_wakeup - now); + return; + } + const struct timeval save = c->c2.timeval; c->c2.timeval.tv_sec = BIG_TIMEOUT; c->c2.timeval.tv_usec = 0; @@ -717,20 +723,6 @@ check_coarse_timers_dowork(struct context *c) } } -static inline void -check_coarse_timers(struct context *c) -{ - const time_t local_now = now; - if (local_now >= c->c2.coarse_timer_wakeup) - { - check_coarse_timers_dowork(c); - } - else - { - context_reschedule_sec(c, c->c2.coarse_timer_wakeup - local_now); - } -} - static void check_timeout_random_component_dowork(struct context *c) { From patchwork Mon Aug 10 14:37:03 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [Openvpn-devel,13/17] Remove S_OP_NORMAL key state. X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 1359 Message-Id: <20200810143707.5834-14-arne@rfc2549.org> To: openvpn-devel@lists.sourceforge.net Date: Mon, 10 Aug 2020 16:37:03 +0200 From: Arne Schwabe List-Id: The key state is virtually identical S_ACTIVE and we only did the state state transition form S_ACTIVE to S_OP_NORMAL at the point where we normally would have timed out the TLS negotiation. This is a very useful to have and indeed we never that information. Signed-off-by: Arne Schwabe Acked-by: Gert Doering --- src/openvpn/ssl.c | 24 +++++++----------------- src/openvpn/ssl_common.h | 9 ++++----- 2 files changed, 11 insertions(+), 22 deletions(-) diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index a43ee985..0d54c9ed 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -755,9 +755,6 @@ state_name(int state) case S_ACTIVE: return "S_ACTIVE"; - case S_NORMAL_OP: - return "S_NORMAL_OP"; - case S_ERROR: return "S_ERROR"; @@ -2705,21 +2702,12 @@ tls_process(struct tls_multi *multi, } /* Are we timed out on receive? */ - if (now >= ks->must_negotiate) + if (now >= ks->must_negotiate && ks->state < S_ACTIVE) { - if (ks->state < S_ACTIVE) - { - msg(D_TLS_ERRORS, - "TLS Error: TLS key negotiation failed to occur within %d seconds (check your network connectivity)", - session->opt->handshake_window); - goto error; - } - else /* assume that ks->state == S_ACTIVE */ - { - dmsg(D_TLS_DEBUG_MED, "STATE S_NORMAL_OP"); - ks->state = S_NORMAL_OP; - ks->must_negotiate = 0; - } + msg(D_TLS_ERRORS, + "TLS Error: TLS key negotiation failed to occur within %d seconds (check your network connectivity)", + session->opt->handshake_window); + goto error; } /* Wait for Initial Handshake ACK */ @@ -2759,6 +2747,8 @@ tls_process(struct tls_multi *multi, } state_change = true; ks->state = S_ACTIVE; + /* Cancel negotiation timeout */ + ks->must_negotiate = 0; INCR_SUCCESS; /* Set outgoing address for data channel packets */ diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h index 9f777750..96897e48 100644 --- a/src/openvpn/ssl_common.h +++ b/src/openvpn/ssl_common.h @@ -64,8 +64,7 @@ * material. * -# \c S_GOT_KEY, have received remote part of \c key_source2 random * material. - * -# \c S_ACTIVE, normal operation during remaining handshake window. - * -# \c S_NORMAL_OP, normal operation. + * -# \c S_ACTIVE, normal operation * * Servers follow the same order, except for \c S_SENT_KEY and \c * S_GOT_KEY being reversed, because the server first receives the @@ -94,9 +93,9 @@ * immediately after negotiation has * completed while still within the * handshake window. */ -/* ready to exchange data channel packets */ -#define S_NORMAL_OP 7 /**< Normal operational \c key_state - * state. */ +/* Note that earlier versions also had a S_OP_NORMAL state that was + * virtually identical with S_ACTIVE and the code still assumes everything + * >= S_ACTIVE to be fully operational */ /** @} name Control channel negotiation states */ /** @} addtogroup control_processor */ From patchwork Mon Aug 10 14:37:04 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [Openvpn-devel,v2,14/17] Skip existing interfaces on opening the first available utun on macOS X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 1354 Message-Id: <20200810143707.5834-15-arne@rfc2549.org> To: openvpn-devel@lists.sourceforge.net Date: Mon, 10 Aug 2020 16:37:04 +0200 From: Arne Schwabe List-Id: This avoids the error messages trying to open already used utuns. Signed-off-by: Arne Schwabe Acked-by: Lev Stipakov --- src/openvpn/tun.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c index cc7b65cf..30454454 100644 --- a/src/openvpn/tun.c +++ b/src/openvpn/tun.c @@ -3021,8 +3021,15 @@ open_darwin_utun(const char *dev, const char *dev_type, const char *dev_node, st /* try to open first available utun device if no specific utun is requested */ if (utunnum == -1) { - for (utunnum = 0; utunnum<255; utunnum++) + for (utunnum = 0; utunnum < 255; utunnum++) { + char ifname[20]; + /* if the interface exists silently skip it */ + ASSERT(snprintf(ifname, sizeof(ifname), "utun%d", utunnum) > 0); + if (if_nametoindex(ifname)) + { + continue; + } fd = utun_open_helper(ctlInfo, utunnum); /* Break if the fd is valid, * or if early initialization failed (-2) */ From patchwork Mon Aug 10 14:37:05 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [Openvpn-devel,15/17] Refactor key_state_export_keying_material functions X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 1365 Message-Id: <20200810143707.5834-16-arne@rfc2549.org> To: openvpn-devel@lists.sourceforge.net Date: Mon, 10 Aug 2020 16:37:05 +0200 From: Arne Schwabe List-Id: This refactors the common code between mbed SSL and OpenSSL into export_user_keying_material and also prepares the backend functions to export more than one key. Signed-off-by: Arne Schwabe --- src/openvpn/ssl.c | 32 +++++++++++++++++++++++++++++++- src/openvpn/ssl_backend.h | 14 ++++++++++++-- src/openvpn/ssl_mbedtls.c | 22 ++++++++-------------- src/openvpn/ssl_openssl.c | 34 ++++++++++++++++------------------ 4 files changed, 67 insertions(+), 35 deletions(-) diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 0d54c9ed..774ba7ed 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -2412,6 +2412,36 @@ error: return false; } +static void +export_user_keying_material(struct key_state_ssl *ssl, + struct tls_session *session) +{ + if (session->opt->ekm_size > 0) + { + unsigned int size = session->opt->ekm_size; + struct gc_arena gc = gc_new(); + + unsigned const char *ekm; + if ((ekm = key_state_export_keying_material(ssl, session, + EXPORT_KEY_USER, &gc))) + { + unsigned int len = (size * 2) + 2; + + const char *key = format_hex_ex(ekm, size, len, 0, NULL, &gc); + setenv_str(session->opt->es, "exported_keying_material", key); + + dmsg(D_TLS_DEBUG_MED, "%s: exported keying material: %s", + __func__, key); + } + else + { + msg(M_WARN, "WARNING: Export keying material failed!"); + setenv_del(session->opt->es, "exported_keying_material"); + } + gc_free(&gc); + } +} + /** * Handle reading key data, peer-info, username/password, OCC * from the TLS control channel (cleartext). @@ -2541,7 +2571,7 @@ key_method_2_read(struct buffer *buf, struct tls_multi *multi, struct tls_sessio if ((ks->authenticated > KS_AUTH_FALSE) && plugin_defined(session->opt->plugins, OPENVPN_PLUGIN_TLS_FINAL)) { - key_state_export_keying_material(&ks->ks_ssl, session); + export_user_keying_material(&ks->ks_ssl, session); if (plugin_call(session->opt->plugins, OPENVPN_PLUGIN_TLS_FINAL, NULL, NULL, session->opt->es) != OPENVPN_PLUGIN_FUNC_SUCCESS) { diff --git a/src/openvpn/ssl_backend.h b/src/openvpn/ssl_backend.h index 7f52ab1e..40e9106a 100644 --- a/src/openvpn/ssl_backend.h +++ b/src/openvpn/ssl_backend.h @@ -389,6 +389,12 @@ void key_state_ssl_free(struct key_state_ssl *ks_ssl); void backend_tls_ctx_reload_crl(struct tls_root_ctx *ssl_ctx, const char *crl_file, bool crl_inline); + +/* defines the different RFC5705 that are used in OpenVPN */ +enum export_key_identifier { + EXPORT_KEY_USER +}; + /** * Keying Material Exporters [RFC 5705] allows additional keying material to be * derived from existing TLS channel. This exported keying material can then be @@ -396,11 +402,15 @@ void backend_tls_ctx_reload_crl(struct tls_root_ctx *ssl_ctx, * * @param ks_ssl The SSL channel's state info * @param session The session associated with the given key_state + * @param key The key to export. + * @returns The exported key material */ -void +unsigned const char* key_state_export_keying_material(struct key_state_ssl *ks_ssl, - struct tls_session *session) __attribute__((nonnull)); + struct tls_session *session, + enum export_key_identifier export_key, + struct gc_arena *gc) __attribute__((nonnull)); /**************************************************************************/ /** @addtogroup control_tls diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c index 9c874788..4a6fad5f 100644 --- a/src/openvpn/ssl_mbedtls.c +++ b/src/openvpn/ssl_mbedtls.c @@ -231,24 +231,18 @@ mbedtls_ssl_export_keys_cb(void *p_expkey, const unsigned char *ms, } #endif /* HAVE_EXPORT_KEYING_MATERIAL */ -void +const unsigned char * key_state_export_keying_material(struct key_state_ssl *ssl, - struct tls_session *session) + struct tls_session *session, + enum export_key_identifier key_id, + struct gc_arena *gc) { - if (ssl->exported_key_material) + if (key_id == EXPORT_KEY_USER) { - unsigned int size = session->opt->ekm_size; - struct gc_arena gc = gc_new(); - unsigned int len = (size * 2) + 2; - - const char *key = format_hex_ex(ssl->exported_key_material, - size, len, 0, NULL, &gc); - setenv_str(session->opt->es, "exported_keying_material", key); - - dmsg(D_TLS_DEBUG_MED, "%s: exported keying material: %s", - __func__, key); - gc_free(&gc); + return ssl->exported_key_material; } + + return NULL; } diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c index 5ba74402..5cc03cf0 100644 --- a/src/openvpn/ssl_openssl.c +++ b/src/openvpn/ssl_openssl.c @@ -158,35 +158,33 @@ tls_ctx_initialised(struct tls_root_ctx *ctx) return NULL != ctx->ctx; } -void +unsigned const char * key_state_export_keying_material(struct key_state_ssl *ssl, - struct tls_session *session) + struct tls_session *session, + enum export_key_identifier key_id, + struct gc_arena *gc) + { - if (session->opt->ekm_size > 0) + if (key_id == EXPORT_KEY_USER) { unsigned int size = session->opt->ekm_size; - struct gc_arena gc = gc_new(); - unsigned char *ekm = (unsigned char *) gc_malloc(size, true, &gc); + unsigned char *ekm = (unsigned char *) gc_malloc(size, true, gc); if (SSL_export_keying_material(ssl->ssl, ekm, size, - session->opt->ekm_label, - session->opt->ekm_label_size, - NULL, 0, 0)) + session->opt->ekm_label, + session->opt->ekm_label_size, + NULL, 0, 0)) { - unsigned int len = (size * 2) + 2; - - const char *key = format_hex_ex(ekm, size, len, 0, NULL, &gc); - setenv_str(session->opt->es, "exported_keying_material", key); - - dmsg(D_TLS_DEBUG_MED, "%s: exported keying material: %s", - __func__, key); + return ekm; } else { - msg(M_WARN, "WARNING: Export keying material failed!"); - setenv_del(session->opt->es, "exported_keying_material"); + return NULL; } - gc_free(&gc); + } + else + { + return NULL; } } From patchwork Mon Aug 10 14:37:06 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [Openvpn-devel,16/17] Move parsing IV_PROTO to separate function X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 1362 Message-Id: <20200810143707.5834-17-arne@rfc2549.org> To: openvpn-devel@lists.sourceforge.net Date: Mon, 10 Aug 2020 16:37:06 +0200 From: Arne Schwabe List-Id: Signed-off-by: Arne Schwabe Acked-by: Gert Doering --- src/openvpn/multi.c | 49 +++++++++++++++++++++++++++------------------ 1 file changed, 30 insertions(+), 19 deletions(-) diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index b7b7e32f..13738180 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -1771,6 +1771,28 @@ multi_client_connect_setenv(struct multi_context *m, gc_free(&gc); } +/** + * Extracts the IV_PROTO variable and returns its value or 0 + * if it cannot be extracted. + * + */ +static unsigned int +extract_iv_proto(const char *peer_info) +{ + + const char *optstr = peer_info ? strstr(peer_info, "IV_PROTO=") : NULL; + if (optstr) + { + int proto = 0; + int r = sscanf(optstr, "IV_PROTO=%d", &proto); + if (r == 1 && proto > 0) + { + return proto; + } + } + return 0; +} + /** * Calculates the options that depend on the client capabilities * based on local options and available peer info @@ -1780,30 +1802,19 @@ multi_client_connect_setenv(struct multi_context *m, static bool multi_client_set_protocol_options(struct context *c) { - - const char *optstr = NULL; struct tls_multi *tls_multi = c->c2.tls_multi; const char *const peer_info = tls_multi->peer_info; struct options *o = &c->options; - /* Send peer-id if client supports it */ - optstr = peer_info ? strstr(peer_info, "IV_PROTO=") : NULL; - if (optstr) - { - int proto = 0; - int r = sscanf(optstr, "IV_PROTO=%d", &proto); - if (r == 1) - { - if (proto & IV_PROTO_DATA_V2) - { - tls_multi->use_peer_id = true; - } - if (proto & IV_PROTO_REQUEST_PUSH) - { - c->c2.push_request_received = true; - } - } + unsigned int proto = extract_iv_proto(peer_info); + if (proto & IV_PROTO_DATA_V2) + { + tls_multi->use_peer_id = true; + } + if (proto & IV_PROTO_REQUEST_PUSH) + { + c->c2.push_request_received = true; } /* Select cipher if client supports Negotiable Crypto Parameters */ From patchwork Mon Aug 10 14:37:07 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [Openvpn-devel,17/17] Move openvpn specific key expansion into its own function X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 1363 Message-Id: <20200810143707.5834-18-arne@rfc2549.org> To: openvpn-devel@lists.sourceforge.net Date: Mon, 10 Aug 2020 16:37:07 +0200 From: Arne Schwabe List-Id: This moves the OpenVPN specific PRF into its own function also simplifies the code a bit by passing tls_session directly instead of 5 of its fields. Signed-off-by: Arne Schwabe --- src/openvpn/ssl.c | 109 +++++++++++++++++++++++++++++----------------- 1 file changed, 69 insertions(+), 40 deletions(-) diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 774ba7ed..91743862 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -1765,27 +1765,38 @@ openvpn_PRF(const uint8_t *secret, VALGRIND_MAKE_READABLE((void *)output, output_len); } -/* - * Using source entropy from local and remote hosts, mix into - * master key. - */ -static bool -generate_key_expansion(struct key_ctx_bi *key, - const struct key_type *key_type, - const struct key_source2 *key_src, - const struct session_id *client_sid, - const struct session_id *server_sid, - bool server) +static void +init_key_contexts(struct key_ctx_bi *key, + const struct key_type *key_type, + bool server, + struct key2 *key2) +{ + /* Initialize OpenSSL key contexts */ + int key_direction = server ? KEY_DIRECTION_INVERSE : KEY_DIRECTION_NORMAL; + init_key_ctx_bi(key, key2, key_direction, key_type, "Data Channel"); + + /* Initialize implicit IVs */ + key_ctx_update_implicit_iv(&key->encrypt, (*key2).keys[(int)server].hmac, + MAX_HMAC_KEY_LENGTH); + key_ctx_update_implicit_iv(&key->decrypt, (*key2).keys[1-(int)server].hmac, + MAX_HMAC_KEY_LENGTH); + +} + + +static struct key2 +generate_key_expansion_oepnvpn_prf(const struct tls_session *session) { + uint8_t master[48] = { 0 }; - struct key2 key2 = { 0 }; - bool ret = false; - if (key->initialized) - { - msg(D_TLS_ERRORS, "TLS Error: key already initialized"); - goto exit; - } + const struct key_state *ks = &session->key[KS_PRIMARY]; + const struct key_source2 *key_src = ks->key_src; + + const struct session_id *client_sid = session->opt->server ? + &ks->session_id_remote : &session->session_id; + const struct session_id *server_sid = !session->opt->server ? + &ks->session_id_remote : &session->session_id; /* debugging print of source key material */ key_source2_print(key_src); @@ -1803,6 +1814,7 @@ generate_key_expansion(struct key_ctx_bi *key, master, sizeof(master)); + struct key2 key2; /* compute key expansion */ openvpn_PRF(master, sizeof(master), @@ -1815,41 +1827,62 @@ generate_key_expansion(struct key_ctx_bi *key, server_sid, (uint8_t *)key2.keys, sizeof(key2.keys)); + secure_memzero(&master, sizeof(master)); + /* We use the DES fixup here so we can drop it once we + * drop DES support and non RFC5705 key derivation */ + for (int i = 0; i < 2; ++i) + { + fixup_key(&key2.keys[i], &session->opt->key_type); + } key2.n = 2; - key2_print(&key2, key_type, "Master Encrypt", "Master Decrypt"); + return key2; +} + +/* + * Using source entropy from local and remote hosts, mix into + * master key. + */ +static bool +generate_key_expansion(struct key_ctx_bi *key, + const struct tls_session *session) +{ + bool ret = false; + + if (key->initialized) + { + msg(D_TLS_ERRORS, "TLS Error: key already initialized"); + goto exit; + } + + + bool server = session->opt->server; + + struct key2 key2 = generate_key_expansion_oepnvpn_prf(session); + + key2_print(&key2, &session->opt->key_type, + "Master Encrypt", "Master Decrypt"); /* check for weak keys */ for (int i = 0; i < 2; ++i) { - fixup_key(&key2.keys[i], key_type); - if (!check_key(&key2.keys[i], key_type)) + if (!check_key(&key2.keys[i], &session->opt->key_type)) { msg(D_TLS_ERRORS, "TLS Error: Bad dynamic key generated"); goto exit; } } - - /* Initialize OpenSSL key contexts */ - int key_direction = server ? KEY_DIRECTION_INVERSE : KEY_DIRECTION_NORMAL; - init_key_ctx_bi(key, &key2, key_direction, key_type, "Data Channel"); - - /* Initialize implicit IVs */ - key_ctx_update_implicit_iv(&key->encrypt, key2.keys[(int)server].hmac, - MAX_HMAC_KEY_LENGTH); - key_ctx_update_implicit_iv(&key->decrypt, key2.keys[1-(int)server].hmac, - MAX_HMAC_KEY_LENGTH); - + init_key_contexts(key, &session->opt->key_type, server, &key2); ret = true; exit: - secure_memzero(&master, sizeof(master)); secure_memzero(&key2, sizeof(key2)); return ret; } + static void key_ctx_update_implicit_iv(struct key_ctx *ctx, uint8_t *key, size_t key_len) { @@ -1879,10 +1912,7 @@ tls_session_generate_data_channel_keys(struct tls_session *session) { bool ret = false; struct key_state *ks = &session->key[KS_PRIMARY]; /* primary key */ - const struct session_id *client_sid = session->opt->server ? - &ks->session_id_remote : &session->session_id; - const struct session_id *server_sid = !session->opt->server ? - &ks->session_id_remote : &session->session_id; + if (ks->authenticated == KS_AUTH_FALSE) { @@ -1891,9 +1921,8 @@ tls_session_generate_data_channel_keys(struct tls_session *session) } ks->crypto_options.flags = session->opt->crypto_flags; - if (!generate_key_expansion(&ks->crypto_options.key_ctx_bi, - &session->opt->key_type, ks->key_src, client_sid, server_sid, - session->opt->server)) + + if (!generate_key_expansion(&ks->crypto_options.key_ctx_bi, session)) { msg(D_TLS_ERRORS, "TLS Error: generate_key_expansion failed"); goto cleanup;