@@ -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
@@ -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
@@ -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
@@ -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
@@ -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)
{
@@ -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)
Attention is currently required from: flichtenheld. Hello flichtenheld, I'd like you to do a code review. Please visit http://gerrit.openvpn.net/c/openvpn/+/449?usp=email to review the following change. Change subject: Allow the TLS session to send out TLS alerts ...................................................................... Allow the TLS session to send out TLS alerts Previous OpenVPN versions shut down the TLS control channel immediately when encountering an error. This also meant that we would not send out TLS alerts to notify a client about potential problems like mismatching TLS versions or having no common cipher. This commit adds a new key_state S_ERROR_PRE which still allows to send out the remaining TLS packets of the control session which are typically the alert message and then going to S_ERROR. We do not wait for retries. So this is more a one-shot notify but that is acceptable in this situation. Sending out alerts is a slight compromise in security as alerts give out a bit of information that otherwise is not given out. But since all other consumers TLS implementation are already doing this and TLS implementation (nowadays) are very careful not to leak (sensitive) information by alerts and since the user experience is much better with alerts, this compromise is worth it. Change-Id: I0ad48915004ddee587e97c8ed190ba8ee989e48d --- M Changes.rst M src/openvpn/ssl.c M src/openvpn/ssl_backend.h M src/openvpn/ssl_common.h M src/openvpn/ssl_mbedtls.c M src/openvpn/ssl_openssl.c 6 files changed, 98 insertions(+), 18 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/49/449/1