From 39a13231816bd7e27efd6d56de9629f40ef07cda Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 23 Nov 2018 13:42:13 +0100 Subject: [PATCH 1/4] dhcp6-client: handle IAID with value zero config_parse_iaid(), dhcp_identifier_set_iaid() and sd_dhcp6_client_set_iaid() all allow for the IAID to be zero. Also, RFC 3315 makes no mention that zero would be invalid. However, client_ensure_iaid() would take an IAID of zero as a sign that the values was unset. Fix that by keeping track whether IAID is initialized. https://github.com/systemd/systemd/pull/10895 https://github.com/systemd/systemd/commit/0e408b82b8bd7675234cf58009475d4f4c0a491a --- src/systemd/src/libsystemd-network/sd-dhcp6-client.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/systemd/src/libsystemd-network/sd-dhcp6-client.c b/src/systemd/src/libsystemd-network/sd-dhcp6-client.c index 45b5b53e54..a1c9da7303 100644 --- a/src/systemd/src/libsystemd-network/sd-dhcp6-client.c +++ b/src/systemd/src/libsystemd-network/sd-dhcp6-client.c @@ -58,6 +58,7 @@ struct sd_dhcp6_client { struct sd_dhcp6_lease *lease; int fd; bool information_request; + bool has_iaid; be16_t *req_opts; size_t req_opts_allocated; size_t req_opts_len; @@ -274,6 +275,7 @@ int sd_dhcp6_client_set_iaid(sd_dhcp6_client *client, uint32_t iaid) { client->ia_na.ia_na.id = htobe32(iaid); client->ia_pd.ia_pd.id = htobe32(iaid); + client->has_iaid = true; return 0; } @@ -798,7 +800,7 @@ static int client_ensure_iaid(sd_dhcp6_client *client) { assert(client); - if (client->ia_na.ia_na.id) + if (client->has_iaid) return 0; r = dhcp_identifier_set_iaid(client->ifindex, client->mac_addr, client->mac_addr_len, true, &iaid); @@ -807,6 +809,7 @@ static int client_ensure_iaid(sd_dhcp6_client *client) { client->ia_na.ia_na.id = iaid; client->ia_pd.ia_pd.id = iaid; + client->has_iaid = true; return 0; } From 0ae18f06804b9c67aebd3970856273e8d8cc9bf0 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 23 Nov 2018 13:00:07 +0100 Subject: [PATCH 2/4] core: add nm_utils_create_dhcp_iaid() util Split out nm_utils_create_dhcp_iaid(), we will need it later. This is also a re-implementation of systemd's dhcp_identifier_set_iaid(). --- src/nm-core-utils.c | 62 ++++++++++++++++++++++++++++++---------- src/nm-core-utils.h | 4 +++ src/tests/test-general.c | 4 +++ 3 files changed, 55 insertions(+), 15 deletions(-) diff --git a/src/nm-core-utils.c b/src/nm-core-utils.c index 0ad28c8244..1c939495e6 100644 --- a/src/nm-core-utils.c +++ b/src/nm-core-utils.c @@ -3290,6 +3290,49 @@ nm_utils_hw_addr_gen_stable_eth (NMUtilsStableType stable_type, /*****************************************************************************/ +#define HASH_KEY ((const guint8[16]) { 0x80, 0x11, 0x8c, 0xc2, 0xfe, 0x4a, 0x03, 0xee, 0x3e, 0xd6, 0x0c, 0x6f, 0x36, 0x39, 0x14, 0x09 }) + +/** + * nm_utils_create_dhcp_iaid: + * @legacy_unstable_byteorder: legacy behavior is to generate a u32 iaid which + * is endianness dependant. This is to preserve backward compatibility. + * For non-legacy behavior, the returned integer is in stable endianness, + * and corresponds to legacy behavior on little endian systems. + * @interface_id: the seed for hashing when generating the ID. Usually, + * this is the interface name. + * @interface_id_len: length of @interface_id + * + * This corresponds to systemd's dhcp_identifier_set_iaid() for generating + * a IAID for the interface. + * + * Returns: the IAID in host byte order. */ +guint32 +nm_utils_create_dhcp_iaid (gboolean legacy_unstable_byteorder, + const guint8 *interface_id, + gsize interface_id_len) +{ + guint64 u64; + guint32 u32; + + u64 = c_siphash_hash (HASH_KEY, interface_id, interface_id_len); + u32 = (u64 & 0xffffffffu) ^ (u64 >> 32); + if (legacy_unstable_byteorder) { + /* legacy systemd code dhcp_identifier_set_iaid() generates the iaid + * dependent on the host endianness. Since this function returns the IAID + * in native-byte order, we need to account for that. + * + * On little endian systems, we want the legacy-behavior is identical to + * the endianness-agnostic behavior. So, we need to swap the bytes on + * big-endian systems. + * + * (https://github.com/systemd/systemd/pull/10614). */ + return htole32 (u32); + } else { + /* we return the value as-is, in native byte order. */ + return u32; + } +} + /** * nm_utils_dhcp_client_id_systemd_node_specific_full: * @legacy_unstable_byteorder: historically, the code would generate a iaid @@ -3315,7 +3358,6 @@ nm_utils_dhcp_client_id_systemd_node_specific_full (gboolean legacy_unstable_byt const guint8 *machine_id, gsize machine_id_len) { - const guint8 HASH_KEY[16] = { 0x80, 0x11, 0x8c, 0xc2, 0xfe, 0x4a, 0x03, 0xee, 0x3e, 0xd6, 0x0c, 0x6f, 0x36, 0x39, 0x14, 0x09 }; const guint16 DUID_TYPE_EN = 2; const guint32 SYSTEMD_PEN = 43793; struct _nm_packed { @@ -3344,20 +3386,10 @@ nm_utils_dhcp_client_id_systemd_node_specific_full (gboolean legacy_unstable_byt client_id->type = 255; - u64 = c_siphash_hash (HASH_KEY, interface_id, interface_id_len); - u32 = (u64 & 0xffffffffu) ^ (u64 >> 32); - if (legacy_unstable_byteorder) { - /* original systemd code dhcp_identifier_set_iaid() generates the iaid - * in native endianness. Do that too, to preserve compatibility - * (https://github.com/systemd/systemd/pull/10614). */ - u32 = bswap_32 (u32); - } else { - /* generate fixed byteorder, in a way that on little endian systems - * the values agree. Meaning: legacy behavior is identical to this - * on little endian. */ - u32 = be32toh (u32); - } - unaligned_write_ne32 (&client_id->iaid, u32); + u32 = nm_utils_create_dhcp_iaid (legacy_unstable_byteorder, + interface_id, + interface_id_len); + unaligned_write_be32 (&client_id->iaid, u32); unaligned_write_be16 (&client_id->duid.type, DUID_TYPE_EN); diff --git a/src/nm-core-utils.h b/src/nm-core-utils.h index e59ef41d11..d75ef9ae48 100644 --- a/src/nm-core-utils.h +++ b/src/nm-core-utils.h @@ -376,6 +376,10 @@ char *nm_utils_hw_addr_gen_stable_eth (NMUtilsStableType stable_type, /*****************************************************************************/ +guint32 nm_utils_create_dhcp_iaid (gboolean legacy_unstable_byteorder, + const guint8 *interface_id, + gsize interface_id_len); + GBytes *nm_utils_dhcp_client_id_systemd_node_specific_full (gboolean legacy_unstable_byteorder, const guint8 *interface_id, gsize interface_id_len, diff --git a/src/tests/test-general.c b/src/tests/test-general.c index a2fb1495c4..e99158345d 100644 --- a/src/tests/test-general.c +++ b/src/tests/test-general.c @@ -2029,6 +2029,10 @@ test_nm_utils_dhcp_client_id_systemd_node_specific (gconstpointer test_data) g_assert_cmpmem (&cid[5], 2, &duid_type_en, sizeof (duid_type_en)); g_assert_cmpmem (&cid[7], 4, &systemd_pen, sizeof (systemd_pen)); g_assert_cmpmem (&cid[11], 8, &d->duid_id, sizeof (d->duid_id)); + + g_assert_cmpint (iaid, ==, htonl (nm_utils_create_dhcp_iaid (legacy_unstable_byteorder, + (const guint8 *) d->ifname, + strlen (d->ifname)))); } } From f2f44a7acaeec4681d37c3cffffb5601c2e421f2 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 23 Nov 2018 14:03:08 +0100 Subject: [PATCH 3/4] dhcp: always explicitly set IAID of internal DHCPv6 client During start, sd_dhcp6_client would call client_ensure_iaid(), to initialize the IAID. First of all, the IAID is important to us, we should not leave the used IAID up to an implementation detail. In the future, we possibly want to make this configurable. The problem is that client_ensure_iaid() calls dhcp_identifier_set_iaid() which looks up the interface name via if_indextoname() (in our fork). The problem is that dhcp_identifier_set_iaid() looks up information by quering the system, which makes it hard for testing and also makes it unpredictable. It's better to use our implementation nm_utils_create_dhcp_iaid(), which is solely based on the interface-name, which is given as a well defined parameter to the DHCP client instance. --- src/dhcp/nm-dhcp-systemd.c | 12 ++++++++++++ src/systemd/src/libsystemd-network/sd-dhcp6-client.c | 2 -- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/dhcp/nm-dhcp-systemd.c b/src/dhcp/nm-dhcp-systemd.c index c33de3accd..bba0cde34a 100644 --- a/src/dhcp/nm-dhcp-systemd.c +++ b/src/dhcp/nm-dhcp-systemd.c @@ -894,6 +894,7 @@ ip6_start (NMDhcpClient *client, nm_auto (sd_dhcp6_client_unrefp) sd_dhcp6_client *sd_client = NULL; GBytes *hwaddr; const char *hostname; + const char *iface; int r, i; const guint8 *duid_arr; gsize duid_len; @@ -928,6 +929,17 @@ ip6_start (NMDhcpClient *client, if (nm_dhcp_client_get_info_only (client)) sd_dhcp6_client_set_information_request (sd_client, 1); + iface = nm_dhcp_client_get_iface (client); + + r = sd_dhcp6_client_set_iaid (sd_client, + nm_utils_create_dhcp_iaid (TRUE, + (const guint8 *) iface, + strlen (iface))); + if (r < 0) { + nm_utils_error_set_errno (error, r, "failed to set IAID: %s"); + return FALSE; + } + r = sd_dhcp6_client_set_duid (sd_client, unaligned_read_be16 (&duid_arr[0]), &duid_arr[2], diff --git a/src/systemd/src/libsystemd-network/sd-dhcp6-client.c b/src/systemd/src/libsystemd-network/sd-dhcp6-client.c index a1c9da7303..e39b185f2e 100644 --- a/src/systemd/src/libsystemd-network/sd-dhcp6-client.c +++ b/src/systemd/src/libsystemd-network/sd-dhcp6-client.c @@ -262,7 +262,6 @@ int sd_dhcp6_client_set_duid( return dhcp6_client_set_duid_internal(client, duid_type, duid, duid_len, 0); } -#if 0 /* NM_IGNORED */ int sd_dhcp6_client_set_duid_llt( sd_dhcp6_client *client, usec_t llt_time) { @@ -279,7 +278,6 @@ int sd_dhcp6_client_set_iaid(sd_dhcp6_client *client, uint32_t iaid) { return 0; } -#endif /* NM_IGNORED */ int sd_dhcp6_client_set_fqdn( sd_dhcp6_client *client, From 80ac8e40a258ba85fb9a2f079ecf5bdf5dcde0da Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 23 Nov 2018 12:11:38 +0100 Subject: [PATCH 4/4] dhcp: disable systemd's dhcp_identifier_set_iaid() dhcp_identifier_set_iaid() is no longer called. Replace the code with an assertion and always fail. The function was already annoying previously, because it would require udev API to generate the IAID. So, we were already required to patch this function. --- .../src/libsystemd-network/dhcp-identifier.c | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/src/systemd/src/libsystemd-network/dhcp-identifier.c b/src/systemd/src/libsystemd-network/dhcp-identifier.c index 9cb64344ec..a9fc8de164 100644 --- a/src/systemd/src/libsystemd-network/dhcp-identifier.c +++ b/src/systemd/src/libsystemd-network/dhcp-identifier.c @@ -15,11 +15,6 @@ #include "sparse-endian.h" #include "virt.h" -#if 0 /* NM_IGNORED */ -#else /* NM_IGNORED */ -#include -#endif /* NM_IGNORED */ - #define SYSTEMD_PEN 43793 #define HASH_KEY SD_ID128_MAKE(80,11,8c,c2,fe,4a,03,ee,3e,d6,0c,6f,36,39,14,09) #define APPLICATION_ID SD_ID128_MAKE(a5,0a,d1,12,bf,60,45,77,a2,fb,74,1a,b1,95,5b,03) @@ -169,14 +164,10 @@ int dhcp_identifier_set_iaid( /* name is a pointer to memory in the sd_device struct, so must * have the same scope */ _cleanup_(sd_device_unrefp) sd_device *device = NULL; -#else /* NM_IGNORED */ - char name_buf[IF_NAMESIZE]; -#endif /* NM_IGNORED */ const char *name = NULL; uint64_t id; uint32_t id32; -#if 0 /* NM_IGNORED */ if (detect_container() <= 0) { /* not in a container, udev will be around */ char ifindex_str[2 + DECIMAL_STR_MAX(int)]; @@ -194,9 +185,6 @@ int dhcp_identifier_set_iaid( name = net_get_name(device); } } -#else /* NM_IGNORED */ - name = if_indextoname(ifindex, name_buf); -#endif /* NM_IGNORED */ if (name) id = siphash24(name, strlen(name), HASH_KEY.bytes); @@ -218,4 +206,9 @@ int dhcp_identifier_set_iaid( unaligned_write_ne32(_id, id32); return 0; +#else /* NM_IGNORED */ + /* for NetworkManager, we don't use this function and we should never call here. + * This got replaced by nm_utils_create_dhcp_iaid(). */ + g_return_val_if_reached (-EINVAL); +#endif /* NM_IGNORED */ }