From bfbe33bc081e1015805f6c801a2a116a09bba90e Mon Sep 17 00:00:00 2001 From: Daniel Lenski Date: Thu, 11 Feb 2021 17:48:40 -0800 Subject: [PATCH] Add start_dtls_anon_handshake() for PPP protocols Instead of PSK or fake-resumed sessions, F5 and Fortinet just do normal DTLS. Which isn't actually "normal" for us, so add start_dtls_anon_handshake() to support it. This needs to add certificate validation, which wasn't necessary with PSK or session resumption which were effectively mutual authentication anyway. It is invoked whenever the vpninfo->dtls_cipher field is empty. The Cisco AnyConnect support *always* sets that; the normal DTLS protocols never will. Rename and repurpose the vpninfo->cisco_dtls12 field to ->dtls12. For Cisco it doesn't change its semantics, but for normal DTLS it indicates whether we can try DTLSv1.2 or not. It doesn't guarantee that the server *will* do DTLSv1.2; it only lets us *try*. So it should be set everywhere except for the older F5 BIG-IP servers (v15 and earlier) which don't set the tag and which actually crap themselves and abort the connection if we try DTLSv1.2 instead of negotiating down to DTLSv1.0 sanely. Signed-off-by: Daniel Lenski Signed-off-by: David Woodhouse --- cstp.c | 2 +- dtls.c | 5 +- gnutls-dtls.c | 70 +++++++++++++- openconnect-internal.h | 13 ++- openssl-dtls.c | 208 +++++++++++++++++++++++++---------------- 5 files changed, 207 insertions(+), 91 deletions(-) diff --git a/cstp.c b/cstp.c index 6c1f9373..17d0f914 100644 --- a/cstp.c +++ b/cstp.c @@ -517,7 +517,7 @@ static int start_cstp_connection(struct openconnect_info *vpninfo) } } else if (!strcmp(buf + i, "CipherSuite")) { /* Remember if it came from a 'X-DTLS12-CipherSuite:' header */ - vpninfo->cisco_dtls12 = (i == 9); + vpninfo->dtls12 = (i == 9); vpninfo->dtls_cipher = strdup(colon); } else if (!strcmp(buf + i, "Port")) { int dtls_port = atol(colon); diff --git a/dtls.c b/dtls.c index 96754486..92819a9f 100644 --- a/dtls.c +++ b/dtls.c @@ -113,7 +113,7 @@ static int connect_dtls_socket(struct openconnect_info *vpninfo, int *timeout) return -EINVAL; } - if (!vpninfo->dtls_cipher) { + if (vpninfo->proto->proto == PROTO_ANYCONNECT && !vpninfo->dtls_cipher) { /* We probably didn't offer it any ciphers it liked */ vpn_progress(vpninfo, PRG_ERR, _("Server offered no DTLS cipher option\n")); vpninfo->dtls_attempt_period = 0; @@ -626,6 +626,9 @@ void dtls_detect_mtu(struct openconnect_info *vpninfo) int prev_mtu = vpninfo->ip_info.mtu; unsigned char *buf; + if (vpninfo->proto->proto != PROTO_ANYCONNECT) + return; + if (vpninfo->ip_info.mtu < 1 + sizeof(uint32_t)) return; diff --git a/gnutls-dtls.c b/gnutls-dtls.c index f14a6880..d36924bb 100644 --- a/gnutls-dtls.c +++ b/gnutls-dtls.c @@ -277,7 +277,7 @@ static int start_dtls_resume_handshake(struct openconnect_info *vpninfo, gnutls_ int cipher; for (cipher = 0; cipher < sizeof(gnutls_dtls_ciphers)/sizeof(gnutls_dtls_ciphers[0]); cipher++) { - if (gnutls_dtls_ciphers[cipher].cisco_dtls12 != vpninfo->cisco_dtls12 || + if (gnutls_dtls_ciphers[cipher].cisco_dtls12 != vpninfo->dtls12 || gnutls_check_version(gnutls_dtls_ciphers[cipher].min_gnutls_version) == NULL) continue; if (!strcmp(vpninfo->dtls_cipher, gnutls_dtls_ciphers[cipher].name)) @@ -317,6 +317,44 @@ static int start_dtls_resume_handshake(struct openconnect_info *vpninfo, gnutls_ return 0; } +static int start_dtls_anon_handshake(struct openconnect_info *vpninfo, gnutls_session_t dtls_ssl) +{ + char *prio = vpninfo->ciphersuite_config; + int ret; + + /* + * Use the same cred store as for the HTTPS session. That has + * our own verify_peer() callback installed, and will validate + * just like we do for the HTTPS service. + * + * There is also perhaps a case to be made for *only* accepting + * precisely the same cert that we get from the HTTPS service, + * but we tried that for EAP-TTLS in the Pulse protocol and the + * theory was disproven, so we ended up doing this there too. + */ + gnutls_credentials_set(dtls_ssl, GNUTLS_CRD_CERTIFICATE, vpninfo->https_cred); + + /* The F5 BIG-IP server before v16, will crap itself if we + * even *try* to do DTLSv1.2 */ + if (!vpninfo->dtls12 && + asprintf(&prio, "%s:-VERS-DTLS1.2:+VERS-DTLS1.0", + vpninfo->ciphersuite_config) < 0) + return -ENOMEM; + + ret = gnutls_priority_set_direct(dtls_ssl, prio ? : vpninfo->ciphersuite_config, + NULL); + if (ret) { + vpn_progress(vpninfo, PRG_ERR, + _("Failed to set DTLS priority: '%s': %s\n"), + prio, gnutls_strerror(ret)); + } + + if (prio != vpninfo->ciphersuite_config) + free(prio); + + return ret; +} + /* * GnuTLS version between 3.6.3 and 3.6.12 send zero'ed ClientHello. Make sure * we are not hitting that bug. Adapted from: @@ -375,10 +413,16 @@ int start_dtls_handshake(struct openconnect_info *vpninfo, int dtls_fd) gnutls_transport_set_ptr(dtls_ssl, (gnutls_transport_ptr_t)(intptr_t)dtls_fd); - if (!strcmp(vpninfo->dtls_cipher, "PSK-NEGOTIATE")) + if (!vpninfo->dtls_cipher) { + /* Anonymous DTLS (PPP protocols) */ + ret = start_dtls_anon_handshake(vpninfo, dtls_ssl); + } else if (!strcmp(vpninfo->dtls_cipher, "PSK-NEGOTIATE")) { + /* For OpenConnect/ocserv protocol */ ret = start_dtls_psk_handshake(vpninfo, dtls_ssl); - else + } else { + /* Nonexistent session resume hack (Cisco AnyConnect) */ ret = start_dtls_resume_handshake(vpninfo, dtls_ssl); + } if (ret) { if (ret != -EAGAIN) @@ -403,8 +447,23 @@ int dtls_try_handshake(struct openconnect_info *vpninfo, int *timeout) char *str; if (!err) { - if (!strcmp(vpninfo->dtls_cipher, "PSK-NEGOTIATE")) { - /* For PSK-NEGOTIATE, we have to determine the tunnel MTU + if (!vpninfo->dtls_cipher) { + /* Anonymous DTLS (PPP protocols) will set vpninfo->ip_info.mtu + * in PPP negotiation. + * + * XX: Needs forthcoming overhaul to detect MTU correctly and offer + * reasonable MRU values during PPP negotiation. + */ + int data_mtu = vpninfo->cstp_basemtu = 1500; + if (vpninfo->peer_addr->sa_family == IPPROTO_IPV6) + data_mtu -= 40; /* IPv6 header */ + else + data_mtu -= 20; /* Legacy IP header */ + data_mtu -= 8; /* UDP header */ + dtls_set_mtu(vpninfo, data_mtu); + } else if (!strcmp(vpninfo->dtls_cipher, "PSK-NEGOTIATE")) { + /* For PSK-NEGOTIATE (OpenConnect/ocserv protocol) + * we have to determine the tunnel MTU * for ourselves based on the base MTU */ int data_mtu = vpninfo->cstp_basemtu; if (vpninfo->peer_addr->sa_family == IPPROTO_IPV6) @@ -428,6 +487,7 @@ int dtls_try_handshake(struct openconnect_info *vpninfo, int *timeout) vpninfo->ip_info.mtu = data_mtu; } } else { + /* Nonexistent session resume hack (Cisco AnyConnect) */ if (!gnutls_session_is_resumed(vpninfo->dtls_ssl)) { /* Someone attempting to hijack the DTLS session? * A real server would never allow a full session diff --git a/openconnect-internal.h b/openconnect-internal.h index f03d17ae..ec396d7c 100644 --- a/openconnect-internal.h +++ b/openconnect-internal.h @@ -633,8 +633,17 @@ struct openconnect_info { uint32_t ift_seq; - int cisco_dtls12; /* If Cisco server sent X-DTLS12-CipherSuite header, rather than X-DTLS-CipherSuite */ - char *dtls_cipher; /* Value of aforementioned header (PSK-NEGOTIATE, or an OpenSSL cipher name) */ + int dtls12; /* For PPP protocols with anonymous DTLS, this being zero indicates that + * the server *cannot* handle DTLSv1.2 and we mustn't even try to negotiate + * it (e.g. F5 BIG-IP v15 and lower). If it's 1, we can try anything; + * even DTLSv1.3. + * + * For AnyConnect, it means the Cisco server sent the X-DTLS12-CipherSuite + * header, rather than X-DTLS-CipherSuite which indicates their DTLSv0.9. + */ + + char *dtls_cipher; /* Only set for AnyConnect. Defines the session to be "resumed" + * ("PSK-NEGOTIATE", or an OpenSSL cipher name). */ char *vpnc_script; #ifndef _WIN32 diff --git a/openssl-dtls.c b/openssl-dtls.c index d47dd701..7bd38f3d 100644 --- a/openssl-dtls.c +++ b/openssl-dtls.c @@ -331,9 +331,15 @@ int start_dtls_handshake(struct openconnect_info *vpninfo, int dtls_fd) int dtlsver = DTLS1_BAD_VER; const char *cipher = vpninfo->dtls_cipher; + if (!cipher) { + dtlsver = 0; #ifdef HAVE_DTLS12 + /* The F5 BIG-IP server before v16, will crap itself if we + * even *try* to do DTLSv1.2 */ + if (!vpninfo->dtls12) + dtlsver = DTLS1_VERSION; /* These things should never happen unless they're supported */ - if (vpninfo->cisco_dtls12) { + } else if (vpninfo->dtls12) { dtlsver = DTLS1_2_VERSION; } else if (!strcmp(cipher, "OC-DTLS1_2-AES128-GCM")) { dtlsver = DTLS1_2_VERSION; @@ -345,19 +351,17 @@ int start_dtls_handshake(struct openconnect_info *vpninfo, int dtls_fd) } else if (!strcmp(cipher, "PSK-NEGOTIATE")) { dtlsver = 0; /* Let it negotiate */ #endif - } #endif + } if (!vpninfo->dtls_ctx) { #ifdef HAVE_DTLS12 /* If we can use SSL_CTX_set_min_proto_version, do so. */ dtls_method = DTLS_client_method(); -#endif -#ifndef HAVE_SSL_CTX_PROTOVER - /* If !HAVE_DTLS12, dtlsver *MUST* be DTLS1_BAD_VER because it's set - * at the top of the function and nothing can change it. */ - if (dtlsver == DTLS1_BAD_VER) - dtls_method = DTLSv1_client_method(); +#elif !defined(HAVE_SSL_CTX_PROTOVER) + dtls_method = DTLSv1_client_method(); +#else +#error "This can't happen as DTLS1.2 came before SSL_CTX_set_mXX_proto_version()" #endif vpninfo->dtls_ctx = SSL_CTX_new(dtls_method); @@ -386,75 +390,90 @@ int start_dtls_handshake(struct openconnect_info *vpninfo, int dtls_fd) if (dtlsver == DTLS1_BAD_VER) SSL_CTX_set_options(vpninfo->dtls_ctx, SSL_OP_CISCO_ANYCONNECT); #endif + /* If we don't readahead, then we do short reads and throw + away the tail of data packets. */ + SSL_CTX_set_read_ahead(vpninfo->dtls_ctx, 1); + + if (vpninfo->proto->proto == PROTO_ANYCONNECT) { + /* All the AnyConnect hackery about saved sessions and PSK */ #if defined (HAVE_DTLS12) && !defined(OPENSSL_NO_PSK) - if (!dtlsver) { - SSL_CTX_set_psk_client_callback(vpninfo->dtls_ctx, psk_callback); - /* For PSK we override the DTLS master secret with one derived - * from the HTTPS session. */ - if (!SSL_export_keying_material(vpninfo->https_ssl, - vpninfo->dtls_secret, PSK_KEY_SIZE, - PSK_LABEL, PSK_LABEL_SIZE, NULL, 0, 0)) { - vpn_progress(vpninfo, PRG_ERR, - _("Failed to generate DTLS key\n")); - openconnect_report_ssl_errors(vpninfo); - SSL_CTX_free(vpninfo->dtls_ctx); - vpninfo->dtls_ctx = NULL; - vpninfo->dtls_attempt_period = 0; - return -EINVAL; + if (!dtlsver) { + SSL_CTX_set_psk_client_callback(vpninfo->dtls_ctx, psk_callback); + /* For PSK we override the DTLS master secret with one derived + * from the HTTPS session. */ + if (!SSL_export_keying_material(vpninfo->https_ssl, + vpninfo->dtls_secret, PSK_KEY_SIZE, + PSK_LABEL, PSK_LABEL_SIZE, NULL, 0, 0)) { + vpn_progress(vpninfo, PRG_ERR, + _("Failed to generate DTLS key\n")); + openconnect_report_ssl_errors(vpninfo); + SSL_CTX_free(vpninfo->dtls_ctx); + vpninfo->dtls_ctx = NULL; + vpninfo->dtls_attempt_period = 0; + return -EINVAL; + } + /* For SSL_CTX_set_cipher_list() */ + cipher = "PSK"; } - /* For SSL_CTX_set_cipher_list() */ - cipher = "PSK"; - } #endif /* OPENSSL_NO_PSK */ #ifdef SSL_OP_NO_ENCRYPT_THEN_MAC - /* - * I'm fairly sure I wasn't lying when I said I had tested - * https://github.com/openssl/openssl/commit/e23d5071ec4c7aa6bb2b - * against GnuTLS both with and without EtM in 2016. - * - * Nevertheless, in 2019 it seems to be failing to negotiate - * at least for DTLS1_BAD_VER against ocserv with GnuTLS 3.6.7: - * https://gitlab.com/gnutls/gnutls/issues/139 — I think because - * GnuTLS isn't actually doing EtM after negotiating it (like - * OpenSSL 1.1.0 used to). - * - * Just turn it off. Real Cisco servers don't do it for - * DTLS1_BAD_VER, and against ocserv (and newer Cisco) we should - * be using DTLSv1.2 with AEAD ciphersuites anyway so EtM is - * irrelevant. - */ - SSL_CTX_set_options(vpninfo->dtls_ctx, SSL_OP_NO_ENCRYPT_THEN_MAC); + /* + * I'm fairly sure I wasn't lying when I said I had tested + * https://github.com/openssl/openssl/commit/e23d5071ec4c7aa6bb2b + * against GnuTLS both with and without EtM in 2016. + * + * Nevertheless, in 2019 it seems to be failing to negotiate + * at least for DTLS1_BAD_VER against ocserv with GnuTLS 3.6.7: + * https://gitlab.com/gnutls/gnutls/issues/139 — I think because + * GnuTLS isn't actually doing EtM after negotiating it (like + * OpenSSL 1.1.0 used to). + * + * Just turn it off. Real Cisco servers don't do it for + * DTLS1_BAD_VER, and against ocserv (and newer Cisco) we should + * be using DTLSv1.2 with AEAD ciphersuites anyway so EtM is + * irrelevant. + */ + SSL_CTX_set_options(vpninfo->dtls_ctx, SSL_OP_NO_ENCRYPT_THEN_MAC); #endif #ifdef SSL_OP_NO_EXTENDED_MASTER_SECRET - /* RFC7627 says: - * - * If the original session did not use the "extended_master_secret" - * extension but the new ClientHello contains the extension, then the - * server MUST NOT perform the abbreviated handshake. Instead, it - * SHOULD continue with a full handshake (as described in - * Section 5.2) to negotiate a new session. - * - * Now that would be distinctly suboptimal, since we have no way to do - * a full handshake (we even explicitly protect against it, in case a - * MITM server attempts to hijack our deliberately-resumed session). - * - * So where OpenSSL provides the choice, tell it not to use extms on - * resumed sessions. - */ - if (dtlsver) - SSL_CTX_set_options(vpninfo->dtls_ctx, SSL_OP_NO_EXTENDED_MASTER_SECRET); + /* RFC7627 says: + * + * If the original session did not use the "extended_master_secret" + * extension but the new ClientHello contains the extension, then the + * server MUST NOT perform the abbreviated handshake. Instead, it + * SHOULD continue with a full handshake (as described in + * Section 5.2) to negotiate a new session. + * + * Now that would be distinctly suboptimal, since we have no way to do + * a full handshake (we even explicitly protect against it, in case a + * MITM server attempts to hijack our deliberately-resumed session). + * + * So where OpenSSL provides the choice, tell it not to use extms on + * resumed sessions. + */ + if (dtlsver) + SSL_CTX_set_options(vpninfo->dtls_ctx, SSL_OP_NO_EXTENDED_MASTER_SECRET); #endif - /* If we don't readahead, then we do short reads and throw - away the tail of data packets. */ - SSL_CTX_set_read_ahead(vpninfo->dtls_ctx, 1); - - if (!SSL_CTX_set_cipher_list(vpninfo->dtls_ctx, cipher)) { - vpn_progress(vpninfo, PRG_ERR, - _("Set DTLS cipher list failed\n")); - SSL_CTX_free(vpninfo->dtls_ctx); - vpninfo->dtls_ctx = NULL; - vpninfo->dtls_attempt_period = 0; - return -EINVAL; + if (!SSL_CTX_set_cipher_list(vpninfo->dtls_ctx, cipher)) { + vpn_progress(vpninfo, PRG_ERR, + _("Set DTLS cipher list failed\n")); + SSL_CTX_free(vpninfo->dtls_ctx); + vpninfo->dtls_ctx = NULL; + vpninfo->dtls_attempt_period = 0; + return -EINVAL; + } + } else { + /* Protocols other than AnyConnect are plain DTLS and do + * need to check the server certificate properly (which + * AnyConnect can skip because it all depends on PSK or + * session resumption anyway */ + int ret = openconnect_install_ctx_verify(vpninfo, vpninfo->dtls_ctx); + if (ret) { + SSL_CTX_free(vpninfo->dtls_ctx); + vpninfo->dtls_ctx = NULL; + vpninfo->dtls_attempt_period = 0; + return ret; + } } } @@ -462,8 +481,17 @@ int start_dtls_handshake(struct openconnect_info *vpninfo, int dtls_fd) SSL_set_connect_state(dtls_ssl); SSL_set_app_data(dtls_ssl, vpninfo); - - if (dtlsver) { + /* + * AnyConnect always sets cipher; nothing else does. Checking this + * instead of vpninfo->proto->proto == PROTO_ANYCONNECT makes the + * static analyser less unhappy because it doesn't know that, so it + * would think we can dereference cipher when it's NULL. + */ + if (!cipher) { + /* Non-AnyConnect protocols need to verify the peer */ + SSL_set_verify(dtls_ssl, SSL_VERIFY_PEER, NULL); + } else if (dtlsver) { + /* This is the actual Cisco AnyConnect method, using session resume */ STACK_OF(SSL_CIPHER) *ciphers = SSL_get_ciphers(dtls_ssl); const SSL_CIPHER *ssl_ciph = NULL; int i; @@ -497,11 +525,11 @@ int start_dtls_handshake(struct openconnect_info *vpninfo, int dtls_fd) if (!SSL_set_session(dtls_ssl, dtls_session)) { vpn_progress(vpninfo, PRG_ERR, - _("SSL_set_session() failed with old protocol version 0x%x\n" + _("SSL_set_session() failed with DTLS protocol version 0x%x\n" "Are you using a version of OpenSSL older than 0.9.8m?\n" "See http://rt.openssl.org/Ticket/Display.html?id=1751\n" "Use the --no-dtls command line option to avoid this message\n"), - DTLS1_BAD_VER); + dtlsver); SSL_CTX_free(vpninfo->dtls_ctx); SSL_free(dtls_ssl); vpninfo->dtls_ctx = NULL; @@ -513,10 +541,13 @@ int start_dtls_handshake(struct openconnect_info *vpninfo, int dtls_fd) SSL_SESSION_free(dtls_session); } else if (vpninfo->dtls_app_id_size > 0) { + /* + * For ocserv PSK-NEGOTIATE we abuse the session resume + * protocol just to pass an 'App ID' in our ClientHello + * in the session identifier. This session doesn't exist + * and isn't actually going to be resumed at all. + */ const uint8_t cs[2] = {0x00, 0x2F}; /* RSA-AES-128 */ - /* we generate a session with a random key which cannot be resumed; - * we want to set the client identifier we received from the server - * as a session ID. */ dtls_session = generate_dtls_session(vpninfo, DTLS1_VERSION, SSL_CIPHER_find(dtls_ssl, cs), 1); @@ -527,7 +558,7 @@ int start_dtls_handshake(struct openconnect_info *vpninfo, int dtls_fd) vpninfo->dtls_attempt_period = 0; return -EINVAL; } - + if (!SSL_set_session(dtls_ssl, dtls_session)) { vpn_progress(vpninfo, PRG_ERR, _("SSL_set_session() failed\n")); @@ -540,8 +571,7 @@ int start_dtls_handshake(struct openconnect_info *vpninfo, int dtls_fd) } /* We don't need our own refcount on it any more */ SSL_SESSION_free(dtls_session); - } - + } /* else it's ocserv PSK-NEGOTIATE without an App-ID */ dtls_bio = BIO_new_socket(dtls_fd, BIO_NOCLOSE); /* Set non-blocking */ @@ -560,7 +590,21 @@ int dtls_try_handshake(struct openconnect_info *vpninfo, int *timeout) if (ret == 1) { const char *c; - if (!strcmp(vpninfo->dtls_cipher, "PSK-NEGOTIATE")) { + if (!vpninfo->dtls_cipher) { + /* Anonymous DTLS (PPP protocols) will set vpninfo->ip_info.mtu + * in PPP negotiation. + * + * XX: Needs forthcoming overhaul to detect MTU correctly and offer + * reasonable MRU values during PPP negotiation. + */ + int data_mtu = vpninfo->cstp_basemtu = 1500; + if (vpninfo->peer_addr->sa_family == IPPROTO_IPV6) + data_mtu -= 40; /* IPv6 header */ + else + data_mtu -= 20; /* Legacy IP header */ + data_mtu -= 8; /* UDP header */ + dtls_set_mtu(vpninfo, data_mtu); + } else if (!strcmp(vpninfo->dtls_cipher, "PSK-NEGOTIATE")) { /* For PSK-NEGOTIATE, we have to determine the tunnel MTU * for ourselves based on the base MTU */ int data_mtu = vpninfo->cstp_basemtu; @@ -612,7 +656,7 @@ int dtls_try_handshake(struct openconnect_info *vpninfo, int *timeout) _("DTLS connection compression using %s.\n"), c); } - vpninfo->dtls_times.last_rekey = vpninfo->dtls_times.last_rx = + vpninfo->dtls_times.last_rekey = vpninfo->dtls_times.last_rx = vpninfo->dtls_times.last_tx = time(NULL); /* From about 8.4.1(11) onwards, the ASA seems to get -- 2.50.1