From patchwork Fri Jan 19 08:22:52 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Steffan Karger X-Patchwork-Id: 199 Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director5.mail.ord1d.rsapps.net ([172.28.255.1]) by backend31.mail.ord1d.rsapps.net (Dovecot) with LMTP id C7AkKOZFYloscgAAgoeIoA for ; Fri, 19 Jan 2018 14:24:22 -0500 Received: from director8.mail.ord1c.rsapps.net ([172.28.255.1]) by director5.mail.ord1d.rsapps.net (Dovecot) with LMTP id u8/7BuZFYlrRIAAAsdCWiw ; Fri, 19 Jan 2018 14:24:22 -0500 Received: from smtp49.gate.ord1a ([172.28.255.1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by director8.mail.ord1c.rsapps.net (Dovecot) with LMTP id 3CSsC+ZFYloVWwAAPBwpBw ; Fri, 19 Jan 2018 14:24:22 -0500 X-Spam-Threshold: 95 X-Spam-Score: 0 X-Spam-Flag: NO X-Virus-Scanned: OK X-Orig-To: openvpnslackdevel@openvpn.net X-Originating-Ip: [216.34.181.88] Authentication-Results: smtp49.gate.ord1a.rsapps.net; iprev=pass policy.iprev="216.34.181.88"; spf=pass smtp.mailfrom="openvpn-devel-bounces@lists.sourceforge.net" smtp.helo="lists.sourceforge.net"; dkim=fail (signature verification failed) header.d=sourceforge.net; dkim=fail (signature verification failed) header.d=sf.net; dkim=fail (signature verification failed) header.d=karger-me.20150623.gappssmtp.com; dmarc=none (p=nil; dis=none) header.from=karger.me X-Classification-ID: 5a2c4752-fd4e-11e7-a870-001c23cf89bd-1-1 Received: from [216.34.181.88] ([216.34.181.88:59266] helo=lists.sourceforge.net) by smtp49.gate.ord1a.rsapps.net (envelope-from ) (ecelerity 4.2.1.56364 r(Core:4.2.1.14)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id F4/EA-02239-6E5426A5; Fri, 19 Jan 2018 14:24:22 -0500 Received: from localhost ([127.0.0.1] helo=sfs-ml-1.v29.ch3.sourceforge.com) by sfs-ml-1.v29.ch3.sourceforge.com with esmtp (Exim 4.89) (envelope-from ) id 1eccFn-0008Af-7h; Fri, 19 Jan 2018 19:23:07 +0000 Received: from sfi-mx-4.v28.ch3.sourceforge.com ([172.29.28.194] helo=mx.sourceforge.net) by sfs-ml-1.v29.ch3.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.89) (envelope-from ) id 1eccFl-0008AW-JT for openvpn-devel@lists.sourceforge.net; Fri, 19 Jan 2018 19:23:05 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=References:In-Reply-To:Message-Id:Date:Subject:Cc: To:From:Sender:Reply-To:MIME-Version:Content-Type:Content-Transfer-Encoding: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=qyKdJAktI0ve5V8edBHVaCg8TBq8ELpvvEUcS2bnXOU=; b=dvR7W/mhz+hDtkumxRb8jpn9NH 5qcpL3wNIWUSl1csg5G9LBpRwuA2eqYdkAta7jfe80XbmKIagJ8AZ5mi90Krra+II8cDepvbnn1M6 sRUVvlFPlpw2FvkktHCwR8cYDR2QTD9SfTeGoioacgnFLZyF5GKsZ7iJ3U5lMWoedvYY=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To :MIME-Version:Content-Type:Content-Transfer-Encoding:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=qyKdJAktI0ve5V8edBHVaCg8TBq8ELpvvEUcS2bnXOU=; b=J6MVDjV8y6xDJ40wjjhfk3TCWB ex2Y1wsg1lssdAsiaWfp1ox45YUWh7Fv++lOBXTG+jAp4oHRFufE3pORFevuoKth1O33iRAJh6hQz eISDDo+9gRAsWeF+UDd4qkIz8k4Vmue3MWKlBSbgp4P0fN+guJXW3UdHIInaaD6j+jpg=; Received: from mail-wm0-f66.google.com ([74.125.82.66]) by sfi-mx-4.v28.ch3.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES128-GCM-SHA256:128) (Exim 4.89) id 1eccFi-0005qy-6L for openvpn-devel@lists.sourceforge.net; Fri, 19 Jan 2018 19:23:05 +0000 Received: by mail-wm0-f66.google.com with SMTP id b141so5454618wme.1 for ; Fri, 19 Jan 2018 11:23:02 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=karger-me.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=qyKdJAktI0ve5V8edBHVaCg8TBq8ELpvvEUcS2bnXOU=; b=DwEBSglCK6GeJDeueVNCp7yJp2zVXFChtFthVfmDE8AkgPmsZt/y10GsGZ2fMLnBop 0RB34LQB09e8LlSvhCK499URUEwSSAN+32Mc2Ktb9+EKoUlRuq6tCX1d0aO+4WRrDggr SPFO3jaNwoNa5dciUTAdizarlCjAQ8eBucw41cF/oyy0ErxYyzhiabPzYS2WajmkMTuj 5ffC1MtHaQnjeabap9ahmm+wg07gK7jusjBYSZ2VBR5qHhSDib2WyC/SUGsKmU3zxSyN tdX48j6wnJbMwCOfDCrMhrjkUspPMOtiVkNMCMxj9VQ2/YWiIjwLvJwkoffoddG1jUTa 2WHg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=qyKdJAktI0ve5V8edBHVaCg8TBq8ELpvvEUcS2bnXOU=; b=Ppn0h/37SvpeoEknXMtbw25fF0DWY00J9sMt4PRrnlaZs2cMfCDqdGI3K3Thkc11VD MvmtzbCKnjC1o+xmmyL15NEG+p9cQU39ZzFQ5grxaxXM1VU7HjY/oZ9Hd58QxSDjsZ4A V9jLcX1tibMW0FZ3ykHoxwk2IlRCCbPFuJk6ToCPxYY+IB871L6l43x9vo4NXC9G2nHd 7SvIENcmO2M2VDGaMRXVYmxMHUpLy+ZYvX88MH4bcjrVVjyXYjZL/Th4OYlqLjYUfUED JIPh7o3OBRZC3ebzrk8tp8/EB4AfxKXwIG6OKbRmD6ojkqDzfOk/XbGHy5C1Fv4zM2Nk DLNQ== X-Gm-Message-State: AKwxytcAHysRFa/aPwd9b869PR7c24PDmtiZcuWatFs14wiknyjVX863 RbkZFewBr56ssKIV+ILUSHvkqDiLxXE= X-Google-Smtp-Source: ACJfBotru18bRrMOEYqyIh+2ZvjgwMi2u2ciYGFq+9egHirR/Qdj3gEoEXySiX01F1v5HpbJgC6GCQ== X-Received: by 10.80.136.83 with SMTP id c19mr14248877edc.302.1516389775811; Fri, 19 Jan 2018 11:22:55 -0800 (PST) Received: from vesta.fritz.box ([2001:985:e54:1:983:56e0:915c:a8f2]) by smtp.gmail.com with ESMTPSA id g59sm6578745ede.52.2018.01.19.11.22.54 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 19 Jan 2018 11:22:54 -0800 (PST) From: Steffan Karger To: openvpn-devel@lists.sourceforge.net Date: Fri, 19 Jan 2018 20:22:52 +0100 Message-Id: <20180119192252.6229-1-steffan@karger.me> X-Mailer: git-send-email 2.14.1 In-Reply-To: References: X-Spam-Report: Spam Filtering performed by mx.sourceforge.net. See http://spamassassin.org/tag/ for more details. -0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at http://www.dnswl.org/, no trust [74.125.82.66 listed in list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature X-Headers-End: 1eccFi-0005qy-6L Subject: [Openvpn-devel] [PATCH 1/3 v3] Fix --tls-version-min and --tls-version-max for OpenSSL 1.1+ X-BeenThere: openvpn-devel@lists.sourceforge.net X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox As described in <80e6b449-c536-dc87-7215-3693872bce5a@birkenwald.de> on the openvpn-devel mailing list, --tls-version-min no longer works with OpenSSL 1.1. Kurt Roeckx posted in a debian bug report: "This is marked as important because if you switch to openssl 1.1.0 the defaults minimum version in Debian is currently TLS 1.2 and you can't override it with the options that you're currently using (and are deprecated)." This patch is loosely based on the original patch by Kurt, but solves the issue by adding functions to openssl-compat.h, like we also did for all other openssl 1.1. breakage. This results in not having to add more ifdefs in ssl_openssl.c and thus cleaner code. Signed-off-by: Steffan Karger Signed-off-by: Steffan Karger <steffan@karger.me>
--- v2: fix define name, obey system lib default minimum version v3: add error handling, fix bug in setting default minimum src/openvpn/openssl_compat.h | 63 +++++++++++++++++++++++++++ src/openvpn/ssl.c | 5 ++- src/openvpn/ssl_backend.h | 4 +- src/openvpn/ssl_mbedtls.c | 3 +- src/openvpn/ssl_openssl.c | 100 +++++++++++++++++++++++++++---------------- 5 files changed, 136 insertions(+), 39 deletions(-) diff --git a/src/openvpn/openssl_compat.h b/src/openvpn/openssl_compat.h index 05ec4e95..4931fad3 100644 --- a/src/openvpn/openssl_compat.h +++ b/src/openvpn/openssl_compat.h @@ -661,4 +661,67 @@ EC_GROUP_order_bits(const EC_GROUP *group) #define RSA_F_RSA_OSSL_PRIVATE_ENCRYPT RSA_F_RSA_EAY_PRIVATE_ENCRYPT #endif +#ifndef SSL_CTX_get_min_proto_version +/** Dummy SSL_CTX_get_min_proto_version for OpenSSL < 1.1 (not really needed) */ +static inline int +SSL_CTX_get_min_proto_version(SSL_CTX *ctx) +{ + return 0; +} +#endif /* SSL_CTX_get_min_proto_version */ + +#ifndef SSL_CTX_set_min_proto_version +/** Mimics SSL_CTX_set_min_proto_version for OpenSSL < 1.1 */ +static inline void +SSL_CTX_set_min_proto_version(SSL_CTX *ctx, long tls_ver_min) +{ + long sslopt = SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3; /* Never do < TLS 1.0 */ + + if (tls_ver_min > TLS1_VERSION) + { + sslopt |= SSL_OP_NO_TLSv1; + } +#ifdef SSL_OP_NO_TLSv1_1 + if (tls_ver_min > TLS1_1_VERSION) + { + sslopt |= SSL_OP_NO_TLSv1_1; + } +#endif +#ifdef SSL_OP_NO_TLSv1_2 + if (tls_ver_min > TLS1_2_VERSION) + { + sslopt |= SSL_OP_NO_TLSv1_2; + } +#endif + SSL_CTX_set_options(ctx, sslopt); +} +#endif /* SSL_CTX_set_min_proto_version */ + +#ifndef SSL_CTX_set_max_proto_version +/** Mimics SSL_CTX_set_max_proto_version for OpenSSL < 1.1 */ +static inline void +SSL_CTX_set_max_proto_version(SSL_CTX *ctx, long tls_ver_max) +{ + long sslopt = 0; + + if (tls_ver_max < TLS1_VERSION) + { + sslopt |= SSL_OP_NO_TLSv1; + } +#ifdef SSL_OP_NO_TLSv1_1 + if (tls_ver_max < TLS1_1_VERSION) + { + sslopt |= SSL_OP_NO_TLSv1_1; + } +#endif +#ifdef SSL_OP_NO_TLSv1_2 + if (tls_ver_max < TLS1_2_VERSION) + { + sslopt |= SSL_OP_NO_TLSv1_2; + } +#endif + SSL_CTX_set_options(ctx, sslopt); +} +#endif /* SSL_CTX_set_max_proto_version */ + #endif /* OPENSSL_COMPAT_H_ */ diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 7b428455..6c27050f 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -622,7 +622,10 @@ init_ssl(const struct options *options, struct tls_root_ctx *new_ctx) * cipher restrictions before loading certificates */ tls_ctx_restrict_ciphers(new_ctx, options->cipher_list); - tls_ctx_set_options(new_ctx, options->ssl_flags); + if (!tls_ctx_set_options(new_ctx, options->ssl_flags)) + { + goto err; + } if (options->pkcs12_file) { diff --git a/src/openvpn/ssl_backend.h b/src/openvpn/ssl_backend.h index 7cf5d830..444fb2f9 100644 --- a/src/openvpn/ssl_backend.h +++ b/src/openvpn/ssl_backend.h @@ -162,8 +162,10 @@ bool tls_ctx_initialised(struct tls_root_ctx *ctx); * * @param ctx TLS context to set options on * @param ssl_flags SSL flags to set + * + * @return true on success, false otherwise. */ -void tls_ctx_set_options(struct tls_root_ctx *ctx, unsigned int ssl_flags); +bool tls_ctx_set_options(struct tls_root_ctx *ctx, unsigned int ssl_flags); /** * Restrict the list of ciphers that can be used within the TLS context. diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c index 8ac52d55..d503162a 100644 --- a/src/openvpn/ssl_mbedtls.c +++ b/src/openvpn/ssl_mbedtls.c @@ -206,9 +206,10 @@ key_state_export_keying_material(struct key_state_ssl *ssl, { } -void +bool tls_ctx_set_options(struct tls_root_ctx *ctx, unsigned int ssl_flags) { + return true; } static const char * diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c index 86318d4c..50d68280 100644 --- a/src/openvpn/ssl_openssl.c +++ b/src/openvpn/ssl_openssl.c @@ -206,16 +206,65 @@ info_callback(INFO_CALLBACK_SSL_CONST SSL *s, int where, int ret) int tls_version_max(void) { -#if defined(SSL_OP_NO_TLSv1_2) +#if defined(TLS1_2_VERSION) || defined(SSL_OP_NO_TLSv1_2) return TLS_VER_1_2; -#elif defined(SSL_OP_NO_TLSv1_1) +#elif defined(TLS1_1_VERSION) || defined(SSL_OP_NO_TLSv1_1) return TLS_VER_1_1; #else return TLS_VER_1_0; #endif } -void +/** Convert internal version number to openssl version number */ +static int +openssl_tls_version(int ver) +{ + if (ver == TLS_VER_1_0) + { + return TLS1_VERSION; + } + else if (ver == TLS_VER_1_1) + { + return TLS1_1_VERSION; + } + else if (ver == TLS_VER_1_2) + { + return TLS1_2_VERSION; + } + return 0; +} + +static bool +tls_ctx_set_tls_versions(struct tls_root_ctx *ctx, unsigned int ssl_flags) +{ + int tls_ver_min = openssl_tls_version( + (ssl_flags >> SSLF_TLS_VERSION_MIN_SHIFT) & SSLF_TLS_VERSION_MIN_MASK); + int tls_ver_max = openssl_tls_version( + (ssl_flags >> SSLF_TLS_VERSION_MAX_SHIFT) & SSLF_TLS_VERSION_MAX_MASK); + + if (!tls_ver_min) + { + /* Enforce at least TLS 1.0 */ + int cur_min = SSL_CTX_get_min_proto_version(ctx->ctx); + tls_ver_min = cur_min < TLS1_VERSION ? TLS1_VERSION : cur_min; + } + + if (!SSL_CTX_set_min_proto_version(ctx->ctx, tls_ver_min)) + { + msg(D_TLS_ERRORS, "%s: failed to set minimum TLS version", __func__); + return false; + } + + if (tls_ver_max && !SSL_CTX_set_max_proto_version(ctx->ctx, tls_ver_max)) + { + msg(D_TLS_ERRORS, "%s: failed to set maximum TLS version", __func__); + return false; + } + + return true; +} + +bool tls_ctx_set_options(struct tls_root_ctx *ctx, unsigned int ssl_flags) { ASSERT(NULL != ctx); @@ -223,41 +272,18 @@ tls_ctx_set_options(struct tls_root_ctx *ctx, unsigned int ssl_flags) /* default certificate verification flags */ int flags = SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT; - /* process SSL options including minimum TLS version we will accept from peer */ - { - long sslopt = SSL_OP_SINGLE_DH_USE | SSL_OP_NO_TICKET | SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3; - int tls_ver_max = TLS_VER_UNSPEC; - const int tls_ver_min = - (ssl_flags >> SSLF_TLS_VERSION_MIN_SHIFT) & SSLF_TLS_VERSION_MIN_MASK; - - tls_ver_max = - (ssl_flags >> SSLF_TLS_VERSION_MAX_SHIFT) & SSLF_TLS_VERSION_MAX_MASK; - if (tls_ver_max <= TLS_VER_UNSPEC) - { - tls_ver_max = tls_version_max(); - } - - if (tls_ver_min > TLS_VER_1_0 || tls_ver_max < TLS_VER_1_0) - { - sslopt |= SSL_OP_NO_TLSv1; - } -#ifdef SSL_OP_NO_TLSv1_1 - if (tls_ver_min > TLS_VER_1_1 || tls_ver_max < TLS_VER_1_1) - { - sslopt |= SSL_OP_NO_TLSv1_1; - } -#endif -#ifdef SSL_OP_NO_TLSv1_2 - if (tls_ver_min > TLS_VER_1_2 || tls_ver_max < TLS_VER_1_2) - { - sslopt |= SSL_OP_NO_TLSv1_2; - } -#endif + /* process SSL options */ + long sslopt = SSL_OP_SINGLE_DH_USE | SSL_OP_NO_TICKET; #ifdef SSL_OP_CIPHER_SERVER_PREFERENCE - sslopt |= SSL_OP_CIPHER_SERVER_PREFERENCE; + sslopt |= SSL_OP_CIPHER_SERVER_PREFERENCE; #endif - sslopt |= SSL_OP_NO_COMPRESSION; - SSL_CTX_set_options(ctx->ctx, sslopt); + sslopt |= SSL_OP_NO_COMPRESSION; + + SSL_CTX_set_options(ctx->ctx, sslopt); + + if (!tls_ctx_set_tls_versions(ctx, ssl_flags)) + { + return false; } #ifdef SSL_MODE_RELEASE_BUFFERS @@ -280,6 +306,8 @@ tls_ctx_set_options(struct tls_root_ctx *ctx, unsigned int ssl_flags) SSL_CTX_set_verify(ctx->ctx, flags, verify_callback); SSL_CTX_set_info_callback(ctx->ctx, info_callback); + + return true; } void