picnoir/Systemd
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

dhcp: ignore padding of 'chaddr' in DHCP server response

Browse source 
The "chaddr" field is 16 bytes long, with "hlen" being the
length of the address.

https://tools.ietf.org/html/rfc2131#section-4.3.1 says:

    The server MUST return to the client:

    ...

    o Any parameters specific to this client (as identified by
      the contents of 'chaddr' or 'client identifier' in the DHCPDISCOVER
      or DHCPREQUEST message), e.g., as configured by the network
      administrator,

It's not clear, whether only the first 'hlen' bytes of 'chaddr'
must correspond or all 16 bytes.

Note that https://tools.ietf.org/html/rfc4390#section-2.1 says for IPoIB

    "chaddr" (client hardware address) field MUST be zeroed.

with having "hlen" zero. This indicates that at least in this case, the
bytes after "hlen" would matter.

As the DHCP client always sets the trailing bytes to zero, we would expect
that the server also replies as such and we could just compare all 16 bytes.
However, let's be liberal and accept any padding here.

This in practice only changes behavior for infiniband, where we
previously would enforce that the first ETH_ALEN bytes are zero.
That seems arbitrary for IPoIB. We should either check all bytes or
none of them. Let's do the latter and don't enforce RFC 4390 in this
regard.
This commit is contained in:
Thomas Haller 2019-02-06 18:13:20 +01:00 committed by Lennart Poettering
parent 8e6b3f49fe
commit 13f1fd0376
2 changed files with 11 additions and 11 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

11
src/libsystemd-network/dhcp-network.c
View file

@ -50,12 +50,16 @@ static int _bind_raw_socket(int ifindex, union sockaddr_union *link,
BPF_STMT(BPF_LD + BPF_B + BPF_ABS, offsetof(DHCPPacket, dhcp.htype)), /* A <- DHCP header type */
BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, arp_type, 1, 0), /* header type == arp_type ? */
BPF_STMT(BPF_RET + BPF_K, 0), /* ignore */
BPF_STMT(BPF_LD + BPF_B + BPF_ABS, offsetof(DHCPPacket, dhcp.hlen)), /* A <- MAC address length */
BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, dhcp_hlen, 1, 0), /* address length == dhcp_hlen ? */
BPF_STMT(BPF_RET + BPF_K, 0), /* ignore */
BPF_STMT(BPF_LD + BPF_W + BPF_ABS, offsetof(DHCPPacket, dhcp.xid)), /* A <- client identifier */
BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, xid, 1, 0), /* client identifier == xid ? */
BPF_STMT(BPF_RET + BPF_K, 0), /* ignore */
BPF_STMT(BPF_LD + BPF_B + BPF_ABS, offsetof(DHCPPacket, dhcp.hlen)), /* A <- MAC address length */
BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, dhcp_hlen, 1, 0), /* address length == dhcp_hlen ? */
BPF_STMT(BPF_RET + BPF_K, 0), /* ignore */
/* We only support MAC address length to be either 0 or 6 (ETH_ALEN). Optionally
* compare chaddr for ETH_ALEN bytes. */
BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, ETH_ALEN, 0, 12), /* A (the MAC address length) == ETH_ALEN ? */
BPF_STMT(BPF_LD + BPF_IMM, unaligned_read_be32(&eth_mac->ether_addr_octet[0])), /* A <- 4 bytes of client's MAC */
BPF_STMT(BPF_MISC + BPF_TAX, 0), /* X <- A */
BPF_STMT(BPF_LD + BPF_W + BPF_ABS, offsetof(DHCPPacket, dhcp.chaddr)), /* A <- 4 bytes of MAC from dhcp.chaddr */
@ -68,6 +72,7 @@ static int _bind_raw_socket(int ifindex, union sockaddr_union *link,
BPF_STMT(BPF_ALU + BPF_XOR + BPF_X, 0), /* A xor X */
BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, 0, 1, 0), /* A == 0 ? */
BPF_STMT(BPF_RET + BPF_K, 0), /* ignore */
BPF_STMT(BPF_LD + BPF_W + BPF_ABS, offsetof(DHCPPacket, dhcp.magic)), /* A <- DHCP magic cookie */
BPF_JUMP(BPF_JMP + BPF_JEQ + BPF_K, DHCP_MAGIC_COOKIE, 1, 0), /* cookie == DHCP magic cookie ? */
BPF_STMT(BPF_RET + BPF_K, 0), /* ignore */

11
src/libsystemd-network/sd-dhcp-client.c
View file

@ -1676,8 +1676,7 @@ static int client_receive_message_udp(
sd_dhcp_client *client = userdata;
_cleanup_free_ DHCPMessage *message = NULL;
const struct ether_addr zero_mac = {};
const struct ether_addr *expected_chaddr = NULL;
const uint8_t *expected_chaddr = NULL;
uint8_t expected_hlen = 0;
ssize_t len, buflen;
@ -1722,11 +1721,7 @@ static int client_receive_message_udp(
if (client->arp_type == ARPHRD_ETHER) {
expected_hlen = ETH_ALEN;
expected_chaddr = (const struct ether_addr *) &client->mac_addr;
} else {
/* Non-Ethernet links expect zero chaddr */
expected_hlen = 0;
expected_chaddr = &zero_mac;
expected_chaddr = &client->mac_addr[0];
}
if (message->hlen != expected_hlen) {
@ -1734,7 +1729,7 @@ static int client_receive_message_udp(
return 0;
}
if (memcmp(&message->chaddr[0], expected_chaddr, ETH_ALEN)) {
if (expected_hlen > 0 && memcmp(&message->chaddr[0], expected_chaddr, expected_hlen)) {
log_dhcp_client(client, "Received chaddr does not match expected: ignoring");
return 0;
}