From 8326c7f789bad623a5705b04b78d104d993a90ee Mon Sep 17 00:00:00 2001 From: Daniel Mack Date: Tue, 28 Jul 2015 18:09:08 +0200 Subject: [PATCH 1/5] resolved: remove runtime check for previously asserted condition --- src/resolve/resolved-dns-transaction.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index bbbc6377a81..af654cbc692 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -186,9 +186,6 @@ void dns_transaction_complete(DnsTransaction *t, DnsTransactionState state) { assert(t); assert(!IN_SET(state, DNS_TRANSACTION_NULL, DNS_TRANSACTION_PENDING)); - if (!IN_SET(t->state, DNS_TRANSACTION_NULL, DNS_TRANSACTION_PENDING)) - return; - /* Note that this call might invalidate the query. Callers * should hence not attempt to access the query or transaction * after calling this function. */ From 106784ebb7b303ae471851100a773ad2aebf5b80 Mon Sep 17 00:00:00 2001 From: Daniel Mack Date: Sat, 11 Jul 2015 16:21:26 -0400 Subject: [PATCH 2/5] resolved: use switch-case statements for protocol details With more protocols to come, switch repetitive if-else blocks with a switch-case statements. --- src/resolve/resolved-dns-packet.c | 43 +++++++++++++------------- src/resolve/resolved-dns-packet.h | 9 ++++-- src/resolve/resolved-dns-scope.c | 27 +++++++++------- src/resolve/resolved-dns-transaction.c | 23 +++++++++++--- 4 files changed, 64 insertions(+), 38 deletions(-) diff --git a/src/resolve/resolved-dns-packet.c b/src/resolve/resolved-dns-packet.c index bebd1ee4a6e..784d949ccea 100644 --- a/src/resolve/resolved-dns-packet.c +++ b/src/resolve/resolved-dns-packet.c @@ -64,7 +64,7 @@ int dns_packet_new(DnsPacket **ret, DnsProtocol protocol, size_t mtu) { int dns_packet_new_query(DnsPacket **ret, DnsProtocol protocol, size_t mtu) { DnsPacket *p; DnsPacketHeader *h; - int r; + int r, rd; assert(ret); @@ -74,26 +74,27 @@ int dns_packet_new_query(DnsPacket **ret, DnsProtocol protocol, size_t mtu) { h = DNS_PACKET_HEADER(p); - if (protocol == DNS_PROTOCOL_LLMNR) - h->flags = htobe16(DNS_PACKET_MAKE_FLAGS(0 /* qr */, - 0 /* opcode */, - 0 /* c */, - 0 /* tc */, - 0 /* t */, - 0 /* ra */, - 0 /* ad */, - 0 /* cd */, - 0 /* rcode */)); - else - h->flags = htobe16(DNS_PACKET_MAKE_FLAGS(0 /* qr */, - 0 /* opcode */, - 0 /* aa */, - 0 /* tc */, - 1 /* rd (ask for recursion) */, - 0 /* ra */, - 0 /* ad */, - 0 /* cd */, - 0 /* rcode */)); + switch (protocol) { + case DNS_PROTOCOL_LLMNR: + /* no recursion for link-local resolving protocols */ + rd = 0; + break; + + default: + /* ask for recursion */ + rd = 1; + break; + } + + h->flags = htobe16(DNS_PACKET_MAKE_FLAGS(0 /* qr */, + 0 /* opcode */, + 0 /* aa */, + 0 /* tc */, + rd /* rd */, + 0 /* ra */, + 0 /* ad */, + 0 /* cd */, + 0 /* rcode */)); *ret = p; return 0; diff --git a/src/resolve/resolved-dns-packet.h b/src/resolve/resolved-dns-packet.h index e81f8a82020..8a72b898da8 100644 --- a/src/resolve/resolved-dns-packet.h +++ b/src/resolve/resolved-dns-packet.h @@ -239,11 +239,16 @@ static inline uint64_t SD_RESOLVED_FLAGS_MAKE(DnsProtocol protocol, int family) /* Converts a protocol + family into a flags field as used in queries */ - if (protocol == DNS_PROTOCOL_DNS) + switch (protocol) { + case DNS_PROTOCOL_DNS: return SD_RESOLVED_DNS; - if (protocol == DNS_PROTOCOL_LLMNR) + case DNS_PROTOCOL_LLMNR: return family == AF_INET6 ? SD_RESOLVED_LLMNR_IPV6 : SD_RESOLVED_LLMNR_IPV4; + default: + break; + } + return 0; } diff --git a/src/resolve/resolved-dns-scope.c b/src/resolve/resolved-dns-scope.c index 336269ca8a6..77034a0be86 100644 --- a/src/resolve/resolved-dns-scope.c +++ b/src/resolve/resolved-dns-scope.c @@ -166,7 +166,8 @@ int dns_scope_emit(DnsScope *s, int fd, DnsPacket *p) { } else mtu = manager_find_mtu(s->manager); - if (s->protocol == DNS_PROTOCOL_DNS) { + switch (s->protocol) { + case DNS_PROTOCOL_DNS: if (DNS_PACKET_QDCOUNT(p) > 1) return -EOPNOTSUPP; @@ -180,8 +181,9 @@ int dns_scope_emit(DnsScope *s, int fd, DnsPacket *p) { if (r < 0) return r; - } else if (s->protocol == DNS_PROTOCOL_LLMNR) { + break; + case DNS_PROTOCOL_LLMNR: if (DNS_PACKET_QDCOUNT(p) > 1) return -EOPNOTSUPP; @@ -205,8 +207,12 @@ int dns_scope_emit(DnsScope *s, int fd, DnsPacket *p) { r = manager_send(s->manager, fd, ifindex, family, &addr, port, p); if (r < 0) return r; - } else + + break; + + default: return -EAFNOSUPPORT; + } return 1; } @@ -341,27 +347,25 @@ DnsScopeMatch dns_scope_good_domain(DnsScope *s, int ifindex, uint64_t flags, co if (dns_name_endswith(domain, *i) > 0) return DNS_SCOPE_YES; - if (s->protocol == DNS_PROTOCOL_DNS) { + switch (s->protocol) { + case DNS_PROTOCOL_DNS: if (dns_name_endswith(domain, "254.169.in-addr.arpa") == 0 && dns_name_endswith(domain, "0.8.e.f.ip6.arpa") == 0 && dns_name_single_label(domain) == 0) return DNS_SCOPE_MAYBE; return DNS_SCOPE_NO; - } - if (s->protocol == DNS_PROTOCOL_MDNS) { + case DNS_PROTOCOL_MDNS: if ((s->family == AF_INET && dns_name_endswith(domain, "in-addr.arpa") > 0) || (s->family == AF_INET6 && dns_name_endswith(domain, "ip6.arpa") > 0) || (dns_name_endswith(domain, "local") > 0 && /* only resolve names ending in .local via mDNS */ dns_name_equal(domain, "local") == 0 && /* but not the single-label "local" name itself */ manager_is_own_hostname(s->manager, domain) <= 0)) /* never resolve the local hostname via mDNS */ - return DNS_SCOPE_MAYBE; return DNS_SCOPE_NO; - } - if (s->protocol == DNS_PROTOCOL_LLMNR) { + case DNS_PROTOCOL_LLMNR: if ((s->family == AF_INET && dns_name_endswith(domain, "in-addr.arpa") > 0) || (s->family == AF_INET6 && dns_name_endswith(domain, "ip6.arpa") > 0) || (dns_name_single_label(domain) > 0 && /* only resolve single label names via LLMNR */ @@ -370,9 +374,10 @@ DnsScopeMatch dns_scope_good_domain(DnsScope *s, int ifindex, uint64_t flags, co return DNS_SCOPE_MAYBE; return DNS_SCOPE_NO; - } - assert_not_reached("Unknown scope protocol"); + default: + assert_not_reached("Unknown scope protocol"); + } } int dns_scope_good_key(DnsScope *s, DnsResourceKey *key) { diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index af654cbc692..faea0585d36 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -260,10 +260,12 @@ static int dns_transaction_open_tcp(DnsTransaction *t) { if (t->stream) return 0; - if (t->scope->protocol == DNS_PROTOCOL_DNS) + switch (t->scope->protocol) { + case DNS_PROTOCOL_DNS: fd = dns_scope_tcp_socket(t->scope, AF_UNSPEC, NULL, 53, &server); - else if (t->scope->protocol == DNS_PROTOCOL_LLMNR) { + break; + case DNS_PROTOCOL_LLMNR: /* When we already received a reply to this (but it was truncated), send to its sender address */ if (t->received) fd = dns_scope_tcp_socket(t->scope, t->received->family, &t->received->sender, t->received->sender_port, NULL); @@ -285,8 +287,12 @@ static int dns_transaction_open_tcp(DnsTransaction *t) { fd = dns_scope_tcp_socket(t->scope, family, &address, LLMNR_PORT, NULL); } - } else + + break; + + default: return -EAFNOSUPPORT; + } if (fd < 0) return fd; @@ -342,7 +348,8 @@ void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p) { * should hence not attempt to access the query or transaction * after calling this function. */ - if (t->scope->protocol == DNS_PROTOCOL_LLMNR) { + switch (t->scope->protocol) { + case DNS_PROTOCOL_LLMNR: assert(t->scope->link); /* For LLMNR we will not accept any packets from other @@ -361,6 +368,14 @@ void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p) { dns_transaction_tentative(t, p); return; } + + break; + + case DNS_PROTOCOL_DNS: + break; + + default: + break; } if (t->received != p) { From 9c56a6f3e2b1fc193cf49f622b57f7ee17766cac Mon Sep 17 00:00:00 2001 From: Daniel Mack Date: Tue, 4 Aug 2015 10:37:59 +0200 Subject: [PATCH 3/5] resolved: move assertion Make a scope with invalid protocol state fail as soon as possible. --- src/resolve/resolved-dns-transaction.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index faea0585d36..7b84c1bab86 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -375,7 +375,7 @@ void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p) { break; default: - break; + assert_not_reached("Invalid DNS protocol."); } if (t->received != p) { @@ -412,7 +412,7 @@ void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p) { break; default: - assert_not_reached("Invalid DNS protocol."); + break; } if (DNS_PACKET_TC(p)) { From a7e5da6e33de9ad9b5bc594fdc74e3e4098a5751 Mon Sep 17 00:00:00 2001 From: Daniel Mack Date: Tue, 28 Jul 2015 15:00:59 +0200 Subject: [PATCH 4/5] sd-network: make LLMNR specific config parser generic Rename the enum, the lookup functions and the parser for LLMNRSupport so the type can be reused for mDNS. --- src/network/networkd-link.c | 2 +- src/network/networkd-network-gperf.gperf | 2 +- src/network/networkd-network.c | 30 ++++++++++++------------ src/network/networkd.h | 28 +++++++++++----------- 4 files changed, 31 insertions(+), 31 deletions(-) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 91b9cdf30d4..cc9dc393c6a 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -2378,7 +2378,7 @@ int link_save(Link *link) { yes_no(link->network->wildcard_domain)); fprintf(f, "LLMNR=%s\n", - llmnr_support_to_string(link->network->llmnr)); + resolve_support_to_string(link->network->llmnr)); } if (!hashmap_isempty(link->bound_to_links)) { diff --git a/src/network/networkd-network-gperf.gperf b/src/network/networkd-network-gperf.gperf index 8735b395812..7ac7ef1ea3a 100644 --- a/src/network/networkd-network-gperf.gperf +++ b/src/network/networkd-network-gperf.gperf @@ -45,7 +45,7 @@ Network.Address, config_parse_address, 0 Network.Gateway, config_parse_gateway, 0, 0 Network.Domains, config_parse_domains, 0, offsetof(Network, domains) Network.DNS, config_parse_strv, 0, offsetof(Network, dns) -Network.LLMNR, config_parse_llmnr, 0, offsetof(Network, llmnr) +Network.LLMNR, config_parse_resolve, 0, offsetof(Network, llmnr) Network.NTP, config_parse_strv, 0, offsetof(Network, ntp) Network.IPForward, config_parse_address_family_boolean_with_kernel,0, offsetof(Network, ip_forward) Network.IPMasquerade, config_parse_bool, 0, offsetof(Network, ip_masquerade) diff --git a/src/network/networkd-network.c b/src/network/networkd-network.c index 3f7e77da3ee..6587ea994cf 100644 --- a/src/network/networkd-network.c +++ b/src/network/networkd-network.c @@ -111,7 +111,7 @@ static int network_load_one(Manager *manager, const char *filename) { network->allow_port_to_be_root = true; network->unicast_flood = true; - network->llmnr = LLMNR_SUPPORT_YES; + network->llmnr = RESOLVE_SUPPORT_YES; network->link_local = ADDRESS_FAMILY_IPV6; @@ -632,15 +632,15 @@ static const char* const dhcp_client_identifier_table[_DHCP_CLIENT_ID_MAX] = { DEFINE_PRIVATE_STRING_TABLE_LOOKUP_FROM_STRING(dhcp_client_identifier, DCHPClientIdentifier); DEFINE_CONFIG_PARSE_ENUM(config_parse_dhcp_client_identifier, dhcp_client_identifier, DCHPClientIdentifier, "Failed to parse client identifier type"); -static const char* const llmnr_support_table[_LLMNR_SUPPORT_MAX] = { - [LLMNR_SUPPORT_NO] = "no", - [LLMNR_SUPPORT_YES] = "yes", - [LLMNR_SUPPORT_RESOLVE] = "resolve", +static const char* const resolve_support_table[_RESOLVE_SUPPORT_MAX] = { + [RESOLVE_SUPPORT_NO] = "no", + [RESOLVE_SUPPORT_YES] = "yes", + [RESOLVE_SUPPORT_RESOLVE] = "resolve", }; -DEFINE_STRING_TABLE_LOOKUP(llmnr_support, LLMNRSupport); +DEFINE_STRING_TABLE_LOOKUP(resolve_support, ResolveSupport); -int config_parse_llmnr( +int config_parse_resolve( const char* unit, const char *filename, unsigned line, @@ -652,32 +652,32 @@ int config_parse_llmnr( void *data, void *userdata) { - LLMNRSupport *llmnr = data; + ResolveSupport *resolve = data; int k; assert(filename); assert(lvalue); assert(rvalue); - assert(llmnr); + assert(resolve); /* Our enum shall be a superset of booleans, hence first try * to parse as boolean, and then as enum */ k = parse_boolean(rvalue); if (k > 0) - *llmnr = LLMNR_SUPPORT_YES; + *resolve = RESOLVE_SUPPORT_YES; else if (k == 0) - *llmnr = LLMNR_SUPPORT_NO; + *resolve = RESOLVE_SUPPORT_NO; else { - LLMNRSupport s; + ResolveSupport s; - s = llmnr_support_from_string(rvalue); + s = resolve_support_from_string(rvalue); if (s < 0){ - log_syntax(unit, LOG_ERR, filename, line, -s, "Failed to parse LLMNR option, ignoring: %s", rvalue); + log_syntax(unit, LOG_ERR, filename, line, -s, "Failed to parse %s option, ignoring: %s", lvalue, rvalue); return 0; } - *llmnr = s; + *resolve = s; } return 0; diff --git a/src/network/networkd.h b/src/network/networkd.h index a285a4b08ff..5340922bf14 100644 --- a/src/network/networkd.h +++ b/src/network/networkd.h @@ -64,13 +64,13 @@ typedef enum AddressFamilyBoolean { _ADDRESS_FAMILY_BOOLEAN_INVALID = -1, } AddressFamilyBoolean; -typedef enum LLMNRSupport { - LLMNR_SUPPORT_NO, - LLMNR_SUPPORT_YES, - LLMNR_SUPPORT_RESOLVE, - _LLMNR_SUPPORT_MAX, - _LLMNR_SUPPORT_INVALID = -1, -} LLMNRSupport; +typedef enum ResolveSupport { + RESOLVE_SUPPORT_NO, + RESOLVE_SUPPORT_YES, + RESOLVE_SUPPORT_RESOLVE, + _RESOLVE_SUPPORT_MAX, + _RESOLVE_SUPPORT_INVALID = -1, +} ResolveSupport; typedef enum LinkOperationalState { LINK_OPERSTATE_OFF, @@ -178,7 +178,7 @@ struct Network { bool wildcard_domain; char **domains, **dns, **ntp, **bind_carrier; - LLMNRSupport llmnr; + ResolveSupport llmnr; LIST_FIELDS(Network, networks); }; @@ -421,14 +421,14 @@ int config_parse_ipv6token(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); -/* LLMNR support */ +/* Resolve protocols support */ -const char* llmnr_support_to_string(LLMNRSupport i) _const_; -LLMNRSupport llmnr_support_from_string(const char *s) _pure_; +const char* resolve_support_to_string(ResolveSupport i) _const_; +ResolveSupport resolve_support_from_string(const char *s) _pure_; -int config_parse_llmnr(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); +int config_parse_resolve(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); /* Address Pool */ From eff91ee0070f85432e4926403b58ed10cbea1af5 Mon Sep 17 00:00:00 2001 From: Daniel Mack Date: Tue, 4 Aug 2015 13:53:02 +0200 Subject: [PATCH 5/5] resolved: allow dns_cache_put() without a question Currently, dns_cache_put() does a number of things: 1) It unconditionally removes all keys contained in the passed question before adding keys from the newly arrived answers. 2) It puts positive entries into the cache for all RRs contained in the answer. 3) It creates negative entries in the cache for all keys in the question that are not answered. Allow passing q = NULL in the parameters and skip 1) and 3), so we can use that function for mDNS responses. In this case, the question is irrelevant, we are interested in all answers we got. --- src/resolve/resolved-dns-cache.c | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/src/resolve/resolved-dns-cache.c b/src/resolve/resolved-dns-cache.c index efc407dbc64..7ee098397cb 100644 --- a/src/resolve/resolved-dns-cache.c +++ b/src/resolve/resolved-dns-cache.c @@ -411,16 +411,17 @@ int dns_cache_put( int owner_family, const union in_addr_union *owner_address) { - unsigned i; + unsigned cache_keys, i; int r; assert(c); - assert(q); - /* First, delete all matching old RRs, so that we only keep - * complete by_key in place. */ - for (i = 0; i < q->n_keys; i++) - dns_cache_remove(c, q->keys[i]); + if (q) { + /* First, if we were passed a question, delete all matching old RRs, + * so that we only keep complete by_key in place. */ + for (i = 0; i < q->n_keys; i++) + dns_cache_remove(c, q->keys[i]); + } if (!answer) return 0; @@ -435,8 +436,13 @@ int dns_cache_put( if (!IN_SET(rcode, DNS_RCODE_SUCCESS, DNS_RCODE_NXDOMAIN)) return 0; + cache_keys = answer->n_rrs; + + if (q) + cache_keys += q->n_keys; + /* Make some space for our new entries */ - dns_cache_make_space(c, answer->n_rrs + q->n_keys); + dns_cache_make_space(c, cache_keys); if (timestamp <= 0) timestamp = now(clock_boottime_or_monotonic()); @@ -448,6 +454,9 @@ int dns_cache_put( goto fail; } + if (!q) + return 0; + /* Third, add in negative entries for all keys with no RR */ for (i = 0; i < q->n_keys; i++) { DnsResourceRecord *soa = NULL; @@ -479,8 +488,11 @@ fail: /* Adding all RRs failed. Let's clean up what we already * added, just in case */ - for (i = 0; i < q->n_keys; i++) - dns_cache_remove(c, q->keys[i]); + if (q) { + for (i = 0; i < q->n_keys; i++) + dns_cache_remove(c, q->keys[i]); + } + for (i = 0; i < answer->n_rrs; i++) dns_cache_remove(c, answer->items[i].rr->key);