Add a short name attribute to peers for human friendly output

``` $ sudo wg show interface: wg0 public key: 8Lv...s= private key: (hidden) listening port: 5555 fwmark: 0xca6c peer: TqV...xWQ= endpoint: ... allowed ips: ... latest handshake: ... transfer: ... name: jabid.in peer: ubKn...yM= allowed ips: 10.0.0.2/32 ``` Checklist - [✓] Set and update name with config file - [✓] Set and update name with CLI interface - [✓] Implement the functionality in kernel module - [✗] Implement the functionality in userspace implementation - [✗] Implement and test removing name attribute
jaseemabid · Jan 29, 2019 · 544e59b4cee39ae81d68eb91242c2edf391f3639 · 544e59b
1 parent 340beb1
commit 544e59b4cee39ae81d68eb91242c2edf391f3639
Unified Split

Showing with 61 additions and 3 deletions.

+18 −1 src/netlink.c

+3 −0 src/peer.h

+19 −0 src/tools/config.c

+3 −1 src/tools/containers.h

+11 −0 src/tools/ipc.c

+1 −1 src/tools/set.c

+2 −0 src/tools/show.c

+4 −0 src/uapi/wireguard.h
diff --git a/src/netlink.c b/src/netlink.c
@@ -30,6 +30,7 @@ static const struct nla_policy device_policy[WGDEVICE_A_MAX + 1] = {
 static const struct nla_policy peer_policy[WGPEER_A_MAX + 1] = {
 	[WGPEER_A_PUBLIC_KEY]				= { .len = NOISE_PUBLIC_KEY_LEN },
 	[WGPEER_A_PRESHARED_KEY]			= { .len = NOISE_SYMMETRIC_KEY_LEN },
+  
+	[WGPEER_A_NAME]					= { .type = NLA_NUL_STRING, .len = WG_NAME_LEN - 1},
 	[WGPEER_A_FLAGS]				= { .type = NLA_U32 },
 	[WGPEER_A_ENDPOINT]				= { .len = sizeof(struct sockaddr) },
 	[WGPEER_A_PERSISTENT_KEEPALIVE_INTERVAL]	= { .type = NLA_U16 },
@@ -122,6 +123,7 @@ static int get_peer(struct wg_peer *peer, struct allowedips_cursor *rt_cursor,
 		if (nla_put(skb, WGPEER_A_LAST_HANDSHAKE_TIME,
 			    sizeof(last_handshake), &last_handshake) ||
+  
+		    nla_put(skb, WGPEER_A_NAME, sizeof(peer->name), &peer->name) ||
 		    nla_put_u16(skb, WGPEER_A_PERSISTENT_KEEPALIVE_INTERVAL,
 				peer->persistent_keepalive_interval) ||
 		    nla_put_u64_64bit(skb, WGPEER_A_TX_BYTES, peer->tx_bytes,
@@ -345,7 +347,7 @@ static int set_allowedip(struct wg_peer *peer, struct nlattr **attrs)
 static int set_peer(struct wg_device *wg, struct nlattr **attrs)
 {
-  
-	u8 *public_key = NULL, *preshared_key = NULL;
+  
+	u8 *public_key = NULL, *preshared_key = NULL, *name = NULL;
 	struct wg_peer *peer = NULL;
 	u32 flags = 0;
 	int ret;
@@ -429,6 +431,21 @@ static int set_peer(struct wg_device *wg, struct nlattr **attrs)
 		}
 	}
+  
+	if (attrs[WGPEER_A_NAME]) {
+  
+		// NOTE: What kind of validation is required before copying the
+  
+		// data to kernel? I don't want this line to become a CVE.
+  
+		if (attrs[WGPEER_A_NAME] &&
+  
+		    nla_len(attrs[WGPEER_A_NAME]) <= WG_NAME_LEN) {
+  
+			name = nla_data(attrs[WGPEER_A_NAME]);
+  
+			// Clear previous name before updating, wont work for renaming
+  
+			// to a shorter name otherwise.
+  
+			memset(&peer->name, 0, WG_NAME_LEN);
+  
+			memcpy(&peer->name, name, strlen(name));
+  
+		}
+  
+		else
+  
+			goto out;
+  
+	}
+  
+
 	if (flags & WGPEER_F_REPLACE_ALLOWEDIPS)
 		wg_allowedips_remove_by_peer(&wg->peer_allowedips, peer,
 					     &wg->device_update_lock);

diff --git a/src/peer.h b/src/peer.h
@@ -40,6 +40,9 @@ struct wg_peer {
 	struct sk_buff_head staged_packet_queue;
 	int serial_work_cpu;
 	struct noise_keypairs keypairs;
+  
+	u8 name[128];  // NOTE: This is defined in wireguard.h as WG_NAME_LEN,
+  
+		       // but not imported here. Is this duplication OK or
+  
+		       // should that be defined differently?
 	struct endpoint endpoint;
 	struct dst_cache endpoint_cache;
 	rwlock_t endpoint_lock;

diff --git a/src/tools/config.c b/src/tools/config.c
@@ -104,6 +104,18 @@ static inline bool parse_fwmark(uint32_t *fwmark, uint32_t *flags, const char *v
 	return false;
 }
+  
+static inline bool parse_name(uint8_t name[static WG_NAME_LEN], uint32_t *flags,  const char *value)
+  
+{
+  
+	if (strlen(value) >= WG_NAME_LEN) {
+  
+		fprintf(stderr, "Name is too long, must be less than %d chars: `%s'\n", WG_NAME_LEN, value);
+  
+		return false;
+  
+	}
+  
+
+  
+	*flags |= WGPEER_HAS_NAME;
+  
+	memcpy(name, value, strlen(value));
+  
+	return true;
+  
+}
+  
+
 static inline bool parse_key(uint8_t key[static WG_KEY_LEN], const char *value)
 {
 	if (!key_from_base64(key, value)) {
@@ -407,6 +419,8 @@ static bool process_line(struct config_ctx *ctx, const char *line)
 			ret = parse_key(ctx->last_peer->public_key, value);
 			if (ret)
 				ctx->last_peer->flags |= WGPEER_HAS_PUBLIC_KEY;
+  
+		} else if (key_match("Name")) {
+  
+			ret = parse_name(ctx->last_peer->name, &ctx->last_peer->flags, value);
 		} else if (key_match("AllowedIPs"))
 			ret = parse_allowedips(ctx->last_peer, &ctx->last_allowedip, value);
 		else if (key_match("PersistentKeepalive"))
@@ -580,6 +594,11 @@ struct wgdevice *config_read_cmd(char *argv[], int argc)
 				goto error;
 			argv += 2;
 			argc -= 2;
+  
+		} else if (!strcmp(argv[0], "name") && argc >= 2 && peer) {
+  
+			if (!parse_name(peer->name, &peer->flags, argv[1]))
+  
+				goto error;
+  
+			argv += 2;
+  
+			argc -= 2;
 		} else if (!strcmp(argv[0], "preshared-key") && argc >= 2 && peer) {
 			if (!parse_keyfile(peer->preshared_key, argv[1]))
 				goto error;

diff --git a/src/tools/containers.h b/src/tools/containers.h
@@ -36,14 +36,16 @@ enum {
 	WGPEER_REPLACE_ALLOWEDIPS = 1U << 1,
 	WGPEER_HAS_PUBLIC_KEY = 1U << 2,
 	WGPEER_HAS_PRESHARED_KEY = 1U << 3,
-  
-	WGPEER_HAS_PERSISTENT_KEEPALIVE_INTERVAL = 1U << 4
+  
+	WGPEER_HAS_PERSISTENT_KEEPALIVE_INTERVAL = 1U << 4,
+  
+	WGPEER_HAS_NAME = 1U << 5,
 };
 struct wgpeer {
 	uint32_t flags;
 	uint8_t public_key[WG_KEY_LEN];
 	uint8_t preshared_key[WG_KEY_LEN];
+  
+	uint8_t name[WG_NAME_LEN];
 	union {
 		struct sockaddr addr;

diff --git a/src/tools/ipc.c b/src/tools/ipc.c
@@ -594,6 +594,8 @@ static int kernel_set_device(struct wgdevice *dev)
 			goto toobig_peers;
 		if (!mnl_attr_put_check(nlh, SOCKET_BUFFER_SIZE, WGPEER_A_PUBLIC_KEY, sizeof(peer->public_key), peer->public_key))
 			goto toobig_peers;
+  
+		if (!mnl_attr_put_check(nlh, SOCKET_BUFFER_SIZE, WGPEER_A_NAME, sizeof(peer->name), peer->name))
+  
+			goto toobig_peers;
 		if (peer->flags & WGPEER_REMOVE_ME)
 			flags |= WGPEER_F_REMOVE_ME;
 		if (!allowedip) {
@@ -755,6 +757,15 @@ static int parse_peer(const struct nlattr *attr, void *data)
 				peer->flags |= WGPEER_HAS_PRESHARED_KEY;
 		}
 		break;
+  
+	case WGPEER_A_NAME:
+  
+		// NOTE: What validations should a client do for data that comes
+  
+		// from the kernel? Can this be blindly trusted?
+  
+		if (mnl_attr_get_payload_len(attr) != 0) {
+  
+			memcpy(peer->name, mnl_attr_get_payload(attr), sizeof(peer->name));
+  
+			if (*peer->name != '\0')
+  
+				peer->flags |= WGPEER_HAS_NAME;
+  
+		}
+  
+		break;
 	case WGPEER_A_ENDPOINT: {
 		struct sockaddr *addr;

diff --git a/src/tools/set.c b/src/tools/set.c
@@ -18,7 +18,7 @@ int set_main(int argc, char *argv[])
 	int ret = 1;
 	if (argc < 3) {
-  
-		fprintf(stderr, "Usage: %s %s <interface> [listen-port <port>] [fwmark <mark>] [private-key <file path>] [peer <base64 public key> [remove] [preshared-key <file path>] [endpoint <ip>:<port>] [persistent-keepalive <interval seconds>] [allowed-ips <ip1>/<cidr1>[,<ip2>/<cidr2>]...] ]...\n", PROG_NAME, argv[0]);
+  
+		fprintf(stderr, "Usage: %s %s <interface> [listen-port <port>] [fwmark <mark>] [private-key <file path>] [peer <base64 public key> [remove] [name <name>] [preshared-key <file path>] [endpoint <ip>:<port>] [persistent-keepalive <interval seconds>] [allowed-ips <ip1>/<cidr1>[,<ip2>/<cidr2>]...] ]...\n", PROG_NAME, argv[0]);
 		return 1;
 	}

diff --git a/src/tools/show.c b/src/tools/show.c
@@ -246,6 +246,8 @@ static void pretty_print(struct wgdevice *device)
 		}
 		if (peer->persistent_keepalive_interval)
 			terminal_printf("  " TERMINAL_BOLD "persistent keepalive" TERMINAL_RESET ": %s\n", every(peer->persistent_keepalive_interval));
+  
+		if (peer->flags & WGPEER_HAS_NAME)
+  
+			terminal_printf("  " TERMINAL_BOLD "name" TERMINAL_RESET ": %s\n", peer->name);
 		if (peer->next_peer)
 			terminal_printf("\n");
 	}

diff --git a/src/uapi/wireguard.h b/src/uapi/wireguard.h
@@ -33,6 +33,7 @@
 *        0: NLA_NESTED
 *            WGPEER_A_PUBLIC_KEY: len WG_KEY_LEN
 *            WGPEER_A_PRESHARED_KEY: len WG_KEY_LEN
+  
+ *            WGPEER_A_NAME: NLA_NUL_STRING, maxlen WG_NAME_LEN - 1
 *            WGPEER_A_ENDPOINT: struct sockaddr_in or struct sockaddr_in6
 *            WGPEER_A_PERSISTENT_KEEPALIVE_INTERVAL: NLA_U16
 *            WGPEER_A_LAST_HANDSHAKE_TIME: struct __kernel_timespec
@@ -92,6 +93,7 @@
 *                            if all current allowed IPs of this peer should be
 *                            removed prior to adding the list below.
 *            WGPEER_A_PRESHARED_KEY: len WG_KEY_LEN, all zeros to remove
+  
+ *            WGPEER_A_NAME: len WG_NAME_LEN, NULL pointer to remove
 *            WGPEER_A_ENDPOINT: struct sockaddr_in or struct sockaddr_in6
 *            WGPEER_A_PERSISTENT_KEEPALIVE_INTERVAL: NLA_U16, 0 to disable
 *            WGPEER_A_ALLOWEDIPS: NLA_NESTED
@@ -133,6 +135,7 @@
 #define WG_GENL_VERSION 1
 #define WG_KEY_LEN 32
+  
+#define WG_NAME_LEN 128
 enum wg_cmd {
 	WG_CMD_GET_DEVICE,
@@ -166,6 +169,7 @@ enum wgpeer_attribute {
 	WGPEER_A_UNSPEC,
 	WGPEER_A_PUBLIC_KEY,
 	WGPEER_A_PRESHARED_KEY,
+  
+	WGPEER_A_NAME,
 	WGPEER_A_FLAGS,
 	WGPEER_A_ENDPOINT,
 	WGPEER_A_PERSISTENT_KEEPALIVE_INTERVAL,