WireGuard: what's wrong with this automatic IP assignment

Question

WireGuard is extremely simple and fast kernel-space VPN based on modern cryptography. I want to use it in production and need automatic IP assignment for new peers. The project provides two short scripts for server and client that do just this. However it states:

Do not use these scripts in production. They are simply a demonstration of how easy the wg(8) tool is at the command line, but by no means should you actually attempt to use these. They are horribly insecure and defeat the purpose of WireGuard. STAY AWAY!

The scripts are:
Server:

#!/bin/bash
# SPDX-License-Identifier: GPL-2.0
#
# Copyright (C) 2015-2018 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.

if [[ -z $NCAT_REMOTE_ADDR ]]; then
    ip link del dev wg0 2>/dev/null
    set -e
    ip link add dev wg0 type wireguard
    ip address add 192.168.4.1/24 dev wg0
    wg set wg0 private-key <(wg genkey) listen-port 12912
    ip link set up dev wg0
    exec ncat -e "$(readlink -f "$0")" -k -l -p 42912 -v
fi
read -r public_key
[[ $(wg show wg0 peers | wc -l) -ge 253 ]] && wg set wg0 peer $(wg show wg0 latest-handshakes | sort -k 2 -b -n | head -n 1 | cut -f 1) remove
next_ip=$(all="$(wg show wg0 allowed-ips)"; for ((i=2; i<=254; i++)); do ip="192.168.4.$i"; [[ $all != *$ip/32* ]] && echo $ip && break; done)
wg set wg0 peer "$public_key" allowed-ips $next_ip/32 2>/dev/null && echo "OK:$(wg show wg0 private-key | wg pubkey):$(wg show wg0 listen-port):$next_ip" || echo ERROR

Client:

#!/bin/bash
# SPDX-License-Identifier: GPL-2.0
#
# Copyright (C) 2015-2018 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.

set -e
[[ $UID == 0 ]] || { echo "You must be root to run this."; exit 1; }
umask 077
trap 'rm -f /tmp/wg_private_key' EXIT INT TERM
exec 3<>/dev/tcp/demo.wireguard.com/42912
wg genkey | tee /tmp/wg_private_key | wg pubkey >&3
IFS=: read -r status server_pubkey server_port internal_ip <&3
[[ $status == OK ]]
ip link del dev wg0 2>/dev/null || true
ip link add dev wg0 type wireguard
wg set wg0 private-key /tmp/wg_private_key peer "$server_pubkey" allowed-ips 0.0.0.0/0 endpoint "demo.wireguard.com:$server_port" persistent-keepalive 25
ip address add "$internal_ip"/24 dev wg0
ip link set up dev wg0
if [ "$1" == "default-route" ]; then
    host="$(wg show wg0 endpoints | sed -n 's/.*\t\(.*\):.*/\1/p')"
    ip route add $(ip route get $host | sed '/ via [0-9]\{1,3\}\.[0-9]\{1,3\}\.[0-9]\{1,3\}\.[0-9]\{1,3\}/{s/^\(.* via [0-9]\{1,3\}\.[0-9]\{1,3\}\.[0-9]\{1,3\}\.[0-9]\{1,3\}\).*/\1/}' | head -n 1) 2>/dev/null || true
    ip route add 0/1 dev wg0
    ip route add 128/1 dev wg0
fi

Questions:

1. What is wrong with those scripts? What is the worst case?
2. Is there a way to fix those issues?
3. Could somebody write a short comment what each line of those scripts does?

Update: the author of WireGuard has stated that "The problem is that it uses unauthenticated TCP." So what is the worst case and how can it be fixed? Can one provide this TCP socket inside of an SSH tunnel?

Answer 1

The problem boils down to your third point. You do not know what it does without reading it in detail and it contains quite a lot of unclean code.

It probably "just works", but if anything fails there is no error handling implemented. In addition it screen scrapes a few shell utilities without knowing if their output format is fixed. Maybe you upgrade your distribution, get a new version of one of the tools and suddenly the script fails.

What is the worst case

Some of the output parsing fails and the next command gets garbled output and deletes /home. Not as an actual result of analyzing the script, but something which could happen with shell scripts without proper error handling and did in the past (I.e. rm -r $uninitialized/*).

The most likely error to happen is leaving you with broken network and as you have to ask for explanation of the script (which is fine. I would need to study it quite some time before fully understanding the risks of it) this would probably force you to reboot to get the system into a consistent state again.

This is not per se insecure, but it is sloppy and may be insecure. For a script which runs as root and setups important things there should be careful error handling for each thing which may fail.

The general devise is that everything has a risk of being insecure until proven to be secure. You could try to verify the script if it is secure, but it is not worth it as there are several anti-patterns (i.e. parsing output which is not guaranteed (yet) to be stable, processing IPs with regex, no cleanup when a line fails) which make it advisable to rewrite it in a more secure manner.

People may for example rely on the VPN to prevent sending confidential company data unencrypted over the internet. When the VPN script (silently) did not succeed setting up the VPN, the data may get sent insecurely.

Is there a way to fix those issues?

Rewriting it with careful error handling after each command. Probably in a better suited language than bash and reading the data using an API instead of screen scraping other programs.

Answer 2

The worst case is you are connecting directly to your attacker and read unsanitized input from him.

This script connects your shell directly to an outside host only learned from DNS, probably half around the world, which might not be the party you expect, since a selective MITM would work quite nicely.