|
| 1 | +From d9355a0deafcb0b25b43b0a1de8fa6d83ed844a9 Mon Sep 17 00:00:00 2001 |
| 2 | +From: Kevin Lockwood <v-klockwood@microsoft.com> |
| 3 | +Date: Fri, 14 Feb 2025 09:25:29 -0800 |
| 4 | +Subject: [PATCH] [Medium] Patch netplan for CVE-2022-4968 |
| 5 | + |
| 6 | +Link: https://github.com/canonical/netplan/commit/4c39b75b5c6ae7d976bda6da68da60d9a7f085ee.patch |
| 7 | +--- |
| 8 | + src/networkd.c | 28 +++--------- |
| 9 | + src/networkd.h | 2 + |
| 10 | + src/nm.c | 4 +- |
| 11 | + src/util.c | 47 +++++++++++++++++++ |
| 12 | + src/util.h | 2 + |
| 13 | + tests/generator/test_wifis.py | 2 +- |
| 14 | + tests/integration/base.py | 86 ++++++++++++++++++++++++++++++++++- |
| 15 | + 7 files changed, 146 insertions(+), 25 deletions(-) |
| 16 | + |
| 17 | +diff --git a/src/networkd.c b/src/networkd.c |
| 18 | +index 41c2998..ea29470 100644 |
| 19 | +--- a/src/networkd.c |
| 20 | ++++ b/src/networkd.c |
| 21 | +@@ -125,7 +125,6 @@ static void |
| 22 | + write_link_file(net_definition* def, const char* rootdir, const char* path) |
| 23 | + { |
| 24 | + GString* s = NULL; |
| 25 | +- mode_t orig_umask; |
| 26 | + |
| 27 | + /* Don't write .link files for virtual devices; they use .netdev instead */ |
| 28 | + if (def->type >= ND_VIRTUAL) |
| 29 | +@@ -150,9 +149,7 @@ write_link_file(net_definition* def, const char* rootdir, const char* path) |
| 30 | + g_string_append_printf(s, "MACAddress=%s\n", def->set_mac); |
| 31 | + |
| 32 | + |
| 33 | +- orig_umask = umask(022); |
| 34 | +- g_string_free_to_file(s, rootdir, path, ".link"); |
| 35 | +- umask(orig_umask); |
| 36 | ++ _netplan_g_string_free_to_file_with_permissions(s, rootdir, path, ".link", "root", "root", 0640); |
| 37 | + } |
| 38 | + |
| 39 | + |
| 40 | +@@ -314,11 +311,7 @@ write_netdev_file(net_definition* def, const char* rootdir, const char* path) |
| 41 | + // LCOV_EXCL_STOP |
| 42 | + } |
| 43 | + |
| 44 | +- /* these do not contain secrets and need to be readable by |
| 45 | +- * systemd-networkd - LP: #1736965 */ |
| 46 | +- orig_umask = umask(022); |
| 47 | +- g_string_free_to_file(s, rootdir, path, ".netdev"); |
| 48 | +- umask(orig_umask); |
| 49 | ++ _netplan_g_string_free_to_file_with_permissions(s, rootdir, path, ".netdev", "root", NETWORKD_GROUP, 0640); |
| 50 | + } |
| 51 | + |
| 52 | + static void |
| 53 | +@@ -420,7 +413,6 @@ write_network_file(net_definition* def, const char* rootdir, const char* path) |
| 54 | + GString* network = NULL; |
| 55 | + GString* link = NULL; |
| 56 | + GString* s = NULL; |
| 57 | +- mode_t orig_umask; |
| 58 | + |
| 59 | + /* Prepare the [Link] section of the .network file. */ |
| 60 | + link = g_string_sized_new(200); |
| 61 | +@@ -590,9 +582,9 @@ write_network_file(net_definition* def, const char* rootdir, const char* path) |
| 62 | + |
| 63 | + /* these do not contain secrets and need to be readable by |
| 64 | + * systemd-networkd - LP: #1736965 */ |
| 65 | +- orig_umask = umask(022); |
| 66 | +- g_string_free_to_file(s, rootdir, path, ".network"); |
| 67 | +- umask(orig_umask); |
| 68 | ++ /* This should allow systemd-networkd access to the file, but still |
| 69 | ++ * protect against malicious use */ |
| 70 | ++ _netplan_g_string_free_to_file_with_permissions(s, rootdir, path, ".network", "root", NETWORKD_GROUP, 0640); |
| 71 | + } |
| 72 | + } |
| 73 | + |
| 74 | +@@ -601,7 +593,6 @@ write_rules_file(net_definition* def, const char* rootdir) |
| 75 | + { |
| 76 | + GString* s = NULL; |
| 77 | + g_autofree char* path = g_strjoin(NULL, "run/udev/rules.d/99-netplan-", def->id, ".rules", NULL); |
| 78 | +- mode_t orig_umask; |
| 79 | + |
| 80 | + /* do we need to write a .rules file? |
| 81 | + * It's only required for reliably setting the name of a physical device |
| 82 | +@@ -635,9 +626,7 @@ write_rules_file(net_definition* def, const char* rootdir) |
| 83 | + |
| 84 | + g_string_append_printf(s, "NAME=\"%s\"\n", def->set_name); |
| 85 | + |
| 86 | +- orig_umask = umask(022); |
| 87 | +- g_string_free_to_file(s, rootdir, path, NULL); |
| 88 | +- umask(orig_umask); |
| 89 | ++ _netplan_g_string_free_to_file_with_permissions(s, rootdir, path, NULL, "root", "root", 0640); |
| 90 | + } |
| 91 | + |
| 92 | + static void |
| 93 | +@@ -711,7 +700,6 @@ write_wpa_conf(net_definition* def, const char* rootdir) |
| 94 | + GHashTableIter iter; |
| 95 | + GString* s = g_string_new("ctrl_interface=/run/wpa_supplicant\n\n"); |
| 96 | + g_autofree char* path = g_strjoin(NULL, "run/netplan/wpa-", def->id, ".conf", NULL); |
| 97 | +- mode_t orig_umask; |
| 98 | + |
| 99 | + g_debug("%s: Creating wpa_supplicant configuration file %s", def->id, path); |
| 100 | + if (def->type == ND_WIFI) { |
| 101 | +@@ -749,9 +737,7 @@ write_wpa_conf(net_definition* def, const char* rootdir) |
| 102 | + } |
| 103 | + |
| 104 | + /* use tight permissions as this contains secrets */ |
| 105 | +- orig_umask = umask(077); |
| 106 | +- g_string_free_to_file(s, rootdir, path, NULL); |
| 107 | +- umask(orig_umask); |
| 108 | ++ _netplan_g_string_free_to_file_with_permissions(s, rootdir, path, NULL, "root", "root", 0600); |
| 109 | + } |
| 110 | + |
| 111 | + /** |
| 112 | +diff --git a/src/networkd.h b/src/networkd.h |
| 113 | +index 6660f9c..9bcfb55 100644 |
| 114 | +--- a/src/networkd.h |
| 115 | ++++ b/src/networkd.h |
| 116 | +@@ -19,6 +19,8 @@ |
| 117 | + |
| 118 | + #include "parse.h" |
| 119 | + |
| 120 | ++#define NETWORKD_GROUP "systemd-network" |
| 121 | ++ |
| 122 | + gboolean write_networkd_conf(net_definition* def, const char* rootdir); |
| 123 | + void cleanup_networkd_conf(const char* rootdir); |
| 124 | + void enable_networkd(const char* generator_dir); |
| 125 | +diff --git a/src/nm.c b/src/nm.c |
| 126 | +index d8a6a7c..156b199 100644 |
| 127 | +--- a/src/nm.c |
| 128 | ++++ b/src/nm.c |
| 129 | +@@ -666,13 +666,13 @@ write_nm_conf_finish(const char* rootdir) |
| 130 | + len = s->len; |
| 131 | + g_hash_table_foreach(netdefs, nd_append_non_nm_ids, s); |
| 132 | + if (s->len > len) |
| 133 | +- g_string_free_to_file(s, rootdir, "run/NetworkManager/conf.d/netplan.conf", NULL); |
| 134 | ++ _netplan_g_string_free_to_file_with_permissions(s, rootdir, "run/NetworkManager/conf.d/netplan.conf", NULL, "root", "root", 0640); |
| 135 | + else |
| 136 | + g_string_free(s, TRUE); |
| 137 | + |
| 138 | + /* write generated udev rules */ |
| 139 | + if (udev_rules) |
| 140 | +- g_string_free_to_file(udev_rules, rootdir, "run/udev/rules.d/90-netplan.rules", NULL); |
| 141 | ++ _netplan_g_string_free_to_file_with_permissions(udev_rules, rootdir, "run/udev/rules.d/90-netplan.rules", NULL, "root", "root", 0640); |
| 142 | + } |
| 143 | + |
| 144 | + /** |
| 145 | +diff --git a/src/util.c b/src/util.c |
| 146 | +index e3441d5..2a5c4bb 100644 |
| 147 | +--- a/src/util.c |
| 148 | ++++ b/src/util.c |
| 149 | +@@ -18,6 +18,9 @@ |
| 150 | + #include <stdlib.h> |
| 151 | + #include <unistd.h> |
| 152 | + #include <glob.h> |
| 153 | ++#include <sys/types.h> |
| 154 | ++#include <pwd.h> |
| 155 | ++#include <grp.h> |
| 156 | + |
| 157 | + #include <glib.h> |
| 158 | + #include <glib/gprintf.h> |
| 159 | +@@ -65,6 +68,50 @@ void g_string_free_to_file(GString* s, const char* rootdir, const char* path, co |
| 160 | + } |
| 161 | + } |
| 162 | + |
| 163 | ++void _netplan_g_string_free_to_file_with_permissions(GString* s, const char* rootdir, const char* path, const char* suffix, const char* owner, const char* group, mode_t mode) |
| 164 | ++{ |
| 165 | ++ g_autofree char* full_path = NULL; |
| 166 | ++ g_autofree char* path_suffix = NULL; |
| 167 | ++ g_autofree char* contents = g_string_free(s, FALSE); |
| 168 | ++ GError* error = NULL; |
| 169 | ++ struct passwd* pw = NULL; |
| 170 | ++ struct group* gr = NULL; |
| 171 | ++ int ret = 0; |
| 172 | ++ |
| 173 | ++ path_suffix = g_strjoin(NULL, path, suffix, NULL); |
| 174 | ++ full_path = g_build_path(G_DIR_SEPARATOR_S, rootdir ?: G_DIR_SEPARATOR_S, path_suffix, NULL); |
| 175 | ++ safe_mkdir_p_dir(full_path); |
| 176 | ++ if (!g_file_set_contents_full(full_path, contents, -1, G_FILE_SET_CONTENTS_CONSISTENT | G_FILE_SET_CONTENTS_ONLY_EXISTING, mode, &error)) { |
| 177 | ++ /* the mkdir() just succeeded, there is no sensible |
| 178 | ++ * method to test this without root privileges, bind mounts, and |
| 179 | ++ * simulating ENOSPC */ |
| 180 | ++ // LCOV_EXCL_START |
| 181 | ++ g_fprintf(stderr, "ERROR: cannot create file %s: %s\n", path, error->message); |
| 182 | ++ exit(1); |
| 183 | ++ // LCOV_EXCL_STOP |
| 184 | ++ } |
| 185 | ++ |
| 186 | ++ /* Here we take the owner and group names and look up for their IDs in the passwd and group files. |
| 187 | ++ * It's OK to fail to set the owners and mode as this code will be called from unit tests. |
| 188 | ++ * The autopkgtests will check if the owner/group and mode are correctly set. |
| 189 | ++ */ |
| 190 | ++ pw = getpwnam(owner); |
| 191 | ++ if (!pw) { |
| 192 | ++ g_debug("Failed to determine the UID of user %s: %s", owner, strerror(errno)); // LCOV_EXCL_LINE |
| 193 | ++ } |
| 194 | ++ gr = getgrnam(group); |
| 195 | ++ if (!gr) { |
| 196 | ++ g_debug("Failed to determine the GID of group %s: %s", group, strerror(errno)); // LCOV_EXCL_LINE |
| 197 | ++ } |
| 198 | ++ if (pw && gr) { |
| 199 | ++ ret = chown(full_path, pw->pw_uid, gr->gr_gid); |
| 200 | ++ if (ret != 0) { |
| 201 | ++ g_debug("Failed to set owner and group for file %s: %s", full_path, strerror(errno)); |
| 202 | ++ } |
| 203 | ++ } |
| 204 | ++} |
| 205 | ++ |
| 206 | ++ |
| 207 | + /** |
| 208 | + * Remove all files matching given glob. |
| 209 | + */ |
| 210 | +diff --git a/src/util.h b/src/util.h |
| 211 | +index 4e85a98..bcdfb45 100644 |
| 212 | +--- a/src/util.h |
| 213 | ++++ b/src/util.h |
| 214 | +@@ -19,4 +19,6 @@ |
| 215 | + |
| 216 | + void safe_mkdir_p_dir(const char* file_path); |
| 217 | + void g_string_free_to_file(GString* s, const char* rootdir, const char* path, const char* suffix); |
| 218 | ++void _netplan_g_string_free_to_file_with_permissions(GString* s, const char* rootdir, const char* path, const char* suffix, const char* owner, const char* group, mode_t mode); |
| 219 | ++ |
| 220 | + void unlink_glob(const char* rootdir, const char* _glob); |
| 221 | +diff --git a/tests/generator/test_wifis.py b/tests/generator/test_wifis.py |
| 222 | +index 6bebe57..becfeae 100644 |
| 223 | +--- a/tests/generator/test_wifis.py |
| 224 | ++++ b/tests/generator/test_wifis.py |
| 225 | +@@ -65,7 +65,7 @@ network={ |
| 226 | + key_mgmt=NONE |
| 227 | + } |
| 228 | + ''') |
| 229 | +- self.assertEqual(stat.S_IMODE(os.fstat(f.fileno()).st_mode), 0o600) |
| 230 | ++ self.assertEqual(stat.S_IMODE(os.fstat(f.fileno()).st_mode), 0o640) |
| 231 | + self.assertTrue(os.path.islink(os.path.join( |
| 232 | + self.workdir.name, 'run/systemd/system/multi-user.target.wants/netplan-wpa@wl0.service'))) |
| 233 | + |
| 234 | +diff --git a/tests/integration/base.py b/tests/integration/base.py |
| 235 | +index 1c9de58..e7c89aa 100644 |
| 236 | +--- a/tests/integration/base.py |
| 237 | ++++ b/tests/integration/base.py |
| 238 | +@@ -28,6 +28,8 @@ import subprocess |
| 239 | + import tempfile |
| 240 | + import unittest |
| 241 | + import shutil |
| 242 | ++import pwd |
| 243 | ++import grp |
| 244 | + |
| 245 | + test_backends = "networkd NetworkManager" if "NETPLAN_TEST_BACKENDS" not in os.environ else os.environ["NETPLAN_TEST_BACKENDS"] |
| 246 | + |
| 247 | +@@ -354,7 +356,89 @@ class IntegrationTestsBase(unittest.TestCase): |
| 248 | + self.fail('timed out waiting for networkd to settle down:\n%s\n%s\n%s' % (st, st_e, st_e2)) |
| 249 | + |
| 250 | + if subprocess.call(['nm-online', '--quiet', '--timeout=120', '--wait-for-startup']) != 0: |
| 251 | +- self.fail('timed out waiting for NetworkManager to settle down') |
| 252 | ++ # Assert file permissions |
| 253 | ++ self.assert_file_permissions() |
| 254 | ++ |
| 255 | ++ def assert_file_permissions(self): |
| 256 | ++ """ Check if the generated files have the expected permissions """ |
| 257 | ++ |
| 258 | ++ nd_expected_mode = 0o100640 |
| 259 | ++ nd_expected_owner = 'root' |
| 260 | ++ nd_expected_group = 'systemd-network' |
| 261 | ++ |
| 262 | ++ sd_expected_mode = 0o100640 |
| 263 | ++ sd_expected_owner = 'root' |
| 264 | ++ sd_expected_group = 'root' |
| 265 | ++ |
| 266 | ++ udev_expected_mode = 0o100640 |
| 267 | ++ udev_expected_owner = 'root' |
| 268 | ++ udev_expected_group = 'root' |
| 269 | ++ |
| 270 | ++ nm_expected_mode = 0o100600 |
| 271 | ++ nm_expected_owner = 'root' |
| 272 | ++ nm_expected_group = 'root' |
| 273 | ++ |
| 274 | ++ wpa_expected_mode = 0o100600 |
| 275 | ++ wpa_expected_owner = 'root' |
| 276 | ++ wpa_expected_group = 'root' |
| 277 | ++ |
| 278 | ++ # Check systemd-networkd files |
| 279 | ++ base_path = '/run/systemd/network' |
| 280 | ++ files = glob.glob(f'{base_path}/*.network') + glob.glob(f'{base_path}/*.netdev') |
| 281 | ++ for file in files: |
| 282 | ++ res = os.stat(file) |
| 283 | ++ user = pwd.getpwuid(res.st_uid) |
| 284 | ++ group = grp.getgrgid(res.st_gid) |
| 285 | ++ self.assertEqual(res.st_mode, nd_expected_mode, f'file {file}') |
| 286 | ++ self.assertEqual(user.pw_name, nd_expected_owner, f'file {file}') |
| 287 | ++ self.assertEqual(group.gr_name, nd_expected_group, f'file {file}') |
| 288 | ++ |
| 289 | ++ # Check Network Manager files |
| 290 | ++ base_path = '/run/NetworkManager/system-connections' |
| 291 | ++ files = glob.glob(f'{base_path}/*.nmconnection') |
| 292 | ++ for file in files: |
| 293 | ++ res = os.stat(file) |
| 294 | ++ user = pwd.getpwuid(res.st_uid) |
| 295 | ++ group = grp.getgrgid(res.st_gid) |
| 296 | ++ self.assertEqual(res.st_mode, nm_expected_mode, f'file {file}') |
| 297 | ++ self.assertEqual(user.pw_name, nm_expected_owner, f'file {file}') |
| 298 | ++ self.assertEqual(group.gr_name, nm_expected_group, f'file {file}') |
| 299 | ++ |
| 300 | ++ # Check wpa_supplicant configuration files |
| 301 | ++ base_path = '/run/netplan' |
| 302 | ++ files = glob.glob(f'{base_path}/wpa-*.conf') |
| 303 | ++ for file in files: |
| 304 | ++ res = os.stat(file) |
| 305 | ++ user = pwd.getpwuid(res.st_uid) |
| 306 | ++ group = grp.getgrgid(res.st_gid) |
| 307 | ++ self.assertEqual(res.st_mode, wpa_expected_mode, f'file {file}') |
| 308 | ++ self.assertEqual(user.pw_name, wpa_expected_owner, f'file {file}') |
| 309 | ++ self.assertEqual(group.gr_name, wpa_expected_group, f'file {file}') |
| 310 | ++ |
| 311 | ++ # Check systemd service unit files |
| 312 | ++ base_path = '/run/systemd/system/' |
| 313 | ++ files = glob.glob(f'{base_path}/netplan-*.service') |
| 314 | ++ files += glob.glob(f'{base_path}/systemd-networkd-wait-online.service.d/*.conf') |
| 315 | ++ for file in files: |
| 316 | ++ res = os.stat(file) |
| 317 | ++ user = pwd.getpwuid(res.st_uid) |
| 318 | ++ group = grp.getgrgid(res.st_gid) |
| 319 | ++ self.assertEqual(res.st_mode, sd_expected_mode, f'file {file}') |
| 320 | ++ self.assertEqual(user.pw_name, sd_expected_owner, f'file {file}') |
| 321 | ++ self.assertEqual(group.gr_name, sd_expected_group, f'file {file}') |
| 322 | ++ |
| 323 | ++ # Check systemd-udevd files |
| 324 | ++ udev_path = '/run/udev/rules.d' |
| 325 | ++ link_path = '/run/systemd/network' |
| 326 | ++ files = glob.glob(f'{udev_path}/*-netplan*.rules') + glob.glob(f'{link_path}/*.link') |
| 327 | ++ for file in files: |
| 328 | ++ res = os.stat(file) |
| 329 | ++ user = pwd.getpwuid(res.st_uid) |
| 330 | ++ group = grp.getgrgid(res.st_gid) |
| 331 | ++ self.assertEqual(res.st_mode, udev_expected_mode, f'file {file}') |
| 332 | ++ self.assertEqual(user.pw_name, udev_expected_owner, f'file {file}') |
| 333 | ++ self.assertEqual(group.gr_name, udev_expected_group, f'file {file}') |
| 334 | ++ self.fail('timed out waiting for NetworkManager to settle down') |
| 335 | + |
| 336 | + def nm_wait_connected(self, iface, timeout): |
| 337 | + for t in range(timeout): |
| 338 | +-- |
| 339 | +2.34.1 |
| 340 | + |
0 commit comments