Development discussion of WireGuard
 help / color / mirror / Atom feed
* [PATCH v2] wg: Allow config to read secret keys from file
@ 2023-02-19 18:23 Daniel Gröber
  2023-02-19 18:41 ` [PATCH v2] " karog
  0 siblings, 1 reply; 2+ messages in thread
From: Daniel Gröber @ 2023-02-19 18:23 UTC (permalink / raw)
  To: wireguard; +Cc: Jason A . Donenfeld, Daniel Gröber, Michael Tokarev

This adds two new config keys PrivateKeyFile= and PresharedKeyFile= that
simply hook up the existing code for the `wg set ... private-key /file`
codepath.

By using the new options wireguard configs can become a lot easier to
manage and deploy as we don't have to treat them as secrets anymore. This
way they can, for example, be tracked in public git repos while the secret
keys can be provisioned using an out of band system or with a manual
one-time step instead.

Before this patch we were using an ugly hack: it's possible to simply omit
PrivateKey= and set it using `PostUp = wg set %i private-key /some/file`.
However this breaks when we try to use setconf or synconf as they
will (rightly) unset the private key when it's missing in the underlying
config file breaking connectivity.

Reviewed-By: Michael Tokarev <mjt@tls.msk.ru>
Signed-off-by: Daniel Gröber <dxld@darkboxed.org>
---
 src/config.c | 8 ++++++++
 src/man/wg.8 | 4 ++++
 2 files changed, 12 insertions(+)

diff --git a/src/config.c b/src/config.c
index e8db900..f9980fe 100644
--- a/src/config.c
+++ b/src/config.c
@@ -464,6 +464,10 @@ static bool process_line(struct config_ctx *ctx, const char *line)
 			ret = parse_key(ctx->device->private_key, value);
 			if (ret)
 				ctx->device->flags |= WGDEVICE_HAS_PRIVATE_KEY;
+		} else if (key_match("PrivateKeyFile")) {
+			ret = parse_keyfile(ctx->device->private_key, value);
+			if (ret)
+				ctx->device->flags |= WGDEVICE_HAS_PRIVATE_KEY;
 		} else
 			goto error;
 	} else if (ctx->is_peer_section) {
@@ -483,6 +487,10 @@ static bool process_line(struct config_ctx *ctx, const char *line)
 			ret = parse_key(ctx->last_peer->preshared_key, value);
 			if (ret)
 				ctx->last_peer->flags |= WGPEER_HAS_PRESHARED_KEY;
+		} else if (key_match("PresharedKeyFile")) {
+			ret = parse_keyfile(ctx->last_peer->preshared_key, value);
+			if (ret)
+				ctx->last_peer->flags |= WGPEER_HAS_PRESHARED_KEY;
 		} else
 			goto error;
 	} else
diff --git a/src/man/wg.8 b/src/man/wg.8
index fd9fde7..48f084d 100644
--- a/src/man/wg.8
+++ b/src/man/wg.8
@@ -134,6 +134,8 @@ The \fIInterface\fP section may contain the following fields:
 .IP \(bu
 PrivateKey \(em a base64 private key generated by \fIwg genkey\fP. Required.
 .IP \(bu
+PrivateKeyFile \(em path to a file containing a base64 private key. May be used instead of \fIPrivateKey\fP. Optional.
+.IP \(bu
 ListenPort \(em a 16-bit port for listening. Optional; if not specified, chosen
 randomly.
 .IP \(bu
@@ -151,6 +153,8 @@ and may be omitted. This option adds an additional layer of symmetric-key
 cryptography to be mixed into the already existing public-key cryptography,
 for post-quantum resistance.
 .IP \(bu
+PresharedKeyFile \(em path to a file containing a base64 preshared key. May be used instead of \fIPresharedKey\fP. Optional.
+.IP \(bu
 AllowedIPs \(em a comma-separated list of IP (v4 or v6) addresses with
 CIDR masks from which incoming traffic for this peer is allowed and to
 which outgoing traffic for this peer is directed. The catch-all
-- 
2.30.2


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

* Re: [PATCH v2] Allow config to read secret keys from file
  2023-02-19 18:23 [PATCH v2] wg: Allow config to read secret keys from file Daniel Gröber
@ 2023-02-19 18:41 ` karog
  0 siblings, 0 replies; 2+ messages in thread
From: karog @ 2023-02-19 18:41 UTC (permalink / raw)
  To: Daniel Gröber; +Cc: wireguard, Jason A . Donenfeld, Michael Tokarev

Instead of using new config keys, did you consider using special values for this case like

PrivateKey=file:/path/to/key
PresharedKey=file:/path/to/preshared

In addition to not proliferating new config keys, it also prevents the possibility of erring by including both PrivateKey and PrivateKeyFile

This kind of syntax is used in systemd service files for things like StandardOut and StandardError

karog

> On Feb 19, 2023, at 1:23 PM, Daniel Gröber <dxld@darkboxed.org> wrote:
> 
> This adds two new config keys PrivateKeyFile= and PresharedKeyFile= that
> simply hook up the existing code for the `wg set ... private-key /file`
> codepath.
> 
> By using the new options wireguard configs can become a lot easier to
> manage and deploy as we don't have to treat them as secrets anymore. This
> way they can, for example, be tracked in public git repos while the secret
> keys can be provisioned using an out of band system or with a manual
> one-time step instead.
> 
> Before this patch we were using an ugly hack: it's possible to simply omit
> PrivateKey= and set it using `PostUp = wg set %i private-key /some/file`.
> However this breaks when we try to use setconf or synconf as they
> will (rightly) unset the private key when it's missing in the underlying
> config file breaking connectivity.
> 
> Reviewed-By: Michael Tokarev <mjt@tls.msk.ru>
> Signed-off-by: Daniel Gröber <dxld@darkboxed.org>
> ---
> src/config.c | 8 ++++++++
> src/man/wg.8 | 4 ++++
> 2 files changed, 12 insertions(+)
> 
> diff --git a/src/config.c b/src/config.c
> index e8db900..f9980fe 100644
> --- a/src/config.c
> +++ b/src/config.c
> @@ -464,6 +464,10 @@ static bool process_line(struct config_ctx *ctx, const char *line)
> 			ret = parse_key(ctx->device->private_key, value);
> 			if (ret)
> 				ctx->device->flags |= WGDEVICE_HAS_PRIVATE_KEY;
> +		} else if (key_match("PrivateKeyFile")) {
> +			ret = parse_keyfile(ctx->device->private_key, value);
> +			if (ret)
> +				ctx->device->flags |= WGDEVICE_HAS_PRIVATE_KEY;
> 		} else
> 			goto error;
> 	} else if (ctx->is_peer_section) {
> @@ -483,6 +487,10 @@ static bool process_line(struct config_ctx *ctx, const char *line)
> 			ret = parse_key(ctx->last_peer->preshared_key, value);
> 			if (ret)
> 				ctx->last_peer->flags |= WGPEER_HAS_PRESHARED_KEY;
> +		} else if (key_match("PresharedKeyFile")) {
> +			ret = parse_keyfile(ctx->last_peer->preshared_key, value);
> +			if (ret)
> +				ctx->last_peer->flags |= WGPEER_HAS_PRESHARED_KEY;
> 		} else
> 			goto error;
> 	} else
> diff --git a/src/man/wg.8 b/src/man/wg.8
> index fd9fde7..48f084d 100644
> --- a/src/man/wg.8
> +++ b/src/man/wg.8
> @@ -134,6 +134,8 @@ The \fIInterface\fP section may contain the following fields:
> .IP \(bu
> PrivateKey \(em a base64 private key generated by \fIwg genkey\fP. Required.
> .IP \(bu
> +PrivateKeyFile \(em path to a file containing a base64 private key. May be used instead of \fIPrivateKey\fP. Optional.
> +.IP \(bu
> ListenPort \(em a 16-bit port for listening. Optional; if not specified, chosen
> randomly.
> .IP \(bu
> @@ -151,6 +153,8 @@ and may be omitted. This option adds an additional layer of symmetric-key
> cryptography to be mixed into the already existing public-key cryptography,
> for post-quantum resistance.
> .IP \(bu
> +PresharedKeyFile \(em path to a file containing a base64 preshared key. May be used instead of \fIPresharedKey\fP. Optional.
> +.IP \(bu
> AllowedIPs \(em a comma-separated list of IP (v4 or v6) addresses with
> CIDR masks from which incoming traffic for this peer is allowed and to
> which outgoing traffic for this peer is directed. The catch-all
> -- 
> 2.30.2


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

end of thread, other threads:[~2023-02-20 11:37 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-19 18:23 [PATCH v2] wg: Allow config to read secret keys from file Daniel Gröber
2023-02-19 18:41 ` [PATCH v2] " karog

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).