From 9ef84af8c0bc31d1e56d0a66a9ed909c1edfdd5d Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Fri, 22 Sep 2017 04:04:00 +0200 Subject: [PATCH] wg: use key_is_zero for comparing to zeros Maybe an attacker on the system could use the infoleak in /proc to gauge how long a wg(8) process takes to complete and determine the number of leading zeros. This is somewhat ridiculous, but it's possible somebody somewhere might at somepoint care in the future, so alright. Signed-off-by: Jason A. Donenfeld --- src/config.c | 10 ++-------- src/encoding.c | 14 ++++++++++++-- src/encoding.h | 2 ++ src/ipc.c | 5 ++--- src/show.c | 10 ++++------ src/showconf.c | 5 ++--- 6 files changed, 24 insertions(+), 22 deletions(-) diff --git a/src/config.c b/src/config.c index 2f61d5b..af74bda 100644 --- a/src/config.c +++ b/src/config.c @@ -387,17 +387,11 @@ bool config_read_init(struct config_ctx *ctx, struct wgdevice **device, bool app return true; } -static inline bool key_is_valid(uint8_t key[WG_KEY_LEN]) -{ - static const uint8_t zero[WG_KEY_LEN] = { 0 }; - return !!memcmp(key, zero, WG_KEY_LEN); -} - bool config_read_finish(struct config_ctx *ctx) { size_t i; struct wgpeer *peer; - if (ctx->buf.dev->flags & WGDEVICE_REPLACE_PEERS && !key_is_valid(ctx->buf.dev->private_key)) { + if (ctx->buf.dev->flags & WGDEVICE_REPLACE_PEERS && key_is_zero(ctx->buf.dev->private_key)) { fprintf(stderr, "No private key configured\n"); goto err; } @@ -405,7 +399,7 @@ bool config_read_finish(struct config_ctx *ctx) ctx->buf.dev->flags |= WGDEVICE_REMOVE_FWMARK; for_each_wgpeer(ctx->buf.dev, peer, i) { - if (!key_is_valid(peer->public_key)) { + if (key_is_zero(peer->public_key)) { fprintf(stderr, "A peer is missing a public key\n"); goto err; } diff --git a/src/encoding.c b/src/encoding.c index 389bbf7..3d5e94b 100644 --- a/src/encoding.c +++ b/src/encoding.c @@ -77,12 +77,12 @@ void key_to_hex(char hex[static WG_KEY_LEN_HEX], const uint8_t key[static WG_KEY bool key_from_hex(uint8_t key[static WG_KEY_LEN], const char *hex) { - uint8_t i, c, c_acc = 0, c_alpha0, c_alpha, c_num0, c_num, c_val, state = 0; + uint8_t c, c_acc = 0, c_alpha0, c_alpha, c_num0, c_num, c_val, state = 0; if (strlen(hex) != WG_KEY_LEN_HEX - 1) return false; - for (i = 0; i < WG_KEY_LEN_HEX - 1; ++i) { + for (unsigned int i = 0; i < WG_KEY_LEN_HEX - 1; ++i) { c = (uint8_t)hex[i]; c_num = c ^ 48U; c_num0 = (c_num - 10U) >> 8; @@ -99,3 +99,13 @@ bool key_from_hex(uint8_t key[static WG_KEY_LEN], const char *hex) } return true; } + +bool key_is_zero(const uint8_t key[static WG_KEY_LEN]) +{ + uint8_t acc = 0; + for (unsigned int i = 0; i < WG_KEY_LEN; ++i) { + acc |= key[i]; + __asm__ ("" : "=r" (acc) : "0" (acc)); + } + return acc == 0; +} diff --git a/src/encoding.h b/src/encoding.h index f4fe51e..9db4c6e 100644 --- a/src/encoding.h +++ b/src/encoding.h @@ -16,4 +16,6 @@ bool key_from_base64(uint8_t key[static WG_KEY_LEN], const char *base64); void key_to_hex(char hex[static WG_KEY_LEN_HEX], const uint8_t key[static WG_KEY_LEN]); bool key_from_hex(uint8_t key[static WG_KEY_LEN], const char *hex); +bool key_is_zero(const uint8_t key[static WG_KEY_LEN]); + #endif diff --git a/src/ipc.c b/src/ipc.c index 48d06aa..d0b5a46 100644 --- a/src/ipc.c +++ b/src/ipc.c @@ -170,7 +170,6 @@ out: static int userspace_set_device(struct wgdevice *dev) { - static const uint8_t zero[WG_KEY_LEN] = { 0 }; char hex[WG_KEY_LEN_HEX], ip[INET6_ADDRSTRLEN], host[4096 + 1], service[512 + 1]; struct wgpeer *peer; struct wgipmask *ipmask; @@ -186,7 +185,7 @@ static int userspace_set_device(struct wgdevice *dev) if (dev->flags & WGDEVICE_REMOVE_PRIVATE_KEY) fprintf(f, "private_key=\n"); - else if (memcmp(dev->private_key, zero, WG_KEY_LEN)) { + else if (!key_is_zero(dev->private_key)) { key_to_hex(hex, dev->private_key); fprintf(f, "private_key=%s\n", hex); } @@ -208,7 +207,7 @@ static int userspace_set_device(struct wgdevice *dev) } if (peer->flags & WGPEER_REMOVE_PRESHARED_KEY) fprintf(f, "preshared_key=\n"); - else if (memcmp(peer->preshared_key, zero, WG_KEY_LEN)) { + else if (!key_is_zero(peer->preshared_key)) { key_to_hex(hex, peer->preshared_key); fprintf(f, "preshared_key=%s\n", hex); } diff --git a/src/show.c b/src/show.c index 4eb096f..6e5de96 100644 --- a/src/show.c +++ b/src/show.c @@ -75,12 +75,10 @@ static void sort_peers(struct wgdevice *device) free(new_device); } -static const uint8_t zero[WG_KEY_LEN] = { 0 }; - static char *key(const uint8_t key[static WG_KEY_LEN]) { static char base64[WG_KEY_LEN_BASE64]; - if (!memcmp(key, zero, WG_KEY_LEN)) + if (key_is_zero(key)) return "(none)"; key_to_base64(base64, key); return base64; @@ -212,9 +210,9 @@ static void pretty_print(struct wgdevice *device) terminal_printf(TERMINAL_RESET); terminal_printf(TERMINAL_FG_GREEN TERMINAL_BOLD "interface" TERMINAL_RESET ": " TERMINAL_FG_GREEN "%s" TERMINAL_RESET "\n", device->interface); - if (memcmp(device->public_key, zero, WG_KEY_LEN)) + if (!key_is_zero(device->public_key)) terminal_printf(" " TERMINAL_BOLD "public key" TERMINAL_RESET ": %s\n", key(device->public_key)); - if (memcmp(device->private_key, zero, WG_KEY_LEN)) + if (!key_is_zero(device->private_key)) terminal_printf(" " TERMINAL_BOLD "private key" TERMINAL_RESET ": %s\n", masked_key(device->private_key)); if (device->port) terminal_printf(" " TERMINAL_BOLD "listening port" TERMINAL_RESET ": %u\n", device->port); @@ -226,7 +224,7 @@ static void pretty_print(struct wgdevice *device) } for_each_wgpeer(device, peer, i) { terminal_printf(TERMINAL_FG_YELLOW TERMINAL_BOLD "peer" TERMINAL_RESET ": " TERMINAL_FG_YELLOW "%s" TERMINAL_RESET "\n", key(peer->public_key)); - if (memcmp(peer->preshared_key, zero, WG_KEY_LEN)) + if (!key_is_zero(peer->preshared_key)) terminal_printf(" " TERMINAL_BOLD "preshared key" TERMINAL_RESET ": %s\n", masked_key(peer->preshared_key)); if (peer->endpoint.addr.sa_family == AF_INET || peer->endpoint.addr.sa_family == AF_INET6) terminal_printf(" " TERMINAL_BOLD "endpoint" TERMINAL_RESET ": %s\n", endpoint(&peer->endpoint.addr)); diff --git a/src/showconf.c b/src/showconf.c index 2453c86..09dc2ec 100644 --- a/src/showconf.c +++ b/src/showconf.c @@ -16,7 +16,6 @@ int showconf_main(int argc, char *argv[]) { - static const uint8_t zero[WG_KEY_LEN] = { 0 }; char base64[WG_KEY_LEN_BASE64]; char ip[INET6_ADDRSTRLEN]; struct wgdevice *device = NULL; @@ -46,7 +45,7 @@ int showconf_main(int argc, char *argv[]) printf("ListenPort = %u\n", device->port); if (device->fwmark) printf("FwMark = 0x%x\n", device->fwmark); - if (memcmp(device->private_key, zero, WG_KEY_LEN)) { + if (!key_is_zero(device->private_key)) { key_to_base64(base64, device->private_key); printf("PrivateKey = %s\n", base64); } @@ -54,7 +53,7 @@ int showconf_main(int argc, char *argv[]) for_each_wgpeer(device, peer, i) { key_to_base64(base64, peer->public_key); printf("[Peer]\nPublicKey = %s\n", base64); - if (memcmp(peer->preshared_key, zero, WG_KEY_LEN)) { + if (!key_is_zero(peer->preshared_key)) { key_to_base64(base64, peer->preshared_key); printf("PresharedKey = %s\n", base64); }