From fdd5f8ff835edb7af044bdab298f2f1fcd6431eb Mon Sep 17 00:00:00 2001 From: "W.C.A. Wijngaards" Date: Thu, 7 Sep 2023 14:44:48 +0200 Subject: [PATCH] - Fix to add EDE text when RRs have been removed due to length. --- doc/Changelog | 1 + iterator/iter_scrub.c | 25 ++++- iterator/iter_scrub.h | 5 +- iterator/iterator.c | 19 +++- testdata/iter_scrub_rr_length.rpl | 30 +++++- testdata/val_scrub_rr_length.rpl | 164 ++++++++++++++++++++++++++++++ util/module.c | 18 ++++ util/module.h | 8 ++ 8 files changed, 260 insertions(+), 10 deletions(-) create mode 100644 testdata/val_scrub_rr_length.rpl diff --git a/doc/Changelog b/doc/Changelog index a9f74ab75..e35f9c389 100644 --- a/doc/Changelog +++ b/doc/Changelog @@ -2,6 +2,7 @@ - Fix to scrub resource records of type A and AAAA that have an inappropriate size. They are removed from responses. - Fix to move msgparse_rrset_remove_rr code to util/msgparse.c. + - Fix to add EDE text when RRs have been removed due to length. 6 September 2023: Wouter - Merge #931: Prevent warnings from -Wmissing-prototypes. diff --git a/iterator/iter_scrub.c b/iterator/iter_scrub.c index a7d60e4b2..5f2e30337 100644 --- a/iterator/iter_scrub.c +++ b/iterator/iter_scrub.c @@ -720,7 +720,8 @@ static int sanitize_nsec_is_overreach(sldns_buffer* pkt, * 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 rrset_parse* prev, struct rrset_parse** rrset, int* added_ede, + struct module_qstate* qstate) { struct rr_parse* rr, *rr_prev = NULL; for(rr = (*rrset)->rr_first; rr; rr = rr->next) { @@ -729,6 +730,11 @@ scrub_sanitize_rr_length(sldns_buffer* pkt, struct msg_parse* msg, * 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(!*added_ede) { + *added_ede = 1; + errinf_ede(qstate, "sanitize: records of inappropriate length have been removed.", + LDNS_EDE_OTHER); + } 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:", @@ -742,6 +748,11 @@ scrub_sanitize_rr_length(sldns_buffer* pkt, struct msg_parse* msg, * 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(!*added_ede) { + *added_ede = 1; + errinf_ede(qstate, "sanitize: records of inappropriate length have been removed.", + LDNS_EDE_OTHER); + } 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:", @@ -767,15 +778,17 @@ scrub_sanitize_rr_length(sldns_buffer* pkt, struct msg_parse* msg, * @param zonename: name of server zone. * @param env: module environment with config and cache. * @param ie: iterator environment with private address data. + * @param qstate: for setting errinf for EDE error messages. * @return 0 on error. */ static int scrub_sanitize(sldns_buffer* pkt, struct msg_parse* msg, struct query_info* qinfo, uint8_t* zonename, struct module_env* env, - struct iter_env* ie) + struct iter_env* ie, struct module_qstate* qstate) { int del_addi = 0; /* if additional-holding rrsets are deleted, we do not trust the normalized additional-A-AAAA any more */ + int added_rrlen_ede = 0; struct rrset_parse* rrset, *prev; prev = NULL; rrset = msg->rrset_first; @@ -823,7 +836,8 @@ scrub_sanitize(sldns_buffer* pkt, struct msg_parse* msg, /* 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)) + if(scrub_sanitize_rr_length(pkt, msg, prev, &rrset, + &added_rrlen_ede, qstate)) continue; } @@ -900,7 +914,8 @@ scrub_sanitize(sldns_buffer* pkt, struct msg_parse* msg, int scrub_message(sldns_buffer* pkt, struct msg_parse* msg, struct query_info* qinfo, uint8_t* zonename, struct regional* region, - struct module_env* env, struct iter_env* ie) + struct module_env* env, struct module_qstate* qstate, + struct iter_env* ie) { /* basic sanity checks */ log_nametypeclass(VERB_ALGO, "scrub for", zonename, LDNS_RR_TYPE_NS, @@ -932,7 +947,7 @@ scrub_message(sldns_buffer* pkt, struct msg_parse* msg, if(!scrub_normalize(pkt, msg, qinfo, region, env)) return 0; /* delete all out-of-zone information */ - if(!scrub_sanitize(pkt, msg, qinfo, zonename, env, ie)) + if(!scrub_sanitize(pkt, msg, qinfo, zonename, env, ie, qstate)) return 0; return 1; } diff --git a/iterator/iter_scrub.h b/iterator/iter_scrub.h index cbbaf73c9..4d6ce7166 100644 --- a/iterator/iter_scrub.h +++ b/iterator/iter_scrub.h @@ -48,6 +48,7 @@ struct query_info; struct regional; struct module_env; struct iter_env; +struct module_qstate; /** * Cleanup the passed dns message. @@ -59,11 +60,13 @@ struct iter_env; * Used to determine out of bailiwick information. * @param regional: where to allocate (new) parts of the message. * @param env: module environment with config settings and cache. + * @param qstate: for setting errinf for EDE error messages. * @param ie: iterator module environment data. * @return: false if the message is total waste. true if scrubbed with success. */ int scrub_message(struct sldns_buffer* pkt, struct msg_parse* msg, struct query_info* qinfo, uint8_t* zonename, struct regional* regional, - struct module_env* env, struct iter_env* ie); + struct module_env* env, struct module_qstate* qstate, + struct iter_env* ie); #endif /* ITERATOR_ITER_SCRUB_H */ diff --git a/iterator/iterator.c b/iterator/iterator.c index 1548dfcae..9f78aa17d 100644 --- a/iterator/iterator.c +++ b/iterator/iterator.c @@ -3874,6 +3874,23 @@ processFinished(struct module_qstate* qstate, struct iter_qstate* iq, /* explicitly set the EDE string to NULL */ iq->response->rep->reason_bogus_str = NULL; + if((qstate->env->cfg->val_log_level >= 2 || + qstate->env->cfg->log_servfail) && qstate->errinf && + !qstate->env->cfg->val_log_squelch) { + char* err_str = errinf_to_str_misc(qstate); + if(err_str) { + size_t err_str_len = strlen(err_str); + verbose(VERB_ALGO, "iterator EDE: %s", err_str); + /* allocate space and store the error + * string */ + iq->response->rep->reason_bogus_str = regional_alloc( + qstate->region, + sizeof(char) * (err_str_len+1)); + memcpy(iq->response->rep->reason_bogus_str, + err_str, err_str_len+1); + } + free(err_str); + } /* we have finished processing this query */ qstate->ext_state[id] = module_finished; @@ -4098,7 +4115,7 @@ process_response(struct module_qstate* qstate, struct iter_qstate* iq, /* normalize and sanitize: easy to delete items from linked lists */ if(!scrub_message(pkt, prs, &iq->qinfo_out, iq->dp->name, - qstate->env->scratch, qstate->env, ie)) { + qstate->env->scratch, qstate->env, qstate, ie)) { /* if 0x20 enabled, start fallback, but we have no message */ if(event == module_event_capsfail && !iq->caps_fallback) { iq->caps_fallback = 1; diff --git a/testdata/iter_scrub_rr_length.rpl b/testdata/iter_scrub_rr_length.rpl index 14bd55265..70c6fc54b 100644 --- a/testdata/iter_scrub_rr_length.rpl +++ b/testdata/iter_scrub_rr_length.rpl @@ -4,6 +4,8 @@ server: qname-minimisation: "no" minimal-responses: no rrset-roundrobin: no + ede: yes + log-servfail: yes stub-zone: name: "." @@ -13,7 +15,7 @@ CONFIG_END SCENARIO_BEGIN Test scrub of RRs of inappropriate length ; K.ROOT-SERVERS.NET. -RANGE_BEGIN 0 100 +RANGE_BEGIN 0 200 ADDRESS 193.0.14.129 ENTRY_BEGIN MATCH opcode qtype qname @@ -41,7 +43,7 @@ ENTRY_END RANGE_END ; a.gtld-servers.net. -RANGE_BEGIN 0 100 +RANGE_BEGIN 0 200 ADDRESS 192.5.6.30 ENTRY_BEGIN MATCH opcode qtype qname @@ -69,7 +71,7 @@ ENTRY_END RANGE_END ; ns.example.com. -RANGE_BEGIN 0 100 +RANGE_BEGIN 0 200 ADDRESS 1.2.3.4 ENTRY_BEGIN MATCH opcode qtype qname @@ -271,4 +273,26 @@ ns.example.com. IN A 1.2.3.6 ns.example.com. IN A 1.2.3.7 ENTRY_END +STEP 100 QUERY +ENTRY_BEGIN +REPLY RD CD DO +SECTION QUESTION +www.example.com. IN A +ENTRY_END + +STEP 110 CHECK_ANSWER +ENTRY_BEGIN +MATCH all ede=any +REPLY QR RD CD RA DO 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.6 +ns.example.com. IN A 1.2.3.7 +ENTRY_END + SCENARIO_END diff --git a/testdata/val_scrub_rr_length.rpl b/testdata/val_scrub_rr_length.rpl new file mode 100644 index 000000000..f57cf773a --- /dev/null +++ b/testdata/val_scrub_rr_length.rpl @@ -0,0 +1,164 @@ +; config options +; The island of trust is at example.com +server: + trust-anchor: "example.com. IN DS 55566 8 2 9c148338951ce1c3b5cd3da532f3d90dfcf92595148022f2c2fd98e5deee90af" + val-override-date: "20070916134226" + target-fetch-policy: "0 0 0 0 0" + qname-minimisation: "no" + trust-anchor-signaling: no + minimal-responses: no + rrset-roundrobin: no + ede: yes + log-servfail: yes + +stub-zone: + name: "." + stub-addr: 193.0.14.129 # K.ROOT-SERVERS.NET. +CONFIG_END + +SCENARIO_BEGIN Test validator with scrub of RR for 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. +example.com. 3600 IN RRSIG NS 8 2 3600 20070926134150 20070829134150 55566 example.com. cHdLVCzujUQs6b67c1SmCX+/br4tgOg86Gj/R/x+PKUQmWHyeVwBSTlJuLOHbca3CQoyIQc+V2ilK6fjwjbY/dLk4uOlux8L+Zn7HsUXSOwJPIjsM3LuTa8CYDMvYhOP7KGR+vNpJVSsQ25pyDn6Rzsdl3E7DAf7uSkPV8VJwa8= +SECTION ADDITIONAL +ns.example.com. IN A 1.2.3.4 +ns.example.com. 3600 IN RRSIG A 8 3 3600 20070926134150 20070829134150 55566 example.com. PBwNifMNxTXlDorHX1neq1wUhWLmqk+PZ+PBZCI5BJAmakdgOXdLQiVqlKaErJyA/4uN+99fUf6/DqxwgxL8FIPdBkxMOTJaKrCFjEhL6qozTd3+DI6qFJPgTm1lrkpvb9W72MtK2vxAyT5I/bG2SWKdpzOaQXysbDb2hnxq3as= +ENTRY_END + +; response to DNSKEY priming query +ENTRY_BEGIN +MATCH opcode qtype qname +ADJUST copy_id +REPLY QR NOERROR +SECTION QUESTION +example.com. IN DNSKEY +SECTION ANSWER +example.com. IN DNSKEY 256 3 8 AwEAAdug/L739i0mgN2nuK/bhxu3wFn5Ud9nK2+XUmZQlPUEZUC5YZvm1rfMmEWTGBn87fFxEu/kjFZHJ55JLzqsbbpVHLbmKCTT2gYR2FV2WDKROGKuYbVkJIXdKAjJ0ONuK507NinYvlWXIoxHn22KAWOd9wKgSTNHBlmGkX+ts3hh ;{id = 55566 (zsk), size = 1024b} +example.com. 3600 IN RRSIG DNSKEY 8 2 3600 20070926134150 20070829134150 55566 example.com. Ni7Q17l2dzKcAnHdU3Mycpdwo0I6qgGxRvBhBNI43xIUFHJpgKpbeMFxKvVTkbwHyMPMIuHmOaC82IBhOpGD10SExVh4erQhWS3Hvl+m4Cwl3WI9N+AW6CTB9yj+d4xzX3bHjjBt6MSk4bU8ABR7qIoAjgjY7zdtUDWQlaM+d18= +SECTION AUTHORITY +example.com. IN NS ns.example.com. +example.com. 3600 IN RRSIG NS 8 2 3600 20070926134150 20070829134150 55566 example.com. cHdLVCzujUQs6b67c1SmCX+/br4tgOg86Gj/R/x+PKUQmWHyeVwBSTlJuLOHbca3CQoyIQc+V2ilK6fjwjbY/dLk4uOlux8L+Zn7HsUXSOwJPIjsM3LuTa8CYDMvYhOP7KGR+vNpJVSsQ25pyDn6Rzsdl3E7DAf7uSkPV8VJwa8= +SECTION ADDITIONAL +ns.example.com. IN A 1.2.3.4 +ns.example.com. 3600 IN RRSIG A 8 3 3600 20070926134150 20070829134150 55566 example.com. PBwNifMNxTXlDorHX1neq1wUhWLmqk+PZ+PBZCI5BJAmakdgOXdLQiVqlKaErJyA/4uN+99fUf6/DqxwgxL8FIPdBkxMOTJaKrCFjEhL6qozTd3+DI6qFJPgTm1lrkpvb9W72MtK2vxAyT5I/bG2SWKdpzOaQXysbDb2hnxq3as= +ENTRY_END + +ENTRY_BEGIN +MATCH opcode qtype qname +ADJUST copy_id +REPLY QR AA NOERROR +SECTION QUESTION +ns.example.com. IN AAAA +SECTION AUTHORITY +example.com. IN NS ns.example.com. +example.com. 3600 IN RRSIG NS 8 2 3600 20070926134150 20070829134150 55566 example.com. cHdLVCzujUQs6b67c1SmCX+/br4tgOg86Gj/R/x+PKUQmWHyeVwBSTlJuLOHbca3CQoyIQc+V2ilK6fjwjbY/dLk4uOlux8L+Zn7HsUXSOwJPIjsM3LuTa8CYDMvYhOP7KGR+vNpJVSsQ25pyDn6Rzsdl3E7DAf7uSkPV8VJwa8= +SECTION ADDITIONAL +ns.example.com. IN A 1.2.3.4 +ns.example.com. 3600 IN RRSIG A 8 3 3600 20070926134150 20070829134150 55566 example.com. PBwNifMNxTXlDorHX1neq1wUhWLmqk+PZ+PBZCI5BJAmakdgOXdLQiVqlKaErJyA/4uN+99fUf6/DqxwgxL8FIPdBkxMOTJaKrCFjEhL6qozTd3+DI6qFJPgTm1lrkpvb9W72MtK2vxAyT5I/bG2SWKdpzOaQXysbDb2hnxq3as= +ENTRY_END + +; response to query of interest +ENTRY_BEGIN +MATCH opcode qtype qname +ADJUST copy_id +REPLY QR NOERROR +SECTION QUESTION +www.example.com. IN A +SECTION ANSWER +www.example.com. IN A 10.20.30.40 +www.example.com. IN A \# 5 0102030405 +; RRSIG includes the malformed record. +www.example.com. 3600 IN RRSIG A 8 3 3600 20070926134150 20070829134150 55566 example.com. W4WFu9B81uRvp3Dj8uLIscypznKWuLuKrZqVg1on5/45/3/xyjHvj3TjTL3gruWFXPiQpldvOstXLZ5eN3OpqILdkVey0eqVATujpHwIruY6GWztVx5WptmFfK6E6zzshZ3RmAARqq/czQ+IZli2A9xixdY2H0o1dSU6gohEjjE= +SECTION AUTHORITY +example.com. IN NS ns.example.com. +example.com. 3600 IN RRSIG NS 8 2 3600 20070926134150 20070829134150 55566 example.com. cHdLVCzujUQs6b67c1SmCX+/br4tgOg86Gj/R/x+PKUQmWHyeVwBSTlJuLOHbca3CQoyIQc+V2ilK6fjwjbY/dLk4uOlux8L+Zn7HsUXSOwJPIjsM3LuTa8CYDMvYhOP7KGR+vNpJVSsQ25pyDn6Rzsdl3E7DAf7uSkPV8VJwa8= +SECTION ADDITIONAL +ns.example.com. IN A 1.2.3.4 +ns.example.com. 3600 IN RRSIG A 8 3 3600 20070926134150 20070829134150 55566 example.com. PBwNifMNxTXlDorHX1neq1wUhWLmqk+PZ+PBZCI5BJAmakdgOXdLQiVqlKaErJyA/4uN+99fUf6/DqxwgxL8FIPdBkxMOTJaKrCFjEhL6qozTd3+DI6qFJPgTm1lrkpvb9W72MtK2vxAyT5I/bG2SWKdpzOaQXysbDb2hnxq3as= +ENTRY_END +RANGE_END + +STEP 1 QUERY +ENTRY_BEGIN +REPLY RD DO +SECTION QUESTION +www.example.com. IN A +ENTRY_END + +; recursion happens here. +STEP 10 CHECK_ANSWER +ENTRY_BEGIN +MATCH all ede=any +REPLY QR RD RA DO SERVFAIL +SECTION QUESTION +www.example.com. IN A +SECTION ANSWER +ENTRY_END + +SCENARIO_END diff --git a/util/module.c b/util/module.c index 773dab853..62e5de4a0 100644 --- a/util/module.c +++ b/util/module.c @@ -194,6 +194,24 @@ char* errinf_to_str_servfail(struct module_qstate* qstate) return p; } +char* errinf_to_str_misc(struct module_qstate* qstate) +{ + char buf[20480]; + char* p = buf; + size_t left = sizeof(buf); + struct errinf_strlist* s; + if(!qstate->errinf) + snprintf(p, left, "misc failure"); + else for(s=qstate->errinf; s; s=s->next) { + snprintf(p, left, "%s%s", (s==qstate->errinf?"":" "), s->str); + left -= strlen(p); p += strlen(p); + } + p = strdup(buf); + if(!p) + log_err("malloc failure in errinf_to_str"); + return p; +} + void errinf_rrset(struct module_qstate* qstate, struct ub_packed_rrset_key *rr) { char buf[1024]; diff --git a/util/module.h b/util/module.h index 5b6fcc93c..d25bebd15 100644 --- a/util/module.h +++ b/util/module.h @@ -842,6 +842,14 @@ sldns_ede_code errinf_to_reason_bogus(struct module_qstate* qstate); */ char* errinf_to_str_servfail(struct module_qstate* qstate); +/** + * Create error info in string. For misc failures that are not servfail. + * @param qstate: query state. + * @return string or NULL on malloc failure (already logged). + * This string is malloced and has to be freed by caller. + */ +char* errinf_to_str_misc(struct module_qstate* qstate); + /** * Initialize the edns known options by allocating the required space. * @param env: the module environment. -- 2.47.3