1
0
mirror of https://github.com/systemd/systemd.git synced 2025-01-26 14:04:03 +03:00

Merge pull request #22607 from yuwata/network-address-hash-func

network: compare addresses more strictly
This commit is contained in:
Zbigniew Jędrzejewski-Szmek 2022-02-24 10:28:30 +01:00 committed by GitHub
commit c00e68c7bd
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 199 additions and 125 deletions

View File

@ -174,8 +174,9 @@ void link_mark_addresses(Link *link, NetworkConfigSource source, const struct in
}
}
static bool address_may_have_broadcast(const Address *a) {
static bool address_needs_to_set_broadcast(const Address *a, Link *link) {
assert(a);
assert(link);
if (a->family != AF_INET)
return false;
@ -188,38 +189,26 @@ static bool address_may_have_broadcast(const Address *a) {
if (a->prefixlen > 30)
return false;
/* If explicitly configured, do not update the address. */
if (in4_addr_is_set(&a->broadcast))
return false;
if (a->set_broadcast >= 0)
return a->set_broadcast;
return true; /* Defaults to true. */
/* Defaults to true, except for wireguard, as typical configuration for wireguard does not set
* broadcast. */
return !streq_ptr(link->kind, "wireguard");
}
void address_set_broadcast(Address *a) {
assert(a);
if (!address_may_have_broadcast(a))
return;
/* If explicitly configured, do not update the address. */
if (in4_addr_is_set(&a->broadcast))
return;
/* If Address= is 0.0.0.0, then the broadcast address will be set later in address_acquire(). */
if (in4_addr_is_null(&a->in_addr.in))
return;
a->broadcast.s_addr = a->in_addr.in.s_addr | htobe32(UINT32_C(0xffffffff) >> a->prefixlen);
}
static bool address_may_set_broadcast(const Address *a, const Link *link) {
void address_set_broadcast(Address *a, Link *link) {
assert(a);
assert(link);
if (!address_may_have_broadcast(a))
return false;
if (!address_needs_to_set_broadcast(a, link))
return;
/* Typical configuration for wireguard does not set broadcast. */
return !streq_ptr(link->kind, "wireguard");
a->broadcast.s_addr = a->in_addr.in.s_addr | htobe32(UINT32_C(0xffffffff) >> a->prefixlen);
}
static struct ifa_cacheinfo *address_set_cinfo(const Address *a, struct ifa_cacheinfo *cinfo) {
@ -271,7 +260,7 @@ static uint32_t address_prefix(const Address *a) {
return be32toh(a->in_addr.in.s_addr) >> (32 - a->prefixlen);
}
void address_hash_func(const Address *a, struct siphash *state) {
static void address_kernel_hash_func(const Address *a, struct siphash *state) {
assert(a);
siphash24_compress(&a->family, sizeof(a->family), state);
@ -293,7 +282,7 @@ void address_hash_func(const Address *a, struct siphash *state) {
}
}
int address_compare_func(const Address *a1, const Address *a2) {
static int address_kernel_compare_func(const Address *a1, const Address *a2) {
int r;
r = CMP(a1->family, a2->family);
@ -322,10 +311,68 @@ int address_compare_func(const Address *a1, const Address *a2) {
}
DEFINE_PRIVATE_HASH_OPS(
address_hash_ops,
address_kernel_hash_ops,
Address,
address_hash_func,
address_compare_func);
address_kernel_hash_func,
address_kernel_compare_func);
void address_hash_func(const Address *a, struct siphash *state) {
assert(a);
siphash24_compress(&a->family, sizeof(a->family), state);
/* treat any other address family as AF_UNSPEC */
if (!IN_SET(a->family, AF_INET, AF_INET6))
return;
siphash24_compress(&a->prefixlen, sizeof(a->prefixlen), state);
siphash24_compress(&a->in_addr, FAMILY_ADDRESS_SIZE(a->family), state);
siphash24_compress(&a->in_addr_peer, FAMILY_ADDRESS_SIZE(a->family), state);
if (a->family == AF_INET) {
/* On update, the kernel ignores the address label and broadcast address, hence we need
* to distinguish addresses with different labels or broadcast addresses. Otherwise,
* the label or broadcast address change will not be applied when we reconfigure the
* interface. */
siphash24_compress_string(a->label, state);
siphash24_compress(&a->broadcast, sizeof(a->broadcast), state);
}
}
int address_compare_func(const Address *a1, const Address *a2) {
int r;
r = CMP(a1->family, a2->family);
if (r != 0)
return r;
if (!IN_SET(a1->family, AF_INET, AF_INET6))
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 (a1->family == AF_INET) {
r = strcmp_ptr(a1->label, a2->label);
if (r != 0)
return r;
r = CMP(a1->broadcast.s_addr, a2->broadcast.s_addr);
if (r != 0)
return r;
}
return 0;
}
DEFINE_PRIVATE_HASH_OPS_WITH_KEY_DESTRUCTOR(
address_hash_ops_free,
@ -496,93 +543,77 @@ int address_get(Link *link, const Address *in, Address **ret) {
return 0;
}
int link_get_ipv6_address(Link *link, const struct in6_addr *address, Address **ret) {
_cleanup_(address_freep) Address *a = NULL;
int r;
assert(link);
assert(address);
r = address_new(&a);
if (r < 0)
return r;
/* address_compare_func() only compares the local address for IPv6 case. So, it is enough to
* set only family and the address. */
a->family = AF_INET6;
a->in_addr.in6 = *address;
return address_get(link, a, ret);
}
int link_get_ipv4_address(Link *link, const struct in_addr *address, unsigned char prefixlen, Address **ret) {
int r;
assert(link);
assert(address);
if (prefixlen != 0) {
_cleanup_(address_freep) Address *a = NULL;
/* If prefixlen is set, then we can use address_get(). */
r = address_new(&a);
if (r < 0)
return r;
a->family = AF_INET;
a->in_addr.in = *address;
a->prefixlen = prefixlen;
return address_get(link, a, ret);
} else {
Address *a;
SET_FOREACH(a, link->addresses) {
if (a->family != AF_INET)
continue;
if (!in4_addr_equal(&a->in_addr.in, address))
continue;
if (ret)
*ret = a;
return 0;
}
return -ENOENT;
}
}
int manager_has_address(Manager *manager, int family, const union in_addr_union *address, bool check_ready) {
int link_get_address(Link *link, int family, const union in_addr_union *address, unsigned char prefixlen, Address **ret) {
Address *a;
Link *link;
int r;
assert(manager);
assert(link);
assert(IN_SET(family, AF_INET, AF_INET6));
assert(address);
if (family == AF_INET) {
HASHMAP_FOREACH(link, manager->links_by_index)
if (link_get_ipv4_address(link, &address->in, 0, &a) >= 0)
return check_ready ? address_is_ready(a) : address_exists(a);
} else {
/* This find an Address object on the link which matches the given address and prefix length
* and does not have peer address. When the prefixlen is zero, then an Address object with an
* arbitrary prefixlen will be returned. */
if (prefixlen != 0) {
_cleanup_(address_freep) Address *tmp = NULL;
/* If prefixlen is set, then we can use address_get(). */
r = address_new(&tmp);
if (r < 0)
return r;
tmp->family = family;
tmp->in_addr = *address;
tmp->prefixlen = prefixlen;
address_set_broadcast(tmp, link);
HASHMAP_FOREACH(link, manager->links_by_index)
if (address_get(link, tmp, &a) >= 0)
return check_ready ? address_is_ready(a) : address_exists(a);
if (address_get(link, tmp, &a) >= 0) {
if (ret)
*ret = a;
return 0;
}
if (family == AF_INET6)
return -ENOENT;
/* IPv4 addresses may have label and/or non-default broadcast address.
* Hence, we need to always fallback below. */
}
SET_FOREACH(a, link->addresses) {
if (a->family != family)
continue;
if (!in_addr_equal(family, &a->in_addr, address))
continue;
if (in_addr_is_set(family, &a->in_addr_peer))
continue;
if (ret)
*ret = a;
return 0;
}
return -ENOENT;
}
int manager_has_address(Manager *manager, int family, const union in_addr_union *address, bool check_ready) {
Address *a;
Link *link;
assert(manager);
assert(IN_SET(family, AF_INET, AF_INET6));
assert(address);
HASHMAP_FOREACH(link, manager->links_by_index)
if (link_get_address(link, family, address, 0, &a) >= 0)
return check_ready ? address_is_ready(a) : address_exists(a);
return false;
}
@ -963,7 +994,6 @@ static int address_acquire(Link *link, const Address *original, Address **ret) {
return r;
na->in_addr = in_addr;
address_set_broadcast(na);
*ret = TAKE_PTR(na);
return 1;
@ -1025,7 +1055,7 @@ static 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_may_set_broadcast(address, link)) {
} else if (in4_addr_is_set(&address->broadcast)) {
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");
@ -1120,6 +1150,21 @@ int link_request_address(
consume_object = true;
}
if (address_needs_to_set_broadcast(address, link)) {
if (!consume_object) {
Address *a;
r = address_dup(address, &a);
if (r < 0)
return r;
address = a;
consume_object = true;
}
address_set_broadcast(address, link);
}
if (address_get(link, address, &existing) < 0) {
_cleanup_(address_freep) Address *tmp = NULL;
@ -1910,10 +1955,11 @@ static int address_section_verify(Address *address) {
address->section->filename, address->section->line);
}
if (address_may_have_broadcast(address))
address_set_broadcast(address);
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. "
if (in4_addr_is_set(&address->broadcast) &&
(address->family == AF_INET6 || address->prefixlen > 30 ||
in_addr_is_set(address->family, &address->in_addr_peer))) {
log_warning("%s: broadcast address is set for an IPv6 address, "
"an IPv4 address with peer address, or with prefix length larger than 30. "
"Ignoring Broadcast= setting in the [Address] section from line %u.",
address->section->filename, address->section->line);
@ -1980,8 +2026,10 @@ int network_drop_invalid_addresses(Network *network) {
address_free(dup);
}
/* Do not use address_hash_ops_free here. Otherwise, all address settings will be freed. */
r = set_ensure_put(&addresses, &address_hash_ops, address);
/* Use address_kernel_hash_ops here. The function address_kernel_compare_func() matches
* how kernel compares addresses, and is more lenient than address_compare_func().
* Hence, the logic of dedup here is stricter than when address_hash_ops is used. */
r = set_ensure_put(&addresses, &address_kernel_hash_ops, address);
if (r < 0)
return log_oom();
assert(r > 0);

View File

@ -70,7 +70,7 @@ int address_configure_handler_internal(sd_netlink *rtnl, sd_netlink_message *m,
int address_remove(Address *address);
int address_dup(const Address *src, Address **ret);
bool address_is_ready(const Address *a);
void address_set_broadcast(Address *a);
void address_set_broadcast(Address *a, Link *link);
DEFINE_SECTION_CLEANUP_FUNCTIONS(Address, address_free);
@ -79,8 +79,15 @@ int link_drop_foreign_addresses(Link *link);
int link_drop_ipv6ll_addresses(Link *link);
void link_foreignize_addresses(Link *link);
bool link_address_is_dynamic(const Link *link, const Address *address);
int link_get_ipv6_address(Link *link, const struct in6_addr *address, Address **ret);
int link_get_ipv4_address(Link *link, const struct in_addr *address, unsigned char prefixlen, Address **ret);
int link_get_address(Link *link, int family, const union in_addr_union *address, unsigned char prefixlen, Address **ret);
static inline int link_get_ipv6_address(Link *link, const struct in6_addr *address, unsigned char prefixlen, Address **ret) {
assert(address);
return link_get_address(link, AF_INET6, &(union in_addr_union) { .in6 = *address }, prefixlen, ret);
}
static inline int link_get_ipv4_address(Link *link, const struct in_addr *address, unsigned char prefixlen, Address **ret) {
assert(address);
return link_get_address(link, AF_INET, &(union in_addr_union) { .in = *address }, prefixlen, ret);
}
int manager_has_address(Manager *manager, int family, const union in_addr_union *address, bool check_ready);
void address_cancel_request(Address *address);

View File

@ -106,7 +106,7 @@ int link_request_dhcp_server_address(Link *link) {
address->family = AF_INET;
address->in_addr.in = link->network->dhcp_server_address;
address->prefixlen = link->network->dhcp_server_address_prefixlen;
address_set_broadcast(address);
address_set_broadcast(address, link);
if (address_get(link, address, &existing) >= 0 &&
address_exists(existing) &&

View File

@ -925,8 +925,7 @@ static int dhcp4_request_address(Link *link, bool announce) {
addr->lifetime_preferred_usec = lifetime_usec;
addr->lifetime_valid_usec = lifetime_usec;
addr->prefixlen = prefixlen;
if (prefixlen <= 30)
addr->broadcast.s_addr = address.s_addr | ~netmask.s_addr;
address_set_broadcast(addr, link);
SET_FLAG(addr->flags, IFA_F_NOPREFIXROUTE, !link_prefixroute(link));
addr->route_metric = link->network->dhcp_route_metric;
addr->duplicate_address_detection = link->network->dhcp_send_decline ? ADDRESS_FAMILY_IPV4 : ADDRESS_FAMILY_NO;

View File

@ -155,7 +155,7 @@ static int dhcp6_address_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *
return 1;
}
static void log_dhcp6_address(Link *link, const Address *address) {
static int verify_dhcp6_address(Link *link, const Address *address) {
_cleanup_free_ char *buffer = NULL;
bool by_ndisc = false;
Address *existing;
@ -167,7 +167,8 @@ static void log_dhcp6_address(Link *link, const Address *address) {
(void) in6_addr_to_string(&address->in_addr.in6, &buffer);
if (address_get(link, address, &existing) < 0) {
if (address_get(link, address, &existing) < 0 &&
link_get_address(link, AF_INET6, &address->in_addr, 0, &existing) < 0) {
/* New address. */
log_level = LOG_INFO;
goto simple_log;
@ -181,20 +182,23 @@ static void log_dhcp6_address(Link *link, const Address *address) {
if (existing->source == NETWORK_CONFIG_SOURCE_NDISC)
by_ndisc = true;
log_link_warning(link, "DHCPv6 address %s/%u (valid %s, preferred %s) conflicts the address %s/%u%s.",
log_link_warning(link, "Ignoring DHCPv6 address %s/%u (valid %s, preferred %s) which conflicts with %s/%u%s.",
strna(buffer), address->prefixlen,
FORMAT_LIFETIME(address->lifetime_valid_usec),
FORMAT_LIFETIME(address->lifetime_preferred_usec),
strna(buffer), existing->prefixlen,
by_ndisc ? " assigned by NDisc. Please try to use or update IPv6Token= setting "
"to change the address generated by NDISC, or disable UseAutonomousPrefix=" : "");
return;
by_ndisc ? " assigned by NDisc" : "");
if (by_ndisc)
log_link_warning(link, "Hint: use IPv6Token= setting to change the address generated by NDisc or set UseAutonomousPrefix=no.");
return -EEXIST;
simple_log:
log_link_full(link, log_level, "DHCPv6 address %s/%u (valid %s, preferred %s)",
strna(buffer), address->prefixlen,
FORMAT_LIFETIME(address->lifetime_valid_usec),
FORMAT_LIFETIME(address->lifetime_preferred_usec));
return 0;
}
static int dhcp6_request_address(
@ -221,7 +225,8 @@ static int dhcp6_request_address(
addr->lifetime_preferred_usec = lifetime_preferred_usec;
addr->lifetime_valid_usec = lifetime_valid_usec;
log_dhcp6_address(link, addr);
if (verify_dhcp6_address(link, addr) < 0)
return 0;
if (address_get(link, addr, &existing) < 0)
link->dhcp6_configured = false;

View File

@ -34,7 +34,7 @@ static int address_new_from_ipv4ll(Link *link, Address **ret) {
address->prefixlen = 16;
address->scope = RT_SCOPE_LINK;
address->route_metric = IPV4LL_ROUTE_METRIC;
address_set_broadcast(address);
address_set_broadcast(address, link);
*ret = TAKE_PTR(address);
return 0;

View File

@ -332,7 +332,7 @@ static int ndisc_router_process_default(Link *link, sd_ndisc_router *rt) {
if (r < 0)
return log_link_error_errno(link, r, "Failed to get gateway address from RA: %m");
if (link_get_ipv6_address(link, &gateway, NULL) >= 0) {
if (link_get_ipv6_address(link, &gateway, 0, NULL) >= 0) {
if (DEBUG_LOGGING) {
_cleanup_free_ char *buffer = NULL;
@ -644,7 +644,7 @@ static int ndisc_router_process_route(Link *link, sd_ndisc_router *rt) {
if (r < 0)
return log_link_error_errno(link, r, "Failed to get gateway address from RA: %m");
if (link_get_ipv6_address(link, &gateway, NULL) >= 0) {
if (link_get_ipv6_address(link, &gateway, 0, NULL) >= 0) {
if (DEBUG_LOGGING) {
_cleanup_free_ char *buf = NULL;

View File

@ -208,8 +208,10 @@ static void test_address_equality(void) {
assert_se(in_addr_from_string(AF_INET, "192.168.3.9", &a2->in_addr) >= 0);
assert_se(address_equal(a1, a2));
assert_se(in_addr_from_string(AF_INET, "192.168.3.10", &a1->in_addr_peer) >= 0);
assert_se(address_equal(a1, a2));
assert_se(!address_equal(a1, a2));
assert_se(in_addr_from_string(AF_INET, "192.168.3.11", &a2->in_addr_peer) >= 0);
assert_se(!address_equal(a1, a2));
assert_se(in_addr_from_string(AF_INET, "192.168.3.10", &a2->in_addr_peer) >= 0);
assert_se(address_equal(a1, a2));
a1->prefixlen = 10;
assert_se(!address_equal(a1, a2));
@ -225,8 +227,9 @@ static void test_address_equality(void) {
assert_se(address_equal(a1, a2));
a2->prefixlen = 8;
assert_se(address_equal(a1, a2));
assert_se(!address_equal(a1, a2));
a2->prefixlen = 10;
assert_se(in_addr_from_string(AF_INET6, "2001:4ca0:4f01::1", &a2->in_addr) >= 0);
assert_se(!address_equal(a1, a2));
}

View File

@ -2149,6 +2149,18 @@ class NetworkdNetworkTests(unittest.TestCase, Utilities):
call('ip netns del ns99', stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL)
def test_address_static(self):
# test for #22515. The address will be removed and replaced with /64 prefix.
rc = call('ip link add dummy98 type dummy')
self.assertEqual(rc, 0)
rc = call('ip link set dev dummy98 up')
self.assertEqual(rc, 0)
rc = call('ip -6 address add 2001:db8:0:f101::15/128 dev dummy98')
self.assertEqual(rc, 0)
self.wait_address('dummy98', '2001:db8:0:f101::15/128', ipv='-6')
rc = call('ip -4 address add 10.3.2.3/16 brd 10.3.255.250 scope global label dummy98:hoge dev dummy98')
self.assertEqual(rc, 0)
self.wait_address('dummy98', '10.3.2.3/16 brd 10.3.255.250', ipv='-4')
copy_unit_to_networkd_unit_path('25-address-static.network', '12-dummy.netdev')
start_networkd()