Use more permissive umask for .service units (LP: #2072486)

doc/security.md

@@ -19,6 +19,23 @@ Security advice: ensure all YAML files in `/etc/netplan`, `/run/netplan` and
 `/lib/netplan` are not readable by non-privileged users.
 :::
 
+## Systemd `.service` units
+
+Netplan generates many systemd `.service` units, which are world-accessible to
+any local user through systemd APIs by design, e.g. using `systemctl show UNIT_NAME.service`
+
+Such service units are therefore generated with `0o644` permissions. This
+needs to be taken into consideration especially for the `netplan-ovs-*.service`
+units that might contain arbitrary content, for example using the `other-config`
+or `external-ids`. Make sure not to put any secrets into those fields, as those
+will become world-readable.
+
+* `/run/systemd/system/netplan-ovs-*.service`
+* `/run/systemd/system/netplan-sriov-*.service`
+* `/run/systemd/system/netplan-regdom.service`
+* `/run/systemd/system/netplan-wpa-*.service`
+* `/run/systemd/system/systemd-networkd-wait-online.service.d/10-netplan*.conf`
+
 ## Static analysis with Coverity
 
 To ensure that common issues do not sneak undetected in our code base,

src/networkd.c

@@ -408,7 +408,9 @@ write_regdom(const NetplanNetDefinition* def, const char* rootdir, GError** erro
     g_autofree char* new_s = _netplan_scrub_systemd_unit_contents(s->str);
     g_string_free(s, TRUE);
     s = g_string_new(new_s);
-    _netplan_g_string_free_to_file_with_permissions(s, rootdir, path, NULL, "root", "root", 0640);
+    mode_t orig_umask = umask(022);
+    _netplan_g_string_free_to_file(s, rootdir, path, NULL);
+    umask(orig_umask);
     _netplan_safe_mkdir_p_dir(link);
     if (symlink(path, link) < 0 && errno != EEXIST) {
         // LCOV_EXCL_START
@@ -1342,7 +1344,9 @@ write_wpa_unit(const NetplanNetDefinition* def, const char* rootdir)
     g_autofree char* new_s = _netplan_scrub_systemd_unit_contents(s->str);
     g_string_free(s, TRUE);
     s = g_string_new(new_s);
-    _netplan_g_string_free_to_file_with_permissions(s, rootdir, path, NULL, "root", "root", 0640);
+    mode_t orig_umask = umask(022);
+    _netplan_g_string_free_to_file(s, rootdir, path, NULL);
+    umask(orig_umask);
 }
 
 STATIC gboolean
@@ -1613,7 +1617,9 @@ _netplan_networkd_write_wait_online(const NetplanState* np_state, const char* ro
     GString* content = g_string_new("[Unit]\n"
         "ConditionPathIsSymbolicLink=/run/systemd/generator/network-online.target.wants/systemd-networkd-wait-online.service\n");
     if (g_hash_table_size(non_optional_interfaces) == 0) {
-        _netplan_g_string_free_to_file_with_permissions(content, rootdir, override, NULL, "root", "root", 0640);
+        mode_t orig_umask = umask(022);
+        _netplan_g_string_free_to_file(content, rootdir, override, NULL);
+        umask(orig_umask);
         return FALSE;
     }
     // ELSE:
@@ -1657,7 +1663,9 @@ _netplan_networkd_write_wait_online(const NetplanState* np_state, const char* ro
     g_autofree char* new_content = _netplan_scrub_systemd_unit_contents(content->str);
     g_string_free(content, TRUE);
     content = g_string_new(new_content);
-    _netplan_g_string_free_to_file_with_permissions(content, rootdir, override, NULL, "root", "root", 0640);
+    mode_t orig_umask = umask(022);
+    _netplan_g_string_free_to_file(content, rootdir, override, NULL);
+    umask(orig_umask);
     return TRUE;
 }
 

src/openvswitch.c

@@ -18,6 +18,7 @@
 
 #include <unistd.h>
 #include <errno.h>
+#include <sys/stat.h>
 
 #include <glib.h>
 #include <glib/gprintf.h>
@@ -70,7 +71,9 @@ write_ovs_systemd_unit(const char* id, const GString* cmds, const char* rootdir,
     g_autofree char* new_s = _netplan_scrub_systemd_unit_contents(s->str);
     g_string_free(s, TRUE);
     s = g_string_new(new_s);
-    _netplan_g_string_free_to_file_with_permissions(s, rootdir, path, NULL, "root", "root", 0640);
+    mode_t orig_umask = umask(022);
+    _netplan_g_string_free_to_file(s, rootdir, path, NULL);
+    umask(orig_umask);
 
     _netplan_safe_mkdir_p_dir(link);
     if (symlink(path, link) < 0 && errno != EEXIST) {

src/sriov.c

@@ -18,6 +18,7 @@
 
 #include <unistd.h>
 #include <errno.h>
+#include <sys/stat.h>
 
 #include <glib.h>
 #include <glib/gstdio.h>
@@ -58,7 +59,9 @@ write_sriov_rebind_systemd_unit(GHashTable* pfs, const char* rootdir, GError** e
     g_autofree char* new_s = _netplan_scrub_systemd_unit_contents(s->str);
     g_string_free(s, TRUE);
     s = g_string_new(new_s);
-    _netplan_g_string_free_to_file_with_permissions(s, rootdir, path, NULL, "root", "root", 0640);
+    mode_t orig_umask = umask(022);
+    _netplan_g_string_free_to_file(s, rootdir, path, NULL);
+    umask(orig_umask);
     g_string_free(interfaces, TRUE);
 
     _netplan_safe_mkdir_p_dir(link);
@@ -98,7 +101,9 @@ write_sriov_apply_systemd_unit(GHashTable* pfs, const char* rootdir, GError** er
     g_autofree char* new_s = _netplan_scrub_systemd_unit_contents(s->str);
     g_string_free(s, TRUE);
     s = g_string_new(new_s);
-    _netplan_g_string_free_to_file_with_permissions(s, rootdir, path, NULL, "root", "root", 0640);
+    mode_t orig_umask = umask(022);
+    _netplan_g_string_free_to_file(s, rootdir, path, NULL);
+    umask(orig_umask);
 
     _netplan_safe_mkdir_p_dir(link);
     if (symlink(path, link) < 0 && errno != EEXIST) {

tests/generator/test_auth.py

@@ -226,7 +226,7 @@ def test_auth_wired(self):
 
         with open(os.path.join(self.workdir.name, 'run/systemd/system/netplan-wpa-eth0.service')) as f:
             self.assertEqual(f.read(), SD_WPA % {'iface': 'eth0', 'drivers': 'wired'})
-            self.assertEqual(stat.S_IMODE(os.fstat(f.fileno()).st_mode), 0o640)
+            self.assertEqual(stat.S_IMODE(os.fstat(f.fileno()).st_mode), 0o644)
         self.assertTrue(os.path.islink(os.path.join(
             self.workdir.name, 'run/systemd/system/systemd-networkd.service.wants/netplan-wpa-eth0.service')))
 

tests/generator/test_wifis.py

@@ -140,7 +140,7 @@ def test_wifi(self):
             self.workdir.name, 'run/systemd/system/netplan-wpa-wl0.service')))
         with open(os.path.join(self.workdir.name, 'run/systemd/system/netplan-wpa-wl0.service')) as f:
             self.assertEqual(f.read(), SD_WPA % {'iface': 'wl0', 'drivers': 'nl80211,wext'})
-            self.assertEqual(stat.S_IMODE(os.fstat(f.fileno()).st_mode), 0o640)
+            self.assertEqual(stat.S_IMODE(os.fstat(f.fileno()).st_mode), 0o644)
         self.assertTrue(os.path.islink(os.path.join(
             self.workdir.name, 'run/systemd/system/systemd-networkd.service.wants/netplan-wpa-wl0.service')))
 

tests/integration/base.py

@@ -406,7 +406,7 @@ def assert_file_permissions(self):
         nd_expected_owner = 'root'
         nd_expected_group = 'systemd-network'
 
-        sd_expected_mode = 0o100640
+        sd_expected_mode = 0o100644
         sd_expected_owner = 'root'
         sd_expected_group = 'root'