From 6069ef4b8bb620da2329d0e60a0a8a260379d686 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 --- 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 7c9982bbff..62891fa2cf 100644 --- a/src/initrd/nmi-cmdline-reader.c +++ b/src/initrd/nmi-cmdline-reader.c @@ -1154,14 +1154,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 07faf96a1f..a809dff93c 100644 --- a/src/initrd/tests/test-cmdline-reader.c +++ b/src/initrd/tests/test-cmdline-reader.c @@ -1952,58 +1952,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