Development discussion of WireGuard
 help / color / mirror / Atom feed
* [PATCH] Implement reading keys from stdin.
@ 2020-02-07 20:00 Hristo Venev
  2020-02-08 22:20 ` Jason A. Donenfeld
  0 siblings, 1 reply; 6+ messages in thread
From: Hristo Venev @ 2020-02-07 20:00 UTC (permalink / raw)
  To: wireguard

If "-" is specified as a path, the key is read as a single line from stdin. If
multiple "-" keys are specified, the order of the lines containing the keys is
the same as the order of the "-" arguments. An empty line means that there is
no key.
---
 src/config.c   | 68 +++++++++++++++++++++++++++++++++-----------------
 src/fuzz/set.c | 20 +++++++++++----
 2 files changed, 60 insertions(+), 28 deletions(-)

diff --git a/src/config.c b/src/config.c
index b8394a5..c5e5267 100644
--- a/src/config.c
+++ b/src/config.c
@@ -21,6 +21,7 @@
 #include "encoding.h"
 
 #define COMMENT_CHAR '#'
+#define KEY_LINE_MAX_LEN (WG_KEY_LEN_BASE64 + 1)
 
 static const char *get_value(const char *line, const char *key)
 {
@@ -118,42 +119,63 @@ static bool parse_keyfile(uint8_t key[static WG_KEY_LEN], const char *path)
 {
 	FILE *f;
 	int c;
-	char dst[WG_KEY_LEN_BASE64];
+	char dst[KEY_LINE_MAX_LEN];
+	bool is_file = false;
 	bool ret = false;
 
-	f = fopen(path, "r");
-	if (!f) {
-		perror("fopen");
-		return false;
+	if (!strcmp(path, "-"))
+		f = stdin;
+	else {
+		f = fopen(path, "r");
+		if (!f) {
+			perror("fopen");
+			return false;
+		}
+		is_file = true;
 	}
 
-	if (fread(dst, WG_KEY_LEN_BASE64 - 1, 1, f) != 1) {
-		/* If we're at the end and we didn't read anything, we're /dev/null or an empty file. */
-		if (!ferror(f) && feof(f) && !ftell(f)) {
-			memset(key, 0, WG_KEY_LEN);
-			ret = true;
-			goto out;
-		}
+	if (!fgets(dst, KEY_LINE_MAX_LEN, f)) {
+		dst[0] = '\0';
+	}
 
-		fprintf(stderr, "Invalid length key in key file\n");
+	if (ferror(f)) {
+		perror("fgets");
 		goto out;
 	}
-	dst[WG_KEY_LEN_BASE64 - 1] = '\0';
 
-	while ((c = getc(f)) != EOF) {
-		if (!isspace(c)) {
-			fprintf(stderr, "Found trailing character in key file: `%c'\n", c);
+	/* fgets stores the trailing newline into the buffer. If it is not there and
+	 * we have not hit EOF, there must be more of the line left. */
+	size_t n = strlen(dst);
+	if (n != 0 && dst[n - 1] == '\n') {
+		n--;
+		dst[n] = '\0';
+	} else if(!feof(f)) {
+		fprintf(stderr, "Key too long: `%s...'\n", dst);
+		goto out;
+	}
+
+	if (is_file) {
+		while ((c = getc(f)) != EOF) {
+			if (!isspace(c)) {
+				fprintf(stderr, "Found trailing character in key file: `%c'\n", c);
+				goto out;
+			}
+		}
+		if (ferror(f) && errno) {
+			perror("getc");
 			goto out;
 		}
 	}
-	if (ferror(f) && errno) {
-		perror("getc");
-		goto out;
-	}
-	ret = parse_key(key, dst);
+
+	if(n == 0) {
+		memset(key, 0, WG_KEY_LEN);
+		ret = true;
+	} else
+		ret = parse_key(key, dst);
 
 out:
-	fclose(f);
+	if (is_file)
+		fclose(f);
 	return ret;
 }
 
diff --git a/src/fuzz/set.c b/src/fuzz/set.c
index 2f40615..ded26e6 100644
--- a/src/fuzz/set.c
+++ b/src/fuzz/set.c
@@ -37,20 +37,30 @@ static FILE *hacked_fopen(const char *pathname, const char *mode)
 int LLVMFuzzerTestOneInput(const char *data, size_t data_len)
 {
 	char *argv[8192] = { "set", "wg0" }, *args;
-	size_t argc = 2;
+	size_t args_len, stdin_len, argc = 2;
+	FILE *input;
 
-	if (!data_len)
+	args_len = strnlen(data, data_len);
+	stdin_len = data_len - args_len;
+	/* POSIX (and therefore glibc) doesn't like fmemopen(_, 0, _) */
+	if (args_len == 0 || stdin_len == 0)
 		return 0;
 
-	assert((args = malloc(data_len)));
-	memcpy(args, data, data_len);
-	args[data_len - 1] = '\0';
+	assert((args = malloc(args_len + 1)));
+	memcpy(args, data, args_len);
+	args[args_len] = '\0';
+
+	assert((input = fmemopen((void*)(data + args_len), stdin_len, "r")));
+	/* discard null character, also permit tests where stdin is empty */
+	fgetc(input);
 
 	for (char *arg = strtok(args, " \t\n\r"); arg && argc < 8192; arg = strtok(NULL, " \t\n\r")) {
 		if (arg[0])
 			argv[argc++] = arg;
 	}
+	stdin = input;
 	set_main(argc, argv);
 	free(args);
+	fclose(stdin);
 	return 0;
 }
-- 
2.24.1

_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Implement reading keys from stdin.
  2020-02-07 20:00 [PATCH] Implement reading keys from stdin Hristo Venev
@ 2020-02-08 22:20 ` Jason A. Donenfeld
  2020-02-09  0:15   ` Hristo Venev
  2020-02-14 11:12   ` Mantas Mikulėnas
  0 siblings, 2 replies; 6+ messages in thread
From: Jason A. Donenfeld @ 2020-02-08 22:20 UTC (permalink / raw)
  To: Hristo Venev; +Cc: WireGuard mailing list

Thank for the patch, and nice hanging with you at FOSDEM.

Trying to get a handle on the use case for this. Is this so that you
can put the private key and the preshared key in a single file
together? Is there a situation where the shell redirection trick
doesn't cut it? For example:

wg set wg0 private-key <(head -n 1 bothkeys) preshared-key <(tail -n 1 bothkeys)
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Implement reading keys from stdin.
  2020-02-08 22:20 ` Jason A. Donenfeld
@ 2020-02-09  0:15   ` Hristo Venev
  2020-02-14 11:17     ` Jason A. Donenfeld
  2020-02-14 11:12   ` Mantas Mikulėnas
  1 sibling, 1 reply; 6+ messages in thread
From: Hristo Venev @ 2020-02-09  0:15 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: WireGuard mailing list


[-- Attachment #1.1: Type: text/plain, Size: 471 bytes --]

On Sat, 2020-02-08 at 23:20 +0100, Jason A. Donenfeld wrote:
> Trying to get a handle on the use case for this.

I am working on a program [1] that configures a WireGuard interface by
invoking `wg`. Generally there are multiple peers, and some of them may
have preshared keys.

Currently the most reasonable way to pass keys is to write each one to
a temporary file. I think passing all of them over stdin is nicer.

[1] https://git.venev.name/hristo/wgconfd/

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 148 bytes --]

_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Implement reading keys from stdin.
  2020-02-08 22:20 ` Jason A. Donenfeld
  2020-02-09  0:15   ` Hristo Venev
@ 2020-02-14 11:12   ` Mantas Mikulėnas
  1 sibling, 0 replies; 6+ messages in thread
From: Mantas Mikulėnas @ 2020-02-14 11:12 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: WireGuard mailing list


[-- Attachment #1.1: Type: text/plain, Size: 573 bytes --]

On Sun, Feb 9, 2020 at 12:23 AM Jason A. Donenfeld <Jason@zx2c4.com> wrote:

> Thank for the patch, and nice hanging with you at FOSDEM.
>
> Trying to get a handle on the use case for this. Is this so that you
> can put the private key and the preshared key in a single file
> together? Is there a situation where the shell redirection trick
> doesn't cut it? For example:
>
> wg set wg0 private-key <(head -n 1 bothkeys) preshared-key <(tail -n 1
> bothkeys)
>

I would guess there are shells which don't have the <(cmd) bashism...

-- 
Mantas Mikulėnas

[-- Attachment #1.2: Type: text/html, Size: 953 bytes --]

[-- Attachment #2: Type: text/plain, Size: 148 bytes --]

_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Implement reading keys from stdin.
  2020-02-09  0:15   ` Hristo Venev
@ 2020-02-14 11:17     ` Jason A. Donenfeld
  2020-02-14 23:38       ` Hristo Venev
  0 siblings, 1 reply; 6+ messages in thread
From: Jason A. Donenfeld @ 2020-02-14 11:17 UTC (permalink / raw)
  To: Hristo Venev; +Cc: WireGuard mailing list

On Sun, Feb 9, 2020 at 1:15 AM Hristo Venev <hristo@venev.name> wrote:
>
> On Sat, 2020-02-08 at 23:20 +0100, Jason A. Donenfeld wrote:
> > Trying to get a handle on the use case for this.
>
> I am working on a program [1] that configures a WireGuard interface by
> invoking `wg`. Generally there are multiple peers, and some of them may
> have preshared keys.
>
> Currently the most reasonable way to pass keys is to write each one to
> a temporary file. I think passing all of them over stdin is nicer.

Except the command line arguments have length limits you'll hit
anyway. Wouldn't the better way to do this be passing a config file to
`wg setconf wg0 /dev/stdin`?
_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Implement reading keys from stdin.
  2020-02-14 11:17     ` Jason A. Donenfeld
@ 2020-02-14 23:38       ` Hristo Venev
  0 siblings, 0 replies; 6+ messages in thread
From: Hristo Venev @ 2020-02-14 23:38 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: WireGuard mailing list


[-- Attachment #1.1: Type: text/plain, Size: 993 bytes --]

On Fri, 2020-02-14 at 12:17 +0100, Jason A. Donenfeld wrote:
> Except the command line arguments have length limits you'll hit
> anyway. Wouldn't the better way to do this be passing a config file
> to
> `wg setconf wg0 /dev/stdin`?

Yes, they would be better. However each command has slight
inefficiencies for my usecase:
 - `wg setconf` removes the endpoints of peers that don't have a static
endpoint address
 - `wg addconf` cannot remove peers
 - `wg syncconf` needs to be given the exact allowed IPs of all peers
it has to keep, not just the ones that have changed. It will also
remove all peers that were added manually by the user (and not by my
daemon).

For now I will either use `wg syncconf`, or maybe `wg addconf` +
multiple `wg set peer remove`.

I've been thinking, how stable is the IPC protocol? It might be nice to
have a tool/daemon/something that makes it possible to use the protocol
to configure devices that natively use netlink or OpenBSD ioctls.

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 148 bytes --]

_______________________________________________
WireGuard mailing list
WireGuard@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/wireguard

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-02-14 23:38 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-07 20:00 [PATCH] Implement reading keys from stdin Hristo Venev
2020-02-08 22:20 ` Jason A. Donenfeld
2020-02-09  0:15   ` Hristo Venev
2020-02-14 11:17     ` Jason A. Donenfeld
2020-02-14 23:38       ` Hristo Venev
2020-02-14 11:12   ` Mantas Mikulėnas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).