From 39ac79c55d0a20fa0e81608a7c1cda6410a8dbd0 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 14 Dec 2018 11:10:57 +0100 Subject: [PATCH] systemd: dhcp: handle multiple addresses for "Router" (option 3) in DHCP library Imported from systemd: The Router DHCP option may contain a list of one or more routers ([1]). Extend the API of sd_dhcp_lease to return a list instead of only the first. Note that networkd still only uses the first router (if present). Aside from extending the internal API of the DHCP client, there is almost no change in behavior. The only visible difference in behavior is that the "ROUTER" variable in the lease file is now a list of addresses. Note how RFC 2132 does not define certain IP addresses as invalid for the router option. Still, previously sd_dhcp_lease_get_router() would never return a "0.0.0.0" address. In fact, the previous API could not differenciate whether no router option was present, whether it was invalid, or whether its first router was "0.0.0.0". No longer let the DHCP client library impose additional restrictions that are not part of RFC. Instead, the caller should handle this. The patch does that, and networkd only consideres the first router entry if it is not "0.0.0.0". [1] https://tools.ietf.org/html/rfc2132#section-3.5 This also required adjusting "src/dhcp/nm-dhcp-systemd.c" due to the changed internal API. https://github.com/systemd/systemd/commit/f8862395e8f802e4106a07ceaaf02b6a1faa5a6d --- src/dhcp/nm-dhcp-systemd.c | 10 ++-- .../libsystemd-network/dhcp-lease-internal.h | 4 +- .../src/libsystemd-network/sd-dhcp-lease.c | 50 ++++++++++--------- src/systemd/src/systemd/sd-dhcp-lease.h | 2 +- 4 files changed, 37 insertions(+), 29 deletions(-) diff --git a/src/dhcp/nm-dhcp-systemd.c b/src/dhcp/nm-dhcp-systemd.c index edb2dbbfd6..6c7447f3dc 100644 --- a/src/dhcp/nm-dhcp-systemd.c +++ b/src/dhcp/nm-dhcp-systemd.c @@ -267,7 +267,7 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx, gint64 ts_time = time (NULL); struct in_addr a_address; struct in_addr a_netmask; - struct in_addr a_router; + const struct in_addr *a_router; guint32 a_plen; guint32 a_lifetime; @@ -482,8 +482,10 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx, } /* FIXME: internal client only supports returning the first router. */ - if (sd_dhcp_lease_get_router (lease, &a_router) >= 0) { - nm_utils_inet4_ntop (a_router.s_addr, addr_str); + num = sd_dhcp_lease_get_router (lease, &a_router); + if ( num > 0 + && a_router[0].s_addr) { + nm_utils_inet4_ntop (a_router[0].s_addr, addr_str); LOG_LEASE (LOGD_DHCP4, "gateway %s", addr_str); add_option (options, dhcp4_requests, SD_DHCP_OPTION_ROUTER, addr_str); @@ -497,7 +499,7 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx, nm_ip4_config_add_route (ip4_config, &((const NMPlatformIP4Route) { .rt_source = NM_IP_CONFIG_SOURCE_DHCP, - .gateway = a_router.s_addr, + .gateway = a_router[0].s_addr, .table_coerced = nm_platform_route_table_coerce (route_table), .metric = route_metric, }), diff --git a/src/systemd/src/libsystemd-network/dhcp-lease-internal.h b/src/systemd/src/libsystemd-network/dhcp-lease-internal.h index 9d245a9059..122042ab58 100644 --- a/src/systemd/src/libsystemd-network/dhcp-lease-internal.h +++ b/src/systemd/src/libsystemd-network/dhcp-lease-internal.h @@ -41,7 +41,6 @@ struct sd_dhcp_lease { /* each 0 if unset */ be32_t address; be32_t server_address; - be32_t router; be32_t next_server; bool have_subnet_mask; @@ -50,6 +49,9 @@ struct sd_dhcp_lease { bool have_broadcast; be32_t broadcast; + struct in_addr *router; + size_t router_size; + struct in_addr *dns; size_t dns_size; diff --git a/src/systemd/src/libsystemd-network/sd-dhcp-lease.c b/src/systemd/src/libsystemd-network/sd-dhcp-lease.c index 8359d077ee..c4fa62715e 100644 --- a/src/systemd/src/libsystemd-network/sd-dhcp-lease.c +++ b/src/systemd/src/libsystemd-network/sd-dhcp-lease.c @@ -152,15 +152,15 @@ int sd_dhcp_lease_get_root_path(sd_dhcp_lease *lease, const char **root_path) { return 0; } -int sd_dhcp_lease_get_router(sd_dhcp_lease *lease, struct in_addr *addr) { +int sd_dhcp_lease_get_router(sd_dhcp_lease *lease, const struct in_addr **addr) { assert_return(lease, -EINVAL); assert_return(addr, -EINVAL); - if (lease->router == 0) + if (lease->router_size <= 0) return -ENODATA; - addr->s_addr = lease->router; - return 0; + *addr = lease->router; + return (int) lease->router_size; } int sd_dhcp_lease_get_netmask(sd_dhcp_lease *lease, struct in_addr *addr) { @@ -262,6 +262,7 @@ static sd_dhcp_lease *dhcp_lease_free(sd_dhcp_lease *lease) { } free(lease->root_path); + free(lease->router); free(lease->timezone); free(lease->hostname); free(lease->domainname); @@ -388,7 +389,7 @@ static void filter_bogus_addresses(struct in_addr *addresses, size_t *n) { *n = j; } -static int lease_parse_in_addrs(const uint8_t *option, size_t len, struct in_addr **ret, size_t *n_ret) { +static int lease_parse_in_addrs(const uint8_t *option, size_t len, bool filter_bogus, struct in_addr **ret, size_t *n_ret) { assert(option); assert(ret); assert(n_ret); @@ -409,7 +410,8 @@ static int lease_parse_in_addrs(const uint8_t *option, size_t len, struct in_add if (!addresses) return -ENOMEM; - filter_bogus_addresses(addresses, &n_addresses); + if (filter_bogus) + filter_bogus_addresses(addresses, &n_addresses); free(*ret); *ret = addresses; @@ -555,21 +557,19 @@ int dhcp_lease_parse_options(uint8_t code, uint8_t len, const void *option, void break; case SD_DHCP_OPTION_ROUTER: - if (len >= 4) { - r = lease_parse_be32(option, 4, &lease->router); - if (r < 0) - log_debug_errno(r, "Failed to parse router address, ignoring: %m"); - } + r = lease_parse_in_addrs(option, len, false, &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, &lease->dns, &lease->dns_size); + r = lease_parse_in_addrs(option, len, true, &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, &lease->ntp, &lease->ntp_size); + r = lease_parse_in_addrs(option, len, true, &lease->ntp, &lease->ntp_size); if (r < 0) log_debug_errno(r, "Failed to parse NTP server, ignoring: %m"); break; @@ -821,7 +821,6 @@ int dhcp_lease_new(sd_dhcp_lease **ret) { if (!lease) return -ENOMEM; - lease->router = INADDR_ANY; lease->n_ref = 1; *ret = lease; @@ -864,9 +863,12 @@ int dhcp_lease_save(sd_dhcp_lease *lease, const char *lease_file) { if (r >= 0) fprintf(f, "NETMASK=%s\n", inet_ntoa(address)); - r = sd_dhcp_lease_get_router(lease, &address); - if (r >= 0) - fprintf(f, "ROUTER=%s\n", inet_ntoa(address)); + r = sd_dhcp_lease_get_router(lease, &addresses); + if (r > 0) { + fputs("ROUTER=", f); + serialize_in_addrs(f, addresses, r); + fputc('\n', f); + } r = sd_dhcp_lease_get_server_identifier(lease, &address); if (r >= 0) @@ -900,14 +902,14 @@ int dhcp_lease_save(sd_dhcp_lease *lease, const char *lease_file) { if (r > 0) { fputs("DNS=", f); serialize_in_addrs(f, addresses, r); - fputs("\n", f); + fputc('\n', f); } r = sd_dhcp_lease_get_ntp(lease, &addresses); if (r > 0) { fputs("NTP=", f); serialize_in_addrs(f, addresses, r); - fputs("\n", f); + fputc('\n', f); } r = sd_dhcp_lease_get_domainname(lease, &string); @@ -918,7 +920,7 @@ int dhcp_lease_save(sd_dhcp_lease *lease, const char *lease_file) { if (r > 0) { fputs("DOMAIN_SEARCH_LIST=", f); fputstrv(f, search_domains, NULL, NULL); - fputs("\n", f); + fputc('\n', f); } r = sd_dhcp_lease_get_hostname(lease, &string); @@ -1081,9 +1083,11 @@ int dhcp_lease_load(sd_dhcp_lease **ret, const char *lease_file) { } if (router) { - r = inet_pton(AF_INET, router, &lease->router); - if (r <= 0) - log_debug("Failed to parse router %s, ignoring.", router); + r = deserialize_in_addrs(&lease->router, router); + if (r < 0) + log_debug_errno(r, "Failed to deserialize router addresses %s, ignoring: %m", router); + else + lease->router_size = r; } if (netmask) { diff --git a/src/systemd/src/systemd/sd-dhcp-lease.h b/src/systemd/src/systemd/sd-dhcp-lease.h index 4875f10555..d299c79121 100644 --- a/src/systemd/src/systemd/sd-dhcp-lease.h +++ b/src/systemd/src/systemd/sd-dhcp-lease.h @@ -39,7 +39,7 @@ int sd_dhcp_lease_get_t1(sd_dhcp_lease *lease, uint32_t *t1); int sd_dhcp_lease_get_t2(sd_dhcp_lease *lease, uint32_t *t2); int sd_dhcp_lease_get_broadcast(sd_dhcp_lease *lease, struct in_addr *addr); int sd_dhcp_lease_get_netmask(sd_dhcp_lease *lease, struct in_addr *addr); -int sd_dhcp_lease_get_router(sd_dhcp_lease *lease, struct in_addr *addr); +int sd_dhcp_lease_get_router(sd_dhcp_lease *lease, const struct in_addr **addr); int sd_dhcp_lease_get_next_server(sd_dhcp_lease *lease, struct in_addr *addr); int sd_dhcp_lease_get_server_identifier(sd_dhcp_lease *lease, struct in_addr *addr); int sd_dhcp_lease_get_dns(sd_dhcp_lease *lease, const struct in_addr **addr);