From 516433f769d169ff234d921da04f778ec446c6b4 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Wed, 23 Dec 2020 14:21:21 +0100 Subject: [PATCH] initrd: accept a zero-byte prefix for BOOTIF The BOOTIF MAC address can be prefixed with a hardware address type. Typically it is 01 (for ethernet), but the legacy network module accepts (and strips) any byte value. It seems wrong to take any address type without validation. In addition to "01", also accept a zero type which, according to the bugzilla below, is used in some configurations to mean "undefined". While at it, also accept ':' as separator for the first byte. https://bugzilla.redhat.com/show_bug.cgi?id=1904099 https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/713 (cherry picked from commit 6069ef4b8bb620da2329d0e60a0a8a260379d686) --- src/initrd/nmi-cmdline-reader.c | 21 ++++-- src/initrd/tests/test-cmdline-reader.c | 89 +++++++++++++++----------- 2 files changed, 65 insertions(+), 45 deletions(-) diff --git a/src/initrd/nmi-cmdline-reader.c b/src/initrd/nmi-cmdline-reader.c index 1c1b43ebb6..1f3d3a4d0c 100644 --- a/src/initrd/nmi-cmdline-reader.c +++ b/src/initrd/nmi-cmdline-reader.c @@ -1140,14 +1140,21 @@ nmi_cmdline_reader_parse(const char *sysfs_dir, const char *const *argv, char ** NMConnection * connection; NMSettingWired *s_wired; const char * bootif = bootif_val; + char prefix[4]; - if (!nm_utils_hwaddr_valid(bootif, ETH_ALEN) && g_str_has_prefix(bootif, "01-") - && nm_utils_hwaddr_valid(&bootif[3], ETH_ALEN)) { - /* - * BOOTIF MAC address can be prefixed with a hardware type identifier. - * "01" stays for "wired", no other are known. - */ - bootif += 3; + if (!nm_utils_hwaddr_valid(bootif, ETH_ALEN)) { + strncpy(prefix, bootif, 3); + prefix[3] = '\0'; + + if (NM_IN_STRSET(prefix, "01-", "01:", "00-", "00:") + && nm_utils_hwaddr_valid(&bootif[3], ETH_ALEN)) { + /* + * BOOTIF MAC address can be prefixed with a hardware type identifier. + * "01" stays for "wired", "00" is also accepted as it means "undefined". + * No others are known. + */ + bootif += 3; + } } connection = reader_get_connection(reader, NULL, NM_SETTING_WIRED_SETTING_NAME, FALSE); diff --git a/src/initrd/tests/test-cmdline-reader.c b/src/initrd/tests/test-cmdline-reader.c index 14a83c084a..5be03bf506 100644 --- a/src/initrd/tests/test-cmdline-reader.c +++ b/src/initrd/tests/test-cmdline-reader.c @@ -1953,58 +1953,71 @@ static void test_bootif_hwtype(void) { gs_unref_hashtable GHashTable *connections = NULL; - const char *const *ARGV = NM_MAKE_STRV("ip=eth0:dhcp", "BOOTIF=01-00-53-AB-cd-02-03"); + const char *const *ARGV0 = NM_MAKE_STRV("ip=eth0:dhcp", "BOOTIF=01-00-53-AB-cd-02-03"); + const char *const *ARGV1 = NM_MAKE_STRV("ip=eth0:dhcp", "BOOTIF=00-00-53-Ab-cD-02-03"); + const char *const *ARGV[] = {ARGV0, ARGV1}; NMConnection * connection; NMSettingWired * s_wired; NMSettingIPConfig *s_ip4; NMSettingIPConfig *s_ip6; gs_free char * hostname = NULL; + guint i; - connections = nmi_cmdline_reader_parse(TEST_INITRD_DIR "/sysfs", ARGV, &hostname); - g_assert(connections); - g_assert_cmpint(g_hash_table_size(connections), ==, 2); - g_assert_cmpstr(hostname, ==, NULL); + for (i = 0; i < G_N_ELEMENTS(ARGV); i++) { + connections = nmi_cmdline_reader_parse(TEST_INITRD_DIR "/sysfs", ARGV[i], &hostname); + g_assert(connections); + g_assert_cmpint(g_hash_table_size(connections), ==, 2); + g_assert_cmpstr(hostname, ==, NULL); - connection = g_hash_table_lookup(connections, "eth0"); - g_assert(connection); - nmtst_assert_connection_verifies_without_normalization(connection); - g_assert_cmpstr(nm_connection_get_id(connection), ==, "eth0"); + connection = g_hash_table_lookup(connections, "eth0"); + g_assert(connection); + nmtst_assert_connection_verifies_without_normalization(connection); + g_assert_cmpstr(nm_connection_get_id(connection), ==, "eth0"); - s_wired = nm_connection_get_setting_wired(connection); - g_assert(!nm_setting_wired_get_mac_address(s_wired)); - g_assert(s_wired); + s_wired = nm_connection_get_setting_wired(connection); + g_assert(!nm_setting_wired_get_mac_address(s_wired)); + g_assert(s_wired); - s_ip4 = nm_connection_get_setting_ip4_config(connection); - g_assert(s_ip4); - g_assert_cmpstr(nm_setting_ip_config_get_method(s_ip4), ==, NM_SETTING_IP4_CONFIG_METHOD_AUTO); - g_assert(!nm_setting_ip_config_get_ignore_auto_dns(s_ip4)); - g_assert(!nm_setting_ip_config_get_may_fail(s_ip4)); + s_ip4 = nm_connection_get_setting_ip4_config(connection); + g_assert(s_ip4); + g_assert_cmpstr(nm_setting_ip_config_get_method(s_ip4), + ==, + NM_SETTING_IP4_CONFIG_METHOD_AUTO); + g_assert(!nm_setting_ip_config_get_ignore_auto_dns(s_ip4)); + g_assert(!nm_setting_ip_config_get_may_fail(s_ip4)); - s_ip6 = nm_connection_get_setting_ip6_config(connection); - g_assert(s_ip6); - g_assert_cmpstr(nm_setting_ip_config_get_method(s_ip6), ==, NM_SETTING_IP6_CONFIG_METHOD_AUTO); - g_assert(!nm_setting_ip_config_get_ignore_auto_dns(s_ip6)); + s_ip6 = nm_connection_get_setting_ip6_config(connection); + g_assert(s_ip6); + g_assert_cmpstr(nm_setting_ip_config_get_method(s_ip6), + ==, + NM_SETTING_IP6_CONFIG_METHOD_AUTO); + g_assert(!nm_setting_ip_config_get_ignore_auto_dns(s_ip6)); - connection = g_hash_table_lookup(connections, "bootif_connection"); - g_assert(connection); - nmtst_assert_connection_verifies_without_normalization(connection); - g_assert_cmpstr(nm_connection_get_id(connection), ==, "BOOTIF Connection"); + connection = g_hash_table_lookup(connections, "bootif_connection"); + g_assert(connection); + nmtst_assert_connection_verifies_without_normalization(connection); + g_assert_cmpstr(nm_connection_get_id(connection), ==, "BOOTIF Connection"); - s_wired = nm_connection_get_setting_wired(connection); - g_assert_cmpstr(nm_setting_wired_get_mac_address(s_wired), ==, "00:53:AB:CD:02:03"); - g_assert(s_wired); + s_wired = nm_connection_get_setting_wired(connection); + g_assert_cmpstr(nm_setting_wired_get_mac_address(s_wired), ==, "00:53:AB:CD:02:03"); + g_assert(s_wired); - s_ip4 = nm_connection_get_setting_ip4_config(connection); - g_assert(s_ip4); - g_assert_cmpstr(nm_setting_ip_config_get_method(s_ip4), ==, NM_SETTING_IP4_CONFIG_METHOD_AUTO); - g_assert(!nm_setting_ip_config_get_ignore_auto_dns(s_ip4)); - g_assert(nm_setting_ip_config_get_may_fail(s_ip4)); + s_ip4 = nm_connection_get_setting_ip4_config(connection); + g_assert(s_ip4); + g_assert_cmpstr(nm_setting_ip_config_get_method(s_ip4), + ==, + NM_SETTING_IP4_CONFIG_METHOD_AUTO); + g_assert(!nm_setting_ip_config_get_ignore_auto_dns(s_ip4)); + g_assert(nm_setting_ip_config_get_may_fail(s_ip4)); - s_ip6 = nm_connection_get_setting_ip6_config(connection); - g_assert(s_ip6); - g_assert_cmpstr(nm_setting_ip_config_get_method(s_ip6), ==, NM_SETTING_IP6_CONFIG_METHOD_AUTO); - g_assert(!nm_setting_ip_config_get_ignore_auto_dns(s_ip6)); - g_assert(nm_setting_ip_config_get_may_fail(s_ip6)); + s_ip6 = nm_connection_get_setting_ip6_config(connection); + g_assert(s_ip6); + g_assert_cmpstr(nm_setting_ip_config_get_method(s_ip6), + ==, + NM_SETTING_IP6_CONFIG_METHOD_AUTO); + g_assert(!nm_setting_ip_config_get_ignore_auto_dns(s_ip6)); + g_assert(nm_setting_ip_config_get_may_fail(s_ip6)); + } } /* Check that nameservers are assigned to all existing