From 6a3ad9877051f8135ed0f941bfbcff9e14ff8b28 Mon Sep 17 00:00:00 2001 From: David Woodhouse Date: Tue, 11 Feb 2014 12:11:58 +0000 Subject: [PATCH] Kill new_dtls_* variables We once might have kept the old DTLS stuff around while we made the new connection. We don't any more; it's just unneeded baggage. Signed-off-by: David Woodhouse --- dtls.c | 59 ++++++++++++++---------------------------- library.c | 4 +-- mainloop.c | 7 +++-- openconnect-internal.h | 5 +--- 4 files changed, 27 insertions(+), 48 deletions(-) diff --git a/dtls.c b/dtls.c index b2c6f07f..8ae512ac 100644 --- a/dtls.c +++ b/dtls.c @@ -200,14 +200,14 @@ static int start_dtls_handshake(struct openconnect_info *vpninfo, int dtls_fd) SSL_set_options(dtls_ssl, SSL_OP_CISCO_ANYCONNECT); - vpninfo->new_dtls_ssl = dtls_ssl; + vpninfo->dtls_ssl = dtls_ssl; return 0; } int dtls_try_handshake(struct openconnect_info *vpninfo) { - int ret = SSL_do_handshake(vpninfo->new_dtls_ssl); + int ret = SSL_do_handshake(vpninfo->dtls_ssl); if (ret == 1) { vpninfo->dtls_state = DTLS_CONNECTED; @@ -215,13 +215,6 @@ int dtls_try_handshake(struct openconnect_info *vpninfo) _("Established DTLS connection (using OpenSSL). Ciphersuite %s.\n"), vpninfo->dtls_cipher); - dtls_close(vpninfo, 0); - vpninfo->dtls_ssl = vpninfo->new_dtls_ssl; - vpninfo->dtls_fd = vpninfo->new_dtls_fd; - - vpninfo->new_dtls_ssl = NULL; - vpninfo->new_dtls_fd = -1; - vpninfo->dtls_times.last_rx = vpninfo->dtls_times.last_tx = time(NULL); /* From about 8.4.1(11) onwards, the ASA seems to get @@ -284,7 +277,7 @@ int dtls_try_handshake(struct openconnect_info *vpninfo) return 0; } - ret = SSL_get_error(vpninfo->new_dtls_ssl, ret); + ret = SSL_get_error(vpninfo->dtls_ssl, ret); if (ret == SSL_ERROR_WANT_WRITE || ret == SSL_ERROR_WANT_READ) { static int badossl_bitched = 0; if (time(NULL) < vpninfo->new_dtls_started + 5) @@ -304,10 +297,7 @@ int dtls_try_handshake(struct openconnect_info *vpninfo) vpn_progress(vpninfo, PRG_ERR, _("DTLS handshake failed: %d\n"), ret); openconnect_report_ssl_errors(vpninfo); - /* Kill both the new (failed) connection and the old one too. The - only time there'll be a valid existing session is when it was a - rekey, and in that case it's time for the old one to die. */ - dtls_close(vpninfo, 1); + dtls_close(vpninfo); vpninfo->dtls_state = DTLS_SLEEPING; time(&vpninfo->new_dtls_started); @@ -393,19 +383,19 @@ static int start_dtls_handshake(struct openconnect_info *vpninfo, int dtls_fd) return -EINVAL; } - vpninfo->new_dtls_ssl = dtls_ssl; + vpninfo->dtls_ssl = dtls_ssl; return 0; } int dtls_try_handshake(struct openconnect_info *vpninfo) { - int err = gnutls_handshake(vpninfo->new_dtls_ssl); + int err = gnutls_handshake(vpninfo->dtls_ssl); if (!err) { #ifdef HAVE_GNUTLS_DTLS_SET_DATA_MTU /* Make sure GnuTLS's idea of the MTU is sufficient to take a full VPN MTU (with 1-byte header) in a data record. */ - err = gnutls_dtls_set_data_mtu(vpninfo->new_dtls_ssl, vpninfo->ip_info.mtu + 1); + err = gnutls_dtls_set_data_mtu(vpninfo->dtls_ssl, vpninfo->ip_info.mtu + 1); if (err) { vpn_progress(vpninfo, PRG_ERR, _("Failed to set DTLS MTU: %s\n"), @@ -417,7 +407,7 @@ int dtls_try_handshake(struct openconnect_info *vpninfo) we leave enough headroom by adding the worst-case overhead. We only support AES128-CBC and DES-CBC3-SHA anyway, so working out the worst case isn't hard. */ - gnutls_dtls_set_mtu(vpninfo->new_dtls_ssl, + gnutls_dtls_set_mtu(vpninfo->dtls_ssl, vpninfo->ip_info.mtu + 1 /* packet + header */ + 13 /* DTLS header */ + 20 /* biggest supported MAC (SHA1) */ @@ -430,13 +420,6 @@ int dtls_try_handshake(struct openconnect_info *vpninfo) _("Established DTLS connection (using GnuTLS). Ciphersuite %s.\n"), vpninfo->dtls_cipher); - dtls_close(vpninfo, 0); - vpninfo->dtls_ssl = vpninfo->new_dtls_ssl; - vpninfo->dtls_fd = vpninfo->new_dtls_fd; - - vpninfo->new_dtls_ssl = NULL; - vpninfo->new_dtls_fd = -1; - vpninfo->dtls_times.last_rx = vpninfo->dtls_times.last_tx = time(NULL); /* XXX: For OpenSSL we explicitly prevent retransmits here. */ @@ -453,10 +436,7 @@ int dtls_try_handshake(struct openconnect_info *vpninfo) gnutls_strerror(err)); error: - /* Kill both the new (failed) connection and the old one too. The - only time there'll be a valid existing session is when it was a - rekey, and in that case it's time for the old one to die. */ - dtls_close(vpninfo, 1); + dtls_close(vpninfo); vpninfo->dtls_state = DTLS_SLEEPING; time(&vpninfo->new_dtls_started); @@ -468,6 +448,13 @@ int connect_dtls_socket(struct openconnect_info *vpninfo) { int dtls_fd, ret, sndbuf; + /* Sanity check for the removal of new_dtls_{fd,ssl} */ + if (vpninfo->dtls_fd != -1) { + vpn_progress(vpninfo, PRG_ERR, _("DTLS connection attempted with an existing fd\n")); + vpninfo->dtls_attempt_period = 0; + return -EINVAL; + } + if (!vpninfo->dtls_addr) { vpn_progress(vpninfo, PRG_ERR, _("No DTLS address\n")); vpninfo->dtls_attempt_period = 0; @@ -552,7 +539,7 @@ int connect_dtls_socket(struct openconnect_info *vpninfo) vpninfo->dtls_state = DTLS_CONNECTING; - vpninfo->new_dtls_fd = dtls_fd; + vpninfo->dtls_fd = dtls_fd; if (vpninfo->select_nfds <= dtls_fd) vpninfo->select_nfds = dtls_fd + 1; @@ -564,7 +551,7 @@ int connect_dtls_socket(struct openconnect_info *vpninfo) return dtls_try_handshake(vpninfo); } -void dtls_close(struct openconnect_info *vpninfo, int kill_handshake_too) +void dtls_close(struct openconnect_info *vpninfo) { if (vpninfo->dtls_ssl) { DTLS_FREE(vpninfo->dtls_ssl); @@ -575,19 +562,11 @@ void dtls_close(struct openconnect_info *vpninfo, int kill_handshake_too) vpninfo->dtls_ssl = NULL; vpninfo->dtls_fd = -1; } - if (kill_handshake_too && vpninfo->new_dtls_ssl) { - DTLS_FREE(vpninfo->new_dtls_ssl); - closesocket(vpninfo->new_dtls_fd); - FD_CLR(vpninfo->new_dtls_fd, &vpninfo->select_rfds); - FD_CLR(vpninfo->new_dtls_fd, &vpninfo->select_efds); - vpninfo->new_dtls_ssl = NULL; - vpninfo->new_dtls_fd = -1; - } } static int dtls_restart(struct openconnect_info *vpninfo) { - dtls_close(vpninfo, 0); + dtls_close(vpninfo); vpninfo->dtls_state = DTLS_SLEEPING; return connect_dtls_socket(vpninfo); } diff --git a/library.c b/library.c index 6ad8c8dd..f83e10d3 100644 --- a/library.c +++ b/library.c @@ -47,7 +47,7 @@ struct openconnect_info *openconnect_vpninfo_new(char *useragent, if (!vpninfo) return NULL; - vpninfo->tun_fd = vpninfo->ssl_fd = vpninfo->dtls_fd = vpninfo->new_dtls_fd = -1; + vpninfo->tun_fd = vpninfo->ssl_fd = vpninfo->dtls_fd = -1; vpninfo->cmd_fd = vpninfo->cmd_fd_write = -1; vpninfo->cert_expire_warning = 60 * 86400; vpninfo->deflate = 1; @@ -129,7 +129,7 @@ static void free_optlist(struct oc_vpn_option *opt) void openconnect_vpninfo_free(struct openconnect_info *vpninfo) { openconnect_close_https(vpninfo, 1); - dtls_close(vpninfo, 1); + dtls_close(vpninfo); if (vpninfo->cmd_fd_write != -1) { close(vpninfo->cmd_fd); close(vpninfo->cmd_fd_write); diff --git a/mainloop.c b/mainloop.c index 34dc192a..d7663f4a 100644 --- a/mainloop.c +++ b/mainloop.c @@ -103,8 +103,11 @@ int openconnect_mainloop(struct openconnect_info *vpninfo, /* close all connections and wait for the user to call openconnect_mainloop() again */ openconnect_close_https(vpninfo, 0); - dtls_close(vpninfo, 1); - vpninfo->new_dtls_started = 0; + if (vpninfo->dtls_state != DTLS_DISABLED) { + dtls_close(vpninfo); + vpninfo->dtls_state = DTLS_SLEEPING; + vpninfo->new_dtls_started = 0; + } vpninfo->got_pause_cmd = 0; vpn_progress(vpninfo, PRG_INFO, _("Caller paused the connection\n")); diff --git a/openconnect-internal.h b/openconnect-internal.h index 7768dd8b..d57b9f4d 100644 --- a/openconnect-internal.h +++ b/openconnect-internal.h @@ -249,7 +249,6 @@ struct openconnect_info { #if defined(DTLS_OPENSSL) SSL_CTX *dtls_ctx; SSL *dtls_ssl; - SSL *new_dtls_ssl; SSL_SESSION *dtls_session; #elif defined(DTLS_GNUTLS) /* Call these *_ssl rather than *_sess because they're just @@ -258,7 +257,6 @@ struct openconnect_info { differently named to the OpenSSL variant, and forcing us to have ifdefs or accessor macros for them. */ gnutls_session_t dtls_ssl; - gnutls_session_t new_dtls_ssl; #endif int dtls_state; struct keepalive_info dtls_times; @@ -290,7 +288,6 @@ struct openconnect_info { int tun_fd; int ssl_fd; int dtls_fd; - int new_dtls_fd; int cmd_fd; int cmd_fd_write; @@ -423,7 +420,7 @@ unsigned char unhex(const char *data); int dtls_mainloop(struct openconnect_info *vpninfo, int *timeout); int dtls_try_handshake(struct openconnect_info *vpninfo); int connect_dtls_socket(struct openconnect_info *vpninfo); -void dtls_close(struct openconnect_info *vpninfo, int kill_handshake_too); +void dtls_close(struct openconnect_info *vpninfo); /* cstp.c */ int cstp_mainloop(struct openconnect_info *vpninfo, int *timeout);