From df8aa086420e6ce61e32898b2bb7c88b87fd1c43 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 2 Dec 2020 19:19:06 +0900 Subject: [PATCH 1/3] network: do not set broadcast if prefixlen is 31 or 32 After fe841414ef157f7f01d339c5d5730126e7b5fe0a, broadcast address is also compared with existing one to determine whether the address is foregin or not. So, the address object should not contain unnecessary information. Fixes #17803. --- src/network/networkd-dhcp4.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/network/networkd-dhcp4.c b/src/network/networkd-dhcp4.c index 8f661c646f..14e7a28774 100644 --- a/src/network/networkd-dhcp4.c +++ b/src/network/networkd-dhcp4.c @@ -865,7 +865,8 @@ static int dhcp4_update_address(Link *link, bool announce) { addr->cinfo.ifa_prefered = lifetime; addr->cinfo.ifa_valid = lifetime; addr->prefixlen = prefixlen; - addr->broadcast.s_addr = address.s_addr | ~netmask.s_addr; + if (prefixlen <= 30) + addr->broadcast.s_addr = address.s_addr | ~netmask.s_addr; SET_FLAG(addr->flags, IFA_F_NOPREFIXROUTE, !link_prefixroute(link)); /* allow reusing an existing address and simply update its lifetime From 05a7023d242b9012216c661f253df1c9c3d45b39 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 2 Dec 2020 19:26:41 +0900 Subject: [PATCH 2/3] network: fix verification for broadcast address Fixes a bug caused by fe841414ef157f7f01d339c5d5730126e7b5fe0a. --- src/network/networkd-address.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c index 92237c4e0f..0eb47f6e65 100644 --- a/src/network/networkd-address.c +++ b/src/network/networkd-address.c @@ -1809,10 +1809,12 @@ static int address_section_verify(Address *address) { address->section->filename, address->section->line); } - if (address->family == AF_INET && in_addr_is_null(address->family, &address->in_addr_peer) && - address->broadcast.s_addr == 0 && address->prefixlen <= 30) - address->broadcast.s_addr = address->in_addr.in.s_addr | htobe32(0xfffffffflu >> address->prefixlen); - else if (address->broadcast.s_addr != 0) { + if (address->family == AF_INET && + in_addr_is_null(address->family, &address->in_addr_peer) && + address->prefixlen <= 30) { + if (address->broadcast.s_addr == 0) + address->broadcast.s_addr = address->in_addr.in.s_addr | htobe32(0xfffffffflu >> address->prefixlen); + } else if (address->broadcast.s_addr != 0) { log_warning("%s: broadcast address is set for IPv6 address or IPv4 address with prefixlength larger than 30. " "Ignoring Broadcast= setting in the [Address] section from line %u.", address->section->filename, address->section->line); From 2a236f9fc0ff8fb2152032551436fde74da7217a Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 3 Dec 2020 10:19:35 +0900 Subject: [PATCH 3/3] network: ignore broadcast address for /31 or /32 addresses As they do not have broadcast address. See https://tools.ietf.org/html/rfc3021 --- src/network/networkd-address.c | 97 ++++++++++++++++++---------------- 1 file changed, 52 insertions(+), 45 deletions(-) diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c index 0eb47f6e65..0c32ab8ff3 100644 --- a/src/network/networkd-address.c +++ b/src/network/networkd-address.c @@ -144,29 +144,35 @@ Address *address_free(Address *address) { return mfree(address); } +static bool address_may_have_broadcast(const Address *a) { + assert(a); + + /* A /31 or /32 IPv4 address does not have a broadcast address. + * See https://tools.ietf.org/html/rfc3021 */ + + return a->family == AF_INET && in4_addr_is_null(&a->in_addr_peer.in) && a->prefixlen <= 30; +} + void address_hash_func(const Address *a, struct siphash *state) { assert(a); siphash24_compress(&a->family, sizeof(a->family), state); - switch (a->family) { - case AF_INET: - siphash24_compress(&a->broadcast, sizeof(a->broadcast), state); + if (!IN_SET(a->family, AF_INET, AF_INET6)) + /* treat non-IPv4 or IPv6 address family as AF_UNSPEC */ + return; + + if (a->family == AF_INET) siphash24_compress_string(a->label, state); - _fallthrough_; - case AF_INET6: - siphash24_compress(&a->prefixlen, sizeof(a->prefixlen), state); - /* local address */ - siphash24_compress(&a->in_addr, FAMILY_ADDRESS_SIZE(a->family), state); - /* peer address */ - siphash24_compress(&a->in_addr_peer, FAMILY_ADDRESS_SIZE(a->family), state); + siphash24_compress(&a->prefixlen, sizeof(a->prefixlen), state); + /* local address */ + siphash24_compress(&a->in_addr, FAMILY_ADDRESS_SIZE(a->family), state); + /* peer address */ + siphash24_compress(&a->in_addr_peer, FAMILY_ADDRESS_SIZE(a->family), state); - break; - default: - /* treat any other address family as AF_UNSPEC */ - break; - } + if (address_may_have_broadcast(a)) + siphash24_compress(&a->broadcast, sizeof(a->broadcast), state); } int address_compare_func(const Address *a1, const Address *a2) { @@ -176,32 +182,32 @@ int address_compare_func(const Address *a1, const Address *a2) { if (r != 0) return r; - switch (a1->family) { - /* use the same notion of equality as the kernel does */ - case AF_INET: - r = CMP(a1->broadcast.s_addr, a2->broadcast.s_addr); - if (r != 0) - return r; + if (!IN_SET(a1->family, AF_INET, AF_INET6)) + /* treat non-IPv4 or IPv6 address family as AF_UNSPEC */ + return 0; + if (a1->family == AF_INET) { r = strcmp_ptr(a1->label, a2->label); if (r != 0) return r; - - _fallthrough_; - case AF_INET6: - r = CMP(a1->prefixlen, a2->prefixlen); - if (r != 0) - return r; - - r = memcmp(&a1->in_addr, &a2->in_addr, FAMILY_ADDRESS_SIZE(a1->family)); - if (r != 0) - return r; - - return memcmp(&a1->in_addr_peer, &a2->in_addr_peer, FAMILY_ADDRESS_SIZE(a1->family)); - default: - /* treat any other address family as AF_UNSPEC */ - return 0; } + + r = CMP(a1->prefixlen, a2->prefixlen); + if (r != 0) + return r; + + r = memcmp(&a1->in_addr, &a2->in_addr, FAMILY_ADDRESS_SIZE(a1->family)); + if (r != 0) + return r; + + r = memcmp(&a1->in_addr_peer, &a2->in_addr_peer, FAMILY_ADDRESS_SIZE(a1->family)); + if (r != 0) + return r; + + if (address_may_have_broadcast(a1)) + return CMP(a1->broadcast.s_addr, a2->broadcast.s_addr); + + return 0; } DEFINE_HASH_OPS_WITH_KEY_DESTRUCTOR(address_hash_ops, Address, address_hash_func, address_compare_func, address_free); @@ -222,18 +228,21 @@ static int address_copy(Address *dest, const Address *src) { assert(dest); assert(src); - r = free_and_strdup(&dest->label, src->label); - if (r < 0) - return r; + if (src->family == AF_INET) { + r = free_and_strdup(&dest->label, src->label); + if (r < 0) + return r; + } dest->family = src->family; dest->prefixlen = src->prefixlen; dest->scope = src->scope; dest->flags = src->flags; - dest->broadcast = src->broadcast; dest->cinfo = src->cinfo; dest->in_addr = src->in_addr; dest->in_addr_peer = src->in_addr_peer; + if (address_may_have_broadcast(src)) + dest->broadcast = src->broadcast; dest->duplicate_address_detection = src->duplicate_address_detection; return 0; @@ -840,13 +849,13 @@ int address_configure( r = netlink_message_append_in_addr_union(req, IFA_ADDRESS, address->family, &address->in_addr_peer); if (r < 0) return log_link_error_errno(link, r, "Could not append IFA_ADDRESS attribute: %m"); - } else if (address->family == AF_INET && address->prefixlen <= 30) { + } else if (address_may_have_broadcast(address)) { r = sd_netlink_message_append_in_addr(req, IFA_BROADCAST, &address->broadcast); if (r < 0) return log_link_error_errno(link, r, "Could not append IFA_BROADCAST attribute: %m"); } - if (address->label) { + if (address->family == AF_INET && address->label) { r = sd_netlink_message_append_string(req, IFA_LABEL, address->label); if (r < 0) return log_link_error_errno(link, r, "Could not append IFA_LABEL attribute: %m"); @@ -1809,9 +1818,7 @@ static int address_section_verify(Address *address) { address->section->filename, address->section->line); } - if (address->family == AF_INET && - in_addr_is_null(address->family, &address->in_addr_peer) && - address->prefixlen <= 30) { + if (address_may_have_broadcast(address)) { if (address->broadcast.s_addr == 0) address->broadcast.s_addr = address->in_addr.in.s_addr | htobe32(0xfffffffflu >> address->prefixlen); } else if (address->broadcast.s_addr != 0) {