From patchwork Wed Nov 20 17:42:36 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "plaisthos (Code Review)" X-Patchwork-Id: 3948 Return-Path: Delivered-To: patchwork@openvpn.net Received: by 2002:a05:7000:1e84:b0:5d9:9f4c:3bc7 with SMTP id hq4csp156043mab; Wed, 20 Nov 2024 09:43:02 -0800 (PST) X-Forwarded-Encrypted: i=2; AJvYcCWpv2Ni/RWRUpZb5VkP371EuB+dA2vFB4bYYi4HHlMXf6dI1aq9LXRGaPhe4TgZOKfN8rQ+M8F73ec=@openvpn.net X-Google-Smtp-Source: AGHT+IGyl5USU4qfklk7kpCq5rLDgYf0/mj4TtUw4Ur9GyN6EfvxaE/TBfh24iwnLSFGC0Ta2dNB X-Received: by 2002:a05:6870:9619:b0:261:22a4:9235 with SMTP id 586e51a60fabf-296d9e4a4cfmr3421282fac.32.1732124582073; Wed, 20 Nov 2024 09:43:02 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1732124582; cv=none; d=google.com; s=arc-20240605; b=MufkmVpAqzVpymbEQWYBP3/a7dNZzD3Q2gDrqwUvFHzX7OuiBuYzO3kL8v7ClxduYm zMq5/eYQji8q0gnC4sqCla9b/iZ0hI52hfB7aYFiTWSz1Z/ny8fe4S9SVJXaHkvA9nPg iQ8uGt592W8a8F+OWZrfBfP/fEKTWyzHv6HYDILAGipBKEmiEitzGS2OL/0v53wtMTZn vKH7SosCBKQmoubD1SqQbP+LfKUvhlF+dRhzHOD9CPSqYrgyCY0TRlpW+Kvmtr4Lj+U0 vRh9hrvXk1hG2lC/snvr6pz6XLV2WAazefpVvMz90MEVZ/YLN2GEho8REhO4fmB4vguh ko3g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20240605; h=errors-to:cc:reply-to:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence:subject:user-agent :mime-version:message-id:references:auto-submitted:to:date:from :dkim-signature:dkim-signature:dkim-signature; bh=SjntbUwcReBbG+6rGFJQJB2VU0kQRt+6YVzwj1AED5g=; fh=U7wEyxtwz2o5+UdevFSA47vNeG9knhWH0KV//QhD5a0=; b=KEQuTzRK+Mjx3YAnFki82qmXJBB+EdtjOjeghnsIw+o6bIWiobACIbHxcRU0fjml7d UXnrpFkjXCfzh9rZb7/8RT1ohpw+4xA42uWWXCFAPePdCVtnNBz+dWYnUB3o/5PTC8Jn pOE9jJdH8ZrAMU35UV4z0gvQrgO/2I0ppr6JIcGK+4W4rid4K5dEbn6YQ7Fvs5TegXPG gXDH9FAKyu7DM37qxd2CzhkbDhV0/TS/2atVBq1ynw3/NBjFKxThMvoie9KcoA1HNOrv gaEYAheKKCEwRj49M/DYBFHGM1bpGUIaWDCUizzENs82kX/UtNPt6aH7TidrAKjYoTwz EoAA==; dara=google.com ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b="GqEAS/d+"; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=YXfTnisS; dkim=neutral (body hash did not verify) header.i=@openvpn.net header.s=google header.b=VWGUx576; spf=pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) smtp.mailfrom=openvpn-devel-bounces@lists.sourceforge.net; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=openvpn.net; dara=fail header.i=@openvpn.net Received: from lists.sourceforge.net (lists.sourceforge.net. [216.105.38.7]) by mx.google.com with ESMTPS id 46e09a7af769-71aba9e88d0si289033a34.75.2024.11.20.09.43.01 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Wed, 20 Nov 2024 09:43:02 -0800 (PST) Received-SPF: pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) client-ip=216.105.38.7; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b="GqEAS/d+"; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=YXfTnisS; dkim=neutral (body hash did not verify) header.i=@openvpn.net header.s=google header.b=VWGUx576; spf=pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) smtp.mailfrom=openvpn-devel-bounces@lists.sourceforge.net; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=openvpn.net; dara=fail header.i=@openvpn.net Received: from [127.0.0.1] (helo=sfs-ml-2.v29.lw.sourceforge.com) by sfs-ml-2.v29.lw.sourceforge.com with esmtp (Exim 4.95) (envelope-from ) id 1tDoj4-0004AC-57; Wed, 20 Nov 2024 17:42:50 +0000 Received: from [172.30.29.66] (helo=mx.sourceforge.net) by sfs-ml-2.v29.lw.sourceforge.com with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1tDoj2-0004A5-Pt for openvpn-devel@lists.sourceforge.net; Wed, 20 Nov 2024 17:42:48 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=Content-Type:Content-Transfer-Encoding:MIME-Version :Message-ID:Reply-To:References:Subject:List-Unsubscribe:List-Id:Cc:To:Date: From:Sender:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:List-Help: List-Subscribe:List-Post:List-Owner:List-Archive; bh=v4PUp5KUTWtRoKkr4WdPDZT/G9Anw9vjrAoojDd1E8E=; b=GqEAS/d+6SaA8BwOSoPtnEFpuI 6vucM0h5cvEwzNQ5U4DSEyN3X+D1RhwXiqHjctDaEsr/vRuwZ7ex+vgQf256PgpcoQw3A+qwaMaQ6 i+OcJn6a3q/boQHqUbZn7t/KlhU5CRsfN4MCWncFdczQCw3H5t1qAam2iB3a8bj3xoYc=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Content-Type:Content-Transfer-Encoding:MIME-Version:Message-ID:Reply-To: References:Subject:List-Unsubscribe:List-Id:Cc:To:Date:From:Sender:Content-ID :Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To: Resent-Cc:Resent-Message-ID:In-Reply-To:List-Help:List-Subscribe:List-Post: List-Owner:List-Archive; bh=v4PUp5KUTWtRoKkr4WdPDZT/G9Anw9vjrAoojDd1E8E=; b=Y XfTnisSPlze/nCNkudB4CmF4t1QIdNxEIHUuQ7q3OzVNZ+0fuGXHcGJ974jHehcmzK1yvvCVk3E2X 6nT5iJQwglp/OW/pYpsJUHmP4chDdRF5z+b3evzy0MGE2aLNxJE4blTaBGttrpX/FBMkZcJj/W5H+ 1M0JLpZeP4caR4m8=; Received: from mail-wm1-f42.google.com ([209.85.128.42]) by sfi-mx-2.v28.lw.sourceforge.com with esmtps (TLS1.2:ECDHE-RSA-AES128-GCM-SHA256:128) (Exim 4.95) id 1tDoiz-0005d2-13 for openvpn-devel@lists.sourceforge.net; Wed, 20 Nov 2024 17:42:48 +0000 Received: by mail-wm1-f42.google.com with SMTP id 5b1f17b1804b1-432d866f70fso21702945e9.2 for ; Wed, 20 Nov 2024 09:42:44 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=openvpn.net; s=google; t=1732124558; x=1732729358; darn=lists.sourceforge.net; h=user-agent:content-disposition:content-transfer-encoding :mime-version:message-id:reply-to:references:subject :list-unsubscribe:list-id:auto-submitted:cc:to:date:from:from:to:cc :subject:date:message-id:reply-to; bh=v4PUp5KUTWtRoKkr4WdPDZT/G9Anw9vjrAoojDd1E8E=; b=VWGUx576ZPkVFfaLrMVYz+tLFXO+f7A08XC+6+Nfa6LEUZNezMhLgzvWJHm9LCZJu+ c3icDWuNP72Xs6fYu4nmLnZ2DV5xUtJLH1TJOXgJLc9eW2YB7ZwqGfruUayXX4Kgomcy N1cODiDrZE4JulIWL/8cpfg2yuRUoXm4WI3JpgcU1sRknwWl+xglE4Hby/eY9EwM3wwf 42FwFwCHIEEhPRUc8Jddc4AhE6WW+/y1YEs3IICQgbr7uJeH6wJYDC8AUDhpT+i1Q2Zg qwRW+ddNQamym2dx6dp4jUAjsECdMjG5tdyFSgFnpfY0okgDL7roFiC8DqY1LDR3A1i/ Ctzg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1732124558; x=1732729358; h=user-agent:content-disposition:content-transfer-encoding :mime-version:message-id:reply-to:references:subject :list-unsubscribe:list-id:auto-submitted:cc:to:date:from :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=v4PUp5KUTWtRoKkr4WdPDZT/G9Anw9vjrAoojDd1E8E=; b=on1X/hVUVu1cskouMVZkeBXdzjDQnJlzH/X+khwXIPDWmrkntKuljfc10HqXnRO5qa srmk7wyAsMmoG7pXf8tdBe0hP1bITJ2B1Kx8liMREVwM7Rxpb3jmF35k3ELIZ9xHrzA2 SAeXbHjv02bKVn19nS73G6VBQyTHe1V5aQsCCoJz0dPNi6xBtHvL/RvRwDQxVW5hX+fg sT5z6vNoatWA/yOexLx9700E6+75Va/m12JBPmaAzaRS2c7TY0u3G8PWY/vL9zDPtohq vP/7F++hvs7V3PT/7V2+wC4i0JA6RaMAQojmcHaWz53smYKWOhbM/c7+pUUnmQNLnTDW Rm4w== X-Gm-Message-State: AOJu0YzAEo5fNqTFRMxbMptHboFBUl24qYSAMkl/CV/ZViiJYM0CgbZb ci4YEh3evowiSfAki5wI98h2wAn7RcaoEkPoxuBptisJ8M9g1UJKCUaEnJ3zGaM= X-Gm-Gg: ASbGncsKGb6dBHvdDMZwbAuJvXXoK0ZKFP264za+8P9O5F0VJoQS2f4CA96T0fzjx0Y aS9SiYa29ryk8sFWrIa/zkk6q9PgL7vG6pYF13qCauJpzC37KHa9jxRiRZApEvDIVH2I9S5/WmF LWpeiopS1Ga+f4M/bSESRpVLHExBrCdXURF6TV5tqqDotYa80sXIKCSCcuD1bsbz9HXBSWB6nK6 J99Xd9+hVjyAwj70gmdvnuhNxK1nc9fynFbBBBe+3KgAH8HEQGF3BxAMsw3hxeZlwePHF15mj8q ooQhGLpPU+sQzTYczBIappy1mlRtAkjxBDcHJd8a4w== X-Received: by 2002:a05:600c:1f18:b0:431:5df7:b310 with SMTP id 5b1f17b1804b1-433489b292amr32660695e9.8.1732124558007; Wed, 20 Nov 2024 09:42:38 -0800 (PST) Received: from gerrit.openvpn.in (ec2-18-159-0-78.eu-central-1.compute.amazonaws.com. [18.159.0.78]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-433b0158637sm27397465e9.0.2024.11.20.09.42.37 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 20 Nov 2024 09:42:37 -0800 (PST) From: "mrbff (Code Review)" X-Google-Original-From: "mrbff (Code Review)" X-Gerrit-PatchSet: 1 Date: Wed, 20 Nov 2024 17:42:36 +0000 To: plaisthos , flichtenheld Auto-Submitted: auto-generated X-Gerrit-MessageType: newchange X-Gerrit-Change-Id: I6ecd4cb47571cc8c20e46de8595c742aeec6064a X-Gerrit-Change-Number: 808 X-Gerrit-Project: openvpn X-Gerrit-ChangeURL: X-Gerrit-Commit: 413e37b0d5b2c2b0271078dbaf311bcfdbdbc773 References: Message-ID: <448e09f3855ce1ff906f2943e0da836d0b39ce88-HTML@gerrit.openvpn.net> MIME-Version: 1.0 User-Agent: Gerrit/3.8.2 X-Spam-Score: -0.2 (/) X-Spam-Report: Spam detection software, running on the system "util-spamd-2.v13.lw.sourceforge.com", has NOT identified this incoming email as spam. The original message has been attached to this so you can view it or label similar future email. If you have any questions, see the administrator of that system for details. Content preview: Attention is currently required from: flichtenheld, plaisthos. Hello plaisthos, flichtenheld, I'd like you to do a code review. Please visit Content analysis details: (-0.2 points, 6.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at https://www.dnswl.org/, no trust [209.85.128.42 listed in list.dnswl.org] 0.0 RCVD_IN_VALIDITY_RPBL_BLOCKED RBL: ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. [209.85.128.42 listed in bl.score.senderscore.com] 0.0 RCVD_IN_VALIDITY_CERTIFIED_BLOCKED RBL: ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. [209.85.128.42 listed in sa-trusted.bondedsender.org] -0.0 RCVD_IN_MSPIKE_H2 RBL: Average reputation (+2) [209.85.128.42 listed in wl.mailspike.net] 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record -0.0 SPF_PASS SPF: sender matches SPF record 0.0 WEIRD_PORT URI: Uses non-standard port number for HTTP 0.0 HTML_MESSAGE BODY: HTML included in message -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID_EF Message has a valid DKIM or DK signature from envelope-from domain 0.0 T_KAM_HTML_FONT_INVALID Test for Invalidly Named or Formatted Colors in HTML X-Headers-End: 1tDoiz-0005d2-13 Subject: [Openvpn-devel] [L] Change in openvpn[master]: PUSH_UPDATE: Allow OpenVPN in client mode to receive and handle PUSH ... 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: , Reply-To: marco@mandelbit.com, arne-openvpn@rfc2549.org, openvpn-devel@lists.sourceforge.net, frank@lichtenheld.com Cc: openvpn-devel Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox X-GMAIL-THRID: =?utf-8?q?1816264265821922395?= X-GMAIL-MSGID: =?utf-8?q?1816264265821922395?= X-getmail-filter-classifier: gerrit message type newchange Attention is currently required from: flichtenheld, plaisthos. Hello plaisthos, flichtenheld, I'd like you to do a code review. Please visit http://gerrit.openvpn.net/c/openvpn/+/808?usp=email to review the following change. Change subject: PUSH_UPDATE: Allow OpenVPN in client mode to receive and handle PUSH UPDATE control messages to allow options updating at runtime. ...................................................................... PUSH_UPDATE: Allow OpenVPN in client mode to receive and handle PUSH UPDATE control messages to allow options updating at runtime. * Added IV_PROTO_PUSH_UPDATE flag bit to support push-updates. * Added process_incoming_push_update(), in a separate file to create tests more easily. * Modified incoming_push_message(), process_incoming_push_msg(), apply_push_options(), apply_pull_filter() to process also push-update messages. * Added the check_push_update_option_flags() function used in apply_pull_filter() to check options formatting inside push-update messages, if the options are updatables and to check for '?' and '-' flags that may be present in front of the options. The '-' flag is used to indicate that the option in question should be removed, while the '?' indicates that the option is optional and to do not generate errors if the client cannot update that option. For more info you can read the RFC at https://github.com/OpenVPN/openvpn-rfc . * Created some unit tests for the push-update message handling in test_push_update_msg.c. Change-Id: I6ecd4cb47571cc8c20e46de8595c742aeec6064a Signed-off-by: Marco Baffo --- M CMakeLists.txt M src/openvpn/Makefile.am M src/openvpn/forward.c M src/openvpn/init.c M src/openvpn/options.c M src/openvpn/options.h M src/openvpn/options_util.c M src/openvpn/options_util.h M src/openvpn/push.c M src/openvpn/push.h A src/openvpn/push_util.c M src/openvpn/ssl.c M src/openvpn/ssl.h M tests/unit_tests/openvpn/Makefile.am A tests/unit_tests/openvpn/test_push_update_msg.c 15 files changed, 611 insertions(+), 82 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/08/808/1 diff --git a/CMakeLists.txt b/CMakeLists.txt index 5db207d..9dd67bb 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -496,6 +496,7 @@ src/openvpn/ps.c src/openvpn/ps.h src/openvpn/push.c + src/openvpn/push_util.c src/openvpn/push.h src/openvpn/pushlist.h src/openvpn/reflect_filter.c diff --git a/src/openvpn/Makefile.am b/src/openvpn/Makefile.am index ecb2bcf..55ec565 100644 --- a/src/openvpn/Makefile.am +++ b/src/openvpn/Makefile.am @@ -114,7 +114,7 @@ proto.c proto.h \ proxy.c proxy.h \ ps.c ps.h \ - push.c push.h \ + push.c push_util.c push.h \ pushlist.h \ reflect_filter.c reflect_filter.h \ reliable.c reliable.h \ diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c index d50b24c..2c75953 100644 --- a/src/openvpn/forward.c +++ b/src/openvpn/forward.c @@ -2031,7 +2031,7 @@ } /* check for incoming control messages on the control channel like - * push request/reply, or authentication failure and 2FA messages */ + * push request/reply/update, or authentication failure and 2FA messages */ if (tls_test_payload_len(c->c2.tls_multi) > 0) { check_incoming_control_channel(c); diff --git a/src/openvpn/init.c b/src/openvpn/init.c index 9371024..f8d9d06 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -2030,8 +2030,8 @@ /* possibly add routes */ if ((route_order(c->c1.tuntap) == ROUTE_AFTER_TUN) && (!c->options.route_delay_defined)) { - int status = do_route(&c->options, c->c1.route_list, c->c1.route_ipv6_list, - c->c1.tuntap, c->plugins, c->c2.es, &c->net_ctx); + bool status = do_route(&c->options, c->c1.route_list, c->c1.route_ipv6_list, + c->c1.tuntap, c->plugins, c->c2.es, &c->net_ctx); *error_flags |= (status ? 0 : ISC_ROUTE_ERRORS); } diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 61f6285..e772a54 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -63,6 +63,7 @@ #include #include "memdbg.h" +#include "options_util.h" const char title_string[] = PACKAGE_STRING @@ -927,24 +928,6 @@ } } -struct pull_filter -{ -#define PUF_TYPE_UNDEF 0 /** undefined filter type */ -#define PUF_TYPE_ACCEPT 1 /** filter type to accept a matching option */ -#define PUF_TYPE_IGNORE 2 /** filter type to ignore a matching option */ -#define PUF_TYPE_REJECT 3 /** filter type to reject and trigger SIGUSR1 */ - int type; - int size; - char *pattern; - struct pull_filter *next; -}; - -struct pull_filter_list -{ - struct pull_filter *head; - struct pull_filter *tail; -}; - #ifndef ENABLE_SMALL static const char * @@ -5469,54 +5452,6 @@ } } -/** - * Filter an option line by all pull filters. - * - * If a match is found, the line is modified depending on - * the filter type, and returns true. If the filter type is - * reject, SIGUSR1 is triggered and the return value is false. - * In that case the caller must end the push processing. - */ -static bool -apply_pull_filter(const struct options *o, char *line) -{ - struct pull_filter *f; - - if (!o->pull_filter_list) - { - return true; - } - - /* skip leading spaces matching the behaviour of parse_line */ - while (isspace(*line)) - { - line++; - } - - for (f = o->pull_filter_list->head; f; f = f->next) - { - if (f->type == PUF_TYPE_ACCEPT && strncmp(line, f->pattern, f->size) == 0) - { - msg(D_LOW, "Pushed option accepted by filter: '%s'", line); - return true; - } - else if (f->type == PUF_TYPE_IGNORE && strncmp(line, f->pattern, f->size) == 0) - { - msg(D_PUSH, "Pushed option removed by filter: '%s'", line); - *line = '\0'; - return true; - } - else if (f->type == PUF_TYPE_REJECT && strncmp(line, f->pattern, f->size) == 0) - { - msg(M_WARN, "Pushed option rejected by filter: '%s'. Restarting.", line); - *line = '\0'; - throw_signal_soft(SIGUSR1, "Offending option received from server"); - return false; - } - } - return true; -} - bool apply_push_options(struct options *options, struct buffer *buf, @@ -5679,6 +5614,43 @@ { return options->forward_compatible ? M_WARN : msglevel; } +bool +apply_push_options(struct context *c, + struct options *options, + struct buffer *buf, + unsigned int permission_mask, + unsigned int *option_types_found, + struct env_set *es, + bool is_update) +{ + char line[OPTION_PARM_SIZE]; + int line_num = 0; + const char *file = "[PUSH-OPTIONS]"; + const int msglevel = D_PUSH_ERRORS|M_OPTERR; + + while (buf_parse(buf, ',', line, sizeof(line))) + { + char *p[MAX_PARMS+1]; + CLEAR(p); + ++line_num; + unsigned int push_update_option_flags = 0; + + if (!apply_pull_filter(options, line, is_update, &push_update_option_flags)) + { + throw_signal_soft(SIGUSR1, "Offending option received from server"); + return false; /* Cause push/pull error and stop push processing */ + } + if (parse_line(line, p, SIZE(p)-1, file, line_num, msglevel, &options->gc)) + { + if (!is_update) + { + add_option(options, p, false, file, line_num, 0, msglevel, + permission_mask, option_types_found, es); + } + } + } + return true; +} static void set_user_script(struct options *options, diff --git a/src/openvpn/options.h b/src/openvpn/options.h index ee39dbb..5125f79 100644 --- a/src/openvpn/options.h +++ b/src/openvpn/options.h @@ -777,6 +777,33 @@ #define MAN_CLIENT_AUTH_ENABLED(opt) (false) #endif +/* + * some PUSH_UPDATE options + */ +#define OPT_P_U_ROUTE (1<<0) +#define OPT_P_U_ROUTE6 (1<<1) +#define OPT_P_U_DNS (1<<2) +#define OPT_P_U_DHCP (1<<3) +#define OPT_P_U_REDIR_GATEWAY (1<<4) + +struct pull_filter +{ +#define PUF_TYPE_UNDEF 0 /** undefined filter type */ +#define PUF_TYPE_ACCEPT 1 /** filter type to accept a matching option */ +#define PUF_TYPE_IGNORE 2 /** filter type to ignore a matching option */ +#define PUF_TYPE_REJECT 3 /** filter type to reject and trigger SIGUSR1 */ + int type; + int size; + char *pattern; + struct pull_filter *next; +}; + +struct pull_filter_list +{ + struct pull_filter *head; + struct pull_filter *tail; +}; + void parse_argv(struct options *options, const int argc, char *argv[], @@ -845,11 +872,13 @@ void pre_connect_restore(struct options *o, struct gc_arena *gc); -bool apply_push_options(struct options *options, +bool apply_push_options(struct context *c, + struct options *options, struct buffer *buf, unsigned int permission_mask, unsigned int *option_types_found, - struct env_set *es); + struct env_set *es, + bool is_update); void options_detach(struct options *o); diff --git a/src/openvpn/options_util.c b/src/openvpn/options_util.c index a9e020a..87282c3 100644 --- a/src/openvpn/options_util.c +++ b/src/openvpn/options_util.c @@ -30,6 +30,8 @@ #include "options_util.h" +#include "push.h" + const char * parse_auth_failed_temp(struct options *o, const char *reason) { @@ -98,3 +100,176 @@ gc_free(&gc); return message; } + +static const char *updatable_options[] = { + "block-ipv6", + "block-outside-dns", + "dhcp-option", + "dns", + "ifconfig", + "ifconfig-ipv6", + "push-continuation", + "redirect-gateway", + "redirect-private", + "route", + "route-gateway", + "route-ipv6", + "route-metric", + "topology", + "tun-mtu", + "keepalive" +}; + +static bool +check_push_update_option_flags(char **line, unsigned int *flags) +{ + if (!line || !*line || !**line) + { + return false; + } + *flags = 0; + bool opt_is_updatable = false; + char *str = *line; + char c = **line; + + if (c == '?') + { + *flags |= PUSH_OPT_OPTIONAL; + if (str[1]) + { + c = *(++str); + } + else + { + return false; + } + } + else if (c == '-') + { + *flags |= PUSH_OPT_TO_REMOVE; + if (str[1]) + { + c = *(++str); + } + else + { + return false; + } + } + + if (c == '-' && !(*flags & PUSH_OPT_TO_REMOVE)) + { + *flags |= PUSH_OPT_TO_REMOVE; + if (str[1]) + { + c = *(++str); + } + else + { + return false; + } + } + + /* Skip '?' and '-' if they are present */ + size_t len = strlen(str); + memmove(*line, str, len); + (*line)[len] = '\0'; + str = *line; + + int count = sizeof(updatable_options)/sizeof(char *); + for (int i = 0; i < count; ++i) + { + size_t opt_len = strlen(updatable_options[i]); + if (len < opt_len) + { + continue; + } + if (!strncmp(str, updatable_options[i], opt_len) + && (!str[opt_len] || str[opt_len] == ' ')) + { + opt_is_updatable = true; + break; + } + } + + if (!opt_is_updatable) + { + if (*flags & PUSH_OPT_OPTIONAL) + { + msg(D_PUSH, "Pushed option is not updatable: '%s'", *line); + return true; + } + else + { + msg(M_WARN, "Pushed option is not updatable: '%s'. Restarting.", *line); + return false; + } + } + + return true; +} + +/** + * Filter an option line by all pull filters. + * + * If a match is found, the line is modified depending on + * the filter type, and returns true. If the filter type is + * reject, SIGUSR1 is triggered and the return value is false. + * In that case the caller must end the push processing. + */ +bool +apply_pull_filter(const struct options *o, char *line, bool is_update, unsigned int *push_update_option_flags) +{ + struct pull_filter *f; + + if (!o->pull_filter_list && !(is_update)) + { + return true; + } + + /* skip leading spaces matching the behaviour of parse_line */ + while (isspace(*line)) + { + line++; + } + + if (is_update) + { + if (!check_push_update_option_flags(&line, push_update_option_flags)) + { + return false; + } + if (!o->pull_filter_list) + { + return true; + } + } + + for (f = o->pull_filter_list->head; f; f = f->next) + { + if (f->type == PUF_TYPE_ACCEPT && strncmp(line, f->pattern, f->size) == 0) + { + msg(D_LOW, "Pushed option accepted by filter: '%s'", line); + return true; + } + else if (f->type == PUF_TYPE_IGNORE && strncmp(line, f->pattern, f->size) == 0) + { + msg(D_PUSH, "Pushed option removed by filter: '%s'", line); + return true; + } + else if (f->type == PUF_TYPE_REJECT && strncmp(line, f->pattern, f->size) == 0) + { + if (is_update && (*push_update_option_flags & PUSH_OPT_OPTIONAL)) + { + msg(D_PUSH, "Pushed option removed by filter: '%s'", line); + return true; + } + else + { + msg(M_WARN, "Pushed option rejected by filter: '%s'. Restarting.", line); + return false; + } + } + } + return true; +} diff --git a/src/openvpn/options_util.h b/src/openvpn/options_util.h index 29bc9dc..64ee0d6 100644 --- a/src/openvpn/options_util.h +++ b/src/openvpn/options_util.h @@ -30,4 +30,10 @@ const char * parse_auth_failed_temp(struct options *o, const char *reason); +bool +apply_pull_filter(const struct options *o, + char *line, + bool is_update, + unsigned int *push_update_option_flags); + #endif diff --git a/src/openvpn/push.c b/src/openvpn/push.c index 6c06374..71a0372 100644 --- a/src/openvpn/push.c +++ b/src/openvpn/push.c @@ -39,8 +39,6 @@ #include "ssl_util.h" #include "options_util.h" -static char push_reply_cmd[] = "PUSH_REPLY"; - /* * Auth username/password * @@ -519,21 +517,31 @@ { msg(D_PUSH_ERRORS, "WARNING: Received bad push/pull message: %s", sanitize_control_message(BSTR(buffer), &gc)); } - else if (status == PUSH_MSG_REPLY || status == PUSH_MSG_CONTINUATION) + else if (status == PUSH_MSG_REPLY || status == PUSH_MSG_UPDATE || status == PUSH_MSG_CONTINUATION) { c->options.push_option_types_found |= option_types_found; /* delay bringing tun/tap up until --push parms received from remote */ - if (status == PUSH_MSG_REPLY) + if (status == PUSH_MSG_REPLY || status == PUSH_MSG_UPDATE) { if (!options_postprocess_pull(&c->options, c->c2.es)) { goto error; } - if (!do_up(c, true, c->options.push_option_types_found)) + if (status == PUSH_MSG_REPLY) { - msg(D_PUSH_ERRORS, "Failed to open tun/tap interface"); - goto error; + if (!do_up(c, true, c->options.push_option_types_found)) + { + msg(D_PUSH_ERRORS, "Failed to open tun/tap interface"); + goto error; + } + } + else + { + if (!option_types_found) + { + msg(M_WARN, "No option found"); + } } } event_timeout_clear(&c->c2.push_request_interval); @@ -1048,11 +1056,13 @@ md_ctx_init(c->c2.pulled_options_state, "SHA256"); c->c2.pulled_options_digest_init_done = true; } - if (apply_push_options(&c->options, + if (apply_push_options(c, + &c->options, buf, permission_mask, option_types_found, - c->c2.es)) + c->c2.es, + false)) { push_update_digest(c->c2.pulled_options_state, &buf_orig, &c->options); @@ -1103,6 +1113,12 @@ return process_incoming_push_reply(c, permission_mask, option_types_found, &buf); } + else if (honor_received_options + && buf_string_compare_advance(&buf, push_update_cmd)) + { + return process_incoming_push_update(c, permission_mask, + option_types_found, &buf); + } else { return PUSH_MSG_ERROR; diff --git a/src/openvpn/push.h b/src/openvpn/push.h index 4a13327..52ca715 100644 --- a/src/openvpn/push.h +++ b/src/openvpn/push.h @@ -33,9 +33,22 @@ #define PUSH_MSG_AUTH_FAILURE 4 #define PUSH_MSG_CONTINUATION 5 #define PUSH_MSG_ALREADY_REPLIED 6 +#define PUSH_MSG_UPDATE 7 + +#define push_reply_cmd "PUSH_REPLY" +#define push_update_cmd "PUSH_UPDATE" + +/* Push-update options flags */ +#define PUSH_OPT_TO_REMOVE (1<<0) +#define PUSH_OPT_OPTIONAL (1<<1) int process_incoming_push_request(struct context *c); +int process_incoming_push_update(struct context *c, + unsigned int permission_mask, + unsigned int *option_types_found, + struct buffer *buf); + int process_incoming_push_msg(struct context *c, const struct buffer *buffer, bool honor_received_options, diff --git a/src/openvpn/push_util.c b/src/openvpn/push_util.c new file mode 100644 index 0000000..b4d1e8b --- /dev/null +++ b/src/openvpn/push_util.c @@ -0,0 +1,44 @@ +#ifdef HAVE_CONFIG_H +#include "config.h" +#endif + +#include "push.h" + +int +process_incoming_push_update(struct context *c, + unsigned int permission_mask, + unsigned int *option_types_found, + struct buffer *buf) +{ + int ret = PUSH_MSG_ERROR; + const uint8_t ch = buf_read_u8(buf); + if (ch == ',') + { + if (apply_push_options(c, + &c->options, + buf, + permission_mask, + option_types_found, + c->c2.es, + true)) + { + switch (c->options.push_continuation) + { + case 0: + case 1: + ret = PUSH_MSG_UPDATE; + break; + + case 2: + ret = PUSH_MSG_CONTINUATION; + break; + } + } + } + else if (ch == '\0') + { + ret = PUSH_MSG_UPDATE; + } + + return ret; +} diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 8040e7b..b006917 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -1947,6 +1947,9 @@ /* support for exit notify via control channel */ iv_proto |= IV_PROTO_CC_EXIT_NOTIFY; + /* support push-updates */ + iv_proto |= IV_PROTO_PUSH_UPDATE; + if (session->opt->pull) { /* support for receiving push_reply before sending diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h index eea1323..53b7f2c 100644 --- a/src/openvpn/ssl.h +++ b/src/openvpn/ssl.h @@ -110,6 +110,9 @@ /** Supports the --dns option after all the incompatible changes */ #define IV_PROTO_DNS_OPTION_V2 (1<<11) +/** Supports push-update */ +#define IV_PROTO_PUSH_UPDATE (1<<12) + /* Default field in X509 to be username */ #define X509_USERNAME_FIELD_DEFAULT "CN" diff --git a/tests/unit_tests/openvpn/Makefile.am b/tests/unit_tests/openvpn/Makefile.am index a4e6235..1b2414c 100644 --- a/tests/unit_tests/openvpn/Makefile.am +++ b/tests/unit_tests/openvpn/Makefile.am @@ -11,7 +11,7 @@ endif test_binaries += crypto_testdriver packet_id_testdriver auth_token_testdriver ncp_testdriver misc_testdriver \ - pkt_testdriver ssl_testdriver user_pass_testdriver + pkt_testdriver ssl_testdriver user_pass_testdriver push_update_msg_testdriver if HAVE_LD_WRAP_SUPPORT if !WIN32 @@ -307,3 +307,25 @@ $(top_srcdir)/src/openvpn/win32-util.c \ $(top_srcdir)/src/openvpn/platform.c \ $(top_srcdir)/src/openvpn/list.c + +push_update_msg_testdriver_CFLAGS = -I$(top_srcdir)/src/openvpn \ + -I$(top_srcdir)/src/compat \ + -I$(top_srcdir)/tests/unit_tests/openvpn \ + -ffunction-sections \ + @TEST_CFLAGS@ \ + @CMOCKA_CFLAGS@ + +push_update_msg_testdriver_LDFLAGS = \ + @TEST_LDFLAGS@ \ + @CMOCKA_LIBS@ \ + -L$(top_srcdir)/src/openvpn \ + $(OPTIONAL_CRYPTO_LIBS) + +push_update_msg_testdriver_SOURCES = test_push_update_msg.c \ + mock_msg.c \ + mock_get_random.c \ + $(top_srcdir)/src/openvpn/buffer.c \ + $(top_srcdir)/src/openvpn/platform.c \ + $(top_srcdir)/src/openvpn/push_util.c \ + $(top_srcdir)/src/openvpn/options_util.c \ + $(top_srcdir)/src/openvpn/otime.c \ No newline at end of file diff --git a/tests/unit_tests/openvpn/test_push_update_msg.c b/tests/unit_tests/openvpn/test_push_update_msg.c new file mode 100644 index 0000000..8574855 --- /dev/null +++ b/tests/unit_tests/openvpn/test_push_update_msg.c @@ -0,0 +1,245 @@ +#ifdef HAVE_CONFIG_H +#include "config.h" +#endif + +#include +#include +#include +#include +#include "push.h" +#include "options_util.h" + +/* mocks */ + +unsigned int +pull_permission_mask(const struct context *c) +{ + unsigned int flags = + OPT_P_UP + | OPT_P_ROUTE_EXTRAS + | OPT_P_SOCKBUF + | OPT_P_SOCKFLAGS + | OPT_P_SETENV + | OPT_P_SHAPER + | OPT_P_TIMER + | OPT_P_COMP + | OPT_P_PERSIST + | OPT_P_MESSAGES + | OPT_P_EXPLICIT_NOTIFY + | OPT_P_ECHO + | OPT_P_PULL_MODE + | OPT_P_PEER_ID + | OPT_P_NCP + | OPT_P_PUSH_MTU + | OPT_P_ROUTE + | OPT_P_DHCPDNS; + return flags; +} + +bool +apply_push_options(struct context *c, + struct options *options, + struct buffer *buf, + unsigned int permission_mask, + unsigned int *option_types_found, + struct env_set *es, + bool is_update) +{ + char line[OPTION_PARM_SIZE]; + + while (buf_parse(buf, ',', line, sizeof(line))) + { + unsigned int push_update_option_flags = 0; + + if (!apply_pull_filter(options, line, is_update, &push_update_option_flags)) + { + msg(M_WARN, "Offending option received from server"); + return false; + } + /* + * No need to test also the application part here + * (add_option/remove_option/update_option) + */ + } + return true; +} + +int +process_incoming_push_msg(struct context *c, + const struct buffer *buffer, + bool honor_received_options, + unsigned int permission_mask, + unsigned int *option_types_found) +{ + struct buffer buf = *buffer; + + if (buf_string_compare_advance(&buf, "PUSH_REQUEST")) + { + return PUSH_MSG_REQUEST; + } + else if (honor_received_options + && buf_string_compare_advance(&buf, push_reply_cmd)) + { + return PUSH_MSG_REPLY; + } + else if (honor_received_options + && buf_string_compare_advance(&buf, push_update_cmd)) + { + return process_incoming_push_update(c, permission_mask, + option_types_found, &buf); + } + else + { + return PUSH_MSG_ERROR; + } +} + +/* tests */ + +static void +test_incoming_push_message_basic(void **state) +{ + struct context *c = *state; + struct buffer buf = alloc_buf(256); + const char *update_msg = "PUSH_UPDATE,dhcp-option DNS 8.8.8.8, route 0.0.0.0 0.0.0.0 10.10.10.1"; + buf_write(&buf, update_msg, strlen(update_msg)); + unsigned int option_types_found = 0; + + assert_int_equal(process_incoming_push_msg(c, &buf, c->options.pull, pull_permission_mask(c), &option_types_found), PUSH_MSG_UPDATE); + + free_buf(&buf); +} + +static void +test_incoming_push_message_error1(void **state) +{ + struct context *c = *state; + struct buffer buf = alloc_buf(256); + const char *update_msg = "PUSH_UPDATEerr,dhcp-option DNS 8.8.8.8"; + buf_write(&buf, update_msg, strlen(update_msg)); + unsigned int option_types_found = 0; + + assert_int_equal(process_incoming_push_msg(c, &buf, c->options.pull, pull_permission_mask(c), &option_types_found), PUSH_MSG_ERROR); + + free_buf(&buf); +} + + +static void +test_incoming_push_message_error2(void **state) +{ + struct context *c = *state; + struct buffer buf = alloc_buf(256); + const char *update_msg = "PUSH_UPDATE ,dhcp-option DNS 8.8.8.8"; + buf_write(&buf, update_msg, strlen(update_msg)); + unsigned int option_types_found = 0; + + assert_int_equal(process_incoming_push_msg(c, &buf, c->options.pull, pull_permission_mask(c), &option_types_found), PUSH_MSG_ERROR); + + free_buf(&buf); +} + +static void +test_incoming_push_message_1(void **state) +{ + struct context *c = *state; + struct buffer buf = alloc_buf(256); + const char *update_msg = "PUSH_UPDATE, ?-dns, route something, ?dhcp-option DNS 8.8.8.8"; + buf_write(&buf, update_msg, strlen(update_msg)); + unsigned int option_types_found = 0; + + assert_int_equal(process_incoming_push_msg(c, &buf, c->options.pull, pull_permission_mask(c), &option_types_found), PUSH_MSG_UPDATE); + + free_buf(&buf); +} + +static void +test_incoming_push_message_bad_format(void **state) +{ + struct context *c = *state; + struct buffer buf = alloc_buf(256); + const char *update_msg = "PUSH_UPDATE, -dhcp-option, -?dns"; + buf_write(&buf, update_msg, strlen(update_msg)); + unsigned int option_types_found = 0; + + assert_int_equal(process_incoming_push_msg(c, &buf, c->options.pull, pull_permission_mask(c), &option_types_found), PUSH_MSG_ERROR); + + free_buf(&buf); +} + +static void +test_incoming_push_message_not_updatable_option(void **state) +{ + struct context *c = *state; + struct buffer buf = alloc_buf(256); + const char *update_msg = "PUSH_UPDATE, dev tun"; + buf_write(&buf, update_msg, strlen(update_msg)); + unsigned int option_types_found = 0; + + assert_int_equal(process_incoming_push_msg(c, &buf, c->options.pull, pull_permission_mask(c), &option_types_found), PUSH_MSG_ERROR); + + free_buf(&buf); +} + +static void +test_incoming_push_message_mix(void **state) +{ + struct context *c = *state; + struct buffer buf = alloc_buf(256); + const char *update_msg = "PUSH_UPDATE,-dhcp-option, route 10.10.10.0, dhcp-option DNS 1.1.1.1, route 10.11.12.0, dhcp-option DOMAIN corp.local, keepalive 10 60"; + buf_write(&buf, update_msg, strlen(update_msg)); + unsigned int option_types_found = 0; + + assert_int_equal(process_incoming_push_msg(c, &buf, c->options.pull, pull_permission_mask(c), &option_types_found), PUSH_MSG_UPDATE); + + free_buf(&buf); +} + +static void +test_incoming_push_message_mix2(void **state) +{ + struct context *c = *state; + struct buffer buf = alloc_buf(256); + const char *update_msg = "PUSH_UPDATE,-dhcp-option,dhcp-option DNS 8.8.8.8,redirect-gateway local,route 192.168.1.0 255.255.255.0"; + buf_write(&buf, update_msg, strlen(update_msg)); + unsigned int option_types_found = 0; + + assert_int_equal(process_incoming_push_msg(c, &buf, c->options.pull, pull_permission_mask(c), &option_types_found), PUSH_MSG_UPDATE); + + free_buf(&buf); +} + +static int +setup(void **state) +{ + struct context *c = calloc(1, sizeof(struct context)); + c->options.pull = true; + c->options.route_nopull = false; + *state = c; + return 0; +} + +static int +teardown(void **state) +{ + struct context *c = *state; + free(c); + return 0; +} + +int +main(void) +{ + const struct CMUnitTest tests[] = { + cmocka_unit_test_setup_teardown(test_incoming_push_message_basic, setup, teardown), + cmocka_unit_test_setup_teardown(test_incoming_push_message_error1, setup, teardown), + cmocka_unit_test_setup_teardown(test_incoming_push_message_error2, setup, teardown), + cmocka_unit_test_setup_teardown(test_incoming_push_message_not_updatable_option, setup, teardown), + cmocka_unit_test_setup_teardown(test_incoming_push_message_1, setup, teardown), + cmocka_unit_test_setup_teardown(test_incoming_push_message_bad_format, setup, teardown), + cmocka_unit_test_setup_teardown(test_incoming_push_message_mix, setup, teardown), + cmocka_unit_test_setup_teardown(test_incoming_push_message_mix2, setup, teardown) + }; + + return cmocka_run_group_tests(tests, NULL, NULL); +}