Development discussion of WireGuard
 help / color / mirror / Atom feed
* [PATCH] wg-quick linux: Add strip-and-eval cmd to extract keys from PostUp
@ 2020-10-04 21:20 Robin Schneider
  2020-10-09 12:20 ` Jason A. Donenfeld
  0 siblings, 1 reply; 6+ messages in thread
From: Robin Schneider @ 2020-10-04 21:20 UTC (permalink / raw)
  To: wireguard

The manpage mentions the trick to use PostUp to read the PrivateKey (or
PresharedKey) from a command (or file). However, when you actually use
that you notice that this is currently not fully supported. The issue is
that

```Shell
wg syncconf wgnet0 <(wg-quick strip wgnet0)
```

from the manpage now breaks the VPN because it *removes* the private key
from the WireGuard interface. The reason is that `strip` removes PostUp
of course.

This patch tries to add full support to read WireGuard keys from files
or command outputs by evaluating PostUp using a best effort approach
(using regex). It will not work for everything but when you follow the
manpage closely, it will work.

I also propose to update the systemd template to make seamless use of
this. This is not a must because the sysadmin can easily change the
ExecReload using systemd drop-in files.

Note that the patchset is incomplete (currently only for Linux).
I don’t have all the other OSes laying around. When the patch looks ok, 
I can apply it to the other versions also.

Example use of this patch:
https://github.com/ypid/ansible-wireguard/tree/prepare-for-debops

Signed-off-by: Robin Schneider <ypid@riseup.net>
---
 src/man/wg-quick.8            |  9 +++++++++
 src/systemd/wg-quick@.service |  2 +-
 src/wg-quick/linux.bash       | 36 ++++++++++++++++++++++++++++++++---
 3 files changed, 43 insertions(+), 4 deletions(-)

diff --git a/src/man/wg-quick.8 b/src/man/wg-quick.8
index b84eb64..f620704 100644
--- a/src/man/wg-quick.8
+++ b/src/man/wg-quick.8
@@ -13,6 +13,8 @@ wg-quick - set up a WireGuard interface simply
 .I save
 |
 .I strip
+|
+.I strip-and-eval
 ] [
 .I CONFIG_FILE
 |
@@ -34,6 +36,7 @@ with all
 .BR wg-quick (8)-specific
 options removed, suitable for use with
 .BR wg (8).
+Use \fIstrip-and-eval\fP in case you use `PostUp' to read PrivateKey or PresharedKey from a file or command and need them in the output.
 
 \fICONFIG_FILE\fP is a configuration file, whose filename is the interface name
 followed by `.conf'. Otherwise, \fIINTERFACE\fP is an interface name, with configuration
@@ -256,6 +259,12 @@ sessions:
 
 \fB    # wg syncconf wgnet0 <(wg-quick strip wgnet0)\fP
 
+\fIstrip-and-eval\fP additionally extracts PrivateKey and PresharedKey from `PostUp` statements and translates them into configuration that
+.BR wg (8)
+does understand:
+
+\fB    # wg syncconf wgnet0 <(wg-quick strip-and-eval wgnet0)\fP
+
 .SH SEE ALSO
 .BR wg (8),
 .BR ip (8),
diff --git a/src/systemd/wg-quick@.service b/src/systemd/wg-quick@.service
index dbdab44..1d59446 100644
--- a/src/systemd/wg-quick@.service
+++ b/src/systemd/wg-quick@.service
@@ -15,7 +15,7 @@ Type=oneshot
 RemainAfterExit=yes
 ExecStart=/usr/bin/wg-quick up %i
 ExecStop=/usr/bin/wg-quick down %i
-ExecReload=/bin/bash -c 'exec /usr/bin/wg syncconf %i <(exec /usr/bin/wg-quick strip %i)'
+ExecReload=/bin/bash -c 'exec /usr/bin/wg syncconf %i <(exec /usr/bin/wg-quick strip-and-eval %i)'
 Environment=WG_ENDPOINT_RESOLUTION_RETRIES=infinity
 
 [Install]
diff --git a/src/wg-quick/linux.bash b/src/wg-quick/linux.bash
index e4d4c4f..89f9ef9 100755
--- a/src/wg-quick/linux.bash
+++ b/src/wg-quick/linux.bash
@@ -38,8 +38,11 @@ die() {
 }
 
 parse_options() {
+	local parsing_mode part_of_command peer_pubkey
 	local interface_section=0 line key value stripped v
+	declare -A peer_pubkey_to_psk
 	CONFIG_FILE="$1"
+	parsing_mode="${2:-safe}"
 	[[ $CONFIG_FILE =~ ^[a-zA-Z0-9_=+.-]{1,15}$ ]] && CONFIG_FILE="/etc/wireguard/$CONFIG_FILE.conf"
 	[[ -e $CONFIG_FILE ]] || die "\`$CONFIG_FILE' does not exist"
 	[[ $CONFIG_FILE =~ (^|/)([a-zA-Z0-9_=+.-]{1,15})\.conf$ ]] || die "The config file must be a valid interface name, followed by .conf"
@@ -63,12 +66,35 @@ parse_options() {
 			Table) TABLE="$value"; continue ;;
 			PreUp) PRE_UP+=( "$value" ); continue ;;
 			PreDown) PRE_DOWN+=( "$value" ); continue ;;
-			PostUp) POST_UP+=( "$value" ); continue ;;
+			PostUp) POST_UP+=( "$value" );
+				if [[ $parsing_mode == "unsafe" ]]; then
+					part_of_command=""
+					if [[ $value =~ ^wg\ +set\ +%i\ +private-key\ +(.+)$ ]]; then
+						key='PrivateKey'
+						part_of_command="${BASH_REMATCH[1]}"
+					elif [[ $value =~ ^wg\ +set\ +%i\ +peer\ +(.+)\ +preshared-key\ +(.+)$ ]]; then
+						key='PresharedKey'
+						peer_pubkey="${BASH_REMATCH[1]}"
+						part_of_command="${BASH_REMATCH[2]}"
+					fi
+					if [[ -n "$part_of_command" ]]; then
+						part_of_command="${part_of_command//%i/$INTERFACE}"
+						value="$(eval "cat $part_of_command")"
+						case "$key" in
+							PresharedKey) peer_pubkey_to_psk["$peer_pubkey"]="$value" ;;
+							*) WG_CONFIG+="$key = $value"$'\n' ;;
+						esac
+					fi
+				fi
+				continue ;;
 			PostDown) POST_DOWN+=( "$value" ); continue ;;
 			SaveConfig) read_bool SAVE_CONFIG "$value"; continue ;;
 			esac
 		fi
 		WG_CONFIG+="$line"$'\n'
+		if [[ $interface_section -eq 0 && $key == 'PublicKey' && -n "${peer_pubkey_to_psk[$value]}" ]]; then
+			WG_CONFIG+="PresharedKey = ${peer_pubkey_to_psk[$value]}"$'\n'
+		fi
 	done < "$CONFIG_FILE"
 	shopt -u nocasematch
 }
@@ -224,7 +250,7 @@ add_default() {
 	cmd ip $proto rule add not fwmark $table table $table
 	cmd ip $proto rule add table main suppress_prefixlength 0
 
-	local marker="-m comment --comment \"wg-quick(8) rule for $INTERFACE\"" restore=$'*raw\n' nftable="wg-quick-$INTERFACE" nftcmd 
+	local marker="-m comment --comment \"wg-quick(8) rule for $INTERFACE\"" restore=$'*raw\n' nftable="wg-quick-$INTERFACE" nftcmd
 	printf -v nftcmd '%sadd table %s %s\n' "$nftcmd" "$pf" "$nftable"
 	printf -v nftcmd '%sadd chain %s %s preraw { type filter hook prerouting priority -300; }\n' "$nftcmd" "$pf" "$nftable"
 	printf -v nftcmd '%sadd chain %s %s premangle { type filter hook prerouting priority -150; }\n' "$nftcmd" "$pf" "$nftable"
@@ -298,7 +324,7 @@ execute_hooks() {
 
 cmd_usage() {
 	cat >&2 <<-_EOF
-	Usage: $PROGRAM [ up | down | save | strip ] [ CONFIG_FILE | INTERFACE ]
+	Usage: $PROGRAM [ up | down | save | strip | strip-and-eval ] [ CONFIG_FILE | INTERFACE ]
 
 	  CONFIG_FILE is a configuration file, whose filename is the interface name
 	  followed by \`.conf'. Otherwise, INTERFACE is an interface name, with
@@ -381,6 +407,10 @@ elif [[ $# -eq 2 && $1 == strip ]]; then
 	auto_su
 	parse_options "$2"
 	cmd_strip
+elif [[ $# -eq 2 && $1 == strip-and-eval ]]; then
+	auto_su
+	parse_options "$2" "unsafe"
+	cmd_strip
 else
 	cmd_usage
 	exit 1
-- 
2.20.1


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

* Re: [PATCH] wg-quick linux: Add strip-and-eval cmd to extract keys from PostUp
  2020-10-04 21:20 [PATCH] wg-quick linux: Add strip-and-eval cmd to extract keys from PostUp Robin Schneider
@ 2020-10-09 12:20 ` Jason A. Donenfeld
  2020-10-10 15:57   ` Robin Schneider
  0 siblings, 1 reply; 6+ messages in thread
From: Jason A. Donenfeld @ 2020-10-09 12:20 UTC (permalink / raw)
  To: Robin Schneider; +Cc: WireGuard mailing list

This seems like a weird inconsistent hack. Strip should return
something that is acted on by something else, and not also do things.

But I have another suggestion on how to achieve what you want:

wg syncconf wg0 <(printf '[Interface]\nPrivateKey=%s\n' "$(wg show wg0
private-key)"; wg-quick strip wg0)

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

* Re: [PATCH] wg-quick linux: Add strip-and-eval cmd to extract keys from PostUp
  2020-10-09 12:20 ` Jason A. Donenfeld
@ 2020-10-10 15:57   ` Robin Schneider
  2020-10-12 15:01     ` Jason A. Donenfeld
  0 siblings, 1 reply; 6+ messages in thread
From: Robin Schneider @ 2020-10-10 15:57 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: WireGuard mailing list

Hi Jason,

On 2020-10-09 14:20, Jason A. Donenfeld wrote:
> This seems like a weird inconsistent hack. Strip should return
> something that is acted on by something else, and not also do things.

I know. Thats why I made this clear by making it a separate subcommand so that users can choose if they want this hack or not.

> 
> But I have another suggestion on how to achieve what you want:
> 
> wg syncconf wg0 <(printf '[Interface]\nPrivateKey=%s\n' "$(wg show wg0
> private-key)"; wg-quick strip wg0)
> 

At first I considered/implemented such a workaround on the systemd level. It is good to know that I would not have to care about merging two INI files as `wg` can handle two `Interface` sections and merge them together itself as it seems.

The proposed workaround has two issues:

1. It does not allow to replace the PrivateKey or PresharedKey using the `syncconf` now from the config file which is what I want to have.
2. It only outputs the PrivateKey and not the PresharedKey for each peer. Sure, this could be done with a for loop.

-- 
Live long and prosper
Robin `ypid` Schneider -- https://me.ypid.de/

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

* Re: [PATCH] wg-quick linux: Add strip-and-eval cmd to extract keys from PostUp
  2020-10-10 15:57   ` Robin Schneider
@ 2020-10-12 15:01     ` Jason A. Donenfeld
  2020-10-12 19:54       ` Robin Schneider
  0 siblings, 1 reply; 6+ messages in thread
From: Jason A. Donenfeld @ 2020-10-12 15:01 UTC (permalink / raw)
  To: Robin Schneider; +Cc: WireGuard mailing list

On Sat, Oct 10, 2020 at 5:58 PM Robin Schneider <ypid@riseup.net> wrote:
> The proposed workaround has two issues:
>
> 1. It does not allow to replace the PrivateKey or PresharedKey using the `syncconf` now from the config file which is what I want to have.
> 2. It only outputs the PrivateKey and not the PresharedKey for each peer. Sure, this could be done with a for loop.

Sounds like something easily achievable with scripting.

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

* Re: [PATCH] wg-quick linux: Add strip-and-eval cmd to extract keys from PostUp
  2020-10-12 15:01     ` Jason A. Donenfeld
@ 2020-10-12 19:54       ` Robin Schneider
  2020-10-12 22:31         ` Jason A. Donenfeld
  0 siblings, 1 reply; 6+ messages in thread
From: Robin Schneider @ 2020-10-12 19:54 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: WireGuard mailing list

On 2020-10-12 17:01, Jason A. Donenfeld wrote:
> On Sat, Oct 10, 2020 at 5:58 PM Robin Schneider <ypid@riseup.net> wrote:
>> The proposed workaround has two issues:
>>
>> 1. It does not allow to replace the PrivateKey or PresharedKey using the `syncconf` now from the config file which is what I want to have.
>> 2. It only outputs the PrivateKey and not the PresharedKey for each peer. Sure, this could be done with a for loop.
> 
> Sounds like something easily achievable with scripting.
> 

So you suggest to write a 3party script instead of putting it into wg-quick? Because a inline script with support for all of this wound not exactly be a one-liner :) Could such a script be included in the wireguard-tools?

-- 
Live long and prosper
Robin `ypid` Schneider -- https://me.ypid.de/

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

* Re: [PATCH] wg-quick linux: Add strip-and-eval cmd to extract keys from PostUp
  2020-10-12 19:54       ` Robin Schneider
@ 2020-10-12 22:31         ` Jason A. Donenfeld
  0 siblings, 0 replies; 6+ messages in thread
From: Jason A. Donenfeld @ 2020-10-12 22:31 UTC (permalink / raw)
  To: Robin Schneider; +Cc: WireGuard mailing list

On Mon, Oct 12, 2020 at 9:54 PM Robin Schneider <ypid@riseup.net> wrote:
>
> On 2020-10-12 17:01, Jason A. Donenfeld wrote:
> > On Sat, Oct 10, 2020 at 5:58 PM Robin Schneider <ypid@riseup.net> wrote:
> >> The proposed workaround has two issues:
> >>
> >> 1. It does not allow to replace the PrivateKey or PresharedKey using the `syncconf` now from the config file which is what I want to have.
> >> 2. It only outputs the PrivateKey and not the PresharedKey for each peer. Sure, this could be done with a for loop.
> >
> > Sounds like something easily achievable with scripting.
> >
>
> So you suggest to write a 3party script instead of putting it into wg-quick? Because a inline script with support for all of this wound not exactly be a one-liner :) Could such a script be included in the wireguard-tools?

Yes, write your own script for this kind of custom merging of
PostUp/PreUp state modification, instead of trying to parse bash with
a regex as this commit does.

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

end of thread, other threads:[~2020-10-12 22:31 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-04 21:20 [PATCH] wg-quick linux: Add strip-and-eval cmd to extract keys from PostUp Robin Schneider
2020-10-09 12:20 ` Jason A. Donenfeld
2020-10-10 15:57   ` Robin Schneider
2020-10-12 15:01     ` Jason A. Donenfeld
2020-10-12 19:54       ` Robin Schneider
2020-10-12 22:31         ` Jason A. Donenfeld

Development discussion of WireGuard

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://inbox.vuxu.org/wireguard

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 wireguard wireguard/ http://inbox.vuxu.org/wireguard \
		wireguard@lists.zx2c4.com
	public-inbox-index wireguard

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.vuxu.org/vuxu.archive.wireguard


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git