From: W.C.A. Wijngaards Date: Thu, 7 Sep 2023 09:08:04 +0000 (+0200) Subject: - Fix to scrub resource records of type A and AAAA that have an X-Git-Tag: release-1.19.0rc1~55 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=dfc00271d17752481506b9a4b473c4eb177ac980;p=thirdparty%2Funbound.git - Fix to scrub resource records of type A and AAAA that have an inappropriate size. They are removed from responses. --- diff --git a/doc/Changelog b/doc/Changelog index bc6c14321..fe03f4378 100644 --- a/doc/Changelog +++ b/doc/Changelog @@ -1,3 +1,7 @@ +7 September 2023: Wouter + - Fix to scrub resource records of type A and AAAA that have an + inappropriate size. They are removed from responses. + 6 September 2023: Wouter - Merge #931: Prevent warnings from -Wmissing-prototypes. diff --git a/iterator/iter_priv.c b/iterator/iter_priv.c index 90bea1746..675c93907 100644 --- a/iterator/iter_priv.c +++ b/iterator/iter_priv.c @@ -208,14 +208,17 @@ size_t priv_get_mem(struct iter_priv* priv) } /** remove RR from msgparse RRset, return true if rrset is entirely bad */ -static int -remove_rr(const char* str, sldns_buffer* pkt, struct rrset_parse* rrset, +int +msgparse_rrset_remove_rr(const char* str, sldns_buffer* pkt, struct rrset_parse* rrset, struct rr_parse* prev, struct rr_parse** rr, struct sockaddr_storage* addr, socklen_t addrlen) { if(verbosity >= VERB_QUERY && rrset->dname_len <= LDNS_MAX_DOMAINLEN && str) { uint8_t buf[LDNS_MAX_DOMAINLEN+1]; dname_pkt_copy(pkt, buf, rrset->dname); - log_name_addr(VERB_QUERY, str, buf, addr, addrlen); + if(addr) + log_name_addr(VERB_QUERY, str, buf, addr, addrlen); + else log_nametypeclass(VERB_QUERY, str, buf, + rrset->type, ntohs(rrset->rrset_class)); } if(prev) prev->next = (*rr)->next; @@ -261,7 +264,7 @@ int priv_rrset_bad(struct iter_priv* priv, sldns_buffer* pkt, INET_SIZE); memmove(&addr, &sa, len); if(priv_lookup_addr(priv, &addr, len)) { - if(remove_rr("sanitize: removing public name with private address", pkt, rrset, prev, &rr, &addr, len)) + if(msgparse_rrset_remove_rr("sanitize: removing public name with private address", pkt, rrset, prev, &rr, &addr, len)) return 1; continue; } @@ -284,7 +287,7 @@ int priv_rrset_bad(struct iter_priv* priv, sldns_buffer* pkt, INET6_SIZE); memmove(&addr, &sa, len); if(priv_lookup_addr(priv, &addr, len)) { - if(remove_rr("sanitize: removing public name with private address", pkt, rrset, prev, &rr, &addr, len)) + if(msgparse_rrset_remove_rr("sanitize: removing public name with private address", pkt, rrset, prev, &rr, &addr, len)) return 1; continue; } diff --git a/iterator/iter_priv.h b/iterator/iter_priv.h index 0430d57e3..7f58ec2f7 100644 --- a/iterator/iter_priv.h +++ b/iterator/iter_priv.h @@ -48,6 +48,8 @@ struct iter_env; struct config_file; struct regional; struct rrset_parse; +struct rr_parse; +struct rrset_parse; /** * Iterator priv structure @@ -109,4 +111,9 @@ int priv_rrset_bad(struct iter_priv* priv, struct sldns_buffer* pkt, */ size_t priv_get_mem(struct iter_priv* priv); +/** remove RR from msgparse RRset, return true if rrset is entirely bad */ +int msgparse_rrset_remove_rr(const char* str, struct sldns_buffer* pkt, + struct rrset_parse* rrset, struct rr_parse* prev, struct rr_parse** rr, + struct sockaddr_storage* addr, socklen_t addrlen); + #endif /* ITERATOR_ITER_PRIV_H */ diff --git a/iterator/iter_scrub.c b/iterator/iter_scrub.c index d1fedcd0f..c34ccfd3c 100644 --- a/iterator/iter_scrub.c +++ b/iterator/iter_scrub.c @@ -716,6 +716,45 @@ static int sanitize_nsec_is_overreach(sldns_buffer* pkt, return 0; } +/** Remove individual RRs, if the length is wrong. Returns true if the RRset + * has been removed. */ +static int +scrub_sanitize_rr_length(sldns_buffer* pkt, struct msg_parse* msg, + struct rrset_parse* prev, struct rrset_parse** rrset) +{ + struct rr_parse* rr, *rr_prev = NULL; + for(rr = (*rrset)->rr_first; rr; rr = rr->next) { + + /* Sanity check for length of records + * An A record should be 6 bytes only + * (2 bytes for length and 4 for IPv4 addr)*/ + if((*rrset)->type == LDNS_RR_TYPE_A && rr->size != 6 ) { + if(msgparse_rrset_remove_rr("sanitize: removing type A RR of inappropriate length:", + pkt, *rrset, rr_prev, &rr, NULL, 0)) { + remove_rrset("sanitize: removing type A RRset of inappropriate length:", + pkt, msg, prev, rrset); + return 1; + } + continue; + } + + /* Sanity check for length of records + * An AAAA record should be 18 bytes only + * (2 bytes for length and 16 for IPv6 addr)*/ + if((*rrset)->type == LDNS_RR_TYPE_AAAA && rr->size != 18 ) { + if(msgparse_rrset_remove_rr("sanitize: removing type AAAA RR of inappropriate length:", + pkt, *rrset, rr_prev, &rr, NULL, 0)) { + remove_rrset("sanitize: removing type AAAA RRset of inappropriate length:", + pkt, msg, prev, rrset); + return 1; + } + continue; + } + rr_prev = rr; + } + return 0; +} + /** * Given a response event, remove suspect RRsets from the response. * "Suspect" rrsets are potentially poison. Note that this routine expects @@ -781,6 +820,13 @@ scrub_sanitize(sldns_buffer* pkt, struct msg_parse* msg, rrset = msg->rrset_first; while(rrset) { + /* Sanity check for length of records */ + if(rrset->type == LDNS_RR_TYPE_A || + rrset->type == LDNS_RR_TYPE_AAAA) { + if(scrub_sanitize_rr_length(pkt, msg, prev, &rrset)) + continue; + } + /* remove private addresses */ if( (rrset->type == LDNS_RR_TYPE_A || rrset->type == LDNS_RR_TYPE_AAAA)) { diff --git a/testdata/dns64_lookup.rpl b/testdata/dns64_lookup.rpl index 898d0d01a..7986fc8fc 100644 --- a/testdata/dns64_lookup.rpl +++ b/testdata/dns64_lookup.rpl @@ -140,33 +140,6 @@ SECTION ADDITIONAL ns.example.com. IN A 1.2.3.4 ENTRY_END -ENTRY_BEGIN -MATCH opcode qtype qname -ADJUST copy_id -REPLY QR NOERROR -SECTION QUESTION -broken.example.com. IN AAAA -SECTION ANSWER -; NO AAAA present -SECTION AUTHORITY -example.com. IN SOA a. b. 1 2 3 4 5 -ENTRY_END - -ENTRY_BEGIN -MATCH opcode qtype qname -ADJUST copy_id -REPLY QR NOERROR -SECTION QUESTION -broken.example.com. IN A -SECTION ANSWER -broken.example.com. IN A 5.6.7.8 -broken.example.com. IN A \# 3 030405 -SECTION AUTHORITY -example.com. IN NS ns.example.com. -SECTION ADDITIONAL -ns.example.com. IN A 1.2.3.4 -ENTRY_END - ENTRY_BEGIN MATCH opcode qtype qname ADJUST copy_id @@ -284,25 +257,4 @@ SECTION AUTHORITY 7.6.5.in-addr.arpa. IN NS ns.example.com. ENTRY_END -; synthesize from broken, malformed A records -STEP 80 QUERY -ENTRY_BEGIN -REPLY RD -SECTION QUESTION -broken.example.com. IN AAAA -ENTRY_END - -; recursion happens here. -STEP 90 CHECK_ANSWER -ENTRY_BEGIN -MATCH all -REPLY QR RD RA NOERROR -SECTION QUESTION -broken.example.com. IN AAAA -SECTION ANSWER -SECTION AUTHORITY -example.com. IN SOA a. b. 1 2 3 4 5 -SECTION ADDITIONAL -ENTRY_END - SCENARIO_END diff --git a/testdata/iter_scrub_rr_length.rpl b/testdata/iter_scrub_rr_length.rpl new file mode 100644 index 000000000..ccb791eb2 --- /dev/null +++ b/testdata/iter_scrub_rr_length.rpl @@ -0,0 +1,273 @@ +; config options +server: + target-fetch-policy: "0 0 0 0 0" + qname-minimisation: "no" + minimal-responses: no + +stub-zone: + name: "." + stub-addr: 193.0.14.129 # K.ROOT-SERVERS.NET. +CONFIG_END + +SCENARIO_BEGIN Test scrub of RRs of inappropriate length + +; K.ROOT-SERVERS.NET. +RANGE_BEGIN 0 100 + ADDRESS 193.0.14.129 +ENTRY_BEGIN +MATCH opcode qtype qname +ADJUST copy_id +REPLY QR NOERROR +SECTION QUESTION +. IN NS +SECTION ANSWER +. IN NS K.ROOT-SERVERS.NET. +SECTION ADDITIONAL +K.ROOT-SERVERS.NET. IN A 193.0.14.129 +ENTRY_END + +ENTRY_BEGIN +MATCH opcode qtype qname +ADJUST copy_id +REPLY QR NOERROR +SECTION QUESTION +www.example.com. IN A +SECTION AUTHORITY +com. IN NS a.gtld-servers.net. +SECTION ADDITIONAL +a.gtld-servers.net. IN A 192.5.6.30 +ENTRY_END +RANGE_END + +; a.gtld-servers.net. +RANGE_BEGIN 0 100 + ADDRESS 192.5.6.30 +ENTRY_BEGIN +MATCH opcode qtype qname +ADJUST copy_id +REPLY QR NOERROR +SECTION QUESTION +com. IN NS +SECTION ANSWER +com. IN NS a.gtld-servers.net. +SECTION ADDITIONAL +a.gtld-servers.net. IN A 192.5.6.30 +ENTRY_END + +ENTRY_BEGIN +MATCH opcode qtype qname +ADJUST copy_id +REPLY QR NOERROR +SECTION QUESTION +www.example.com. IN A +SECTION AUTHORITY +example.com. IN NS ns.example.com. +SECTION ADDITIONAL +ns.example.com. IN A 1.2.3.4 +ENTRY_END +RANGE_END + +; ns.example.com. +RANGE_BEGIN 0 100 + ADDRESS 1.2.3.4 +ENTRY_BEGIN +MATCH opcode qtype qname +ADJUST copy_id +REPLY QR NOERROR +SECTION QUESTION +example.com. IN NS +SECTION ANSWER +example.com. IN NS ns.example.com. +SECTION ADDITIONAL +ns.example.com. IN A 1.2.3.4 +ENTRY_END + +ENTRY_BEGIN +MATCH opcode qtype qname +ADJUST copy_id +REPLY QR AA NOERROR +SECTION QUESTION +www.example.com. IN A +SECTION ANSWER +www.example.com. IN A 10.20.30.40 +www.example.com. IN A \# 3 030405 +SECTION AUTHORITY +example.com. IN NS ns.example.com. +SECTION ADDITIONAL +ns.example.com. IN A 1.2.3.4 +ENTRY_END + +ENTRY_BEGIN +MATCH opcode qtype qname +ADJUST copy_id +REPLY QR AA NOERROR +SECTION QUESTION +www.example.com. IN AAAA +SECTION ANSWER +www.example.com. IN AAAA 2001:db8::1234 +www.example.com. IN AAAA \# 48 000102030405060708090A0B0C0D0E0F101112131415161718191A1B1C1D1E1F202122232425262728292A2B2C2D2E2F +SECTION AUTHORITY +example.com. IN NS ns.example.com. +SECTION ADDITIONAL +ns.example.com. IN A 1.2.3.4 +ENTRY_END + +ENTRY_BEGIN +MATCH opcode qtype qname +ADJUST copy_id +REPLY QR AA NOERROR +SECTION QUESTION +broken1.example.com. IN A +SECTION ANSWER +broken1.example.com. IN A \# 3 030405 +broken1.example.com. IN A \# 3 030406 +SECTION AUTHORITY +example.com. IN NS ns.example.com. +SECTION ADDITIONAL +ns.example.com. IN A 1.2.3.4 +ENTRY_END + +ENTRY_BEGIN +MATCH opcode qtype qname +ADJUST copy_id +REPLY QR AA NOERROR +SECTION QUESTION +broken1.example.com. IN AAAA +SECTION ANSWER +broken1.example.com. IN AAAA \# 48 000102030405060708090A0B0C0D0E0F101112131415161718191A1B1C1D1E1F202122232425262728292A2B2C2D2E2F +broken1.example.com. IN AAAA \# 48 000102030405060708090A0B0C0D0E0F101112131415161718191A1B1C1D1E1F202122232425262728292A2B2C2D2E30 +broken1.example.com. IN AAAA \# 48 000102030405060708090A0B0C0D0E0F101112131415161718191A1B1C1D1E1F202122232425262728292A2B2C2D2E31 +SECTION AUTHORITY +example.com. IN NS ns.example.com. +SECTION ADDITIONAL +ns.example.com. IN A 1.2.3.4 +ENTRY_END + +ENTRY_BEGIN +MATCH opcode qtype qname +ADJUST copy_id +REPLY QR AA NOERROR +SECTION QUESTION +broken2.example.com. IN A +SECTION ANSWER +broken2.example.com. IN A 1.2.3.4 +broken2.example.com. IN A \# 3 030405 +broken2.example.com. IN A 1.2.3.5 +broken2.example.com. IN A \# 3 030406 +SECTION AUTHORITY +example.com. IN NS ns.example.com. +SECTION ADDITIONAL +ns.example.com. IN A \# 3 030407 +ns.example.com. IN A 1.2.3.6 +ns.example.com. IN A \# 3 030408 +ns.example.com. IN A \# 3 030409 +ns.example.com. IN A 1.2.3.7 +ENTRY_END +RANGE_END + +STEP 1 QUERY +ENTRY_BEGIN +REPLY RD +SECTION QUESTION +www.example.com. IN A +ENTRY_END + +STEP 10 CHECK_ANSWER +ENTRY_BEGIN +MATCH all +REPLY QR RD RA NOERROR +SECTION QUESTION +www.example.com. IN A +SECTION ANSWER +www.example.com. IN A 10.20.30.40 +SECTION AUTHORITY +example.com. IN NS ns.example.com. +SECTION ADDITIONAL +ns.example.com. IN A 1.2.3.4 +ENTRY_END + +STEP 20 QUERY +ENTRY_BEGIN +REPLY RD +SECTION QUESTION +www.example.com. IN AAAA +ENTRY_END + +STEP 30 CHECK_ANSWER +ENTRY_BEGIN +MATCH all +REPLY QR RD RA NOERROR +SECTION QUESTION +www.example.com. IN AAAA +SECTION ANSWER +www.example.com. IN AAAA 2001:db8::1234 +SECTION AUTHORITY +example.com. IN NS ns.example.com. +SECTION ADDITIONAL +ns.example.com. IN A 1.2.3.4 +ENTRY_END + +STEP 40 QUERY +ENTRY_BEGIN +REPLY RD +SECTION QUESTION +broken1.example.com. IN A +ENTRY_END + +STEP 50 CHECK_ANSWER +ENTRY_BEGIN +MATCH all +REPLY QR RD RA NOERROR +SECTION QUESTION +broken1.example.com. IN A +SECTION ANSWER +SECTION AUTHORITY +example.com. IN NS ns.example.com. +SECTION ADDITIONAL +ns.example.com. IN A 1.2.3.4 +ENTRY_END + +STEP 60 QUERY +ENTRY_BEGIN +REPLY RD +SECTION QUESTION +broken1.example.com. IN AAAA +ENTRY_END + +STEP 70 CHECK_ANSWER +ENTRY_BEGIN +MATCH all +REPLY QR RD RA NOERROR +SECTION QUESTION +broken1.example.com. IN AAAA +SECTION ANSWER +SECTION AUTHORITY +example.com. IN NS ns.example.com. +SECTION ADDITIONAL +ns.example.com. IN A 1.2.3.4 +ENTRY_END + +STEP 80 QUERY +ENTRY_BEGIN +REPLY RD +SECTION QUESTION +broken2.example.com. IN A +ENTRY_END + +STEP 90 CHECK_ANSWER +ENTRY_BEGIN +MATCH all +REPLY QR RD RA NOERROR +SECTION QUESTION +broken2.example.com. IN A +SECTION ANSWER +broken2.example.com. IN A 1.2.3.4 +broken2.example.com. IN A 1.2.3.5 +SECTION AUTHORITY +example.com. IN NS ns.example.com. +SECTION ADDITIONAL +ns.example.com. IN A 1.2.3.6 +ns.example.com. IN A 1.2.3.7 +ENTRY_END + +SCENARIO_END