[SOLVED] Extraneous Whitespace from WireGuard Key Generation

[utkonos]

I've been digging into the WireGuard configuration for a project I'm working on and noticed that each time the public and private keys are generated, there are extra newlines and whitespace added to the configuration XML. This whitespace is then removed whenever the WireGuard config is modified for any reason going forward. I've tracked down the source: "tee" is used to write the keys to a file and print them to stdout simultanously, the output to stdout being captured in the config.xml file. This command line adds newlines which cannot be silenced (I checked the tee man page).

This is where tee is used:

https://github.com/opnsense/plugins/blob/3bcfab38f6ea265bf23b5b01eccc4e82f75fbb4e/net/wireguard/src/opnsense/scripts/OPNsense/Wireguard/genkey.sh#L37-L45

Here is where that shell script is called in PHP:

https://github.com/opnsense/plugins/blob/3bcfab38f6ea265bf23b5b01eccc4e82f75fbb4e/net/wireguard/src/opnsense/mvc/app/controllers/OPNsense/Wireguard/Api/ServerController.php#L48-L93

There are three potential fixes along with "just ignore it / who cares".

1. In genkey.sh, the output from tee can be piped to tr: "tee ${TMPDIR}/wireguard.priv | tr -d '\n'" as one example of two.
2. In ServerController.php, each of the four locations where the strings are stored in the config get wrapped in rtrim(): "$node->privkey = rtrim($keyspriv);" as one example of four.
3. Whatever function pretty prints the config XML before writing to file should be called after these modifications are done. I have not found this pretty print function or step, so I don't know what to do for it, but there is clearly a pretty print function being called after subsequent modifications to the config because this whitespace is removed any other time the config is modified.

I can make a PR, but I didn't want to until I check here in case the OPNsense maintainers decide that "just ignore it / who cares" is what they wish.



And, yes, I know that is a private key in the image. It was generated for this screenshot and is not used anywhere.

[franco]

There is a trim() missing here

https://github.com/opnsense/plugins/blob/3bcfab38f6ea265bf23b5b01eccc4e82f75fbb4e/net/wireguard/src/opnsense/mvc/app/controllers/OPNsense/Wireguard/Api/ServerController.php#L62-L63

and here

https://github.com/opnsense/plugins/blob/3bcfab38f6ea265bf23b5b01eccc4e82f75fbb4e/net/wireguard/src/opnsense/mvc/app/controllers/OPNsense/Wireguard/Api/ServerController.php#L87-L88

PRs welcome.

Background info: the backend daemon has extra characters at the end to signify the end of the command output. Normally we only parse this or convert from JSON. Here it is injected into the configuration verbatim causing these extra newlines.

Looks like there are a number of calls that use trim() correctly already in plugins alone...

% git grep 'trim.*configd'


Cheers,
Franco

[utkonos]

PR:

https://github.com/opnsense/plugins/pull/2982