From patchwork Sun Jul 12 23:46:46 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arne Schwabe X-Patchwork-Id: 1241 Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director12.mail.ord1d.rsapps.net ([172.31.255.6]) by backend30.mail.ord1d.rsapps.net with LMTP id KG3aJrotDF9HAwAAIUCqbw for ; Mon, 13 Jul 2020 05:47:38 -0400 Received: from proxy20.mail.iad3b.rsapps.net ([172.31.255.6]) by director12.mail.ord1d.rsapps.net with LMTP id YLk/JLotDF8CJQAAIasKDg ; Mon, 13 Jul 2020 05:47:38 -0400 Received: from smtp6.gate.iad3b ([172.31.255.6]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy20.mail.iad3b.rsapps.net with LMTP id sH8lHrotDF+nRgAAcDxLoQ ; Mon, 13 Jul 2020 05:47:38 -0400 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.105.38.7] Authentication-Results: smtp6.gate.iad3b.rsapps.net; iprev=pass policy.iprev="216.105.38.7"; 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; dmarc=none (p=nil; dis=none) header.from=rfc2549.org X-Suspicious-Flag: YES X-Classification-ID: e25c4514-c4ed-11ea-9fef-5254000d607e-1-1 Received: from [216.105.38.7] ([216.105.38.7:47104] helo=lists.sourceforge.net) by smtp6.gate.iad3b.rsapps.net (envelope-from ) (ecelerity 4.2.38.62370 r(:)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id D2/E1-21592-9BD2C0F5; Mon, 13 Jul 2020 05:47:38 -0400 Received: from [127.0.0.1] (helo=sfs-ml-4.v29.lw.sourceforge.com) by sfs-ml-4.v29.lw.sourceforge.com with esmtp (Exim 4.90_1) (envelope-from ) id 1juv35-0005Pm-IE; Mon, 13 Jul 2020 09:46:59 +0000 Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-4.v29.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.90_1) (envelope-from ) id 1juv33-0005Pf-F7 for openvpn-devel@lists.sourceforge.net; Mon, 13 Jul 2020 09:46:57 +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:To: From:Sender:Reply-To:Cc: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=tebGa7dnjdVV+MJHAWb9Ggz1JydROMv14dMiy0xBUZk=; b=b+U9O5UU9YSUwW+8rzU96gO4XD j081546XvgZVVT9d2y7ifZjtQ7kNfWlgaW8mXjwtGP4/bzjpIkGEcrO36/APnoHRhSOjfOhNRnx40 VzWiyMpb6Y5ZFit+q+eFKUkXmG9qIAKdMkrOQp7mhyMKY7ujoDtCeq0jrkaMlnM4pFIk=; 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:To:From:Sender:Reply-To:Cc :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=tebGa7dnjdVV+MJHAWb9Ggz1JydROMv14dMiy0xBUZk=; b=K8dDdURqpZ/L25b57XUgwcoc2A g3PkLg6blEM5/iPXTHonWYbN06yeOCT6PxGkEjySk1VqBmQ+sY+dFsvTQng11bzOETX5EpF69S4Ln MPrF2XEmvTaQasyFD3baR7VL1s5eXoZ69XzNp6R7YEtE74cWOCY5sUuTXpNjOOtLQAsM=; Received: from mail.blinkt.de ([192.26.174.232]) by sfi-mx-4.v28.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.92.2) id 1juv2z-001Hz3-Df for openvpn-devel@lists.sourceforge.net; Mon, 13 Jul 2020 09:46:57 +0000 Received: from kamera.blinkt.de ([2001:638:502:390:20c:29ff:fec8:535c]) by mail.blinkt.de with smtp (Exim 4.94 (FreeBSD)) (envelope-from ) id 1juv2t-000GLx-4d for openvpn-devel@lists.sourceforge.net; Mon, 13 Jul 2020 11:46:47 +0200 Received: (nullmailer pid 13980 invoked by uid 10006); Mon, 13 Jul 2020 09:46:47 -0000 From: Arne Schwabe To: openvpn-devel@lists.sourceforge.net Date: Mon, 13 Jul 2020 11:46:46 +0200 Message-Id: <20200713094646.13929-3-arne@rfc2549.org> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20200713094646.13929-1-arne@rfc2549.org> References: <20200713094646.13929-1-arne@rfc2549.org> X-Spam-Report: Spam Filtering performed by mx.sourceforge.net. See http://spamassassin.org/tag/ for more details. 0.0 HEADER_FROM_DIFFERENT_DOMAINS From and EnvelopeFrom 2nd level mail domains are different 0.0 SPF_NONE SPF: sender does not publish an SPF Record 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record 0.1 AWL AWL: Adjusted score from AWL reputation of From: address X-Headers-End: 1juv2z-001Hz3-Df Subject: [Openvpn-devel] [PATCH 3/3] Remove key-method 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 Signed-off-by: Arne Schwabe --- doc/doxygen/doc_control_processor.h | 6 +- doc/doxygen/doc_key_generation.h | 6 +- doc/doxygen/doc_protocol_overview.h | 2 +- src/openvpn/forward.c | 2 +- src/openvpn/helper.c | 5 - src/openvpn/init.c | 1 - src/openvpn/options.c | 37 +----- src/openvpn/options.h | 4 - src/openvpn/ssl.c | 180 +++------------------------- src/openvpn/ssl.h | 6 +- src/openvpn/ssl_common.h | 1 - 11 files changed, 23 insertions(+), 227 deletions(-) diff --git a/doc/doxygen/doc_control_processor.h b/doc/doxygen/doc_control_processor.h index f87324cc..1bbf2d2d 100644 --- a/doc/doxygen/doc_control_processor.h +++ b/doc/doxygen/doc_control_processor.h @@ -175,11 +175,7 @@ * appropriate messages to be sent. * * @par Functions which control data channel key generation - * - Key method 1 key exchange functions: - * - \c key_method_1_write(), generates and processes key material to - * be sent to the remote OpenVPN peer. - * - \c key_method_1_read(), processes key material received from the - * remote OpenVPN peer. + * - Key method 1 key exchange functions were removed from OpenVPN 2.5 * - Key method 2 key exchange functions: * - \c key_method_2_write(), generates and processes key material to * be sent to the remote OpenVPN peer. diff --git a/doc/doxygen/doc_key_generation.h b/doc/doxygen/doc_key_generation.h index efe61155..4bb9c708 100644 --- a/doc/doxygen/doc_key_generation.h +++ b/doc/doxygen/doc_key_generation.h @@ -131,11 +131,7 @@ S_ACTIVE S_ACTIVE * control_processor Control Channel Processor module's\endlink \c * tls_process() function and control the %key generation and exchange * process as follows: - * - %Key method 1: - * - \c key_method_1_write(): generate random material locally, and load - * as "sending" keys. - * - \c key_method_1_read(): read random material received from remote - * peer, and load as "receiving" keys. + * - %Key method 1 has been removed in OpenVPN 2.5 * - %Key method 2: * - \c key_method_2_write(): generate random material locally, and if * in server mode generate %key expansion. diff --git a/doc/doxygen/doc_protocol_overview.h b/doc/doxygen/doc_protocol_overview.h index 3f48b18a..08212223 100644 --- a/doc/doxygen/doc_protocol_overview.h +++ b/doc/doxygen/doc_protocol_overview.h @@ -150,7 +150,7 @@ * * @subsection network_protocol_control_plaintext Structure of plaintext control channel messages * - * - %Key method 1: + * - %Key method 1 (support removed in OpenVPN 2.5): * - Cipher %key length in bytes (1 byte). * - Cipher %key (n bytes). * - HMAC %key length in bytes (1 byte). diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c index 5c4370a8..34124b6f 100644 --- a/src/openvpn/forward.c +++ b/src/openvpn/forward.c @@ -1100,7 +1100,7 @@ process_incoming_link_part1(struct context *c, struct link_socket_info *lsi, boo floated, &ad_start)) { /* Restore pre-NCP frame parameters */ - if (is_hard_reset(opcode, c->options.key_method)) + if (is_hard_reset(opcode, KEY_METHOD_2)) { c->c2.frame = c->c2.frame_initial; #ifdef ENABLE_FRAGMENT diff --git a/src/openvpn/helper.c b/src/openvpn/helper.c index 6e9cc63c..a1d03070 100644 --- a/src/openvpn/helper.c +++ b/src/openvpn/helper.c @@ -490,11 +490,6 @@ helper_client_server(struct options *o) */ if (o->client) { - if (o->key_method != 2) - { - msg(M_USAGE, "--client requires --key-method 2"); - } - o->pull = true; o->tls_client = true; } diff --git a/src/openvpn/init.c b/src/openvpn/init.c index e9c01629..b96d1471 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -2852,7 +2852,6 @@ do_init_crypto_tls(struct context *c, const unsigned int flags) to.ssl_ctx = c->c1.ks.ssl_ctx; to.key_type = c->c1.ks.key_type; to.server = options->tls_server; - to.key_method = options->key_method; to.replay = options->replay; to.replay_window = options->replay_window; to.replay_time = options->replay_time; diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 345e0b29..e667ca21 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -886,7 +886,6 @@ init_options(struct options *o, const bool init_gc) #ifdef ENABLE_PREDICTION_RESISTANCE o->use_prediction_resistance = false; #endif - o->key_method = 2; o->tls_timeout = 2; o->renegotiate_bytes = -1; o->renegotiate_seconds = 3600; @@ -1709,7 +1708,6 @@ show_settings(const struct options *o) SHOW_BOOL(tls_server); SHOW_BOOL(tls_client); - SHOW_INT(key_method); SHOW_STR(ca_file); SHOW_STR(ca_path); SHOW_STR(dh_file); @@ -2370,10 +2368,6 @@ options_postprocess_verify_ce(const struct options *options, const struct connec { msg(M_USAGE, "--ccd-exclusive must be used with --client-config-dir"); } - if (options->key_method != 2) - { - msg(M_USAGE, "--mode server requires --key-method 2"); - } if (options->auth_token_generate && !options->renegotiate_seconds) { msg(M_USAGE, "--auth-gen-token needs a non-infinite " @@ -2540,13 +2534,6 @@ options_postprocess_verify_ce(const struct options *options, const struct connec "may accept clients which do not present a certificate"); } - if (options->key_method == 1) - { - msg(M_WARN, "WARNING: --key-method 1 is deprecated and will be removed " - "in OpenVPN 2.5. By default --key-method 2 will be used if not set " - "in the configuration file, which is the recommended approach."); - } - const int tls_version_max = (options->ssl_flags >> SSLF_TLS_VERSION_MAX_SHIFT) & SSLF_TLS_VERSION_MAX_MASK; @@ -2788,7 +2775,6 @@ options_postprocess_verify_ce(const struct options *options, const struct connec MUST_BE_UNDEF(push_peer_info); MUST_BE_UNDEF(tls_exit); MUST_BE_UNDEF(crl_file); - MUST_BE_UNDEF(key_method); MUST_BE_UNDEF(ns_cert_type); MUST_BE_UNDEF(remote_cert_ku[0]); MUST_BE_UNDEF(remote_cert_eku); @@ -3817,10 +3803,9 @@ options_string(const struct options *o, * tls-auth/tls-crypt does not match. Removing tls-auth here would * break stuff, so leaving that in place. */ - if (o->key_method > 1) - { - buf_printf(&out, ",key-method %d", o->key_method); - } + + + buf_printf(&out, ",key-method %d", 2); } if (remote) @@ -8459,22 +8444,6 @@ add_option(struct options *options, VERIFY_PERMISSION(OPT_P_GENERAL); options->tls_crypt_v2_verify_script = p[1]; } - else if (streq(p[0], "key-method") && p[1] && !p[2]) - { - int key_method; - - VERIFY_PERMISSION(OPT_P_GENERAL); - key_method = atoi(p[1]); - if (key_method < KEY_METHOD_MIN || key_method > KEY_METHOD_MAX) - { - msg(msglevel, "key_method parameter (%d) must be >= %d and <= %d", - key_method, - KEY_METHOD_MIN, - KEY_METHOD_MAX); - goto err; - } - options->key_method = key_method; - } else if (streq(p[0], "x509-track") && p[1] && !p[2]) { VERIFY_PERMISSION(OPT_P_GENERAL); diff --git a/src/openvpn/options.h b/src/openvpn/options.h index 1b038c91..3546bab3 100644 --- a/src/openvpn/options.h +++ b/src/openvpn/options.h @@ -572,10 +572,6 @@ struct options #ifdef ENABLE_CRYPTOAPI const char *cryptoapi_cert; #endif - - /* data channel key exchange method */ - int key_method; - /* Per-packet timeout on control channel */ int tls_timeout; diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 00b97352..35c70d71 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -1097,23 +1097,14 @@ tls_session_init(struct tls_multi *multi, struct tls_session *session) } /* Are we a TLS server or client? */ - ASSERT(session->opt->key_method >= 1); - if (session->opt->key_method == 1) + if (session->opt->server) { - session->initial_opcode = session->opt->server ? - P_CONTROL_HARD_RESET_SERVER_V1 : P_CONTROL_HARD_RESET_CLIENT_V1; + session->initial_opcode = P_CONTROL_HARD_RESET_SERVER_V2; } - else /* session->opt->key_method >= 2 */ + else { - if (session->opt->server) - { - session->initial_opcode = P_CONTROL_HARD_RESET_SERVER_V2; - } - else - { - session->initial_opcode = session->opt->tls_crypt_v2 ? - P_CONTROL_HARD_RESET_CLIENT_V3 : P_CONTROL_HARD_RESET_CLIENT_V2; - } + session->initial_opcode = session->opt->tls_crypt_v2 ? + P_CONTROL_HARD_RESET_CLIENT_V3 : P_CONTROL_HARD_RESET_CLIENT_V2; } /* Initialize control channel authentication parameters */ @@ -2225,52 +2216,6 @@ read_string_alloc(struct buffer *buf) return str; } -/* - * Handle the reading and writing of key data to and from - * the TLS control channel (cleartext). - */ - -static bool -key_method_1_write(struct buffer *buf, struct tls_session *session) -{ - struct key key; - struct key_state *ks = &session->key[KS_PRIMARY]; /* primary key */ - - ASSERT(session->opt->key_method == 1); - ASSERT(buf_init(buf, 0)); - - generate_key_random(&key, &session->opt->key_type); - if (!check_key(&key, &session->opt->key_type)) - { - msg(D_TLS_ERRORS, "TLS Error: Bad encrypting key generated"); - return false; - } - - if (!write_key(&key, &session->opt->key_type, buf)) - { - msg(D_TLS_ERRORS, "TLS Error: write_key failed"); - return false; - } - - init_key_ctx(&ks->crypto_options.key_ctx_bi.encrypt, &key, - &session->opt->key_type, OPENVPN_OP_ENCRYPT, - "Data Channel Encrypt"); - secure_memzero(&key, sizeof(key)); - - /* send local options string */ - { - const char *local_options = local_options_string(session); - const int optlen = strlen(local_options) + 1; - if (!buf_write(buf, local_options, optlen)) - { - msg(D_TLS_ERRORS, "TLS Error: KM1 write options failed"); - return false; - } - } - - return true; -} - static bool push_peer_info(struct buffer *buf, struct tls_session *session) { @@ -2394,7 +2339,6 @@ key_method_2_write(struct buffer *buf, struct tls_session *session) { struct key_state *ks = &session->key[KS_PRIMARY]; /* primary key */ - ASSERT(session->opt->key_method == 2); ASSERT(buf_init(buf, 0)); /* write a uint32 0 */ @@ -2404,7 +2348,7 @@ key_method_2_write(struct buffer *buf, struct tls_session *session) } /* write key_method + flags */ - if (!buf_write_u8(buf, (session->opt->key_method & KEY_METHOD_MASK))) + if (!buf_write_u8(buf, (KEY_METHOD_2 & KEY_METHOD_MASK))) { goto error; } @@ -2506,73 +2450,11 @@ error: return false; } -static bool -key_method_1_read(struct buffer *buf, struct tls_session *session) -{ - int status; - struct key key; - struct key_state *ks = &session->key[KS_PRIMARY]; /* primary key */ - - ASSERT(session->opt->key_method == 1); - - if (!session->verified) - { - msg(D_TLS_ERRORS, - "TLS Error: Certificate verification failed (key-method 1)"); - goto error; - } - - status = read_key(&key, &session->opt->key_type, buf); - if (status != 1) - { - msg(D_TLS_ERRORS, - "TLS Error: Error reading data channel key from plaintext buffer"); - goto error; - } - - if (!check_key(&key, &session->opt->key_type)) - { - msg(D_TLS_ERRORS, "TLS Error: Bad decrypting key received from peer"); - goto error; - } - - if (buf->len < 1) - { - msg(D_TLS_ERRORS, "TLS Error: Missing options string"); - goto error; - } - -#ifdef ENABLE_OCC - /* compare received remote options string - * with our locally computed options string */ - if (!session->opt->disable_occ - && !options_cmp_equal_safe((char *) BPTR(buf), session->opt->remote_options, buf->len)) - { - options_warning_safe((char *) BPTR(buf), session->opt->remote_options, buf->len); - } -#endif - - buf_clear(buf); - - init_key_ctx(&ks->crypto_options.key_ctx_bi.decrypt, &key, - &session->opt->key_type, OPENVPN_OP_DECRYPT, - "Data Channel Decrypt"); - secure_memzero(&key, sizeof(key)); - ks->authenticated = KS_AUTH_TRUE; - return true; - -error: - buf_clear(buf); - secure_memzero(&key, sizeof(key)); - return false; -} - static bool key_method_2_read(struct buffer *buf, struct tls_multi *multi, struct tls_session *session) { struct key_state *ks = &session->key[KS_PRIMARY]; /* primary key */ - int key_method_flags; bool username_status, password_status; struct gc_arena gc = gc_new(); @@ -2582,8 +2464,6 @@ key_method_2_read(struct buffer *buf, struct tls_multi *multi, struct tls_sessio /* allocate temporary objects */ ALLOC_ARRAY_CLEAR_GC(options, char, TLS_OPTIONS_LEN, &gc); - ASSERT(session->opt->key_method == 2); - /* discard leading uint32 */ if (!buf_advance(buf, 4)) { @@ -2593,7 +2473,7 @@ key_method_2_read(struct buffer *buf, struct tls_multi *multi, struct tls_sessio } /* get key method */ - key_method_flags = buf_read_u8(buf); + int key_method_flags = buf_read_u8(buf); if ((key_method_flags & KEY_METHOD_MASK) != 2) { msg(D_TLS_ERRORS, @@ -3003,23 +2883,9 @@ tls_process(struct tls_multi *multi, if (!buf->len && ((ks->state == S_START && !session->opt->server) || (ks->state == S_GOT_KEY && session->opt->server))) { - if (session->opt->key_method == 1) + if (!key_method_2_write(buf, session)) { - if (!key_method_1_write(buf, session)) - { - goto error; - } - } - else if (session->opt->key_method == 2) - { - if (!key_method_2_write(buf, session)) - { - goto error; - } - } - else - { - ASSERT(0); + goto error; } state_change = true; @@ -3033,23 +2899,9 @@ tls_process(struct tls_multi *multi, && ((ks->state == S_SENT_KEY && !session->opt->server) || (ks->state == S_START && session->opt->server))) { - if (session->opt->key_method == 1) - { - if (!key_method_1_read(buf, session)) - { - goto error; - } - } - else if (session->opt->key_method == 2) - { - if (!key_method_2_read(buf, multi, session)) - { - goto error; - } - } - else + if (!key_method_2_read(buf, multi, session)) { - ASSERT(0); + goto error; } state_change = true; @@ -3547,11 +3399,10 @@ tls_pre_decrypt(struct tls_multi *multi, struct tls_session *session = &multi->session[TM_ACTIVE]; struct key_state *ks = &session->key[KS_PRIMARY]; - if (!is_hard_reset(op, multi->opt.key_method)) + if (!is_hard_reset(op, KEY_METHOD_2)) { msg(D_TLS_ERRORS, "TLS ERROR: initial packet local/remote key_method mismatch, local key_method=%d, op=%s", - multi->opt.key_method, - packet_opcode_name(op)); + KEY_METHOD_2, packet_opcode_name(op)); goto error; } @@ -3614,11 +3465,10 @@ tls_pre_decrypt(struct tls_multi *multi, goto error; } - if (!is_hard_reset(op, multi->opt.key_method)) + if (!is_hard_reset(op, KEY_METHOD_2)) { msg(D_TLS_ERRORS, "TLS ERROR: new session local/remote key_method mismatch, local key_method=%d, op=%s", - multi->opt.key_method, - packet_opcode_name(op)); + KEY_METHOD_2, packet_opcode_name(op)); goto error; } diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h index 86fb6853..3fcc3654 100644 --- a/src/openvpn/ssl.h +++ b/src/openvpn/ssl.h @@ -109,11 +109,7 @@ /* Default field in X509 to be username */ #define X509_USERNAME_FIELD_DEFAULT "CN" -/* - * Range of key exchange methods - */ -#define KEY_METHOD_MIN 1 -#define KEY_METHOD_MAX 2 +#define KEY_METHOD_2 2 /* key method taken from lower 4 bits */ #define KEY_METHOD_MASK 0x0F diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h index e0b3ee56..d904c31f 100644 --- a/src/openvpn/ssl_common.h +++ b/src/openvpn/ssl_common.h @@ -262,7 +262,6 @@ struct tls_options #endif /* from command line */ - int key_method; bool replay; bool single_session; #ifdef ENABLE_OCC