dhcp: move filtering of bogus DNS/NTP addresses out of DHCP client

The DHCP client should not pre-filter addresses beyond what RFC
requires. If a client's user (like networkd) wishes to skip/filter
certain addresses, it's their responsibility.

The point of this is that the DHCP library does not hide/abstract
information that might be relevant for certain users. For example,
NetworkManager exposes DHCP options in its API. When doing that, the
options should be close to the actual lease.

This is related to commit d9ec2e632df4905201facf76d6a205edc952116a
(dhcp4: filter bogus DNS/NTP server addresses silently).

diff --git a/src/basic/in-addr-util.c b/src/basic/in-addr-util.c
index 2bffe473ca696d4fd76f76a07afe20f1d9a34ac5..c715075c14f9f16446a36edbce0acfa6f9d7c43b 100644 (file)
--- a/src/basic/in-addr-util.c
+++ b/src/basic/in-addr-util.c
@@ -68,6 +68,14 @@ bool in4_addr_is_localhost(const struct in_addr *a) {
         return (be32toh(a->s_addr) & UINT32_C(0xFF000000)) == UINT32_C(127) << 24;
 }
 
+bool in4_addr_is_non_local(const struct in_addr *a) {
+        /* Whether the address is not null and not localhost.
+         *
+         * As such, it is suitable to configure as DNS/NTP server from DHCP. */
+        return !in4_addr_is_null(a) &&
+               !in4_addr_is_localhost(a);
+}
+
 int in_addr_is_localhost(int family, const union in_addr_union *u) {
         assert(u);
 

diff --git a/src/basic/in-addr-util.h b/src/basic/in-addr-util.h
index 3069790519b5c5527d1cafa563eab2b29d7cce8a..c21567122caf254d39882f7e8ff620bb56f8332b 100644 (file)
--- a/src/basic/in-addr-util.h
+++ b/src/basic/in-addr-util.h
@@ -30,6 +30,8 @@ int in_addr_is_link_local(int family, const union in_addr_union *u);
 bool in4_addr_is_localhost(const struct in_addr *a);
 int in_addr_is_localhost(int family, const union in_addr_union *u);
 
+bool in4_addr_is_non_local(const struct in_addr *a);
+
 int in_addr_equal(int family, const union in_addr_union *a, const union in_addr_union *b);
 int in_addr_prefix_intersect(int family, const union in_addr_union *a, unsigned aprefixlen, const union in_addr_union *b, unsigned bprefixlen);
 int in_addr_prefix_next(int family, union in_addr_union *u, unsigned prefixlen);

diff --git a/src/libsystemd-network/network-internal.c b/src/libsystemd-network/network-internal.c
index 221c83df5651a083e582686c41b7210f599a1089..465a6f6f030327f5b4bd46428da355c06263bfd4 100644 (file)
--- a/src/libsystemd-network/network-internal.c
+++ b/src/libsystemd-network/network-internal.c
@@ -414,16 +414,31 @@ int config_parse_bridge_port_priority(
         return 0;
 }
 
-void serialize_in_addrs(FILE *f, const struct in_addr *addresses, size_t size) {
-        unsigned i;
+size_t serialize_in_addrs(FILE *f,
+                          const struct in_addr *addresses,
+                          size_t size,
+                          bool with_leading_space,
+                          bool (*predicate)(const struct in_addr *addr)) {
+        size_t count;
+        size_t i;
 
         assert(f);
         assert(addresses);
-        assert(size);
 
-        for (i = 0; i < size; i++)
-                fprintf(f, "%s%s", inet_ntoa(addresses[i]),
-                        (i < (size - 1)) ? " ": "");
+        count = 0;
+
+        for (i = 0; i < size; i++) {
+                if (predicate && !predicate(&addresses[i]))
+                        continue;
+                if (with_leading_space)
+                        fputc(' ', f);
+                else
+                        with_leading_space = true;
+                fputs(inet_ntoa(addresses[i]), f);
+                count++;
+        }
+
+        return count;
 }
 
 int deserialize_in_addrs(struct in_addr **ret, const char *string) {

diff --git a/src/libsystemd-network/network-internal.h b/src/libsystemd-network/network-internal.h
index 0c8da848c16a09bbe5e97c3121292f570e54c943..12c303b1e04b722f55d966044aa7fc4e1d6e97da 100644 (file)
--- a/src/libsystemd-network/network-internal.h
+++ b/src/libsystemd-network/network-internal.h
@@ -40,7 +40,11 @@ CONFIG_PARSER_PROTOTYPE(config_parse_bridge_port_priority);
 int net_get_unique_predictable_data(sd_device *device, uint64_t *result);
 const char *net_get_name(sd_device *device);
 
-void serialize_in_addrs(FILE *f, const struct in_addr *addresses, size_t size);
+size_t serialize_in_addrs(FILE *f,
+                          const struct in_addr *addresses,
+                          size_t size,
+                          bool with_leading_space,
+                          bool (*predicate)(const struct in_addr *addr));
 int deserialize_in_addrs(struct in_addr **addresses, const char *string);
 void serialize_in6_addrs(FILE *f, const struct in6_addr *addresses,
                          size_t size);

diff --git a/src/libsystemd-network/sd-dhcp-lease.c b/src/libsystemd-network/sd-dhcp-lease.c
index 406188c5c69ca2a2da9302d34b26ffe253a7247a..8f179f9708c1ba7148002fb874585c677bbb8386 100644 (file)
--- a/src/libsystemd-network/sd-dhcp-lease.c
+++ b/src/libsystemd-network/sd-dhcp-lease.c
@@ -371,24 +371,7 @@ static int lease_parse_domain(const uint8_t *option, size_t len, char **ret) {
         return 0;
 }
 
-static void filter_bogus_addresses(struct in_addr *addresses, size_t *n) {
-        size_t i, j;
-
-        /* Silently filter DNS/NTP servers supplied to us that do not make outside of the local scope. */
-
-        for (i = 0, j = 0; i < *n; i ++) {
-
-                if (in4_addr_is_null(addresses+i) ||
-                    in4_addr_is_localhost(addresses+i))
-                        continue;
-
-                addresses[j++] = addresses[i];
-        }
-
-        *n = j;
-}
-
-static int lease_parse_in_addrs(const uint8_t *option, size_t len, bool filter_bogus, struct in_addr **ret, size_t *n_ret) {
+static int lease_parse_in_addrs(const uint8_t *option, size_t len, struct in_addr **ret, size_t *n_ret) {
         assert(option);
         assert(ret);
         assert(n_ret);
@@ -409,9 +392,6 @@ static int lease_parse_in_addrs(const uint8_t *option, size_t len, bool filter_b
                 if (!addresses)
                         return -ENOMEM;
 
-                if (filter_bogus)
-                        filter_bogus_addresses(addresses, &n_addresses);
-
                 free(*ret);
                 *ret = addresses;
                 *n_ret = n_addresses;
@@ -556,19 +536,19 @@ int dhcp_lease_parse_options(uint8_t code, uint8_t len, const void *option, void
                 break;
 
         case SD_DHCP_OPTION_ROUTER:
-                r = lease_parse_in_addrs(option, len, false, &lease->router, &lease->router_size);
+                r = lease_parse_in_addrs(option, len, &lease->router, &lease->router_size);
                 if (r < 0)
                         log_debug_errno(r, "Failed to parse router addresses, ignoring: %m");
                 break;
 
         case SD_DHCP_OPTION_DOMAIN_NAME_SERVER:
-                r = lease_parse_in_addrs(option, len, true, &lease->dns, &lease->dns_size);
+                r = lease_parse_in_addrs(option, len, &lease->dns, &lease->dns_size);
                 if (r < 0)
                         log_debug_errno(r, "Failed to parse DNS server, ignoring: %m");
                 break;
 
         case SD_DHCP_OPTION_NTP_SERVER:
-                r = lease_parse_in_addrs(option, len, true, &lease->ntp, &lease->ntp_size);
+                r = lease_parse_in_addrs(option, len, &lease->ntp, &lease->ntp_size);
                 if (r < 0)
                         log_debug_errno(r, "Failed to parse NTP server, ignoring: %m");
                 break;
@@ -865,7 +845,7 @@ int dhcp_lease_save(sd_dhcp_lease *lease, const char *lease_file) {
         r = sd_dhcp_lease_get_router(lease, &addresses);
         if (r > 0) {
                 fputs("ROUTER=", f);
-                serialize_in_addrs(f, addresses, r);
+                serialize_in_addrs(f, addresses, r, false, NULL);
                 fputc('\n', f);
         }
 
@@ -900,14 +880,14 @@ int dhcp_lease_save(sd_dhcp_lease *lease, const char *lease_file) {
         r = sd_dhcp_lease_get_dns(lease, &addresses);
         if (r > 0) {
                 fputs("DNS=", f);
-                serialize_in_addrs(f, addresses, r);
+                serialize_in_addrs(f, addresses, r, false, NULL);
                 fputc('\n', f);
         }
 
         r = sd_dhcp_lease_get_ntp(lease, &addresses);
         if (r > 0) {
                 fputs("NTP=", f);
-                serialize_in_addrs(f, addresses, r);
+                serialize_in_addrs(f, addresses, r, false, NULL);
                 fputc('\n', f);
         }
 

diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c
index 0cff3cf62729c090690ca4e55e86a3d5c539f95d..2aea0b839c1ec91b518acda9a52587431483a06c 100644 (file)
--- a/src/network/networkd-link.c
+++ b/src/network/networkd-link.c
@@ -1058,7 +1058,7 @@ static int link_push_uplink_dns_to_dhcp_server(Link *link, sd_dhcp_server *s) {
 
         if (link->network->dhcp_use_dns && link->dhcp_lease) {
                 const struct in_addr *da = NULL;
-                int n;
+                int j, n;
 
                 n = sd_dhcp_lease_get_dns(link->dhcp_lease, &da);
                 if (n > 0) {
@@ -1066,8 +1066,9 @@ static int link_push_uplink_dns_to_dhcp_server(Link *link, sd_dhcp_server *s) {
                         if (!GREEDY_REALLOC(addresses, n_allocated, n_addresses + n))
                                 return log_oom();
 
-                        memcpy(addresses + n_addresses, da, n * sizeof(struct in_addr));
-                        n_addresses += n;
+                        for (j = 0; j < n; j++)
+                                if (in4_addr_is_non_local(&da[j]))
+                                        addresses[n_addresses++] = da[j];
                 }
         }
 
@@ -1106,7 +1107,7 @@ static int link_push_uplink_ntp_to_dhcp_server(Link *link, sd_dhcp_server *s) {
 
         if (link->network->dhcp_use_ntp && link->dhcp_lease) {
                 const struct in_addr *da = NULL;
-                int n;
+                int j, n;
 
                 n = sd_dhcp_lease_get_ntp(link->dhcp_lease, &da);
                 if (n > 0) {
@@ -1114,8 +1115,9 @@ static int link_push_uplink_ntp_to_dhcp_server(Link *link, sd_dhcp_server *s) {
                         if (!GREEDY_REALLOC(addresses, n_allocated, n_addresses + n))
                                 return log_oom();
 
-                        memcpy(addresses + n_addresses, da, n * sizeof(struct in_addr));
-                        n_addresses += n;
+                        for (j = 0; j < n; j++)
+                                if (in4_addr_is_non_local(&da[j]))
+                                        addresses[n_addresses++] = da[j];
                 }
         }
 
@@ -4004,12 +4006,9 @@ int link_save(Link *link) {
                         const struct in_addr *addresses;
 
                         r = sd_dhcp_lease_get_dns(link->dhcp_lease, &addresses);
-                        if (r > 0) {
-                                if (space)
-                                        fputc(' ', f);
-                                serialize_in_addrs(f, addresses, r);
-                                space = true;
-                        }
+                        if (r > 0)
+                                if (serialize_in_addrs(f, addresses, r, space, in4_addr_is_non_local) > 0)
+                                        space = true;
                 }
 
                 if (link->network->dhcp_use_dns && dhcp6_lease) {
@@ -4050,12 +4049,9 @@ int link_save(Link *link) {
                         const struct in_addr *addresses;
 
                         r = sd_dhcp_lease_get_ntp(link->dhcp_lease, &addresses);
-                        if (r > 0) {
-                                if (space)
-                                        fputc(' ', f);
-                                serialize_in_addrs(f, addresses, r);
-                                space = true;
-                        }
+                        if (r > 0)
+                                if (serialize_in_addrs(f, addresses, r, space, in4_addr_is_non_local) > 0)
+                                        space = true;
                 }
 
                 if (link->network->dhcp_use_ntp && dhcp6_lease) {
@@ -4200,7 +4196,7 @@ int link_save(Link *link) {
                 r = sd_dhcp_lease_get_address(link->dhcp_lease, &address);
                 if (r >= 0) {
                         fputs("DHCP4_ADDRESS=", f);
-                        serialize_in_addrs(f, &address, 1);
+                        serialize_in_addrs(f, &address, 1, false, NULL);
                         fputc('\n', f);
                 }
 
@@ -4220,7 +4216,7 @@ int link_save(Link *link) {
                 r = sd_ipv4ll_get_address(link->ipv4ll, &address);
                 if (r >= 0) {
                         fputs("IPV4LL_ADDRESS=", f);
-                        serialize_in_addrs(f, &address, 1);
+                        serialize_in_addrs(f, &address, 1, false, NULL);
                         fputc('\n', f);
                 }
         }

diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c
index 4b69e6afaeb4020b3bc01bfa684140d3ce2a0018..d4a692c86fe43f61ff0a38e63ffe313b6b5116cb 100644 (file)
--- a/src/network/networkd-manager.c
+++ b/src/network/networkd-manager.c
@@ -1044,14 +1044,19 @@ static int ordered_set_put_in4_addr(OrderedSet *s, const struct in_addr *address
         return r;
 }
 
-static int ordered_set_put_in4_addrv(OrderedSet *s, const struct in_addr *addresses, unsigned n) {
+static int ordered_set_put_in4_addrv(OrderedSet *s,
+                                     const struct in_addr *addresses,
+                                     size_t n,
+                                     bool (*predicate)(const struct in_addr *addr)) {
         int r, c = 0;
-        unsigned i;
+        size_t i;
 
         assert(s);
         assert(n == 0 || addresses);
 
         for (i = 0; i < n; i++) {
+                if (predicate && !predicate(&addresses[i]))
+                        continue;
                 r = ordered_set_put_in4_addr(s, addresses+i);
                 if (r < 0)
                         return r;
@@ -1144,7 +1149,7 @@ static int manager_save(Manager *m) {
 
                         r = sd_dhcp_lease_get_dns(link->dhcp_lease, &addresses);
                         if (r > 0) {
-                                r = ordered_set_put_in4_addrv(dns, addresses, r);
+                                r = ordered_set_put_in4_addrv(dns, addresses, r, in4_addr_is_non_local);
                                 if (r < 0)
                                         return r;
                         } else if (r < 0 && r != -ENODATA)
@@ -1156,7 +1161,7 @@ static int manager_save(Manager *m) {
 
                         r = sd_dhcp_lease_get_ntp(link->dhcp_lease, &addresses);
                         if (r > 0) {
-                                r = ordered_set_put_in4_addrv(ntp, addresses, r);
+                                r = ordered_set_put_in4_addrv(ntp, addresses, r, in4_addr_is_non_local);
                                 if (r < 0)
                                         return r;
                         } else if (r < 0 && r != -ENODATA)