feat(settings): load wireguard inidivual fields as secret files

[DennisGaida]

Currently docker secrets aren't supported for wireguard environment variables, but should.

[qdm12]

Thanks for the contribution!

However, I am closing this because the implementation for secret files is not part of this PR, and because

• feat(settings): support Wireguard ini config files #1120 will get merged soon
• We can then have that single ini wireguard config file as a secret, instead of having to deal with multiple secret files

[DennisGaida]

Oh, I totally forgot to check-in the implementation at /internal/secrets. I could do so, but not sure whether you want it.

The PR #1120 is also interesting, but not what I would actually want: Secrets management is hard and I don't want a config file with my private key just dangling somewhere to be mounted. I want secrets to be in docker secrets since that is what the system is designed for (and I can use third party solutions to manage these secrets).
And we do have exactly these secrets for OpenSSH (OPENVPN_USER_SECRETFILE & OPENVPN_PASSWORD_SECRETFILE), so I believe this should be done for all secrets including Wireguard.

If you are still interested I'll gladly add the implementation to this PR and I don't think this would hinder adding a wireguard config import funtionality.

[DennisGaida]

Sorry for forgetting the implementation:

• secrets\wireguard.go follows openvpn.go closely in style and is able to set the private key, preshared key as well as the addresses
• helpers.go received the readWireguardAddress function from env\wireguard.go to be used from the original file as well as the new secrets\wireguard.go
• vpn.go now also calls readWireguard()

[qdm12]

I fixed & updated your code (compilation, some logic was wrong etc.); however whilst doing this, it still bugs be regarding the ini wireguard secret file to be added soon. Since both are from the 'secrets' settings source, which should have precedence? The precedence system works by source and I want to keep it that way to avoid having horrible shadowy logic here and there.

I still see no point to have separate secret files instead of one standardized wireguard ini file as a secret file (most users would just bring over some wireguard ini file from somewhere).

I'll change this to a draft PR, and will likely close it once we have a secret ini file setup soon-ish (after release v3.33.0). Although you can now build the image using your patch-1 branch and use it if you want 😉

[DennisGaida]

Thanks for the fixes!

Personally I didn't have any wireguard.ini file, just the private key & addresses, hence a complete wireguard.ini would be more hassle than just passing in two environment variables / secrets. The benefit of having separate secret files is also less dependency. I may be able to re-use that wireguard private key for another container / service / app, the wireguard.ini file I may not use like that because maybe the other service is using a different format. The WIREGUARD_ADDRESSES I actually do reuse in multiple ways, especially outside firewall, traffic filter etc. - because I only define these address ranges once, they can be re-used everywhere instead of being hidden within some wireguard conf file.

Please do as you see fit with the PR, you are the maintainer 👍

[qdm12]

Good points, I'll give it more thoughts then! 👍 I'll get back to you 😉

[qdm12]

We'll have secret files for everything using this precedence (blocked by #1759):

1. Secret ini file
2. Secret single values (goal of this pr)
3. ini file
4. environment variables

Both 1 and 2 will be from the same "secrets" source, so there will have to be a bit of ugly logic there to read both and merge settings, but that's fine.

You should also be able to specify a mix of all of them, for example have only a server public key in the ini file and the private key as a single secret file.

Now I'm a bit shared if 1. or 2. should be first, but to respect existing order for non secret sources, I think having the ini secret file have precedence makes more sense.

[DennisGaida]

Totally agreed. If there is an ini file it trumps all. Ultimately that is what WireGuard uses for configuration.

[qdm12]

6096b7a reads a wireguard config file from /run/secrets/wg0.conf (changeable path with WIREGUARD_CONF_SECRETFILE), filling the first step 1..

I've updated your branch to fill the second step 2. 😉 although from a users' standpoint it does exactly what you added (same paths, variable names).

3. is done with PR feat(settings): support Wireguard ini config files #1120 (8 months ago), and env variables were already there for 4.

All in all, I think we should be good now 😉

[qdm12]

FYI now, if a user sets a wireguard secret config file, all fields are extracted, not just private key, preshared key and addresses. This makes more sense than before where it would only extract sensitive fields.