From cb31e7c861f44cb2a50417dde2ca2ea83d6d7129 Mon Sep 17 00:00:00 2001
From: Yu Watanabe <watanabe.yu+github@gmail.com>
Date: Fri, 5 Apr 2019 17:28:46 +0900
Subject: [PATCH] network: make reading PrivateKeyFile= failure always fatal

This also refactor wireguard_read_key_file().
---
 man/systemd.netdev.xml                     |  4 +-
 src/network/netdev/wireguard.c             | 63 +++++++++-------------
 test/test-network/conf/25-wireguard.netdev |  1 -
 3 files changed, 27 insertions(+), 41 deletions(-)

diff --git a/man/systemd.netdev.xml b/man/systemd.netdev.xml
index a44018cad6d..10397402a46 100644
--- a/man/systemd.netdev.xml
+++ b/man/systemd.netdev.xml
@@ -1242,9 +1242,7 @@
         <term><varname>PrivateKeyFile=</varname></term>
         <listitem>
           <para>Takes a absolute path to a file which contains the Base64 encoded private key for the interface.
-          If both <varname>PrivateKey=</varname> and <varname>PrivateKeyFile=</varname> are specified, and if
-          the file specified in <varname>PrivateKeyFile=</varname> contains valid wireguard key, then
-          the key provided by <varname>PrivateKey=</varname> is ignored.
+          When this option is specified, then <varname>PrivateKey=</varname> is ignored.
           Note that the file must be readable by the user <literal>systemd-network</literal>, so it
           should be, e.g., owned by <literal>root:systemd-network</literal> with a
           <literal>0640</literal> file mode.</para>
diff --git a/src/network/netdev/wireguard.c b/src/network/netdev/wireguard.c
index 61fbc24cfbe..d958a5f02f6 100644
--- a/src/network/netdev/wireguard.c
+++ b/src/network/netdev/wireguard.c
@@ -846,41 +846,29 @@ static void wireguard_done(NetDev *netdev) {
         set_free(w->peers_with_failed_endpoint);
 }
 
-static int wireguard_read_private_key_file(Wireguard *w, bool fatal) {
-        _cleanup_free_ char *contents = NULL;
-        _cleanup_free_ void *key = NULL;
-        size_t size, key_len;
-        NetDev *netdev;
-        int level, r;
-
-        assert(w);
-
-        netdev = NETDEV(w);
+static int wireguard_read_key_file(const char *filename, uint8_t dest[static WG_KEY_LEN]) {
+        _cleanup_free_ char *key = NULL;
+        size_t key_len;
+        int r;
 
-        if (!w->private_key_file)
+        if (!filename)
                 return 0;
 
-        level = fatal ? LOG_ERR : LOG_INFO;
-
-        r = read_full_file(w->private_key_file, &contents, &size);
+        r = read_full_file_full(filename, READ_FULL_FILE_SECURE | READ_FULL_FILE_UNBASE64, &key, &key_len);
         if (r < 0)
-                return log_netdev_full(netdev, level, r,
-                                       "Failed to read private key from '%s'%s: %m",
-                                       w->private_key_file, fatal ? "" : ", ignoring");
+                return r;
 
-        r = unbase64mem(contents, size, &key, &key_len);
-        if (r < 0)
-                return log_netdev_full(netdev, level, r,
-                                       "Failed to decode private key%s: %m",
-                                       fatal ? "" : ", ignoring");
+        if (key_len != WG_KEY_LEN) {
+                r = -EINVAL;
+                goto finalize;
+        }
 
-        if (key_len != WG_KEY_LEN)
-                return log_netdev_full(netdev, level, SYNTHETIC_ERRNO(EINVAL),
-                                       "Wireguard private key has invalid length (%zu bytes)%s: %m",
-                                       key_len, fatal ? "" : ", ignoring");
+        memcpy(dest, key, WG_KEY_LEN);
+        r = 0;
 
-        memcpy(w->private_key, key, WG_KEY_LEN);
-        return 0;
+finalize:
+        explicit_bzero_safe(key, key_len);
+        return r;
 }
 
 static int wireguard_peer_verify(WireguardPeer *peer) {
@@ -901,22 +889,23 @@ static int wireguard_peer_verify(WireguardPeer *peer) {
 static int wireguard_verify(NetDev *netdev, const char *filename) {
         WireguardPeer *peer, *peer_next;
         Wireguard *w;
-        bool empty;
         int r;
 
         assert(netdev);
         w = WIREGUARD(netdev);
         assert(w);
 
-        empty = eqzero(w->private_key);
-        if (empty && !w->private_key_file)
-                return log_netdev_error_errno(netdev, SYNTHETIC_ERRNO(EINVAL),
-                                              "%s: Missing PrivateKey= or PrivateKeyFile=, ignoring.",
-                                              filename);
+        r = wireguard_read_key_file(w->private_key_file, w->private_key);
+        if (r < 0)
+                return log_netdev_error_errno(netdev, r,
+                                              "Failed to read private key from %s. Dropping network device %s.",
+                                              w->private_key_file, netdev->ifname);
 
-        r = wireguard_read_private_key_file(w, empty);
-        if (r < 0 && empty)
-                return r;
+        if (eqzero(w->private_key))
+                return log_netdev_error_errno(netdev, SYNTHETIC_ERRNO(EINVAL),
+                                              "%s: Missing PrivateKey= or PrivateKeyFile=, "
+                                              "Dropping network device %s.",
+                                              filename, netdev->ifname);
 
         LIST_FOREACH_SAFE(peers, peer, peer_next, w->peers)
                 if (wireguard_peer_verify(peer) < 0)
diff --git a/test/test-network/conf/25-wireguard.netdev b/test/test-network/conf/25-wireguard.netdev
index 61afd1f5e79..4866c31ccac 100644
--- a/test/test-network/conf/25-wireguard.netdev
+++ b/test/test-network/conf/25-wireguard.netdev
@@ -4,7 +4,6 @@ Kind=wireguard
 
 [WireGuard]
 PrivateKey=EEGlnEPYJV//kbvvIqxKkQwOiS+UENyPncC4bF46ong=
-PrivateKeyFile=/run/systemd/network/not-exist
 ListenPort=51820
 FwMark=1234
 
-- 
2.39.2