From patchwork Thu Mar 8 08:47:48 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Steffan Karger X-Patchwork-Id: 266 X-Patchwork-Delegate: gert@greenie.muc.de Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director12.mail.ord1d.rsapps.net ([172.30.191.6]) by backend30.mail.ord1d.rsapps.net (Dovecot) with LMTP id oP/nFraToVqpaAAAIUCqbw for ; Thu, 08 Mar 2018 14:49:10 -0500 Received: from proxy9.mail.ord1d.rsapps.net ([172.30.191.6]) by director12.mail.ord1d.rsapps.net (Dovecot) with LMTP id pwy5GraToVoUZwAAIasKDg ; Thu, 08 Mar 2018 14:49:10 -0500 Received: from smtp42.gate.ord1c ([172.30.191.6]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy9.mail.ord1d.rsapps.net with LMTP id KB6uM7aToVqQRgAA7h+8OQ ; Thu, 08 Mar 2018 14:49:10 -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.105.38.7] Authentication-Results: smtp42.gate.ord1c.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; 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: c4926596-2309-11e8-b35d-b8ca3a655ab8-1-1 Received: from [216.105.38.7] ([216.105.38.7:39349] helo=lists.sourceforge.net) by smtp42.gate.ord1c.rsapps.net (envelope-from ) (ecelerity 4.2.1.56364 r(Core:4.2.1.14)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id 90/02-28681-6B391AA5; Thu, 08 Mar 2018 14:49:10 -0500 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 1eu1WZ-0000fO-6b; Thu, 08 Mar 2018 19:48:23 +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 1eu1WX-0000en-DK for openvpn-devel@lists.sourceforge.net; Thu, 08 Mar 2018 19:48:22 +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=gDSArQ10tpWZ5ZppwQiKuGlTlJc2eQJ/GytYexjvwOs=; b=Zp36EugblLD5xrJtGaGrrb0fTx E83dAdqrutWVMBpo225iEUqKnEogADsexMj1bC5A7n9IMpCxKNMm0mXs6zHQc621SWXk//eGbuu7J U9mJm2v17JLfGyVaR1fL2a/z/W9MAX60xXVufe8DA/tg7KsSCKn8uc3031o0ftVzP8BY=; 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=gDSArQ10tpWZ5ZppwQiKuGlTlJc2eQJ/GytYexjvwOs=; b=WabEa0Ie2H/dtnF5SiTfTkCkGU k8Rh1zT2X+2NSzl4gR685zTDqHwLqLbZiY+fOy1I8LwW84i8M0gGbUi4tHEUMY4aTrUiwNdsvfxRN kOT1McDXV6P+rcVpCbOVcQ8EIbo+JsaeSkzgbStwn4zNCw0DboDTHTLmufZn/pCketRY=; Received: from sfi-lb-mx.v20.lw.sourceforge.com ([172.30.20.201] helo=mail-wm0-f41.google.com) by sfi-mx-4.v28.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES128-GCM-SHA256:128) (Exim 4.90_1) id 1eu1WA-005SUZ-Fy for openvpn-devel@lists.sourceforge.net; Thu, 08 Mar 2018 19:48:11 +0000 Received: by mail-wm0-f41.google.com with SMTP id z81so1458005wmb.4 for ; Thu, 08 Mar 2018 11:48:01 -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=gDSArQ10tpWZ5ZppwQiKuGlTlJc2eQJ/GytYexjvwOs=; b=FR6Br3+73EwLaXPaft6IqEJqYVltbWiMJYO2rn4OLYUofcMxg0JJZcQ3Ap/L7hWuY+ ++HA7IuxxMgmRw4F4e3cNspke1SxUdNVqfkmsbU2oyikZUfHQAbp78LKE+i9q1DXfdrW Q8vE1i76oCLozRMCeLx0v3ijGR0V6YPYlcp3Jkvkpk2CyS4/iITlpgSbEzYp40TfJlLa ba5piIjjRxmQ+7qRKgOpUA/rW5a94WjQG30LFqvSnpRwYp8vWn3sQM6JNaKu03Ylct8n eBgjfuKGh75UHRp7JSLxuc0WbZrVyHZrXAuflzaqCIx2uP53do6T5bQboxC6JR4U7hGN +8rg== 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=gDSArQ10tpWZ5ZppwQiKuGlTlJc2eQJ/GytYexjvwOs=; b=DZ4e9eH/uk79gflsjS/dGtA+ZsbxAi4g7X9hY2WktM6oWJclDr5oZAwnFUrRwlvLb0 /rPlvhmNRAj32Fz8WltY4ulk4u6znUY0Te6dxleSdpqlL3ORYMl0Lr/ShrQ7+iMryBKr HTXi3XJMOpUehqQie5bY/9JqRyYb5yUFZD0F700fAOjZDIYUEbrERxm4h9Yvl5KsjGfU CfFBeSAsFmIMN0JdUQN4eM2V63bWSzf6wEXc9CvvqSfcRDOvtbedqVHeBLiziQd4tv3g oYGritR2KhVwBamIiRfYzFl0BLo5vOMF6qUa0T7Km234ZbxY9ghI3KPgIqNuz9Wusudt fOCA== X-Gm-Message-State: APf1xPCyV6C/RKJvJbfRTPQFcXWRiUkPQ/ZkEyv+FPaZ7yS/H9v51wWH kYx/v3bRBXwPHoTJx9y2EL0dityeH40= X-Google-Smtp-Source: AG47ELvX/Sdm6oNcY+DrS6h+ryc1a6Zdpsjt2tvpAblmTnACJTNfGLlCKEYlIntwJh/TT+iqp/G4xQ== X-Received: by 10.80.224.9 with SMTP id e9mr33707700edl.218.1520538473960; Thu, 08 Mar 2018 11:47:53 -0800 (PST) Received: from vesta.fritz.box ([2001:985:e54:1:99cf:be5f:7efd:47d1]) by smtp.gmail.com with ESMTPSA id s8sm6531884edk.28.2018.03.08.11.47.52 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 08 Mar 2018 11:47:52 -0800 (PST) From: Steffan Karger To: openvpn-devel@lists.sourceforge.net Date: Thu, 8 Mar 2018 20:47:48 +0100 Message-Id: <20180308194748.4627-1-steffan@karger.me> X-Mailer: git-send-email 2.14.1 In-Reply-To: <1487344823-8125-1-git-send-email-steffan@karger.me> References: <1487344823-8125-1-git-send-email-steffan@karger.me> X-Spam-Report: Spam Filtering performed by mx.sourceforge.net. See http://spamassassin.org/tag/ for more details. 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 0.5 AWL AWL: Adjusted score from AWL reputation of From: address X-Headers-End: 1eu1WA-005SUZ-Fy Subject: [Openvpn-devel] [PATCH] Allow changing cipher from a ccd file 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-Suspicious-Flag: YES X-getmail-retrieved-from-mailbox: Inbox As described in msg <374a7eb7-f539-5231-623b-41f208ed856e@belkam.com> on openvpn-devel@lists.sourceforge.net, clients that are compiled with --disable-occ (included in --enable-small) won't send an options string. Without the options string, the 2.4 server doesn't know which cipher to use for poor man's NCP. This patch allows working around that issue by allowing the 'cipher' directive to be used in --client-config-dir files. That way, a server admin can add ccd files to specify per-client which cipher to use. Because the ccd files are read after where we would normally generate keys, this patch delays key generation for non-NCP p2mp servers until after reading the ccd file. Thanks to a sharp review comment from Gert, it turns out that multi_connections_establised() is actually a better place than incoming_push_message() to do the server-side NCP cipher selection and key generation. Doing it here removes the need for handling with multiple received push requests. Technically, the check for .authenticated before generating keys should even not be necessary, but I think it's good to leave it in as a double-check to prevent future mistakes. Signed-off-by: Steffan Karger --- v2: postpone p2mp non-NCP key generation, such that setting cipher in a ccd file for a non-NCP client actually works. v3: merge all server-side NCP key generation to the new place. src/openvpn/multi.c | 45 +++++++++++++++++++++++++++++++++------------ src/openvpn/options.c | 2 +- src/openvpn/options.h | 2 +- src/openvpn/push.c | 22 ---------------------- src/openvpn/ssl.c | 9 ++++----- 5 files changed, 39 insertions(+), 41 deletions(-) diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index 6a72a1dc..77dd6724 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -1786,6 +1786,7 @@ multi_connection_established(struct multi_context *m, struct multi_instance *mi) { struct gc_arena gc = gc_new(); unsigned int option_types_found = 0; + struct options *options = &mi->context.options; const unsigned int option_permissions_mask = OPT_P_INSTANCE @@ -1810,7 +1811,7 @@ multi_connection_established(struct multi_context *m, struct multi_instance *mi) generate_prefix(mi); /* delete instances of previous clients with same common-name */ - if (!mi->context.options.duplicate_cn) + if (!options->duplicate_cn) { multi_delete_dup(m, mi); } @@ -1822,11 +1823,11 @@ multi_connection_established(struct multi_context *m, struct multi_instance *mi) * Try to source a dynamic config file from the * --client-config-dir directory. */ - if (mi->context.options.client_config_dir) + if (options->client_config_dir) { const char *ccd_file; - ccd_file = gen_path(mi->context.options.client_config_dir, + ccd_file = gen_path(options->client_config_dir, tls_common_name(mi->context.c2.tls_multi, false), &gc); @@ -1842,7 +1843,7 @@ multi_connection_established(struct multi_context *m, struct multi_instance *mi) } else /* try default file */ { - ccd_file = gen_path(mi->context.options.client_config_dir, + ccd_file = gen_path(options->client_config_dir, CCD_DEFAULT, &gc); @@ -1876,7 +1877,7 @@ multi_connection_established(struct multi_context *m, struct multi_instance *mi) if (plugin_defined(mi->context.plugins, OPENVPN_PLUGIN_CLIENT_CONNECT)) { struct argv argv = argv_new(); - const char *dc_file = create_temp_file(mi->context.options.tmp_dir, "cc", &gc); + const char *dc_file = create_temp_file(options->tmp_dir, "cc", &gc); if (!dc_file) { @@ -1931,21 +1932,21 @@ script_depr_failed: /* * Run --client-connect script. */ - if (mi->context.options.client_connect_script && cc_succeeded) + if (options->client_connect_script && cc_succeeded) { struct argv argv = argv_new(); const char *dc_file = NULL; setenv_str(mi->context.c2.es, "script_type", "client-connect"); - dc_file = create_temp_file(mi->context.options.tmp_dir, "cc", &gc); + dc_file = create_temp_file(options->tmp_dir, "cc", &gc); if (!dc_file) { cc_succeeded = false; goto script_failed; } - argv_parse_cmd(&argv, mi->context.options.client_connect_script); + argv_parse_cmd(&argv, options->client_connect_script); argv_printf_cat(&argv, "%s", dc_file); if (openvpn_run_script(&argv, mi->context.c2.es, 0, "--client-connect")) @@ -1983,13 +1984,33 @@ script_failed: * Check for "disable" directive in client-config-dir file * or config file generated by --client-connect script. */ - if (mi->context.options.disable) + if (options->disable) { msg(D_MULTI_ERRORS, "MULTI: client has been rejected due to 'disable' directive"); cc_succeeded = false; cc_succeeded_count = 0; } + /* Determine data channel cipher and generate data channel keys */ + struct tls_multi *multi = mi->context.c2.tls_multi; + if (tls_peer_info_ncp_ver(multi->peer_info) >= 2 && options->ncp_enabled) + { + /* Select the first cipher from --ncp-ciphers for the data channel. + * TODO: actual negotiation, instead of server dictatorship. */ + char *push_cipher = string_alloc(options->ncp_ciphers, &options->gc); + options->ciphername = strtok(push_cipher, ":"); + } + + struct tls_session *session = &multi->session[TM_ACTIVE]; + if (!session->key[KS_PRIMARY].authenticated + || !tls_session_update_crypto_params(session, &mi->context.options, + &mi->context.c2.frame)) + { + msg(D_TLS_ERRORS, "TLS Error: updating crypto parameters failed"); + cc_succeeded = false; + cc_succeeded_count = 0; + } + if (cc_succeeded) { /* @@ -2015,8 +2036,8 @@ script_failed: msg(D_MULTI_ERRORS, "MULTI ERROR: primary virtual IP for %s (%s) violates tunnel network/netmask constraint (%s/%s)", multi_instance_string(mi, false, &gc), print_in_addr_t(mi->context.c2.push_ifconfig_local, 0, &gc), - print_in_addr_t(mi->context.options.push_ifconfig_constraint_network, 0, &gc), - print_in_addr_t(mi->context.options.push_ifconfig_constraint_netmask, 0, &gc)); + print_in_addr_t(options->push_ifconfig_constraint_network, 0, &gc), + print_in_addr_t(options->push_ifconfig_constraint_netmask, 0, &gc)); } /* @@ -2053,7 +2074,7 @@ script_failed: */ remove_iroutes_from_push_route_list(&mi->context.options); } - else if (mi->context.options.iroutes) + else if (options->iroutes) { msg(D_MULTI_ERRORS, "MULTI: --iroute options rejected for %s -- iroute only works with tun-style tunnels", multi_instance_string(mi, false, &gc)); diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 587f73ad..5443b204 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -7505,7 +7505,7 @@ add_option(struct options *options, } else if (streq(p[0], "cipher") && p[1] && !p[2]) { - VERIFY_PERMISSION(OPT_P_NCP); + VERIFY_PERMISSION(OPT_P_NCP|OPT_P_INSTANCE); options->ciphername = p[1]; } else if (streq(p[0], "ncp-ciphers") && p[1] && !p[2]) diff --git a/src/openvpn/options.h b/src/openvpn/options.h index 636bb8db..565b3697 100644 --- a/src/openvpn/options.h +++ b/src/openvpn/options.h @@ -624,7 +624,7 @@ struct options #define OPT_P_MTU (1<<14) /* TODO */ #define OPT_P_NICE (1<<15) #define OPT_P_PUSH (1<<16) -#define OPT_P_INSTANCE (1<<17) +#define OPT_P_INSTANCE (1<<17) /**< Allow usage in ccd file */ #define OPT_P_CONFIG (1<<18) #define OPT_P_EXPLICIT_NOTIFY (1<<19) #define OPT_P_ECHO (1<<20) diff --git a/src/openvpn/push.c b/src/openvpn/push.c index fee3eaab..d58d7119 100644 --- a/src/openvpn/push.c +++ b/src/openvpn/push.c @@ -270,21 +270,6 @@ incoming_push_message(struct context *c, const struct buffer *buffer) } event_timeout_clear(&c->c2.push_request_interval); } - else if (status == PUSH_MSG_REQUEST) - { - if (c->options.mode == MODE_SERVER) - { - struct tls_session *session = &c->c2.tls_multi->session[TM_ACTIVE]; - /* Do not regenerate keys if client send a second push request */ - if (!session->key[KS_PRIMARY].crypto_options.key_ctx_bi.initialized - && !tls_session_update_crypto_params(session, &c->options, - &c->c2.frame)) - { - msg(D_TLS_ERRORS, "TLS Error: initializing data channel failed"); - goto error; - } - } - } goto cleanup; error: @@ -384,13 +369,6 @@ prepare_push_reply(struct context *c, struct gc_arena *gc, "re-sending previously negotiated cipher '%s'", o->ciphername ); } - else - { - /* Push the first cipher from --ncp-ciphers to the client. - * TODO: actual negotiation, instead of server dictatorship. */ - char *push_cipher = string_alloc(o->ncp_ciphers, &o->gc); - o->ciphername = strtok(push_cipher, ":"); - } push_option_fmt(gc, push_list, M_USAGE, "cipher %s", o->ciphername); } else if (o->ncp_enabled) diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 21a29195..48de2e32 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -2426,12 +2426,11 @@ key_method_2_write(struct buffer *buf, struct tls_session *session) } /* Generate tunnel keys if we're a TLS server. - * If we're a p2mp server and IV_NCP >= 2 is negotiated, the first key - * generation is postponed until after the pull/push, so we can process pushed - * cipher directives. + * If we're a p2mp server, the first key generation is postponed so we can + * switch cipher during the connection setup phase. */ - if (session->opt->server && !(session->opt->ncp_enabled - && session->opt->mode == MODE_SERVER && ks->key_id <= 0)) + if (session->opt->server + && !(session->opt->mode == MODE_SERVER && ks->key_id <= 0)) { if (ks->authenticated) {