dhcp: don't enforce hardware address length for sd_dhcp_client_set_client_id()

sd_dhcp_client_set_client_id() is the only API for setting a raw client-id.
All other setters are more restricted and only allow to set a type 255 DUID.

Also, dhcp4_set_client_identifier() is the only caller, which already
does:

                r = sd_dhcp_client_set_client_id(link->dhcp_client,
                                                 ARPHRD_ETHER,
                                                 (const uint8_t *) &link->mac,
                                                 sizeof(link->mac));

and hence ensures that the data length is indeed ETH_ALEN.

Drop additional input validation from sd_dhcp_client_set_client_id(). The client-id
is an opaque blob, and if a caller wishes to set type 1 (ethernet) or type 32
(infiniband) with unexpected address length, it should be allowed. The actual
client-id is not relevant to the DHCP client, and it's the responsibility of the
caller to generate a suitable client-id.

For example, in NetworkManager you can configure all the bytes of the
client-id, including such _invalid_ settings. I think it makes sense,
to allow the user to fully configure the identifier. Even if such configuration
would be rejected, it would be the responsibility of the higher layers (including
a sensible error message to the user) and not fail later during
sd_dhcp_client_set_client_id().

Still log a debug message if the length is unexpected.

1 files changed, 11 insertions, 18 deletions

diff --git a/src/libsystemd-network/sd-dhcp-client.c b/src/libsystemd-network/sd-dhcp-client.c
index ff7f54793..f39af0d32 100644
--- a/src/libsystemd-network/sd-dhcp-client.c
+++ b/src/libsystemd-network/sd-dhcp-client.c
@@ -300,29 +300,22 @@ int sd_dhcp_client_set_client_id(
         assert_return(data, -EINVAL);
         assert_return(data_len > 0 && data_len <= MAX_CLIENT_ID_LEN, -EINVAL);
 
-        switch (type) {
-
-        case ARPHRD_ETHER:
-                if (data_len != ETH_ALEN)
-                        return -EINVAL;
-                break;
-
-        case ARPHRD_INFINIBAND:
-                /* Infiniband addresses are 20 bytes (INFINIBAND_ALEN), however only
-                 * the last 8 bytes are stable and suitable for putting into the client-id. */
-                if (data_len != 8)
-                        return -EINVAL;
-                break;
-
-        default:
-                break;
-        }
-
         if (client->client_id_len == data_len + sizeof(client->client_id.type) &&
             client->client_id.type == type &&
             memcmp(&client->client_id.raw.data, data, data_len) == 0)
                 return 0;
 
+        /* For hardware types, log debug message about unexpected data length.
+         *
+         * Note that infiniband's INFINIBAND_ALEN is 20 bytes long, but only
+         * last last 8 bytes of the address are stable and suitable to put into
+         * the client-id. The caller is advised to account for that. */
+        if ((type == ARPHRD_ETHER && data_len != ETH_ALEN) ||
+            (type == ARPHRD_INFINIBAND && data_len != 8))
+                log_dhcp_client(client, "Changing client ID to hardware type %u with "
+                                "unexpected address length %zu",
+                                type, data_len);
+
         if (!IN_SET(client->state, DHCP_STATE_INIT, DHCP_STATE_STOPPED)) {
                 log_dhcp_client(client, "Changing client ID on running DHCP "
                                 "client, restarting");