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

diff --git a/src/libsystemd-network/dhcp-identifier.c b/src/libsystemd-network/dhcp-identifier.c
index 794e3a6f54ad56965200e64bba52aa84d841cdbb..c768adeb6da39ff2a1f540bd201e4e9606b69b36 100644 (file)
--- 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 64315d3a3bd6f0348d21cfcf45a9bcf221ef6c72..a544f38ab8652a1c073227f24b787b19321aa45d 100644 (file)
--- 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 dde2d0ceb06732a80d4e37fb6e099a614072425d..c538362488fcd6633e63ef9fc9764d4c2901f7ea 100644 (file)
--- 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 2ddfead4b0e6c0d4806372723cd136a9006e7887..15aece87d808ab438ce23539921bce2411297692 100644 (file)
--- 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 e92d4afac0905628f44b2c1d61c1fd88a1631779..fe6788d91b460ab41f337f8b30918d5d0b3c045a 100644 (file)
--- 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);