Development discussion of WireGuard
 help / color / mirror / Atom feed
* [PATCH] wg: Allow config to read private key from file
@ 2022-11-20 22:46 Daniel Gröber
  2022-11-21  6:31 ` Michael Tokarev
  0 siblings, 1 reply; 3+ messages in thread
From: Daniel Gröber @ 2022-11-20 22:46 UTC (permalink / raw)
  To: wireguard; +Cc: Jason A . Donenfeld, Daniel Gröber

This adds a new config key PrivateKeyFile= that simply hooks up the
existing code for the `wg set ... private-key /file` codepath.

Using this new option the interface configs can be much easier to deploy in
an automated fashion as they don't contain secrets anymore. The private key
can easily be provisioned out of band or using a one-time provisioning step
instead.

Before this patch we were using a neat 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 instead of leaving it as-is.
---
 src/config.c | 4 ++++
 src/man/wg.8 | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/src/config.c b/src/config.c
index e8db900..49cbb07 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) {
diff --git a/src/man/wg.8 b/src/man/wg.8
index fd9fde7..1d37338 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 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
-- 
2.30.2


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

* Re: [PATCH] wg: Allow config to read private key from file
  2022-11-20 22:46 [PATCH] wg: Allow config to read private key from file Daniel Gröber
@ 2022-11-21  6:31 ` Michael Tokarev
  2022-11-21 13:28   ` dxld
  0 siblings, 1 reply; 3+ messages in thread
From: Michael Tokarev @ 2022-11-21  6:31 UTC (permalink / raw)
  To: Daniel Gröber, wireguard; +Cc: Jason A . Donenfeld

21.11.2022 01:46, Daniel Gröber wrote:
> This adds a new config key PrivateKeyFile= that simply hooks up the
> existing code for the `wg set ... private-key /file` codepath.
> 
> Using this new option the interface configs can be much easier to deploy in
> an automated fashion as they don't contain secrets anymore. The private key
> can easily be provisioned out of band or using a one-time provisioning step
> instead.

This is definitely a very welcome option in my PoV.

Add my
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>

for this.

> Before this patch we were using a neat hack: it's possible to simply omit
> PrivateKey= and set it using PostUp= wg set %i private-key /some/file.

Well, this isn't really neat, it is a hackish workaround for the missing
functionality ;)

On a side, note, almost a year ago I sent a patch for wg utility to recognize
and discard some keywords which are processed by wg-quick script - like,
Address=. This way, there's no need to pre-process the config file anymore,
and in order to recognize more peers, one doesn't have to restart the
tunnel interface, instead, a regular wg syncconf wgif.conf is sufficient,
and many things can be simplified too (removing the preprocessing).
I've never got any reply for these patches.

/mjt

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

* Re: [PATCH] wg: Allow config to read private key from file
  2022-11-21  6:31 ` Michael Tokarev
@ 2022-11-21 13:28   ` dxld
  0 siblings, 0 replies; 3+ messages in thread
From: dxld @ 2022-11-21 13:28 UTC (permalink / raw)
  To: Michael Tokarev; +Cc: wireguard, Jason A . Donenfeld

Hi Michael,

On Mon, Nov 21, 2022 at 09:31:41AM +0300, Michael Tokarev wrote:
> 21.11.2022 01:46, Daniel Gröber wrote:
> > Using this new option the interface configs can be much easier to deploy in
> > an automated fashion as they don't contain secrets anymore. The private key
> > can easily be provisioned out of band or using a one-time provisioning step
> > instead.
> 
> This is definitely a very welcome option in my PoV.
> 
> Add my
> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>

I think you mean Reviewed-By? Speaking of which I actually forgot the
signoff myself. Doh.

Is Reviewed-By something we do here? I can't find a single such tag with
`git log --grep Reviewed-By`. I appreciate the positive response nontheless
though :)

> > Before this patch we were using a neat hack: it's possible to simply omit
> > PrivateKey= and set it using PostUp= wg set %i private-key /some/file.
> 
> Well, this isn't really neat, it is a hackish workaround for the missing
> functionality ;)

It does work surprisingly well though :D. I just re-set the private-key
after syncconf now, which definetly ought to loose some traffic but it
works at least ;)

> On a side, note, almost a year ago I sent a patch for wg utility to recognize
> and discard some keywords which are processed by wg-quick script - like,
> Address=. This way, there's no need to pre-process the config file anymore,
> and in order to recognize more peers, one doesn't have to restart the
> tunnel interface, instead, a regular wg syncconf wgif.conf is sufficient,
> and many things can be simplified too (removing the preprocessing).

Ok I think I found your patch[1]. So we did actually independently come up
with the idea of PrivateKeyFile, interesting. Also you support PresharedKey
too. I realised I forgot that one right after sending the patch obv. ;)
I'll send a v2 for that soon.

[1]: https://lists.zx2c4.com/pipermail/wireguard/2021-January/006346.html

As for ignoring the wg-quick options, I'm not sure what's the right way to
go there. I don't find the wg-quick strip approach toooo taxing but it sure
would be more convenient to just call one tool.

> I've never got any reply for these patches.

I have another patch pending for a longish while aswell "wg: Support
restricting address family of DNS resolved Endpoint". IMO you should have
just resent your series every couple of months :)

--Daniel

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

end of thread, other threads:[~2022-11-21 13:29 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-20 22:46 [PATCH] wg: Allow config to read private key from file Daniel Gröber
2022-11-21  6:31 ` Michael Tokarev
2022-11-21 13:28   ` dxld

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