diff options
author | 2018-12-20 11:56:02 +0100 | |
---|---|---|
committer | 2018-12-20 13:40:39 +0100 | |
commit | ab4a88bc29e31754ec50c4a865058ee36f6284a6 (patch) | |
tree | 3cf07c9cc723eaa604e666569b507688421005a1 | |
parent | dhcp: don't enforce hardware address length for sd_dhcp_client_set_client_id() (diff) | |
download | systemd-ab4a88bc29e31754ec50c4a865058ee36f6284a6.tar.gz systemd-ab4a88bc29e31754ec50c4a865058ee36f6284a6.tar.bz2 systemd-ab4a88bc29e31754ec50c4a865058ee36f6284a6.zip |
dhcp6: don't enforce DUID content for sd_dhcp6_client_set_duid()
There are various functions to set the DUID of a DHCPv6 client.
However, none of them allows to set arbitrary data. The closest is
sd_dhcp6_client_set_duid(), which would still do validation of the
DUID's content via dhcp_validate_duid_len().
Relax the validation and only log a debug message if the DUID
does not validate.
Note that dhcp_validate_duid_len() already is not very strict. For example
with DUID_TYPE_LLT it only ensures that the length is suitable to contain
hwtype and time. It does not further check that the length of hwaddr is non-zero
or suitable for hwtype. Also, non-well-known DUID types are accepted for
extensibility. Why reject certain DUIDs but allowing clearly wrong formats
otherwise?
The validation and failure should happen earlier, when accepting the
unsuitable DUID. At that point, there is more context of what is wrong,
and a better failure reason (or warning) can be reported to the user. Rejecting
the DUID when setting up the DHCPv6 client seems not optimal, in particular
because the DHCPv6 client does not care about actual content of the
DUID and treats it as opaque blob.
Also, NetworkManager (which uses this code) allows to configure the entire
binary DUID in binary. It intentionally does not validate the binary
content any further. Hence, it needs to be able to set _invalid_ DUIDs,
provided that some basic constraints are satisfied (like the maximum length).
sd_dhcp6_client_set_duid() has two callers: both set the DUID obtained
from link_get_duid(), which comes from configuration.
`man networkd.conf` says: "The configured DHCP DUID should conform to
the specification in RFC 3315, RFC 6355.". It does not not state that
it MUST conform.
Note that dhcp_validate_duid_len() has another caller: DHCPv4's
dhcp_client_set_iaid_duid_internal(). In this case, continue with
strict validation, as the callers are more controlled. Also, there is
already sd_dhcp_client_set_client_id() which can be used to bypass
this check and set arbitrary client identifiers.
-rw-r--r-- | src/libsystemd-network/dhcp-identifier.c | 8 | ||||
-rw-r--r-- | src/libsystemd-network/dhcp-identifier.h | 2 | ||||
-rw-r--r-- | src/libsystemd-network/sd-dhcp-client.c | 2 | ||||
-rw-r--r-- | src/libsystemd-network/sd-dhcp6-client.c | 10 |
4 files changed, 16 insertions, 6 deletions
diff --git a/src/libsystemd-network/dhcp-identifier.c b/src/libsystemd-network/dhcp-identifier.c index c768adeb6..07496adaa 100644 --- a/src/libsystemd-network/dhcp-identifier.c +++ b/src/libsystemd-network/dhcp-identifier.c @@ -18,13 +18,19 @@ #define APPLICATION_ID SD_ID128_MAKE(a5,0a,d1,12,bf,60,45,77,a2,fb,74,1a,b1,95,5b,03) #define USEC_2000 ((usec_t) 946684800000000) /* 2000-01-01 00:00:00 UTC */ -int dhcp_validate_duid_len(uint16_t duid_type, size_t duid_len) { +int dhcp_validate_duid_len(uint16_t duid_type, size_t duid_len, bool strict) { struct duid d; assert_cc(sizeof(d.raw) >= MAX_DUID_LEN); if (duid_len > MAX_DUID_LEN) return -EINVAL; + if (!strict) { + /* Strict validation is not requested. We only ensure that the + * DUID is not too long. */ + return 0; + } + switch (duid_type) { case DUID_TYPE_LLT: if (duid_len <= sizeof(d.llt)) diff --git a/src/libsystemd-network/dhcp-identifier.h b/src/libsystemd-network/dhcp-identifier.h index a544f38ab..b3115125d 100644 --- a/src/libsystemd-network/dhcp-identifier.h +++ b/src/libsystemd-network/dhcp-identifier.h @@ -52,7 +52,7 @@ struct duid { }; } _packed_; -int dhcp_validate_duid_len(uint16_t duid_type, size_t duid_len); +int dhcp_validate_duid_len(uint16_t duid_type, size_t duid_len, bool strict); int dhcp_identifier_set_duid_llt(struct duid *duid, usec_t t, const uint8_t *addr, size_t addr_len, uint16_t arp_type, size_t *len); 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); diff --git a/src/libsystemd-network/sd-dhcp-client.c b/src/libsystemd-network/sd-dhcp-client.c index f39af0d32..2d7ffd22c 100644 --- a/src/libsystemd-network/sd-dhcp-client.c +++ b/src/libsystemd-network/sd-dhcp-client.c @@ -356,7 +356,7 @@ static int dhcp_client_set_iaid_duid_internal( assert_return(duid_len == 0 || duid != NULL, -EINVAL); if (duid != NULL) { - r = dhcp_validate_duid_len(duid_type, duid_len); + r = dhcp_validate_duid_len(duid_type, duid_len, true); if (r < 0) return r; } diff --git a/src/libsystemd-network/sd-dhcp6-client.c b/src/libsystemd-network/sd-dhcp6-client.c index 8ff4585f6..593ffd1b6 100644 --- a/src/libsystemd-network/sd-dhcp6-client.c +++ b/src/libsystemd-network/sd-dhcp6-client.c @@ -206,9 +206,13 @@ static int dhcp6_client_set_duid_internal( assert_return(IN_SET(client->state, DHCP6_STATE_STOPPED), -EBUSY); if (duid != NULL) { - r = dhcp_validate_duid_len(duid_type, duid_len); - if (r < 0) - return r; + r = dhcp_validate_duid_len(duid_type, duid_len, true); + if (r < 0) { + r = dhcp_validate_duid_len(duid_type, duid_len, false); + if (r < 0) + return r; + log_dhcp6_client(client, "Setting DUID of type %u with unexpected content", duid_type); + } client->duid.type = htobe16(duid_type); memcpy(&client->duid.raw.data, duid, duid_len); |