diff --git a/Changes.rst b/Changes.rst index 3676dce..4c8be39 100644 --- a/Changes.rst +++ b/Changes.rst @@ -1,5 +1,16 @@ Overview of changes in 2.7 ========================== +New features +------------ +TLS alerts + OpenVPN 2.7 will send out TLS alerts to peer informing them if the TLS + session shuts down or when the TLS implementation informs the peer about + an error in the TLS session (e.g. mismatching TLS versions). This improves + the user experience as the client shows an error instead of running into + a timeout when the server just stop responding completely. + +Deprecated features +------------------- ``secret`` support has been removed by default. static key mode (non-TLS) is no longer considered "good and secure enough" for today's requirements. Use TLS mode instead. If deploying a PKI CA diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index b4cd8f5..16d3d22 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -853,6 +853,9 @@ case S_ERROR: return "S_ERROR"; + case S_ERROR_PRE: + return "S_ERROR_PRE"; + case S_GENERATED_KEYS: return "S_GENERATED_KEYS"; @@ -2839,6 +2842,35 @@ return true; } +/** + * Shut down an SSL session, so an SSL close notify is sent if there no other + * SSL notify. + * @param ks + */ +void +do_ssl_shutdown(struct key_state *ks) +{ +} + + +static bool +check_outgoing_ciphertext(struct key_state *ks, struct tls_session *session, + bool *state_change) +{ + /* Outgoing Ciphertext to reliable buffer */ + if (ks->state >= S_PRE_START) + { + struct buffer *buf = reliable_get_buf_output_sequenced(ks->send_reliable); + if (buf) + { + if (!write_outgoing_tls_ciphertext(session, state_change)) + { + return false; + } + } + } + return true; +} static bool tls_process_state(struct tls_multi *multi, @@ -2912,6 +2944,16 @@ return false; } + if (ks->state == S_ERROR_PRE) + { + /* When we end up here, we had one last chance to send an outstanding + * packet that contained an alert. We do not ensure that this packet + * has been successfully delivered (ie wait for the ACK etc) + * but rather stop processing now */ + ks->state = S_ERROR; + return false; + } + /* Write incoming ciphertext to TLS object */ struct reliable_entry *entry = reliable_get_entry_sequenced(ks->rec_reliable); if (entry) @@ -2992,29 +3034,31 @@ dmsg(D_TLS_DEBUG, "Outgoing Plaintext -> TLS"); } } - - /* Outgoing Ciphertext to reliable buffer */ - if (ks->state >= S_START) + if (!check_outgoing_ciphertext(ks, session, &state_change)) { - buf = reliable_get_buf_output_sequenced(ks->send_reliable); - if (buf) - { - if (!write_outgoing_tls_ciphertext(session, &state_change)) - { - goto error; - } - } + goto error; } return state_change; error: tls_clear_error(); - ks->state = S_ERROR; + + /* Shut down the TLS session but do a last read from the TLS + * object to be able to read potential TLS alerts */ + do_ssl_shutdown(ks); + check_outgoing_ciphertext(ks, session, &continue_tls_process); + + /* Put ourselves in the pre error state that will only send out the + * control channel packets but nothing else */ + ks->state = S_ERROR_PRE; + msg(D_TLS_ERRORS, "TLS Error: TLS handshake failed"); INCR_ERROR; - return false; - + return true; } + + + /* * This is the primary routine for processing TLS stuff inside the * the main event loop. When this routine exits @@ -3122,7 +3166,7 @@ } /* When should we wake up again? */ - if (ks->state >= S_INITIAL) + if (ks->state >= S_INITIAL || ks->state == S_ERROR_PRE) { compute_earliest_wakeup(wakeup, reliable_send_timeout(ks->send_reliable)); @@ -3275,7 +3319,7 @@ session_id_print(&ks->session_id_remote, &gc), print_link_socket_actual(&ks->remote_addr, &gc)); - if (ks->state >= S_INITIAL && link_socket_actual_defined(&ks->remote_addr)) + if ((ks->state >= S_INITIAL || ks->state == S_ERROR_PRE) && link_socket_actual_defined(&ks->remote_addr)) { struct link_socket_actual *tla = NULL; @@ -3353,7 +3397,8 @@ { msg(D_TLS_ERRORS, "TLS Error: generate_key_expansion failed"); ks->authenticated = KS_AUTH_FALSE; - ks->state = S_ERROR; + do_ssl_shutdown(ks); + ks->state = S_ERROR_PRE; } /* Update auth token on the client if needed on renegotiation diff --git a/src/openvpn/ssl_backend.h b/src/openvpn/ssl_backend.h index 3854d59..ea31902 100644 --- a/src/openvpn/ssl_backend.h +++ b/src/openvpn/ssl_backend.h @@ -372,6 +372,13 @@ const struct tls_root_ctx *ssl_ctx, bool is_server, struct tls_session *session); /** + * Sets a TLS session to be shutdown state, so the TLS library will generate + * a shutdown altert. + */ +void +key_state_ssl_shutdown(struct key_state_ssl *ks_ssl); + +/** * Free the SSL channel part of the given key state. * * @param ks_ssl The SSL channel's state info to free diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h index d3edc5f..d38d8ec 100644 --- a/src/openvpn/ssl_common.h +++ b/src/openvpn/ssl_common.h @@ -74,7 +74,10 @@ * * @{ */ -#define S_ERROR -1 /**< Error state. */ +#define S_ERROR (-2) /**< Error state. */ +#define S_ERROR_PRE (-1) /**< Error state but try to send out alerts + * before killing the keystore and moving + * it to S_ERROR */ #define S_UNDEF 0 /**< Undefined state, used after a \c * key_state is cleaned up. */ #define S_INITIAL 1 /**< Initial \c key_state state after diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c index 9c9167d..c8aca4f 100644 --- a/src/openvpn/ssl_mbedtls.c +++ b/src/openvpn/ssl_mbedtls.c @@ -1275,6 +1275,14 @@ ssl_bio_read, NULL); } + +void +key_state_ssl_shutdown(struct key_state_ssl *ks_ssl) +{ + mbedtls_ssl_send_alert_message(ks_ssl->ctx, MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_CLOSE_NOTIFY); +} + void key_state_ssl_free(struct key_state_ssl *ks_ssl) { diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c index 82872bf..622b9b5 100644 --- a/src/openvpn/ssl_openssl.c +++ b/src/openvpn/ssl_openssl.c @@ -1961,6 +1961,12 @@ } void +key_state_ssl_shutdown(struct key_state_ssl *ks_ssl) +{ + SSL_set_shutdown(ks_ssl->ssl, SSL_SENT_SHUTDOWN | SSL_RECEIVED_SHUTDOWN); +} + +void key_state_ssl_free(struct key_state_ssl *ks_ssl) { if (ks_ssl->ssl)