NetBSD Problem Report #57392
From oster@fween.ca Sat May 6 03:15:39 2023
Return-Path: <oster@fween.ca>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits))
(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
by mollari.NetBSD.org (Postfix) with ESMTPS id 2FAB51A923E
for <gnats-bugs@gnats.NetBSD.org>; Sat, 6 May 2023 03:15:39 +0000 (UTC)
Message-Id: <20230506015500.9C80F1047DD@gonzo.fween.ca>
Date: Fri, 5 May 2023 19:55:00 -0600 (CST)
From: oster@netbsd.org
Reply-To: oster@netbsd.org
To: gnats-bugs@NetBSD.org
Subject: wgconfig add peer accepts anything for options
X-Send-Pr-Version: 3.95
>Number: 57392
>Category: bin
>Synopsis: wgconfig add peer accepts anything for command-line options
>Confidential: no
>Severity: serious
>Priority: medium
>Responsible: bin-bug-people
>State: closed
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Sat May 06 03:20:00 +0000 2023
>Closed-Date: Sun May 07 16:06:44 +0000 2023
>Last-Modified: Thu Jun 08 11:15:01 +0000 2023
>Originator: Greg Oster
>Release: NetBSD 10.0_BETA
>Organization:
>Environment:
System: NetBSD test1.fween.ca 10.0_BETA NetBSD 10.0_BETA (GENERIC) #0: Sat Apr 29 13:22:34 CST 2023 oster@buildbot.fween.ca:/u1/builds/build2/src/obj/amd64/u1/builds/build2/src/sys/arch/amd64/compile/GENERIC amd64
Architecture: x86_64
Machine: amd64
>Description:
When running "wgconfig add peer <peer name> <base64 public key>" one can add,
via options, a preshared key path, an endpoint, or allowed IPs. Unfortunately,
the following:
wgconfig add peer somepeer UNAzPLq7HDvg6P7ezD3v+S0jE+KxcFCKb/zv3Msa9AA= --allowed-ip=192.168.0.1/32
looks valid, returns no error, but is equivalent to:
wgconfig add peer somepeer UNAzPLq7HDvg6P7ezD3v+S0jE+KxcFCKb/zv3Msa9AA=
which would definitely not be what is expected, nor desired. In fact,
wgconfig add peer somepeer UNAzPLq7HDvg6P7ezD3v+S0jE+KxcFCKb/zv3Msa9AA= random text
is also equivalent to:
wgconfig add peer somepeer UNAzPLq7HDvg6P7ezD3v+S0jE+KxcFCKb/zv3Msa9AA=
(again, without any error).
Not erroring out on invalid options is not good, as one ends up with a
configuration that does not match the desired intent, even though the
command appears to have been 'successful'.
>How-To-Repeat:
ifconfig wg0 create 192.168.200.2/24
wgconfig wg0 set private-key /etc/wireguard/wg0
wgconfig wg0 set listen-port 51820
wgconfig wg0 add peer other UNAzPLq7HDvg6P7ezD3v+S0jE+KxcFCKb/zv3Msa9AA= --allowed-ip=192.168.100.1/32
wgconfig wg0 show peer other
scratch head wondering why there is no 'allowed-ips' mentioned in the output:
peer: other
public-key: UNAzPLq7HDvg6P7ezD3v+S0jE+KxcFCKb/zv3Msa9AA=
endpoint: (none)
preshared-key: (hidden)
latest-handshake: (never)
Realize that 'allowed-ip' is not the same as 'allowed-ips'.
>Fix:
Perhaps something like this?
*** wgconfig.c.orig 2023-05-04 16:35:04.400335580 -0600
--- wgconfig.c 2023-05-05 19:27:42.055900579 -0600
***************
*** 680,693 ****
--- 680,697 ----
{
while (argc > 0) {
+ int found = 0;
for (size_t i = 0; i < __arraycount(options); i++) {
const struct option *opt = &options[i];
size_t optlen = strlen(opt->option);
if (strncmp(argv[0], opt->option, optlen) == 0) {
opt->func(argv[0] + optlen, prop_dict);
+ found = 1;
break;
}
}
+ if (found == 0)
+ errx(EXIT_FAILURE, "invalid option: %s", argv[0]);
argc -= 1;
argv += 1;
}
>Release-Note:
>Audit-Trail:
From: David Holland <dholland-bugs@netbsd.org>
To: gnats-bugs@netbsd.org
Cc:
Subject: Re: bin/57392: wgconfig add peer accepts anything for options
Date: Sun, 7 May 2023 06:26:30 +0000
On Sat, May 06, 2023 at 03:20:00AM +0000, oster@netbsd.org wrote:
> Perhaps something like this?
>
> *** wgconfig.c.orig 2023-05-04 16:35:04.400335580 -0600
> --- wgconfig.c 2023-05-05 19:27:42.055900579 -0600
> ***************
> *** 680,693 ****
> --- 680,697 ----
> {
>
> while (argc > 0) {
> + int found = 0;
> for (size_t i = 0; i < __arraycount(options); i++) {
> const struct option *opt = &options[i];
> size_t optlen = strlen(opt->option);
> if (strncmp(argv[0], opt->option, optlen) == 0) {
> opt->func(argv[0] + optlen, prop_dict);
> + found = 1;
> break;
> }
> }
> + if (found == 0)
> + errx(EXIT_FAILURE, "invalid option: %s", argv[0]);
> argc -= 1;
> argv += 1;
> }
Any reason to not just commit that?
--
David A. Holland
dholland@netbsd.org
From: Greg Oster <oster@netbsd.org>
To: gnats-bugs@netbsd.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org
Cc:
Subject: Re: bin/57392: wgconfig add peer accepts anything for options
Date: Sun, 7 May 2023 09:54:52 -0600
On 2023-05-07 00.30, David Holland wrote:
>
> Any reason to not just commit that?
I hadn't done a proper build test yet, and wanted to get the bug documented.
I've done a successful build test, and so will commit the fix shortly.
Later...
Greg Oster
State-Changed-From-To: open->closed
State-Changed-By: oster@NetBSD.org
State-Changed-When: Sun, 07 May 2023 16:06:44 +0000
State-Changed-Why:
Fix committed.
From: "Greg Oster" <oster@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/57392 CVS commit: src/usr.sbin/wgconfig
Date: Sun, 7 May 2023 16:05:07 +0000
Module Name: src
Committed By: oster
Date: Sun May 7 16:05:07 UTC 2023
Modified Files:
src/usr.sbin/wgconfig: wgconfig.c
Log Message:
Don't allow "wgconfig add peer ..." to accept invalid options.
Addresses PR bin/57392 .
To generate a diff of this commit:
cvs rdiff -u -r1.5 -r1.6 src/usr.sbin/wgconfig/wgconfig.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/57392 CVS commit: [netbsd-10] src/usr.sbin/wgconfig
Date: Thu, 8 Jun 2023 11:12:22 +0000
Module Name: src
Committed By: martin
Date: Thu Jun 8 11:12:22 UTC 2023
Modified Files:
src/usr.sbin/wgconfig [netbsd-10]: wgconfig.c
Log Message:
Pull up following revision(s) (requested by oster in ticket #194):
usr.sbin/wgconfig/wgconfig.c: revision 1.6
Don't allow "wgconfig add peer ..." to accept invalid options.
Addresses PR bin/57392 .
To generate a diff of this commit:
cvs rdiff -u -r1.5 -r1.5.6.1 src/usr.sbin/wgconfig/wgconfig.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
>Unformatted:
(Contact us)
$NetBSD: query-full-pr,v 1.47 2022/09/11 19:34:41 kim Exp $
$NetBSD: gnats_config.sh,v 1.9 2014/08/02 14:16:04 spz Exp $
Copyright © 1994-2023
The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.