From 6d13616b9e97743b45794deb892b28b4d6ab1835 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 1 Nov 2018 14:43:11 +0100 Subject: [PATCH] dhcp: support endianness independent dhcp_identifier_set_iaid() The previous code did htole64() followed by unaligned_write_be32() (the XOR and shift in between is endianness agnostic). That means, on every architeture there is always exactly one byte swap and the iaid is dependent on endianness. Since dhcp_identifier_set_iaid() is part of the DUID generation algorithm, this cannot be fixed without changing the client-id. In particular, as the client-id already depends on the machine-id (and is thus inherrently host-specific), it is better to stick to the current behavior. However, add a parameter to switch between old and new behaviour. Since the new behavior is unused, the only real purpose of this change is to self-document the oddity of the function. Fixes: 933f9caeeb2b3c1b951d330e04beb04226e5a890 --- src/libsystemd-network/dhcp-identifier.c | 22 ++++++++++++++++++---- src/libsystemd-network/dhcp-identifier.h | 2 +- src/libsystemd-network/sd-dhcp-client.c | 4 +++- src/libsystemd-network/sd-dhcp6-client.c | 4 ++-- src/libsystemd-network/test-dhcp-client.c | 18 +++++++++++++----- 5 files changed, 37 insertions(+), 13 deletions(-) diff --git a/src/libsystemd-network/dhcp-identifier.c b/src/libsystemd-network/dhcp-identifier.c index 794e3a6f54a..c768adeb6da 100644 --- a/src/libsystemd-network/dhcp-identifier.c +++ b/src/libsystemd-network/dhcp-identifier.c @@ -148,12 +148,18 @@ int dhcp_identifier_set_duid_uuid(struct duid *duid, size_t *len) { return 0; } -int dhcp_identifier_set_iaid(int ifindex, uint8_t *mac, size_t mac_len, void *_id) { +int dhcp_identifier_set_iaid( + int ifindex, + const uint8_t *mac, + size_t mac_len, + bool legacy_unstable_byteorder, + void *_id) { /* 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; const char *name = NULL; uint64_t id; + uint32_t id32; if (detect_container() <= 0) { /* not in a container, udev will be around */ @@ -179,10 +185,18 @@ int dhcp_identifier_set_iaid(int ifindex, uint8_t *mac, size_t mac_len, void *_i /* fall back to MAC address if no predictable name available */ id = siphash24(mac, mac_len, HASH_KEY.bytes); - id = htole64(id); + id32 = (id & 0xffffffff) ^ (id >> 32); - /* fold into 32 bits */ - unaligned_write_be32(_id, (id & 0xffffffff) ^ (id >> 32)); + if (legacy_unstable_byteorder) + /* for historical reasons (a bug), the bits were swapped and thus + * the result was endianness dependant. Preserve that behavior. */ + id32 = __bswap_32(id32); + else + /* the fixed behavior returns a stable byte order. Since LE is expected + * to be more common, swap the bytes on LE to give the same as legacy + * behavior. */ + id32 = be32toh(id32); + unaligned_write_ne32(_id, id32); return 0; } diff --git a/src/libsystemd-network/dhcp-identifier.h b/src/libsystemd-network/dhcp-identifier.h index 64315d3a3bd..a544f38ab86 100644 --- a/src/libsystemd-network/dhcp-identifier.h +++ b/src/libsystemd-network/dhcp-identifier.h @@ -57,4 +57,4 @@ int dhcp_identifier_set_duid_llt(struct duid *duid, usec_t t, const uint8_t *add int dhcp_identifier_set_duid_ll(struct duid *duid, const uint8_t *addr, size_t addr_len, uint16_t arp_type, size_t *len); int dhcp_identifier_set_duid_en(struct duid *duid, size_t *len); int dhcp_identifier_set_duid_uuid(struct duid *duid, size_t *len); -int dhcp_identifier_set_iaid(int ifindex, uint8_t *mac, size_t mac_len, void *_id); +int dhcp_identifier_set_iaid(int ifindex, const uint8_t *mac, size_t mac_len, bool legacy_unstable_byteorder, void *_id); diff --git a/src/libsystemd-network/sd-dhcp-client.c b/src/libsystemd-network/sd-dhcp-client.c index dde2d0ceb06..c538362488f 100644 --- a/src/libsystemd-network/sd-dhcp-client.c +++ b/src/libsystemd-network/sd-dhcp-client.c @@ -371,6 +371,7 @@ static int dhcp_client_set_iaid_duid_internal( if (iaid == 0) { r = dhcp_identifier_set_iaid(client->ifindex, client->mac_addr, client->mac_addr_len, + true, &client->client_id.ns.iaid); if (r < 0) return r; @@ -650,7 +651,8 @@ static int client_message_init( client->client_id.type = 255; - r = dhcp_identifier_set_iaid(client->ifindex, client->mac_addr, client->mac_addr_len, &client->client_id.ns.iaid); + r = dhcp_identifier_set_iaid(client->ifindex, client->mac_addr, client->mac_addr_len, + true, &client->client_id.ns.iaid); if (r < 0) return r; diff --git a/src/libsystemd-network/sd-dhcp6-client.c b/src/libsystemd-network/sd-dhcp6-client.c index 2ddfead4b0e..15aece87d80 100644 --- a/src/libsystemd-network/sd-dhcp6-client.c +++ b/src/libsystemd-network/sd-dhcp6-client.c @@ -807,14 +807,14 @@ error: static int client_ensure_iaid(sd_dhcp6_client *client) { int r; - be32_t iaid; + uint32_t iaid; assert(client); if (client->ia_na.ia_na.id) return 0; - r = dhcp_identifier_set_iaid(client->ifindex, client->mac_addr, client->mac_addr_len, &iaid); + r = dhcp_identifier_set_iaid(client->ifindex, client->mac_addr, client->mac_addr_len, true, &iaid); if (r < 0) return r; diff --git a/src/libsystemd-network/test-dhcp-client.c b/src/libsystemd-network/test-dhcp-client.c index e92d4afac09..fe6788d91b4 100644 --- a/src/libsystemd-network/test-dhcp-client.c +++ b/src/libsystemd-network/test-dhcp-client.c @@ -156,7 +156,8 @@ static void test_checksum(void) { } static void test_dhcp_identifier_set_iaid(void) { - uint32_t iaid; + uint32_t iaid_legacy; + be32_t iaid; int ifindex; for (;;) { @@ -169,11 +170,18 @@ static void test_dhcp_identifier_set_iaid(void) { break; } - assert_se(dhcp_identifier_set_iaid(ifindex, mac_addr, sizeof(mac_addr), &iaid) >= 0); + assert_se(dhcp_identifier_set_iaid(ifindex, mac_addr, sizeof(mac_addr), true, &iaid_legacy) >= 0); + assert_se(dhcp_identifier_set_iaid(ifindex, mac_addr, sizeof(mac_addr), false, &iaid) >= 0); - /* we expect, that the MAC address was hashed. Note that the value is in native + /* we expect, that the MAC address was hashed. The legacy value is in native * endianness. */ - assert_se(iaid == 0x8dde4ba8u); + assert_se(iaid_legacy == 0x8dde4ba8u); + assert_se(iaid == htole32(0x8dde4ba8u)); +#if __BYTE_ORDER == __LITTLE_ENDIAN + assert_se(iaid == iaid_legacy); +#else + assert_se(iaid == __bswap_32(iaid_legacy)); +#endif } static int check_options(uint8_t code, uint8_t len, const void *option, void *userdata) { @@ -185,7 +193,7 @@ static int check_options(uint8_t code, uint8_t len, const void *option, void *us size_t duid_len; assert_se(dhcp_identifier_set_duid_en(&duid, &duid_len) >= 0); - assert_se(dhcp_identifier_set_iaid(42, mac_addr, ETH_ALEN, &iaid) >= 0); + assert_se(dhcp_identifier_set_iaid(42, mac_addr, ETH_ALEN, true, &iaid) >= 0); assert_se(len == sizeof(uint8_t) + sizeof(uint32_t) + duid_len); assert_se(len == 19); -- 2.39.5