From 95dba435427f989717e02ead93fe0a3199efd2b0 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 28 Feb 2019 16:18:57 +0900 Subject: [PATCH 01/14] network: use ltype to determine netdev kind in config_parse_stacked_netdev() --- src/network/networkd-network-gperf.gperf | 10 ++++----- src/network/networkd-network.c | 27 +++++------------------- 2 files changed, 10 insertions(+), 27 deletions(-) diff --git a/src/network/networkd-network-gperf.gperf b/src/network/networkd-network-gperf.gperf index 51123d12ef0..43ee17fe355 100644 --- a/src/network/networkd-network-gperf.gperf +++ b/src/network/networkd-network-gperf.gperf @@ -40,11 +40,11 @@ Link.RequiredForOnline, config_parse_bool, Network.Description, config_parse_string, 0, offsetof(Network, description) Network.Bridge, config_parse_ifname, 0, offsetof(Network, bridge_name) Network.Bond, config_parse_ifname, 0, offsetof(Network, bond_name) -Network.VLAN, config_parse_stacked_netdev, 0, offsetof(Network, stacked_netdev_names) -Network.MACVLAN, config_parse_stacked_netdev, 0, offsetof(Network, stacked_netdev_names) -Network.MACVTAP, config_parse_stacked_netdev, 0, offsetof(Network, stacked_netdev_names) -Network.IPVLAN, config_parse_stacked_netdev, 0, offsetof(Network, stacked_netdev_names) -Network.VXLAN, config_parse_stacked_netdev, 0, offsetof(Network, stacked_netdev_names) +Network.VLAN, config_parse_stacked_netdev, NETDEV_KIND_VLAN, offsetof(Network, stacked_netdev_names) +Network.MACVLAN, config_parse_stacked_netdev, NETDEV_KIND_MACVLAN, offsetof(Network, stacked_netdev_names) +Network.MACVTAP, config_parse_stacked_netdev, NETDEV_KIND_MACVTAP, offsetof(Network, stacked_netdev_names) +Network.IPVLAN, config_parse_stacked_netdev, NETDEV_KIND_IPVLAN, offsetof(Network, stacked_netdev_names) +Network.VXLAN, config_parse_stacked_netdev, NETDEV_KIND_VXLAN, offsetof(Network, stacked_netdev_names) Network.Tunnel, config_parse_stacked_netdev, _NETDEV_KIND_TUNNEL, offsetof(Network, stacked_netdev_names) Network.VRF, config_parse_ifname, 0, offsetof(Network, vrf_name) Network.DHCP, config_parse_dhcp, 0, offsetof(Network, dhcp) diff --git a/src/network/networkd-network.c b/src/network/networkd-network.c index 5f12df907af..1e5684144dc 100644 --- a/src/network/networkd-network.c +++ b/src/network/networkd-network.c @@ -732,35 +732,18 @@ int config_parse_stacked_netdev(const char *unit, const char *rvalue, void *data, void *userdata) { - _cleanup_free_ char *kind_string = NULL, *name = NULL; + _cleanup_free_ char *name = NULL; + NetDevKind kind = ltype; Hashmap **h = data; - NetDevKind kind; - char *p; int r; assert(filename); assert(lvalue); assert(rvalue); assert(data); - - if (ltype == _NETDEV_KIND_TUNNEL) - kind = _NETDEV_KIND_TUNNEL; - else { - kind_string = strdup(lvalue); - if (!kind_string) - return log_oom(); - - /* the keys are CamelCase versions of the kind */ - for (p = kind_string; *p; p++) - *p = tolower(*p); - - kind = netdev_kind_from_string(kind_string); - if (kind < 0 || IN_SET(kind, NETDEV_KIND_BRIDGE, NETDEV_KIND_BOND, NETDEV_KIND_VRF)) { - log_syntax(unit, LOG_ERR, filename, line, 0, - "Invalid NetDev kind: %s", lvalue); - return 0; - } - } + assert(IN_SET(kind, + NETDEV_KIND_VLAN, NETDEV_KIND_MACVLAN, NETDEV_KIND_MACVTAP, + NETDEV_KIND_IPVLAN, NETDEV_KIND_VXLAN, _NETDEV_KIND_TUNNEL)); if (!ifname_valid(rvalue)) { log_syntax(unit, LOG_ERR, filename, line, 0, From 48315d3dcae9bb045ab7d16264904b976123018e Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 28 Feb 2019 16:30:14 +0900 Subject: [PATCH 02/14] network: move NetworkConfigSection and related functions to networkd-util.[ch] --- src/network/networkd-network.c | 36 ---------------------------------- src/network/networkd-network.h | 10 ---------- src/network/networkd-util.c | 36 ++++++++++++++++++++++++++++++++++ src/network/networkd-util.h | 11 +++++++++++ 4 files changed, 47 insertions(+), 46 deletions(-) diff --git a/src/network/networkd-network.c b/src/network/networkd-network.c index 1e5684144dc..341b6335d6c 100644 --- a/src/network/networkd-network.c +++ b/src/network/networkd-network.c @@ -26,42 +26,6 @@ /* Let's assume that anything above this number is a user misconfiguration. */ #define MAX_NTP_SERVERS 128 -static void network_config_hash_func(const NetworkConfigSection *c, struct siphash *state) { - siphash24_compress(c->filename, strlen(c->filename), state); - siphash24_compress(&c->line, sizeof(c->line), state); -} - -static int network_config_compare_func(const NetworkConfigSection *x, const NetworkConfigSection *y) { - int r; - - r = strcmp(x->filename, y->filename); - if (r != 0) - return r; - - return CMP(x->line, y->line); -} - -DEFINE_HASH_OPS(network_config_hash_ops, NetworkConfigSection, network_config_hash_func, network_config_compare_func); - -int network_config_section_new(const char *filename, unsigned line, NetworkConfigSection **s) { - NetworkConfigSection *cs; - - cs = malloc0(offsetof(NetworkConfigSection, filename) + strlen(filename) + 1); - if (!cs) - return -ENOMEM; - - strcpy(cs->filename, filename); - cs->line = line; - - *s = TAKE_PTR(cs); - - return 0; -} - -void network_config_section_free(NetworkConfigSection *cs) { - free(cs); -} - /* Set defaults following RFC7844 */ void network_apply_anonymize_if_set(Network *network) { if (!network->dhcp_anonymize) diff --git a/src/network/networkd-network.h b/src/network/networkd-network.h index 2f7b6133fd5..38f3b7decb3 100644 --- a/src/network/networkd-network.h +++ b/src/network/networkd-network.h @@ -84,16 +84,6 @@ typedef enum RADVPrefixDelegation { _RADV_PREFIX_DELEGATION_INVALID = -1, } RADVPrefixDelegation; -typedef struct NetworkConfigSection { - unsigned line; - char filename[]; -} NetworkConfigSection; - -int network_config_section_new(const char *filename, unsigned line, NetworkConfigSection **s); -void network_config_section_free(NetworkConfigSection *network); -DEFINE_TRIVIAL_CLEANUP_FUNC(NetworkConfigSection*, network_config_section_free); -extern const struct hash_ops network_config_hash_ops; - typedef struct Manager Manager; struct Network { diff --git a/src/network/networkd-util.c b/src/network/networkd-util.c index 9b6bc128581..a392aadd4c4 100644 --- a/src/network/networkd-util.c +++ b/src/network/networkd-util.c @@ -102,3 +102,39 @@ int kernel_route_expiration_supported(void) { } return cached; } + +static void network_config_hash_func(const NetworkConfigSection *c, struct siphash *state) { + siphash24_compress(c->filename, strlen(c->filename), state); + siphash24_compress(&c->line, sizeof(c->line), state); +} + +static int network_config_compare_func(const NetworkConfigSection *x, const NetworkConfigSection *y) { + int r; + + r = strcmp(x->filename, y->filename); + if (r != 0) + return r; + + return CMP(x->line, y->line); +} + +DEFINE_HASH_OPS(network_config_hash_ops, NetworkConfigSection, network_config_hash_func, network_config_compare_func); + +int network_config_section_new(const char *filename, unsigned line, NetworkConfigSection **s) { + NetworkConfigSection *cs; + + cs = malloc0(offsetof(NetworkConfigSection, filename) + strlen(filename) + 1); + if (!cs) + return -ENOMEM; + + strcpy(cs->filename, filename); + cs->line = line; + + *s = TAKE_PTR(cs); + + return 0; +} + +void network_config_section_free(NetworkConfigSection *cs) { + free(cs); +} diff --git a/src/network/networkd-util.h b/src/network/networkd-util.h index 3c0c279b97b..d360035b147 100644 --- a/src/network/networkd-util.h +++ b/src/network/networkd-util.h @@ -2,6 +2,7 @@ #pragma once #include "conf-parser.h" +#include "hash-funcs.h" #include "macro.h" typedef enum AddressFamilyBoolean { @@ -14,6 +15,11 @@ typedef enum AddressFamilyBoolean { _ADDRESS_FAMILY_BOOLEAN_INVALID = -1, } AddressFamilyBoolean; +typedef struct NetworkConfigSection { + unsigned line; + char filename[]; +} NetworkConfigSection; + CONFIG_PARSER_PROTOTYPE(config_parse_address_family_boolean); CONFIG_PARSER_PROTOTYPE(config_parse_address_family_boolean_with_kernel); @@ -21,3 +27,8 @@ const char *address_family_boolean_to_string(AddressFamilyBoolean b) _const_; AddressFamilyBoolean address_family_boolean_from_string(const char *s) _const_; int kernel_route_expiration_supported(void); + +int network_config_section_new(const char *filename, unsigned line, NetworkConfigSection **s); +void network_config_section_free(NetworkConfigSection *network); +DEFINE_TRIVIAL_CLEANUP_FUNC(NetworkConfigSection*, network_config_section_free); +extern const struct hash_ops network_config_hash_ops; From fa7cd7117ff2158c4d175b0321a0c0393cc4e3ec Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 5 Mar 2019 10:51:57 +0900 Subject: [PATCH 03/14] network: add IPv4LL route right after .network file is parsed Previously, the route is added when the .network config is assigned to a Link. So, if multiple links match the .network file, the route entry becomes duplicated in the corresponding Network object. --- src/network/networkd-network.c | 26 ++++---------------------- src/network/networkd-route.c | 28 ++++++++++++++++++++++++++++ src/network/networkd-route.h | 2 ++ 3 files changed, 34 insertions(+), 22 deletions(-) diff --git a/src/network/networkd-network.c b/src/network/networkd-network.c index 341b6335d6c..81d6d35c7c9 100644 --- a/src/network/networkd-network.c +++ b/src/network/networkd-network.c @@ -423,6 +423,10 @@ int network_load_one(Manager *manager, const char *filename) { network_apply_anonymize_if_set(network); + r = network_add_ipv4ll_route(network); + if (r < 0) + log_warning_errno(r, "%s: Failed to add IPv4LL route, ignoring: %m", network->filename); + LIST_PREPEND(networks, manager->networks, network); network->manager = manager; @@ -637,33 +641,11 @@ int network_get(Manager *manager, sd_device *device, } int network_apply(Network *network, Link *link) { - int r; - assert(network); assert(link); link->network = network; - if (network->ipv4ll_route) { - Route *route; - - r = route_new_static(network, NULL, 0, &route); - if (r < 0) - return r; - - r = inet_pton(AF_INET, "169.254.0.0", &route->dst.in); - if (r == 0) - return -EINVAL; - if (r < 0) - return -errno; - - route->family = AF_INET; - route->dst_prefixlen = 16; - route->scope = RT_SCOPE_LINK; - route->priority = IPV4LL_ROUTE_METRIC; - route->protocol = RTPROT_STATIC; - } - if (network->n_dns > 0 || !strv_isempty(network->ntp) || !ordered_set_isempty(network->search_domains) || diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index 20746987805..2e3ea127994 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -677,6 +677,34 @@ int route_configure( return 0; } +int network_add_ipv4ll_route(Network *network) { + _cleanup_(route_freep) Route *n = NULL; + int r; + + assert(network); + + if (!network->ipv4ll_route) + return 0; + + /* IPv4LLRoute= is in [Network] section. */ + r = route_new_static(network, NULL, 0, &n); + if (r < 0) + return r; + + r = in_addr_from_string(AF_INET, "169.254.0.0", &n->dst); + if (r < 0) + return r; + + n->family = AF_INET; + n->dst_prefixlen = 16; + n->scope = RT_SCOPE_LINK; + n->priority = IPV4LL_ROUTE_METRIC; + n->protocol = RTPROT_STATIC; + + TAKE_PTR(n); + return 0; +} + int config_parse_gateway( const char *unit, const char *filename, diff --git a/src/network/networkd-route.h b/src/network/networkd-route.h index 7a9fc161bbc..9c6c4b8b0d5 100644 --- a/src/network/networkd-route.h +++ b/src/network/networkd-route.h @@ -59,6 +59,8 @@ int route_expire_handler(sd_event_source *s, uint64_t usec, void *userdata); DEFINE_TRIVIAL_CLEANUP_FUNC(Route*, route_free); +int network_add_ipv4ll_route(Network *network); + CONFIG_PARSER_PROTOTYPE(config_parse_gateway); CONFIG_PARSER_PROTOTYPE(config_parse_preferred_src); CONFIG_PARSER_PROTOTYPE(config_parse_destination); From 9560e5b3235e30eea2873e507d27ed00331d838c Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 1 Mar 2019 13:19:53 +0900 Subject: [PATCH 04/14] network: make all xxx_new_static() static These functions are called from only config parsers, and the parsers are in the same files. So, let's make them static. --- src/network/networkd-address.c | 2 +- src/network/networkd-address.h | 1 - src/network/networkd-fdb.c | 2 +- src/network/networkd-fdb.h | 1 - src/network/networkd-ipv6-proxy-ndp.c | 2 +- src/network/networkd-ipv6-proxy-ndp.h | 1 - src/network/networkd-radv.c | 4 ++-- src/network/networkd-radv.h | 1 - src/network/networkd-route.c | 2 +- src/network/networkd-route.h | 1 - 10 files changed, 6 insertions(+), 11 deletions(-) diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c index 7712fd6ae3d..06b6b3d20d9 100644 --- a/src/network/networkd-address.c +++ b/src/network/networkd-address.c @@ -39,7 +39,7 @@ int address_new(Address **ret) { return 0; } -int address_new_static(Network *network, const char *filename, unsigned section_line, Address **ret) { +static int address_new_static(Network *network, const char *filename, unsigned section_line, Address **ret) { _cleanup_(network_config_section_freep) NetworkConfigSection *n = NULL; _cleanup_(address_freep) Address *address = NULL; int r; diff --git a/src/network/networkd-address.h b/src/network/networkd-address.h index 4714c07d4da..57d82f82c39 100644 --- a/src/network/networkd-address.h +++ b/src/network/networkd-address.h @@ -46,7 +46,6 @@ struct Address { LIST_FIELDS(Address, addresses); }; -int address_new_static(Network *network, const char *filename, unsigned section, Address **ret); int address_new(Address **ret); void address_free(Address *address); int address_add_foreign(Link *link, int family, const union in_addr_union *in_addr, unsigned char prefixlen, Address **ret); diff --git a/src/network/networkd-fdb.c b/src/network/networkd-fdb.c index 324f6a5a1aa..02724938ae8 100644 --- a/src/network/networkd-fdb.c +++ b/src/network/networkd-fdb.c @@ -18,7 +18,7 @@ #define STATIC_FDB_ENTRIES_PER_NETWORK_MAX 1024U /* create a new FDB entry or get an existing one. */ -int fdb_entry_new_static( +static int fdb_entry_new_static( Network *network, const char *filename, unsigned section_line, diff --git a/src/network/networkd-fdb.h b/src/network/networkd-fdb.h index 55fc3a21703..c0ef27d7d7c 100644 --- a/src/network/networkd-fdb.h +++ b/src/network/networkd-fdb.h @@ -24,7 +24,6 @@ struct FdbEntry { LIST_FIELDS(FdbEntry, static_fdb_entries); }; -int fdb_entry_new_static(Network *network, const char *filename, unsigned section_line, FdbEntry **ret); void fdb_entry_free(FdbEntry *fdb_entry); int fdb_entry_configure(Link *link, FdbEntry *fdb_entry); diff --git a/src/network/networkd-ipv6-proxy-ndp.c b/src/network/networkd-ipv6-proxy-ndp.c index 98d42055478..e2d77e9ad2c 100644 --- a/src/network/networkd-ipv6-proxy-ndp.c +++ b/src/network/networkd-ipv6-proxy-ndp.c @@ -50,7 +50,7 @@ static int ipv6_proxy_ndp_set(Link *link) { return 0; } -int ipv6_proxy_ndp_address_new_static(Network *network, IPv6ProxyNDPAddress **ret) { +static int ipv6_proxy_ndp_address_new_static(Network *network, IPv6ProxyNDPAddress **ret) { _cleanup_(ipv6_proxy_ndp_address_freep) IPv6ProxyNDPAddress *ipv6_proxy_ndp_address = NULL; assert(network); diff --git a/src/network/networkd-ipv6-proxy-ndp.h b/src/network/networkd-ipv6-proxy-ndp.h index 546e8561f1c..4ce5de8d69c 100644 --- a/src/network/networkd-ipv6-proxy-ndp.h +++ b/src/network/networkd-ipv6-proxy-ndp.h @@ -16,7 +16,6 @@ struct IPv6ProxyNDPAddress { LIST_FIELDS(IPv6ProxyNDPAddress, ipv6_proxy_ndp_addresses); }; -int ipv6_proxy_ndp_address_new_static(Network *network, IPv6ProxyNDPAddress ** ipv6_proxy_ndp_address); void ipv6_proxy_ndp_address_free(IPv6ProxyNDPAddress *ipv6_proxy_ndp_address); int ipv6_proxy_ndp_address_configure(Link *link, IPv6ProxyNDPAddress *ipv6_proxy_ndp_address); int ipv6_proxy_ndp_addresses_configure(Link *link); diff --git a/src/network/networkd-radv.c b/src/network/networkd-radv.c index ecf984e226b..3de969764eb 100644 --- a/src/network/networkd-radv.c +++ b/src/network/networkd-radv.c @@ -124,8 +124,8 @@ int prefix_new(Prefix **ret) { return 0; } -int prefix_new_static(Network *network, const char *filename, - unsigned section_line, Prefix **ret) { +static int prefix_new_static(Network *network, const char *filename, + unsigned section_line, Prefix **ret) { _cleanup_(network_config_section_freep) NetworkConfigSection *n = NULL; _cleanup_(prefix_freep) Prefix *prefix = NULL; int r; diff --git a/src/network/networkd-radv.h b/src/network/networkd-radv.h index bb88f8a4074..ebf6d7299b0 100644 --- a/src/network/networkd-radv.h +++ b/src/network/networkd-radv.h @@ -22,7 +22,6 @@ struct Prefix { int prefix_new(Prefix **ret); void prefix_free(Prefix *prefix); -int prefix_new_static(Network *network, const char *filename, unsigned section, Prefix **ret); DEFINE_TRIVIAL_CLEANUP_FUNC(Prefix*, prefix_free); diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index 2e3ea127994..3eb807c9f83 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -67,7 +67,7 @@ int route_new(Route **ret) { return 0; } -int route_new_static(Network *network, const char *filename, unsigned section_line, Route **ret) { +static int route_new_static(Network *network, const char *filename, unsigned section_line, Route **ret) { _cleanup_(network_config_section_freep) NetworkConfigSection *n = NULL; _cleanup_(route_freep) Route *route = NULL; int r; diff --git a/src/network/networkd-route.h b/src/network/networkd-route.h index 9c6c4b8b0d5..c19b7aa3a73 100644 --- a/src/network/networkd-route.h +++ b/src/network/networkd-route.h @@ -43,7 +43,6 @@ struct Route { LIST_FIELDS(Route, routes); }; -int route_new_static(Network *network, const char *filename, unsigned section_line, Route **ret); int route_new(Route **ret); void route_free(Route *route); int route_configure(Route *route, Link *link, link_netlink_message_handler_t callback); From fcbf4cb7270faa2624a0cf149f9518280e8a6714 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 1 Mar 2019 13:27:47 +0900 Subject: [PATCH 05/14] network: drop sections contain invalid settings in network_verify() If e.g., an [Address] section has an invalid setting, then previously assigned settings in the section is freed, and only later settings are stored. That may cause partially broken section stored in Network object. This makes if an invalid setting is found, then set 'invalid' flag instead of freeing it. And invalid sections are dropped later by network_verify(). --- src/network/networkd-address-label.c | 4 +- src/network/networkd-address-label.h | 3 +- src/network/networkd-address.c | 28 ++++++++--- src/network/networkd-address.h | 4 +- src/network/networkd-fdb.c | 4 +- src/network/networkd-fdb.h | 3 +- src/network/networkd-ipv6-proxy-ndp.h | 6 +-- src/network/networkd-neighbor.c | 4 +- src/network/networkd-neighbor.h | 3 +- src/network/networkd-network.c | 51 ++++++++++---------- src/network/networkd-radv.c | 6 +-- src/network/networkd-radv.h | 3 +- src/network/networkd-route.c | 54 ++++++++++++++++------ src/network/networkd-route.h | 4 +- src/network/networkd-routing-policy-rule.c | 18 ++++---- src/network/networkd-routing-policy-rule.h | 3 +- src/network/networkd-util.h | 22 +++++++++ src/network/test-networkd-conf.c | 2 +- 18 files changed, 149 insertions(+), 73 deletions(-) diff --git a/src/network/networkd-address-label.c b/src/network/networkd-address-label.c index 94a12f8bfe7..ab738448c52 100644 --- a/src/network/networkd-address-label.c +++ b/src/network/networkd-address-label.c @@ -159,7 +159,7 @@ int config_parse_address_label_prefix(const char *unit, void *data, void *userdata) { - _cleanup_(address_label_freep) AddressLabel *n = NULL; + _cleanup_(address_label_free_or_set_invalidp) AddressLabel *n = NULL; Network *network = userdata; int r; @@ -196,7 +196,7 @@ int config_parse_address_label( void *data, void *userdata) { - _cleanup_(address_label_freep) AddressLabel *n = NULL; + _cleanup_(address_label_free_or_set_invalidp) AddressLabel *n = NULL; Network *network = userdata; uint32_t k; int r; diff --git a/src/network/networkd-address-label.h b/src/network/networkd-address-label.h index 6922cb0faf8..595072a17e6 100644 --- a/src/network/networkd-address-label.h +++ b/src/network/networkd-address-label.h @@ -11,6 +11,7 @@ typedef struct AddressLabel AddressLabel; #include "networkd-link.h" #include "networkd-network.h" +#include "networkd-util.h" typedef struct Network Network; typedef struct Link Link; @@ -30,7 +31,7 @@ struct AddressLabel { void address_label_free(AddressLabel *label); -DEFINE_TRIVIAL_CLEANUP_FUNC(AddressLabel*, address_label_free); +DEFINE_NETWORK_SECTION_FUNCTIONS(AddressLabel, address_label_free); int address_label_configure(AddressLabel *address, Link *link, link_netlink_message_handler_t callback, bool update); diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c index 06b6b3d20d9..1a9ba4223cd 100644 --- a/src/network/networkd-address.c +++ b/src/network/networkd-address.c @@ -681,7 +681,7 @@ int config_parse_broadcast( void *userdata) { Network *network = userdata; - _cleanup_(address_freep) Address *n = NULL; + _cleanup_(address_free_or_set_invalidp) Address *n = NULL; int r; assert(filename); @@ -725,7 +725,7 @@ int config_parse_address(const char *unit, void *userdata) { Network *network = userdata; - _cleanup_(address_freep) Address *n = NULL; + _cleanup_(address_free_or_set_invalidp) Address *n = NULL; union in_addr_union buffer; unsigned char prefixlen; int r, f; @@ -807,7 +807,7 @@ int config_parse_label( void *data, void *userdata) { - _cleanup_(address_freep) Address *n = NULL; + _cleanup_(address_free_or_set_invalidp) Address *n = NULL; Network *network = userdata; int r; @@ -846,7 +846,7 @@ int config_parse_lifetime(const char *unit, void *data, void *userdata) { Network *network = userdata; - _cleanup_(address_freep) Address *n = NULL; + _cleanup_(address_free_or_set_invalidp) Address *n = NULL; unsigned k; int r; @@ -888,7 +888,7 @@ int config_parse_address_flags(const char *unit, void *data, void *userdata) { Network *network = userdata; - _cleanup_(address_freep) Address *n = NULL; + _cleanup_(address_free_or_set_invalidp) Address *n = NULL; int r; assert(filename); @@ -936,7 +936,7 @@ int config_parse_address_scope(const char *unit, void *data, void *userdata) { Network *network = userdata; - _cleanup_(address_freep) Address *n = NULL; + _cleanup_(address_free_or_set_invalidp) Address *n = NULL; int r; assert(filename); @@ -976,3 +976,19 @@ bool address_is_ready(const Address *a) { else return !(a->flags & (IFA_F_TENTATIVE | IFA_F_DEPRECATED)); } + +int address_section_verify(Address *address) { + if (section_is_invalid(address->section)) + return -EINVAL; + + if (address->family == AF_UNSPEC) { + assert(address->section); + + return log_warning_errno(SYNTHETIC_ERRNO(EINVAL), + "%s: Address section without Address= field configured. " + "Ignoring [Address] section from line %u.", + address->section->filename, address->section->line); + } + + return 0; +} diff --git a/src/network/networkd-address.h b/src/network/networkd-address.h index 57d82f82c39..455985d2252 100644 --- a/src/network/networkd-address.h +++ b/src/network/networkd-address.h @@ -11,6 +11,7 @@ typedef struct Address Address; #include "networkd-link.h" #include "networkd-network.h" +#include "networkd-util.h" #define CACHE_INFO_INFINITY_LIFE_TIME 0xFFFFFFFFU @@ -57,8 +58,9 @@ int address_configure(Address *address, Link *link, link_netlink_message_handler int address_remove(Address *address, Link *link, link_netlink_message_handler_t callback); bool address_equal(Address *a1, Address *a2); bool address_is_ready(const Address *a); +int address_section_verify(Address *a); -DEFINE_TRIVIAL_CLEANUP_FUNC(Address*, address_free); +DEFINE_NETWORK_SECTION_FUNCTIONS(Address, address_free); CONFIG_PARSER_PROTOTYPE(config_parse_address); CONFIG_PARSER_PROTOTYPE(config_parse_broadcast); diff --git a/src/network/networkd-fdb.c b/src/network/networkd-fdb.c index 02724938ae8..fa139495381 100644 --- a/src/network/networkd-fdb.c +++ b/src/network/networkd-fdb.c @@ -189,7 +189,7 @@ int config_parse_fdb_hwaddr( void *userdata) { Network *network = userdata; - _cleanup_(fdb_entry_freep) FdbEntry *fdb_entry = NULL; + _cleanup_(fdb_entry_free_or_set_invalidp) FdbEntry *fdb_entry = NULL; int r; assert(filename); @@ -235,7 +235,7 @@ int config_parse_fdb_vlan_id( void *userdata) { Network *network = userdata; - _cleanup_(fdb_entry_freep) FdbEntry *fdb_entry = NULL; + _cleanup_(fdb_entry_free_or_set_invalidp) FdbEntry *fdb_entry = NULL; int r; assert(filename); diff --git a/src/network/networkd-fdb.h b/src/network/networkd-fdb.h index c0ef27d7d7c..6b7da2e7413 100644 --- a/src/network/networkd-fdb.h +++ b/src/network/networkd-fdb.h @@ -8,6 +8,7 @@ #include "conf-parser.h" #include "list.h" #include "macro.h" +#include "networkd-util.h" typedef struct Network Network; typedef struct FdbEntry FdbEntry; @@ -27,7 +28,7 @@ struct FdbEntry { void fdb_entry_free(FdbEntry *fdb_entry); int fdb_entry_configure(Link *link, FdbEntry *fdb_entry); -DEFINE_TRIVIAL_CLEANUP_FUNC(FdbEntry*, fdb_entry_free); +DEFINE_NETWORK_SECTION_FUNCTIONS(FdbEntry, fdb_entry_free); CONFIG_PARSER_PROTOTYPE(config_parse_fdb_hwaddr); CONFIG_PARSER_PROTOTYPE(config_parse_fdb_vlan_id); diff --git a/src/network/networkd-ipv6-proxy-ndp.h b/src/network/networkd-ipv6-proxy-ndp.h index 4ce5de8d69c..d6666beab5c 100644 --- a/src/network/networkd-ipv6-proxy-ndp.h +++ b/src/network/networkd-ipv6-proxy-ndp.h @@ -10,10 +10,10 @@ typedef struct IPv6ProxyNDPAddress IPv6ProxyNDPAddress; typedef struct Link Link; struct IPv6ProxyNDPAddress { - Network *network; - struct in6_addr in_addr; + Network *network; + struct in6_addr in_addr; - LIST_FIELDS(IPv6ProxyNDPAddress, ipv6_proxy_ndp_addresses); + LIST_FIELDS(IPv6ProxyNDPAddress, ipv6_proxy_ndp_addresses); }; void ipv6_proxy_ndp_address_free(IPv6ProxyNDPAddress *ipv6_proxy_ndp_address); diff --git a/src/network/networkd-neighbor.c b/src/network/networkd-neighbor.c index 713bad2bba3..d0275fdd3ea 100644 --- a/src/network/networkd-neighbor.c +++ b/src/network/networkd-neighbor.c @@ -164,7 +164,7 @@ int config_parse_neighbor_address(const char *unit, void *userdata) { Network *network = userdata; - _cleanup_(neighbor_freep) Neighbor *n = NULL; + _cleanup_(neighbor_free_or_set_invalidp) Neighbor *n = NULL; int r; assert(filename); @@ -200,7 +200,7 @@ int config_parse_neighbor_hwaddr(const char *unit, void *userdata) { Network *network = userdata; - _cleanup_(neighbor_freep) Neighbor *n = NULL; + _cleanup_(neighbor_free_or_set_invalidp) Neighbor *n = NULL; int r; assert(filename); diff --git a/src/network/networkd-neighbor.h b/src/network/networkd-neighbor.h index 094bf7977ed..f591f0b03f3 100644 --- a/src/network/networkd-neighbor.h +++ b/src/network/networkd-neighbor.h @@ -13,6 +13,7 @@ typedef struct Neighbor Neighbor; #include "networkd-link.h" #include "networkd-network.h" +#include "networkd-util.h" struct Neighbor { Network *network; @@ -29,7 +30,7 @@ struct Neighbor { void neighbor_free(Neighbor *neighbor); -DEFINE_TRIVIAL_CLEANUP_FUNC(Neighbor*, neighbor_free); +DEFINE_NETWORK_SECTION_FUNCTIONS(Neighbor, neighbor_free); int neighbor_configure(Neighbor *neighbor, Link *link, link_netlink_message_handler_t callback); diff --git a/src/network/networkd-network.c b/src/network/networkd-network.c index 81d6d35c7c9..027e076c346 100644 --- a/src/network/networkd-network.c +++ b/src/network/networkd-network.c @@ -162,6 +162,11 @@ static uint32_t network_get_stacked_netdevs_mtu(Network *network) { static int network_verify(Network *network) { Address *address, *address_next; Route *route, *route_next; + FdbEntry *fdb, *fdb_next; + Neighbor *neighbor, *neighbor_next; + AddressLabel *label, *label_next; + Prefix *prefix, *prefix_next; + RoutingPolicyRule *rule, *rule_next; uint32_t mtu; assert(network); @@ -249,34 +254,32 @@ static int network_verify(Network *network) { } LIST_FOREACH_SAFE(addresses, address, address_next, network->static_addresses) - if (address->family == AF_UNSPEC) { - log_warning("%s: Address section without Address= field configured. " - "Ignoring [Address] section from line %u.", - network->filename, address->section->line); - + if (address_section_verify(address) < 0) address_free(address); - } - - LIST_FOREACH_SAFE(routes, route, route_next, network->static_routes) { - if (route->family == AF_UNSPEC) { - log_warning("%s: Route section without Gateway=, Destination=, Source=, " - "or PreferredSource= field configured. " - "Ignoring [Route] section from line %u.", - network->filename, route->section->line); + LIST_FOREACH_SAFE(routes, route, route_next, network->static_routes) + if (route_section_verify(route, network) < 0) route_free(route); - continue; - } - if (network->n_static_addresses == 0 && - in_addr_is_null(route->family, &route->gw) == 0 && - route->gateway_onlink < 0) { - log_warning("%s: Gateway= without static address configured. " - "Enabling GatewayOnLink= option.", - network->filename); - route->gateway_onlink = true; - } - } + LIST_FOREACH_SAFE(static_fdb_entries, fdb, fdb_next, network->static_fdb_entries) + if (section_is_invalid(fdb->section)) + fdb_entry_free(fdb); + + LIST_FOREACH_SAFE(neighbors, neighbor, neighbor_next, network->neighbors) + if (section_is_invalid(neighbor->section)) + neighbor_free(neighbor); + + LIST_FOREACH_SAFE(labels, label, label_next, network->address_labels) + if (section_is_invalid(label->section)) + address_label_free(label); + + LIST_FOREACH_SAFE(prefixes, prefix, prefix_next, network->static_prefixes) + if (section_is_invalid(prefix->section)) + prefix_free(prefix); + + LIST_FOREACH_SAFE(rules, rule, rule_next, network->rules) + if (section_is_invalid(rule->section)) + routing_policy_rule_free(rule); return 0; } diff --git a/src/network/networkd-radv.c b/src/network/networkd-radv.c index 3de969764eb..8cb14b588cf 100644 --- a/src/network/networkd-radv.c +++ b/src/network/networkd-radv.c @@ -186,7 +186,7 @@ int config_parse_prefix(const char *unit, void *userdata) { Network *network = userdata; - _cleanup_(prefix_freep) Prefix *p = NULL; + _cleanup_(prefix_free_or_set_invalidp) Prefix *p = NULL; uint8_t prefixlen = 64; union in_addr_union in6addr; int r; @@ -228,7 +228,7 @@ int config_parse_prefix_flags(const char *unit, void *data, void *userdata) { Network *network = userdata; - _cleanup_(prefix_freep) Prefix *p = NULL; + _cleanup_(prefix_free_or_set_invalidp) Prefix *p = NULL; int r, val; assert(filename); @@ -272,7 +272,7 @@ int config_parse_prefix_lifetime(const char *unit, void *data, void *userdata) { Network *network = userdata; - _cleanup_(prefix_freep) Prefix *p = NULL; + _cleanup_(prefix_free_or_set_invalidp) Prefix *p = NULL; usec_t usec; int r; diff --git a/src/network/networkd-radv.h b/src/network/networkd-radv.h index ebf6d7299b0..3192bb8b6c8 100644 --- a/src/network/networkd-radv.h +++ b/src/network/networkd-radv.h @@ -8,6 +8,7 @@ #include "conf-parser.h" #include "networkd-address.h" #include "networkd-link.h" +#include "networkd-util.h" typedef struct Prefix Prefix; @@ -23,7 +24,7 @@ struct Prefix { int prefix_new(Prefix **ret); void prefix_free(Prefix *prefix); -DEFINE_TRIVIAL_CLEANUP_FUNC(Prefix*, prefix_free); +DEFINE_NETWORK_SECTION_FUNCTIONS(Prefix, prefix_free); CONFIG_PARSER_PROTOTYPE(config_parse_router_prefix_delegation); CONFIG_PARSER_PROTOTYPE(config_parse_router_preference); diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index 3eb807c9f83..379077cbfde 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -678,7 +678,7 @@ int route_configure( } int network_add_ipv4ll_route(Network *network) { - _cleanup_(route_freep) Route *n = NULL; + _cleanup_(route_free_or_set_invalidp) Route *n = NULL; int r; assert(network); @@ -718,7 +718,7 @@ int config_parse_gateway( void *userdata) { Network *network = userdata; - _cleanup_(route_freep) Route *n = NULL; + _cleanup_(route_free_or_set_invalidp) Route *n = NULL; int r; assert(filename); @@ -763,7 +763,7 @@ int config_parse_preferred_src( void *userdata) { Network *network = userdata; - _cleanup_(route_freep) Route *n = NULL; + _cleanup_(route_free_or_set_invalidp) Route *n = NULL; int r; assert(filename); @@ -803,7 +803,7 @@ int config_parse_destination( void *userdata) { Network *network = userdata; - _cleanup_(route_freep) Route *n = NULL; + _cleanup_(route_free_or_set_invalidp) Route *n = NULL; union in_addr_union *buffer; unsigned char *prefixlen; int r; @@ -854,7 +854,7 @@ int config_parse_route_priority( void *userdata) { Network *network = userdata; - _cleanup_(route_freep) Route *n = NULL; + _cleanup_(route_free_or_set_invalidp) Route *n = NULL; int r; assert(filename); @@ -891,7 +891,7 @@ int config_parse_route_scope( void *userdata) { Network *network = userdata; - _cleanup_(route_freep) Route *n = NULL; + _cleanup_(route_free_or_set_invalidp) Route *n = NULL; int r; assert(filename); @@ -931,7 +931,7 @@ int config_parse_route_table( void *data, void *userdata) { - _cleanup_(route_freep) Route *n = NULL; + _cleanup_(route_free_or_set_invalidp) Route *n = NULL; Network *network = userdata; int r; @@ -969,7 +969,7 @@ int config_parse_gateway_onlink( void *userdata) { Network *network = userdata; - _cleanup_(route_freep) Route *n = NULL; + _cleanup_(route_free_or_set_invalidp) Route *n = NULL; int r; assert(filename); @@ -1008,7 +1008,7 @@ int config_parse_ipv6_route_preference( void *userdata) { Network *network = userdata; - _cleanup_(route_freep) Route *n = NULL; + _cleanup_(route_free_or_set_invalidp) Route *n = NULL; int r; r = route_new_static(network, filename, section_line, &n); @@ -1043,7 +1043,7 @@ int config_parse_route_protocol( void *userdata) { Network *network = userdata; - _cleanup_(route_freep) Route *n = NULL; + _cleanup_(route_free_or_set_invalidp) Route *n = NULL; int r; r = route_new_static(network, filename, section_line, &n); @@ -1082,7 +1082,7 @@ int config_parse_route_type( void *userdata) { Network *network = userdata; - _cleanup_(route_freep) Route *n = NULL; + _cleanup_(route_free_or_set_invalidp) Route *n = NULL; int r; r = route_new_static(network, filename, section_line, &n); @@ -1121,7 +1121,7 @@ int config_parse_tcp_window( void *data, void *userdata) { - _cleanup_(route_freep) Route *n = NULL; + _cleanup_(route_free_or_set_invalidp) Route *n = NULL; Network *network = userdata; uint64_t k; int r; @@ -1171,7 +1171,7 @@ int config_parse_quickack( void *data, void *userdata) { - _cleanup_(route_freep) Route *n = NULL; + _cleanup_(route_free_or_set_invalidp) Route *n = NULL; Network *network = userdata; int k, r; @@ -1210,7 +1210,7 @@ int config_parse_route_mtu( void *userdata) { Network *network = userdata; - _cleanup_(route_freep) Route *n = NULL; + _cleanup_(route_free_or_set_invalidp) Route *n = NULL; int r; assert(filename); @@ -1230,3 +1230,29 @@ int config_parse_route_mtu( TAKE_PTR(n); return 0; } + +int route_section_verify(Route *route, Network *network) { + if (section_is_invalid(route->section)) + return -EINVAL; + + if (route->family == AF_UNSPEC) { + assert(route->section); + + return log_warning_errno(SYNTHETIC_ERRNO(EINVAL), + "%s: Route section without Gateway=, Destination=, Source=, " + "or PreferredSource= field configured. " + "Ignoring [Route] section from line %u.", + route->section->filename, route->section->line); + } + + if (network->n_static_addresses == 0 && + in_addr_is_null(route->family, &route->gw) == 0 && + route->gateway_onlink < 0) { + log_warning("%s: Gateway= without static address configured. " + "Enabling GatewayOnLink= option.", + network->filename); + route->gateway_onlink = true; + } + + return 0; +} diff --git a/src/network/networkd-route.h b/src/network/networkd-route.h index c19b7aa3a73..1e8320fdc08 100644 --- a/src/network/networkd-route.h +++ b/src/network/networkd-route.h @@ -7,6 +7,7 @@ typedef struct Route Route; typedef struct NetworkConfigSection NetworkConfigSection; #include "networkd-network.h" +#include "networkd-util.h" struct Route { Network *network; @@ -55,8 +56,9 @@ void route_update(Route *route, const union in_addr_union *src, unsigned char sr bool route_equal(Route *r1, Route *r2); int route_expire_handler(sd_event_source *s, uint64_t usec, void *userdata); +int route_section_verify(Route *route, Network *network); -DEFINE_TRIVIAL_CLEANUP_FUNC(Route*, route_free); +DEFINE_NETWORK_SECTION_FUNCTIONS(Route, route_free); int network_add_ipv4ll_route(Network *network); diff --git a/src/network/networkd-routing-policy-rule.c b/src/network/networkd-routing-policy-rule.c index ae94272781e..f6253215ed8 100644 --- a/src/network/networkd-routing-policy-rule.c +++ b/src/network/networkd-routing-policy-rule.c @@ -636,7 +636,7 @@ int config_parse_routing_policy_rule_tos( void *data, void *userdata) { - _cleanup_(routing_policy_rule_freep) RoutingPolicyRule *n = NULL; + _cleanup_(routing_policy_rule_free_or_set_invalidp) RoutingPolicyRule *n = NULL; Network *network = userdata; int r; @@ -673,7 +673,7 @@ int config_parse_routing_policy_rule_priority( void *data, void *userdata) { - _cleanup_(routing_policy_rule_freep) RoutingPolicyRule *n = NULL; + _cleanup_(routing_policy_rule_free_or_set_invalidp) RoutingPolicyRule *n = NULL; Network *network = userdata; int r; @@ -710,7 +710,7 @@ int config_parse_routing_policy_rule_table( void *data, void *userdata) { - _cleanup_(routing_policy_rule_freep) RoutingPolicyRule *n = NULL; + _cleanup_(routing_policy_rule_free_or_set_invalidp) RoutingPolicyRule *n = NULL; Network *network = userdata; int r; @@ -747,7 +747,7 @@ int config_parse_routing_policy_rule_fwmark_mask( void *data, void *userdata) { - _cleanup_(routing_policy_rule_freep) RoutingPolicyRule *n = NULL; + _cleanup_(routing_policy_rule_free_or_set_invalidp) RoutingPolicyRule *n = NULL; Network *network = userdata; int r; @@ -784,7 +784,7 @@ int config_parse_routing_policy_rule_prefix( void *data, void *userdata) { - _cleanup_(routing_policy_rule_freep) RoutingPolicyRule *n = NULL; + _cleanup_(routing_policy_rule_free_or_set_invalidp) RoutingPolicyRule *n = NULL; Network *network = userdata; union in_addr_union *buffer; uint8_t *prefixlen; @@ -831,7 +831,7 @@ int config_parse_routing_policy_rule_device( void *data, void *userdata) { - _cleanup_(routing_policy_rule_freep) RoutingPolicyRule *n = NULL; + _cleanup_(routing_policy_rule_free_or_set_invalidp) RoutingPolicyRule *n = NULL; Network *network = userdata; int r; @@ -876,7 +876,7 @@ int config_parse_routing_policy_rule_port_range( const char *rvalue, void *data, void *userdata) { - _cleanup_(routing_policy_rule_freep) RoutingPolicyRule *n = NULL; + _cleanup_(routing_policy_rule_free_or_set_invalidp) RoutingPolicyRule *n = NULL; Network *network = userdata; uint16_t low, high; int r; @@ -922,7 +922,7 @@ int config_parse_routing_policy_rule_ip_protocol( void *data, void *userdata) { - _cleanup_(routing_policy_rule_freep) RoutingPolicyRule *n = NULL; + _cleanup_(routing_policy_rule_free_or_set_invalidp) RoutingPolicyRule *n = NULL; Network *network = userdata; int r; @@ -961,7 +961,7 @@ int config_parse_routing_policy_rule_invert( void *data, void *userdata) { - _cleanup_(routing_policy_rule_freep) RoutingPolicyRule *n = NULL; + _cleanup_(routing_policy_rule_free_or_set_invalidp) RoutingPolicyRule *n = NULL; Network *network = userdata; int r; diff --git a/src/network/networkd-routing-policy-rule.h b/src/network/networkd-routing-policy-rule.h index b35126e2cfd..e1bd78f8095 100644 --- a/src/network/networkd-routing-policy-rule.h +++ b/src/network/networkd-routing-policy-rule.h @@ -13,6 +13,7 @@ typedef struct RoutingPolicyRule RoutingPolicyRule; #include "networkd-link.h" #include "networkd-network.h" +#include "networkd-util.h" typedef struct Network Network; typedef struct Link Link; @@ -54,7 +55,7 @@ struct RoutingPolicyRule { int routing_policy_rule_new(RoutingPolicyRule **ret); void routing_policy_rule_free(RoutingPolicyRule *rule); -DEFINE_TRIVIAL_CLEANUP_FUNC(RoutingPolicyRule*, routing_policy_rule_free); +DEFINE_NETWORK_SECTION_FUNCTIONS(RoutingPolicyRule, routing_policy_rule_free); int routing_policy_rule_configure(RoutingPolicyRule *address, Link *link, link_netlink_message_handler_t callback, bool update); int routing_policy_rule_remove(RoutingPolicyRule *routing_policy_rule, Link *link, link_netlink_message_handler_t callback); diff --git a/src/network/networkd-util.h b/src/network/networkd-util.h index d360035b147..a49e2893511 100644 --- a/src/network/networkd-util.h +++ b/src/network/networkd-util.h @@ -17,6 +17,7 @@ typedef enum AddressFamilyBoolean { typedef struct NetworkConfigSection { unsigned line; + bool invalid; char filename[]; } NetworkConfigSection; @@ -32,3 +33,24 @@ int network_config_section_new(const char *filename, unsigned line, NetworkConfi void network_config_section_free(NetworkConfigSection *network); DEFINE_TRIVIAL_CLEANUP_FUNC(NetworkConfigSection*, network_config_section_free); extern const struct hash_ops network_config_hash_ops; + +static inline bool section_is_invalid(NetworkConfigSection *section) { + /* If this retuns false, then it does _not_ mean the section is valid. */ + + if (!section) + return false; + + return section->invalid; +} + +#define DEFINE_NETWORK_SECTION_FUNCTIONS(type, free_func) \ + static inline void free_func##_or_set_invalid(type *p) { \ + assert(p); \ + \ + if (p->section) \ + p->section->invalid = true; \ + else \ + free_func(p); \ + } \ + DEFINE_TRIVIAL_CLEANUP_FUNC(type*, free_func); \ + DEFINE_TRIVIAL_CLEANUP_FUNC(type*, free_func##_or_set_invalid); diff --git a/src/network/test-networkd-conf.c b/src/network/test-networkd-conf.c index 86d4e7e733f..3adfdab2bf8 100644 --- a/src/network/test-networkd-conf.c +++ b/src/network/test-networkd-conf.c @@ -173,7 +173,7 @@ static void test_config_parse_address_one(const char *rvalue, int family, unsign assert_se(network = new0(Network, 1)); assert_se(config_parse_address("network", "filename", 1, "section", 1, "Address", 0, rvalue, network, network) == 0); - assert_se(network->n_static_addresses == n_addresses); + assert_se(network->n_static_addresses == 1); if (n_addresses > 0) { assert_se(network->static_addresses); assert_se(network->static_addresses->prefixlen == prefixlen); From 96db641250e2692836badcf8eeb5e32fd7e189d4 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 1 Mar 2019 17:39:45 +0900 Subject: [PATCH 06/14] test: make network_verify() public and use it in test-networkd-conf --- src/network/networkd-network.c | 9 +++++---- src/network/networkd-network.h | 1 + src/network/test-networkd-conf.c | 3 +++ 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/network/networkd-network.c b/src/network/networkd-network.c index 027e076c346..22b240c1bec 100644 --- a/src/network/networkd-network.c +++ b/src/network/networkd-network.c @@ -70,14 +70,15 @@ static int network_resolve_netdev_one(Network *network, const char *name, NetDev NetDev *netdev; int r; + /* For test-networkd-conf, the check must be earlier than the assertions. */ + if (!name) + return 0; + assert(network); assert(network->manager); assert(network->filename); assert(ret_netdev); - if (!name) - return 0; - if (kind == _NETDEV_KIND_TUNNEL) kind_string = "tunnel"; else { @@ -159,7 +160,7 @@ static uint32_t network_get_stacked_netdevs_mtu(Network *network) { return mtu; } -static int network_verify(Network *network) { +int network_verify(Network *network) { Address *address, *address_next; Route *route, *route_next; FdbEntry *fdb, *fdb_next; diff --git a/src/network/networkd-network.h b/src/network/networkd-network.h index 38f3b7decb3..8b65fd16bde 100644 --- a/src/network/networkd-network.h +++ b/src/network/networkd-network.h @@ -285,6 +285,7 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(Network*, network_free); int network_load(Manager *manager); int network_load_one(Manager *manager, const char *filename); +int network_verify(Network *network); int network_get_by_name(Manager *manager, const char *name, Network **ret); int network_get(Manager *manager, sd_device *device, const char *ifname, const struct ether_addr *mac, Network **ret); diff --git a/src/network/test-networkd-conf.c b/src/network/test-networkd-conf.c index 3adfdab2bf8..6408719d15a 100644 --- a/src/network/test-networkd-conf.c +++ b/src/network/test-networkd-conf.c @@ -172,8 +172,11 @@ static void test_config_parse_address_one(const char *rvalue, int family, unsign _cleanup_(network_freep) Network *network = NULL; assert_se(network = new0(Network, 1)); + assert_se(network->filename = strdup("hogehoge.network")); assert_se(config_parse_address("network", "filename", 1, "section", 1, "Address", 0, rvalue, network, network) == 0); assert_se(network->n_static_addresses == 1); + assert_se(network_verify(network) >= 0); + assert_se(network->n_static_addresses == n_addresses); if (n_addresses > 0) { assert_se(network->static_addresses); assert_se(network->static_addresses->prefixlen == prefixlen); From bbb5aebe3e77842ab7c45f013def177fd3cd8abd Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 1 Mar 2019 15:21:22 +0900 Subject: [PATCH 07/14] test-network: add tests for invalid Address sections --- .../25-address-section-miscellaneous.network | 17 +++++++++++++++++ test/test-network/systemd-networkd-tests.py | 3 +++ 2 files changed, 20 insertions(+) diff --git a/test/test-network/conf/25-address-section-miscellaneous.network b/test/test-network/conf/25-address-section-miscellaneous.network index 3a37d036ce2..bfb278f31f2 100644 --- a/test/test-network/conf/25-address-section-miscellaneous.network +++ b/test/test-network/conf/25-address-section-miscellaneous.network @@ -1,6 +1,11 @@ [Match] Name=dummy98 +[Network] +# these lines are ignored +Address=hogehoge +Address=foofoo + [Address] Address=10.2.3.4/16 PreferredLifetime=0 @@ -8,3 +13,15 @@ Scope=link [Address] Address=2001:0db8:0:f101::1/64 + +[Address] +# this section must be ignored +Peer=hoge +Address=10.10.0.1/16 +Label=30 + +[Address] +# this section must be ignored +Label=30 +Peer=hoge +Address=10.10.0.2/16 diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index 856960ad65f..ec8ed00c38b 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -813,6 +813,9 @@ class NetworkdNetWorkTests(unittest.TestCase, Utilities): print(output) self.assertRegex(output, 'inet 10.2.3.4/16 brd 10.2.255.255 scope link deprecated dummy98') self.assertRegex(output, 'inet6 2001:db8:0:f101::1/64 scope global') + # also tests invalid [Address] section + self.assertNotRegex(output, '10.10.0.1/16') + self.assertNotRegex(output, '10.10.0.2/16') def test_ip_route(self): self.copy_unit_to_networkd_unit_path('25-route-section.network', '12-dummy.netdev') From f1368a333e5e08575f0b45dfe41e936b106a8627 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 28 Feb 2019 17:12:02 +0900 Subject: [PATCH 08/14] wireguard: use NetworkConfigSection to manage [WireGuardPeer] sections This also contains huge refactoring of wireguard support. --- src/network/netdev/wireguard.c | 363 ++++++++++++++++++++------------- src/network/netdev/wireguard.h | 30 +-- 2 files changed, 227 insertions(+), 166 deletions(-) diff --git a/src/network/netdev/wireguard.c b/src/network/netdev/wireguard.c index 7d35afae6d7..3560c25eecd 100644 --- a/src/network/netdev/wireguard.c +++ b/src/network/netdev/wireguard.c @@ -24,26 +24,79 @@ static void resolve_endpoints(NetDev *netdev); -static WireguardPeer *wireguard_peer_new(Wireguard *w, unsigned section) { - WireguardPeer *peer; +static void wireguard_peer_free(WireguardPeer *peer) { + WireguardIPmask *mask; + + if (!peer) + return; + + if (peer->wireguard) { + LIST_REMOVE(peers, peer->wireguard->peers, peer); + + set_remove(peer->wireguard->peers_with_unresolved_endpoint, peer); + set_remove(peer->wireguard->peers_with_failed_endpoint, peer); + + if (peer->section) + hashmap_remove(peer->wireguard->peers_by_section, peer->section); + } + + network_config_section_free(peer->section); + + while ((mask = peer->ipmasks)) { + LIST_REMOVE(ipmasks, peer->ipmasks, mask); + free(mask); + } + + free(peer->endpoint_host); + free(peer->endpoint_port); + + free(peer); +} + +DEFINE_NETWORK_SECTION_FUNCTIONS(WireguardPeer, wireguard_peer_free); + +static int wireguard_peer_new_static(Wireguard *w, const char *filename, unsigned section_line, WireguardPeer **ret) { + _cleanup_(network_config_section_freep) NetworkConfigSection *n = NULL; + _cleanup_(wireguard_peer_freep) WireguardPeer *peer = NULL; + int r; assert(w); + assert(ret); + assert(filename); + assert(section_line > 0); - if (w->last_peer_section == section && w->peers) - return w->peers; + r = network_config_section_new(filename, section_line, &n); + if (r < 0) + return r; + + peer = hashmap_get(w->peers_by_section, n); + if (peer) { + *ret = TAKE_PTR(peer); + return 0; + } peer = new(WireguardPeer, 1); if (!peer) - return NULL; + return -ENOMEM; *peer = (WireguardPeer) { .flags = WGPEER_F_REPLACE_ALLOWEDIPS, + .wireguard = w, + .section = TAKE_PTR(n), }; LIST_PREPEND(peers, w->peers, peer); - w->last_peer_section = section; - return peer; + r = hashmap_ensure_allocated(&w->peers_by_section, &network_config_hash_ops); + if (r < 0) + return r; + + r = hashmap_put(w->peers_by_section, peer->section, peer); + if (r < 0) + return r; + + *ret = TAKE_PTR(peer); + return 0; } static int wireguard_set_ipmask_one(NetDev *netdev, sd_netlink_message *message, const WireguardIPmask *mask, uint16_t index) { @@ -226,24 +279,20 @@ static int wireguard_set_interface(NetDev *netdev) { return 0; } -static WireguardEndpoint* wireguard_endpoint_free(WireguardEndpoint *e) { - if (!e) - return NULL; - e->host = mfree(e->host); - e->port = mfree(e->port); - return mfree(e); +static void wireguard_peer_destroy_callback(WireguardPeer *peer) { + NetDev *netdev; + + assert(peer); + assert(peer->wireguard); + + netdev = NETDEV(peer->wireguard); + + if (section_is_invalid(peer->section)) + wireguard_peer_free(peer); + + netdev_unref(netdev); } -static void wireguard_endpoint_destroy_callback(WireguardEndpoint *e) { - assert(e); - assert(e->netdev); - - netdev_unref(e->netdev); - wireguard_endpoint_free(e); -} - -DEFINE_TRIVIAL_CLEANUP_FUNC(WireguardEndpoint*, wireguard_endpoint_free); - static int on_resolve_retry(sd_event_source *s, usec_t usec, void *userdata) { NetDev *netdev = userdata; Wireguard *w; @@ -255,8 +304,9 @@ static int on_resolve_retry(sd_event_source *s, usec_t usec, void *userdata) { if (!netdev_is_managed(netdev)) return 0; - assert(!w->unresolved_endpoints); - w->unresolved_endpoints = TAKE_PTR(w->failed_endpoints); + assert(set_isempty(w->peers_with_unresolved_endpoint)); + + SWAP_TWO(w->peers_with_unresolved_endpoint, w->peers_with_failed_endpoint); resolve_endpoints(netdev); @@ -274,40 +324,53 @@ static int exponential_backoff_milliseconds(unsigned n_retries) { static int wireguard_resolve_handler(sd_resolve_query *q, int ret, const struct addrinfo *ai, - WireguardEndpoint *e) { - _cleanup_(netdev_unrefp) NetDev *netdev_will_unrefed = NULL; + WireguardPeer *peer) { NetDev *netdev; Wireguard *w; int r; - assert(e); - assert(e->netdev); + assert(peer); + assert(peer->wireguard); - netdev = e->netdev; - w = WIREGUARD(netdev); - assert(w); + w = peer->wireguard; + netdev = NETDEV(w); if (!netdev_is_managed(netdev)) return 0; if (ret != 0) { - log_netdev_error(netdev, "Failed to resolve host '%s:%s': %s", e->host, e->port, gai_strerror(ret)); - LIST_PREPEND(endpoints, w->failed_endpoints, e); - (void) sd_resolve_query_set_destroy_callback(q, NULL); /* Avoid freeing endpoint by destroy callback. */ - netdev_will_unrefed = netdev; /* But netdev needs to be unrefed. */ + log_netdev_error(netdev, "Failed to resolve host '%s:%s': %s", peer->endpoint_host, peer->endpoint_port, gai_strerror(ret)); + + r = set_ensure_allocated(&w->peers_with_failed_endpoint, NULL); + if (r < 0) { + log_oom(); + peer->section->invalid = true; + goto resolve_next; + } + + r = set_put(w->peers_with_failed_endpoint, peer); + if (r < 0) { + log_netdev_error(netdev, "Failed to save a peer, dropping the peer: %m"); + peer->section->invalid = true; + goto resolve_next; + } + } else if ((ai->ai_family == AF_INET && ai->ai_addrlen == sizeof(struct sockaddr_in)) || (ai->ai_family == AF_INET6 && ai->ai_addrlen == sizeof(struct sockaddr_in6))) - memcpy(&e->peer->endpoint, ai->ai_addr, ai->ai_addrlen); + memcpy(&peer->endpoint, ai->ai_addr, ai->ai_addrlen); else - log_netdev_error(netdev, "Neither IPv4 nor IPv6 address found for peer endpoint: %s:%s", e->host, e->port); + log_netdev_error(netdev, "Neither IPv4 nor IPv6 address found for peer endpoint %s:%s, ignoring the address.", + peer->endpoint_host, peer->endpoint_port); - if (w->unresolved_endpoints) { +resolve_next: + if (!set_isempty(w->peers_with_unresolved_endpoint)) { resolve_endpoints(netdev); return 0; } (void) wireguard_set_interface(netdev); - if (w->failed_endpoints) { + + if (!set_isempty(w->peers_with_failed_endpoint)) { _cleanup_(sd_event_source_unrefp) sd_event_source *s = NULL; w->n_retries++; @@ -342,24 +405,24 @@ static void resolve_endpoints(NetDev *netdev) { .ai_socktype = SOCK_DGRAM, .ai_protocol = IPPROTO_UDP }; - WireguardEndpoint *endpoint; + WireguardPeer *peer; Wireguard *w; + Iterator i; int r = 0; assert(netdev); w = WIREGUARD(netdev); assert(w); - LIST_FOREACH(endpoints, endpoint, w->unresolved_endpoints) { + SET_FOREACH(peer, w->peers_with_unresolved_endpoint, i) { r = resolve_getaddrinfo(netdev->manager->resolve, NULL, - endpoint->host, - endpoint->port, + peer->endpoint_host, + peer->endpoint_port, &hints, wireguard_resolve_handler, - wireguard_endpoint_destroy_callback, - endpoint); - + wireguard_peer_destroy_callback, + peer); if (r == -ENOBUFS) break; if (r < 0) { @@ -370,7 +433,7 @@ static void resolve_endpoints(NetDev *netdev) { /* Avoid freeing netdev. It will be unrefed by the destroy callback. */ netdev_ref(netdev); - LIST_REMOVE(endpoints, w->unresolved_endpoints, endpoint); + (void) set_remove(w->peers_with_unresolved_endpoint, peer); } } @@ -404,9 +467,12 @@ int config_parse_wireguard_listen_port(const char *unit, assert(data); if (!streq(rvalue, "auto")) { - r = parse_ip_port(rvalue, &port); - if (r < 0) - log_syntax(unit, LOG_ERR, filename, line, r, "Invalid port specification, ignoring assignment: %s", rvalue); + r = parse_ip_port(rvalue, s); + if (r < 0) { + log_syntax(unit, LOG_ERR, filename, line, r, + "Invalid port specification, ignoring assignment: %s", rvalue); + return 0; + } } *s = port; @@ -434,7 +500,8 @@ static int parse_wireguard_key(const char *unit, r = unbase64mem(rvalue, strlen(rvalue), &key, &len); if (r < 0) { - log_syntax(unit, LOG_ERR, filename, line, r, "Could not parse wireguard key \"%s\", ignoring assignment: %m", rvalue); + log_syntax(unit, LOG_ERR, filename, line, r, + "Could not parse wireguard key \"%s\", ignoring assignment: %m", rvalue); return 0; } if (len != WG_KEY_LEN) { @@ -465,16 +532,8 @@ int config_parse_wireguard_private_key(const char *unit, assert(w); - return parse_wireguard_key(unit, - filename, - line, - section, - section_line, - lvalue, - ltype, - rvalue, - data, - &w->private_key); + return parse_wireguard_key(unit, filename, line, section, section_line, + lvalue, ltype, rvalue, data, &w->private_key); } @@ -488,8 +547,10 @@ int config_parse_wireguard_preshared_key(const char *unit, const char *rvalue, void *data, void *userdata) { + + _cleanup_(wireguard_peer_free_or_set_invalidp) WireguardPeer *peer = NULL; Wireguard *w; - WireguardPeer *peer; + int r; assert(data); @@ -497,20 +558,17 @@ int config_parse_wireguard_preshared_key(const char *unit, assert(w); - peer = wireguard_peer_new(w, section_line); - if (!peer) - return log_oom(); + r = wireguard_peer_new_static(w, filename, section_line, &peer); + if (r < 0) + return r; - return parse_wireguard_key(unit, - filename, - line, - section, - section_line, - lvalue, - ltype, - rvalue, - data, - peer->preshared_key); + r = parse_wireguard_key(unit, filename, line, section, section_line, + lvalue, ltype, rvalue, data, peer->preshared_key); + if (r < 0) + return r; + + TAKE_PTR(peer); + return 0; } int config_parse_wireguard_public_key(const char *unit, @@ -523,8 +581,10 @@ int config_parse_wireguard_public_key(const char *unit, const char *rvalue, void *data, void *userdata) { + + _cleanup_(wireguard_peer_free_or_set_invalidp) WireguardPeer *peer = NULL; Wireguard *w; - WireguardPeer *peer; + int r; assert(data); @@ -532,20 +592,17 @@ int config_parse_wireguard_public_key(const char *unit, assert(w); - peer = wireguard_peer_new(w, section_line); - if (!peer) - return log_oom(); + r = wireguard_peer_new_static(w, filename, section_line, &peer); + if (r < 0) + return r; - return parse_wireguard_key(unit, - filename, - line, - section, - section_line, - lvalue, - ltype, - rvalue, - data, - peer->public_key); + r = parse_wireguard_key(unit, filename, line, section, section_line, + lvalue, ltype, rvalue, data, peer->public_key); + if (r < 0) + return r; + + TAKE_PTR(peer); + return 0; } int config_parse_wireguard_allowed_ips(const char *unit, @@ -558,11 +615,12 @@ int config_parse_wireguard_allowed_ips(const char *unit, const char *rvalue, void *data, void *userdata) { + + _cleanup_(wireguard_peer_free_or_set_invalidp) WireguardPeer *peer = NULL; union in_addr_union addr; unsigned char prefixlen; int r, family; Wireguard *w; - WireguardPeer *peer; WireguardIPmask *ipmask; assert(rvalue); @@ -570,9 +628,11 @@ int config_parse_wireguard_allowed_ips(const char *unit, w = WIREGUARD(data); - peer = wireguard_peer_new(w, section_line); - if (!peer) - return log_oom(); + assert(w); + + r = wireguard_peer_new_static(w, filename, section_line, &peer); + if (r < 0) + return r; for (;;) { _cleanup_free_ char *word = NULL; @@ -583,14 +643,16 @@ int config_parse_wireguard_allowed_ips(const char *unit, if (r == -ENOMEM) return log_oom(); if (r < 0) { - log_syntax(unit, LOG_ERR, filename, line, r, "Failed to split allowed ips \"%s\" option: %m", rvalue); + log_syntax(unit, LOG_ERR, filename, line, r, + "Failed to split allowed ips \"%s\" option: %m", rvalue); break; } r = in_addr_prefix_from_string_auto(word, &family, &addr, &prefixlen); if (r < 0) { - log_syntax(unit, LOG_ERR, filename, line, r, "Network address is invalid, ignoring assignment: %s", word); - return 0; + log_syntax(unit, LOG_ERR, filename, line, r, + "Network address is invalid, ignoring assignment: %s", word); + continue; } ipmask = new(WireguardIPmask, 1); @@ -606,6 +668,7 @@ int config_parse_wireguard_allowed_ips(const char *unit, LIST_PREPEND(ipmasks, peer->ipmasks, ipmask); } + TAKE_PTR(peer); return 0; } @@ -619,12 +682,12 @@ int config_parse_wireguard_endpoint(const char *unit, const char *rvalue, void *data, void *userdata) { + + _cleanup_(wireguard_peer_free_or_set_invalidp) WireguardPeer *peer = NULL; + const char *begin, *end; Wireguard *w; - WireguardPeer *peer; size_t len; - const char *begin, *end = NULL; - _cleanup_free_ char *host = NULL, *port = NULL; - _cleanup_(wireguard_endpoint_freep) WireguardEndpoint *endpoint = NULL; + int r; assert(data); assert(rvalue); @@ -633,21 +696,25 @@ int config_parse_wireguard_endpoint(const char *unit, assert(w); - peer = wireguard_peer_new(w, section_line); - if (!peer) - return log_oom(); + r = wireguard_peer_new_static(w, filename, section_line, &peer); + if (r < 0) + return r; if (rvalue[0] == '[') { begin = &rvalue[1]; end = strchr(rvalue, ']'); if (!end) { - log_syntax(unit, LOG_ERR, filename, line, 0, "Unable to find matching brace of endpoint, ignoring assignment: %s", rvalue); + log_syntax(unit, LOG_ERR, filename, line, 0, + "Unable to find matching brace of endpoint, ignoring assignment: %s", + rvalue); return 0; } len = end - begin; ++end; if (*end != ':' || !*(end + 1)) { - log_syntax(unit, LOG_ERR, filename, line, 0, "Unable to find port of endpoint: %s", rvalue); + log_syntax(unit, LOG_ERR, filename, line, 0, + "Unable to find port of endpoint, ignoring assignment: %s", + rvalue); return 0; } ++end; @@ -655,33 +722,32 @@ int config_parse_wireguard_endpoint(const char *unit, begin = rvalue; end = strrchr(rvalue, ':'); if (!end || !*(end + 1)) { - log_syntax(unit, LOG_ERR, filename, line, 0, "Unable to find port of endpoint: %s", rvalue); + log_syntax(unit, LOG_ERR, filename, line, 0, + "Unable to find port of endpoint, ignoring assignment: %s", + rvalue); return 0; } len = end - begin; ++end; } - host = strndup(begin, len); - if (!host) + peer->endpoint_host = strndup(begin, len); + if (!peer->endpoint_host) return log_oom(); - port = strdup(end); - if (!port) + peer->endpoint_port = strdup(end); + if (!peer->endpoint_port) return log_oom(); - endpoint = new(WireguardEndpoint, 1); - if (!endpoint) + r = set_ensure_allocated(&w->peers_with_unresolved_endpoint, NULL); + if (r < 0) return log_oom(); - *endpoint = (WireguardEndpoint) { - .peer = TAKE_PTR(peer), - .host = TAKE_PTR(host), - .port = TAKE_PTR(port), - .netdev = data, - }; - LIST_PREPEND(endpoints, w->unresolved_endpoints, TAKE_PTR(endpoint)); + r = set_put(w->peers_with_unresolved_endpoint, peer); + if (r < 0) + return r; + TAKE_PTR(peer); return 0; } @@ -695,10 +761,11 @@ int config_parse_wireguard_keepalive(const char *unit, const char *rvalue, void *data, void *userdata) { - int r; + + _cleanup_(wireguard_peer_free_or_set_invalidp) WireguardPeer *peer = NULL; uint16_t keepalive = 0; Wireguard *w; - WireguardPeer *peer; + int r; assert(rvalue); assert(data); @@ -707,19 +774,25 @@ int config_parse_wireguard_keepalive(const char *unit, assert(w); - peer = wireguard_peer_new(w, section_line); - if (!peer) - return log_oom(); + r = wireguard_peer_new_static(w, filename, section_line, &peer); + if (r < 0) + return r; if (streq(rvalue, "off")) keepalive = 0; else { r = safe_atou16(rvalue, &keepalive); - if (r < 0) - log_syntax(unit, LOG_ERR, filename, line, r, "The persistent keepalive interval must be 0-65535. Ignore assignment: %s", rvalue); + if (r < 0) { + log_syntax(unit, LOG_ERR, filename, line, r, + "The persistent keepalive interval must be 0-65535. Ignore assignment: %s", + rvalue); + return 0; + } } peer->persistent_keepalive_interval = keepalive; + + TAKE_PTR(peer); return 0; } @@ -737,32 +810,29 @@ static void wireguard_init(NetDev *netdev) { static void wireguard_done(NetDev *netdev) { Wireguard *w; - WireguardPeer *peer; - WireguardIPmask *mask; - WireguardEndpoint *e; assert(netdev); w = WIREGUARD(netdev); assert(w); - while ((peer = w->peers)) { - LIST_REMOVE(peers, w->peers, peer); - while ((mask = peer->ipmasks)) { - LIST_REMOVE(ipmasks, peer->ipmasks, mask); - free(mask); - } - free(peer); - } + hashmap_free_with_destructor(w->peers_by_section, wireguard_peer_free); + set_free(w->peers_with_unresolved_endpoint); + set_free(w->peers_with_failed_endpoint); +} - while ((e = w->unresolved_endpoints)) { - LIST_REMOVE(endpoints, w->unresolved_endpoints, e); - wireguard_endpoint_free(e); - } +static int wireguard_verify(NetDev *netdev, const char *filename) { + WireguardPeer *peer, *peer_next; + Wireguard *w; - while ((e = w->failed_endpoints)) { - LIST_REMOVE(endpoints, w->failed_endpoints, e); - wireguard_endpoint_free(e); - } + assert(netdev); + w = WIREGUARD(netdev); + assert(w); + + LIST_FOREACH_SAFE(peers, peer, peer_next, w->peers) + if (section_is_invalid(peer->section)) + wireguard_peer_free(peer); + + return 0; } const NetDevVTable wireguard_vtable = { @@ -772,4 +842,5 @@ const NetDevVTable wireguard_vtable = { .init = wireguard_init, .done = wireguard_done, .create_type = NETDEV_CREATE_INDEPENDENT, + .config_verify = wireguard_verify, }; diff --git a/src/network/netdev/wireguard.h b/src/network/netdev/wireguard.h index 143e93cdc5f..72e895bf434 100644 --- a/src/network/netdev/wireguard.h +++ b/src/network/netdev/wireguard.h @@ -7,10 +7,6 @@ typedef struct Wireguard Wireguard; #include "socket-util.h" #include "wireguard-netlink.h" -#ifndef IFNAMSIZ -#define IFNAMSIZ 16 -#endif - typedef struct WireguardIPmask { uint16_t family; union in_addr_union ip; @@ -20,43 +16,37 @@ typedef struct WireguardIPmask { } WireguardIPmask; typedef struct WireguardPeer { + Wireguard *wireguard; + NetworkConfigSection *section; + uint8_t public_key[WG_KEY_LEN]; uint8_t preshared_key[WG_KEY_LEN]; uint32_t flags; + uint16_t persistent_keepalive_interval; union sockaddr_union endpoint; - - uint16_t persistent_keepalive_interval; + char *endpoint_host; + char *endpoint_port; LIST_HEAD(WireguardIPmask, ipmasks); LIST_FIELDS(struct WireguardPeer, peers); } WireguardPeer; -typedef struct WireguardEndpoint { - char *host; - char *port; - - NetDev *netdev; - WireguardPeer *peer; - - LIST_FIELDS(struct WireguardEndpoint, endpoints); -} WireguardEndpoint; - struct Wireguard { NetDev meta; unsigned last_peer_section; uint32_t flags; - uint8_t private_key[WG_KEY_LEN]; + uint16_t port; uint32_t fwmark; - uint16_t port; + Hashmap *peers_by_section; + Set *peers_with_unresolved_endpoint; + Set *peers_with_failed_endpoint; LIST_HEAD(WireguardPeer, peers); - LIST_HEAD(WireguardEndpoint, unresolved_endpoints); - LIST_HEAD(WireguardEndpoint, failed_endpoints); unsigned n_retries; }; From 85c987a82d80460ea17b55e40de0d6aade4c5291 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 1 Mar 2019 15:41:47 +0900 Subject: [PATCH 09/14] wireguard: reuse sd_event_source object --- src/network/netdev/wireguard.c | 25 ++++++++----------------- src/network/netdev/wireguard.h | 1 + 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/src/network/netdev/wireguard.c b/src/network/netdev/wireguard.c index 3560c25eecd..44e7aa61098 100644 --- a/src/network/netdev/wireguard.c +++ b/src/network/netdev/wireguard.c @@ -9,6 +9,7 @@ #include "sd-resolve.h" #include "alloc-util.h" +#include "event-util.h" #include "fd-util.h" #include "hexdecoct.h" #include "netlink-util.h" @@ -371,29 +372,17 @@ resolve_next: (void) wireguard_set_interface(netdev); if (!set_isempty(w->peers_with_failed_endpoint)) { - _cleanup_(sd_event_source_unrefp) sd_event_source *s = NULL; + usec_t usec; w->n_retries++; - r = sd_event_add_time(netdev->manager->event, - &s, - CLOCK_MONOTONIC, - now(CLOCK_MONOTONIC) + exponential_backoff_milliseconds(w->n_retries), - 0, - on_resolve_retry, - netdev); + usec = usec_add(now(CLOCK_MONOTONIC), exponential_backoff_milliseconds(w->n_retries)); + r = event_reset_time(netdev->manager->event, &w->resolve_retry_event_source, + CLOCK_MONOTONIC, usec, 0, on_resolve_retry, netdev, + 0, "wireguard-resolve-retry", true); if (r < 0) { log_netdev_warning_errno(netdev, r, "Could not arm resolve retry handler: %m"); return 0; } - - r = sd_event_source_set_destroy_callback(s, (sd_event_destroy_t) netdev_destroy_callback); - if (r < 0) { - log_netdev_warning_errno(netdev, r, "Failed to set destroy callback to event source: %m"); - return 0; - } - - (void) sd_event_source_set_floating(s, true); - netdev_ref(netdev); } return 0; @@ -815,6 +804,8 @@ static void wireguard_done(NetDev *netdev) { w = WIREGUARD(netdev); assert(w); + sd_event_source_unref(w->resolve_retry_event_source); + hashmap_free_with_destructor(w->peers_by_section, wireguard_peer_free); set_free(w->peers_with_unresolved_endpoint); set_free(w->peers_with_failed_endpoint); diff --git a/src/network/netdev/wireguard.h b/src/network/netdev/wireguard.h index 72e895bf434..65e9ac24b9a 100644 --- a/src/network/netdev/wireguard.h +++ b/src/network/netdev/wireguard.h @@ -48,6 +48,7 @@ struct Wireguard { LIST_HEAD(WireguardPeer, peers); unsigned n_retries; + sd_event_source *resolve_retry_event_source; }; DEFINE_NETDEV_CAST(WIREGUARD, Wireguard); From 583eb170f0b09d5f83a9a5d2a1924773077f6c5d Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 4 Mar 2019 14:12:03 +0900 Subject: [PATCH 10/14] wireguard: do not log wireguard key It may be PrivateKey=. --- src/network/netdev/wireguard.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/network/netdev/wireguard.c b/src/network/netdev/wireguard.c index 44e7aa61098..8286b47100c 100644 --- a/src/network/netdev/wireguard.c +++ b/src/network/netdev/wireguard.c @@ -490,12 +490,13 @@ static int parse_wireguard_key(const char *unit, r = unbase64mem(rvalue, strlen(rvalue), &key, &len); if (r < 0) { log_syntax(unit, LOG_ERR, filename, line, r, - "Could not parse wireguard key \"%s\", ignoring assignment: %m", rvalue); + "Failed to decode wireguard key provided by %s=, ignoring assignment: %m", lvalue); return 0; } if (len != WG_KEY_LEN) { - log_syntax(unit, LOG_ERR, filename, line, EINVAL, - "Wireguard key is too short, ignoring assignment: %s", rvalue); + log_syntax(unit, LOG_ERR, filename, line, 0, + "Wireguard key provided by %s= has invalid length (%zu bytes), ignoring assignment.", + lvalue, len); return 0; } From fedcb4c3eb969b4ebae94b75cea106cecf774880 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 5 Mar 2019 12:35:43 +0900 Subject: [PATCH 11/14] wireguard: drop unused arguments and rename parse_wireguard_key() --- src/network/netdev/wireguard.c | 38 +++++++++++++++++----------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/src/network/netdev/wireguard.c b/src/network/netdev/wireguard.c index 8286b47100c..839af53a47f 100644 --- a/src/network/netdev/wireguard.c +++ b/src/network/netdev/wireguard.c @@ -469,23 +469,26 @@ int config_parse_wireguard_listen_port(const char *unit, return 0; } -static int parse_wireguard_key(const char *unit, - const char *filename, - unsigned line, - const char *section, - unsigned section_line, - const char *lvalue, - int ltype, - const char *rvalue, - void *data, - void *userdata) { +static int wireguard_decode_key_and_warn( + const char *rvalue, + uint8_t *ret, + const char *unit, + const char *filename, + unsigned line, + const char *lvalue) { _cleanup_free_ void *key = NULL; size_t len; int r; - assert(filename); assert(rvalue); - assert(userdata); + assert(ret); + assert(filename); + assert(lvalue); + + if (isempty(rvalue)) { + memzero(ret, WG_KEY_LEN); + return 0; + } r = unbase64mem(rvalue, strlen(rvalue), &key, &len); if (r < 0) { @@ -500,7 +503,7 @@ static int parse_wireguard_key(const char *unit, return 0; } - memcpy(userdata, key, WG_KEY_LEN); + memcpy(ret, key, WG_KEY_LEN); return true; } @@ -522,8 +525,7 @@ int config_parse_wireguard_private_key(const char *unit, assert(w); - return parse_wireguard_key(unit, filename, line, section, section_line, - lvalue, ltype, rvalue, data, &w->private_key); + return wireguard_decode_key_and_warn(rvalue, w->private_key, unit, filename, line, lvalue); } @@ -552,8 +554,7 @@ int config_parse_wireguard_preshared_key(const char *unit, if (r < 0) return r; - r = parse_wireguard_key(unit, filename, line, section, section_line, - lvalue, ltype, rvalue, data, peer->preshared_key); + r = wireguard_decode_key_and_warn(rvalue, peer->preshared_key, unit, filename, line, lvalue); if (r < 0) return r; @@ -586,8 +587,7 @@ int config_parse_wireguard_public_key(const char *unit, if (r < 0) return r; - r = parse_wireguard_key(unit, filename, line, section, section_line, - lvalue, ltype, rvalue, data, peer->public_key); + r = wireguard_decode_key_and_warn(rvalue, peer->public_key, unit, filename, line, lvalue); if (r < 0) return r; From 9cc9021aed09c064cfd56d28704474d1cc4f7de7 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 4 Mar 2019 14:31:36 +0900 Subject: [PATCH 12/14] wireguard: check whether PrivateKey= and PublicKey= are set --- src/network/netdev/wireguard.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/src/network/netdev/wireguard.c b/src/network/netdev/wireguard.c index 839af53a47f..c5c60c1752f 100644 --- a/src/network/netdev/wireguard.c +++ b/src/network/netdev/wireguard.c @@ -812,6 +812,21 @@ static void wireguard_done(NetDev *netdev) { set_free(w->peers_with_failed_endpoint); } +static int wireguard_peer_verify(WireguardPeer *peer) { + NetDev *netdev = NETDEV(peer->wireguard); + + if (section_is_invalid(peer->section)) + return -EINVAL; + + if (eqzero(peer->public_key)) + return log_netdev_error_errno(netdev, SYNTHETIC_ERRNO(EINVAL), + "%s: WireGuardPeer section without PublicKey= configured. " + "Ignoring [WireGuardPeer] section from line %u.", + peer->section->filename, peer->section->line); + + return 0; +} + static int wireguard_verify(NetDev *netdev, const char *filename) { WireguardPeer *peer, *peer_next; Wireguard *w; @@ -820,8 +835,13 @@ static int wireguard_verify(NetDev *netdev, const char *filename) { w = WIREGUARD(netdev); assert(w); + if (eqzero(w->private_key)) + return log_netdev_error_errno(netdev, SYNTHETIC_ERRNO(EINVAL), + "%s: Missing PrivateKey= or PrivateKeyFile=, ignoring.", + filename); + LIST_FOREACH_SAFE(peers, peer, peer_next, w->peers) - if (section_is_invalid(peer->section)) + if (wireguard_peer_verify(peer) < 0) wireguard_peer_free(peer); return 0; From 76df77794a902c7a7bce6cf689114fde9e139b22 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 4 Mar 2019 14:19:21 +0900 Subject: [PATCH 13/14] wireguard: add PrivateKeyFile= option Closes #11878. --- man/systemd.netdev.xml | 14 +++- src/network/netdev/netdev-gperf.gperf | 1 + src/network/netdev/wireguard.c | 84 ++++++++++++++++++- src/network/netdev/wireguard.h | 2 + .../fuzz/fuzz-netdev-parser/directives.netdev | 1 + 5 files changed, 100 insertions(+), 2 deletions(-) diff --git a/man/systemd.netdev.xml b/man/systemd.netdev.xml index 69e7d53fc7d..9d8f045da6e 100644 --- a/man/systemd.netdev.xml +++ b/man/systemd.netdev.xml @@ -1113,12 +1113,24 @@ The Base64 encoded private key for the interface. It can be generated using the wg genkey command (see wg8). - This option is mandatory to use WireGuard. + This option or PrivateKeyFile= is mandatory to use WireGuard. Note that because this information is secret, you may want to set the permissions of the .netdev file to be owned by root:systemd-network with a 0640 file mode. + + PrivateKeyFile= + + Takes a absolute path to a file which contains the Base64 encoded private key for the interface. + If both PrivateKey= and PrivateKeyFile= are specified, and if + the file specified in PrivateKeyFile= contains valid wireguard key, then + the key provided by PrivateKey= is ignored. + Note that the file must be readable by the user systemd-network, so it + should be, e.g., owned by root:systemd-network with a + 0640 file mode. + + ListenPort= diff --git a/src/network/netdev/netdev-gperf.gperf b/src/network/netdev/netdev-gperf.gperf index f7ca98fa467..29bdc65a21f 100644 --- a/src/network/netdev/netdev-gperf.gperf +++ b/src/network/netdev/netdev-gperf.gperf @@ -167,6 +167,7 @@ VRF.Table, config_parse_uint32, 0, WireGuard.FwMark, config_parse_unsigned, 0, offsetof(Wireguard, fwmark) WireGuard.ListenPort, config_parse_wireguard_listen_port, 0, offsetof(Wireguard, port) WireGuard.PrivateKey, config_parse_wireguard_private_key, 0, 0 +WireGuard.PrivateKeyFile, config_parse_wireguard_private_key_file, 0, 0 WireGuardPeer.AllowedIPs, config_parse_wireguard_allowed_ips, 0, 0 WireGuardPeer.Endpoint, config_parse_wireguard_endpoint, 0, 0 WireGuardPeer.PublicKey, config_parse_wireguard_public_key, 0, 0 diff --git a/src/network/netdev/wireguard.c b/src/network/netdev/wireguard.c index c5c60c1752f..eefb543c4d3 100644 --- a/src/network/netdev/wireguard.c +++ b/src/network/netdev/wireguard.c @@ -11,12 +11,14 @@ #include "alloc-util.h" #include "event-util.h" #include "fd-util.h" +#include "fileio.h" #include "hexdecoct.h" #include "netlink-util.h" #include "networkd-link.h" #include "networkd-manager.h" #include "networkd-util.h" #include "parse-util.h" +#include "path-util.h" #include "resolve-private.h" #include "string-util.h" #include "strv.h" @@ -529,6 +531,40 @@ int config_parse_wireguard_private_key(const char *unit, } +int config_parse_wireguard_private_key_file( + const char *unit, + const char *filename, + unsigned line, + const char *section, + unsigned section_line, + const char *lvalue, + int ltype, + const char *rvalue, + void *data, + void *userdata) { + + _cleanup_free_ char *path = NULL; + Wireguard *w; + + assert(data); + w = WIREGUARD(data); + assert(w); + + if (isempty(rvalue)) { + w->private_key_file = mfree(w->private_key_file); + return 0; + } + + path = strdup(rvalue); + if (!path) + return log_oom(); + + if (path_simplify_and_warn(path, PATH_CHECK_ABSOLUTE, unit, filename, line, lvalue) < 0) + return 0; + + return free_and_replace(w->private_key_file, path); +} + int config_parse_wireguard_preshared_key(const char *unit, const char *filename, unsigned line, @@ -807,11 +843,50 @@ static void wireguard_done(NetDev *netdev) { sd_event_source_unref(w->resolve_retry_event_source); + free(w->private_key_file); + hashmap_free_with_destructor(w->peers_by_section, wireguard_peer_free); set_free(w->peers_with_unresolved_endpoint); set_free(w->peers_with_failed_endpoint); } +static int wireguard_read_private_key_file(Wireguard *w, bool fatal) { + _cleanup_free_ char *contents = NULL; + _cleanup_free_ void *key = NULL; + size_t size, key_len; + NetDev *netdev; + int level, r; + + assert(w); + + netdev = NETDEV(w); + + if (!w->private_key_file) + return 0; + + level = fatal ? LOG_ERR : LOG_INFO; + + r = read_full_file(w->private_key_file, &contents, &size); + if (r < 0) + return log_netdev_full(netdev, level, r, + "Failed to read private key from '%s'%s: %m", + w->private_key_file, fatal ? "" : ", ignoring"); + + r = unbase64mem(contents, size, &key, &key_len); + if (r < 0) + return log_netdev_full(netdev, level, r, + "Failed to decode private key%s: %m", + fatal ? "" : ", ignoring"); + + if (key_len != WG_KEY_LEN) + return log_netdev_full(netdev, level, SYNTHETIC_ERRNO(EINVAL), + "Wireguard private key has invalid length (%zu bytes)%s: %m", + key_len, fatal ? "" : ", ignoring"); + + memcpy(w->private_key, key, WG_KEY_LEN); + return 0; +} + static int wireguard_peer_verify(WireguardPeer *peer) { NetDev *netdev = NETDEV(peer->wireguard); @@ -830,16 +905,23 @@ static int wireguard_peer_verify(WireguardPeer *peer) { static int wireguard_verify(NetDev *netdev, const char *filename) { WireguardPeer *peer, *peer_next; Wireguard *w; + bool empty; + int r; assert(netdev); w = WIREGUARD(netdev); assert(w); - if (eqzero(w->private_key)) + empty = eqzero(w->private_key); + if (empty && !w->private_key_file) return log_netdev_error_errno(netdev, SYNTHETIC_ERRNO(EINVAL), "%s: Missing PrivateKey= or PrivateKeyFile=, ignoring.", filename); + r = wireguard_read_private_key_file(w, empty); + if (r < 0 && empty) + return r; + LIST_FOREACH_SAFE(peers, peer, peer_next, w->peers) if (wireguard_peer_verify(peer) < 0) wireguard_peer_free(peer); diff --git a/src/network/netdev/wireguard.h b/src/network/netdev/wireguard.h index 65e9ac24b9a..6cf6eec14db 100644 --- a/src/network/netdev/wireguard.h +++ b/src/network/netdev/wireguard.h @@ -38,6 +38,7 @@ struct Wireguard { uint32_t flags; uint8_t private_key[WG_KEY_LEN]; + char *private_key_file; uint16_t port; uint32_t fwmark; @@ -60,5 +61,6 @@ CONFIG_PARSER_PROTOTYPE(config_parse_wireguard_listen_port); CONFIG_PARSER_PROTOTYPE(config_parse_wireguard_public_key); CONFIG_PARSER_PROTOTYPE(config_parse_wireguard_private_key); +CONFIG_PARSER_PROTOTYPE(config_parse_wireguard_private_key_file); CONFIG_PARSER_PROTOTYPE(config_parse_wireguard_preshared_key); CONFIG_PARSER_PROTOTYPE(config_parse_wireguard_keepalive); diff --git a/test/fuzz/fuzz-netdev-parser/directives.netdev b/test/fuzz/fuzz-netdev-parser/directives.netdev index cd7c3aadedb..419ccda7dcc 100644 --- a/test/fuzz/fuzz-netdev-parser/directives.netdev +++ b/test/fuzz/fuzz-netdev-parser/directives.netdev @@ -9,6 +9,7 @@ Mode= [WireGuard] ListenPort= PrivateKey= +PrivateKeyFile= FwMark= [MACVTAP] Mode= From 39bcff3bca345fa9665db9b2cd94009370c9394c Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 4 Mar 2019 14:42:03 +0900 Subject: [PATCH 14/14] test-network: add test for WireGuard.PrivateKeyFile= --- test/test-network/conf/25-wireguard-23-peers.netdev | 4 +++- test/test-network/conf/25-wireguard-private-key.txt | 6 ++++++ test/test-network/conf/25-wireguard.netdev | 1 + test/test-network/systemd-networkd-tests.py | 8 +++++++- 4 files changed, 17 insertions(+), 2 deletions(-) create mode 100644 test/test-network/conf/25-wireguard-private-key.txt diff --git a/test/test-network/conf/25-wireguard-23-peers.netdev b/test/test-network/conf/25-wireguard-23-peers.netdev index 7f77dc17431..61a75e592f4 100644 --- a/test/test-network/conf/25-wireguard-23-peers.netdev +++ b/test/test-network/conf/25-wireguard-23-peers.netdev @@ -7,7 +7,9 @@ Description=For issue #11404 [WireGuard] # 51820 is common port for Wireguard, 4500 is IPSec/UDP ListenPort=4500 -PrivateKey=CJQUtcS9emY2fLYqDlpSZiE/QJyHkPWr+WHtZLZ90FU= +# The key below should be overridden by PrivateKeyFile= +PrivateKey=EEGlnEPYJV//kbvvIqxKkQwOiS+UENyPncC4bF46ong= +PrivateKeyFile=/run/systemd/network/25-wireguard-private-key.txt # peer 1 [WireGuardPeer] diff --git a/test/test-network/conf/25-wireguard-private-key.txt b/test/test-network/conf/25-wireguard-private-key.txt new file mode 100644 index 00000000000..469acd0950f --- /dev/null +++ b/test/test-network/conf/25-wireguard-private-key.txt @@ -0,0 +1,6 @@ +CJQUtcS9emY2fLY + qDlpSZiE/QJyHkP + Wr+WHtZ + + +LZ90FU= diff --git a/test/test-network/conf/25-wireguard.netdev b/test/test-network/conf/25-wireguard.netdev index 4866c31ccac..61afd1f5e79 100644 --- a/test/test-network/conf/25-wireguard.netdev +++ b/test/test-network/conf/25-wireguard.netdev @@ -4,6 +4,7 @@ Kind=wireguard [WireGuard] PrivateKey=EEGlnEPYJV//kbvvIqxKkQwOiS+UENyPncC4bF46ong= +PrivateKeyFile=/run/systemd/network/not-exist ListenPort=51820 FwMark=1234 diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index ec8ed00c38b..17abd043702 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -242,6 +242,7 @@ class NetworkdNetDevTests(unittest.TestCase, Utilities): '25-vxlan.netdev', '25-wireguard-23-peers.netdev', '25-wireguard-23-peers.network', + '25-wireguard-private-key.txt', '25-wireguard.netdev', '6rd.network', 'gre.network', @@ -454,16 +455,21 @@ class NetworkdNetDevTests(unittest.TestCase, Utilities): self.assertTrue(output, 'RDf+LSpeEre7YEIKaxg+wbpsNV7du+ktR99uBEtIiCA=\t20') output = subprocess.check_output(['wg', 'show', 'wg99', 'endpoints']).rstrip().decode('utf-8') self.assertTrue(output, 'RDf+LSpeEre7YEIKaxg+wbpsNV7du+ktR99uBEtIiCA=\t192.168.27.3:51820') + output = subprocess.check_output(['wg', 'show', 'wg99', 'private-key']).rstrip().decode('utf-8') + self.assertTrue(output, 'EEGlnEPYJV//kbvvIqxKkQwOiS+UENyPncC4bF46ong=') self.assertTrue(self.link_exits('wg99')) @expectedFailureIfModuleIsNotAvailable('wireguard') def test_wireguard_23_peers(self): - self.copy_unit_to_networkd_unit_path('25-wireguard-23-peers.netdev', '25-wireguard-23-peers.network') + self.copy_unit_to_networkd_unit_path('25-wireguard-23-peers.netdev', '25-wireguard-23-peers.network', + '25-wireguard-private-key.txt') self.start_networkd() if shutil.which('wg'): subprocess.call('wg') + output = subprocess.check_output(['wg', 'show', 'wg98', 'private-key']).rstrip().decode('utf-8') + self.assertTrue(output, 'CJQUtcS9emY2fLYqDlpSZiE/QJyHkPWr+WHtZLZ90FU=') self.assertTrue(self.link_exits('wg98'))