diff --git a/Changes.rst b/Changes.rst index 4bc3bb3c..439352ab 100644 --- a/Changes.rst +++ b/Changes.rst @@ -1,5 +1,14 @@ Overview of changes in 2.7 ========================== +New features +------------ +TLS alerts + OpenVPN 2.7 will send out TLS alerts to peers 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 stops responding completely. + Deprecated features ------------------- ``secret`` support has been removed by default. diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 7c494519..2054eb47 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -690,6 +690,9 @@ state_name(int state) case S_ERROR: return "S_ERROR"; + case S_ERROR_PRE: + return "S_ERROR_PRE"; + case S_GENERATED_KEYS: return "S_GENERATED_KEYS"; @@ -2682,6 +2685,25 @@ write_outgoing_tls_ciphertext(struct tls_session *session, bool *continue_tls_pr return true; } +static bool +check_outgoing_ciphertext(struct key_state *ks, struct tls_session *session, + bool *continue_tls_process) +{ + /* Outgoing Ciphertext to reliable buffer */ + if (ks->state >= S_START) + { + struct buffer *buf = reliable_get_buf_output_sequenced(ks->send_reliable); + if (buf) + { + if (!write_outgoing_tls_ciphertext(session, continue_tls_process)) + { + return false; + } + } + } + return true; +} + static bool tls_process_state(struct tls_multi *multi, struct tls_session *session, @@ -2706,7 +2728,7 @@ tls_process_state(struct tls_multi *multi, } /* Are we timed out on receive? */ - if (now >= ks->must_negotiate && ks->state < S_ACTIVE) + if (now >= ks->must_negotiate && ks->state >= S_UNDEF && ks->state < S_ACTIVE) { msg(D_TLS_ERRORS, "TLS Error: TLS key negotiation failed to occur within %d seconds (check your network connectivity)", @@ -2760,6 +2782,16 @@ tls_process_state(struct tls_multi *multi, 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) @@ -2840,29 +2872,31 @@ tls_process_state(struct tls_multi *multi, dmsg(D_TLS_DEBUG, "Outgoing Plaintext -> TLS"); } } - - /* Outgoing Ciphertext to reliable buffer */ - if (ks->state >= S_START) + if (!check_outgoing_ciphertext(ks, session, &continue_tls_process)) { - buf = reliable_get_buf_output_sequenced(ks->send_reliable); - if (buf) - { - if (!write_outgoing_tls_ciphertext(session, &continue_tls_process)) - { - goto error; - } - } + goto error; } return continue_tls_process; 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 */ + key_state_ssl_shutdown(&ks->ks_ssl); + 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 @@ -2970,7 +3004,7 @@ tls_process(struct tls_multi *multi, } /* 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)); @@ -3123,7 +3157,7 @@ tls_multi_process(struct tls_multi *multi, 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; @@ -3201,7 +3235,8 @@ tls_multi_process(struct tls_multi *multi, { msg(D_TLS_ERRORS, "TLS Error: generate_key_expansion failed"); ks->authenticated = KS_AUTH_FALSE; - ks->state = S_ERROR; + key_state_ssl_shutdown(&ks->ks_ssl); + 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 8c3403ca..285705f7 100644 --- a/src/openvpn/ssl_backend.h +++ b/src/openvpn/ssl_backend.h @@ -362,6 +362,13 @@ void tls_ctx_personalise_random(struct tls_root_ctx *ctx); void key_state_ssl_init(struct key_state_ssl *ks_ssl, 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 alert. + */ +void +key_state_ssl_shutdown(struct key_state_ssl *ks_ssl); + /** * Free the SSL channel part of the given key state. * diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h index 5a7dd605..5bc2f2aa 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 b0303b60..a68588e8 100644 --- a/src/openvpn/ssl_mbedtls.c +++ b/src/openvpn/ssl_mbedtls.c @@ -1276,6 +1276,14 @@ key_state_ssl_init(struct key_state_ssl *ks_ssl, 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 7e74f2f5..e8a30a31 100644 --- a/src/openvpn/ssl_openssl.c +++ b/src/openvpn/ssl_openssl.c @@ -1940,6 +1940,12 @@ key_state_ssl_init(struct key_state_ssl *ks_ssl, const struct tls_root_ctx *ssl_ BIO_set_ssl(ks_ssl->ssl_bio, ks_ssl->ssl, BIO_NOCLOSE); } +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) {