- Fix to add EDE text when RRs have been removed due to length.

2024-09-21 06:37:08 +00:00 · 2023-09-07 14:44:48 +02:00 · 2023-09-07 14:44:48 +02:00 · fdd5f8ff83
commit fdd5f8ff83
parent 63616a5fce
8 changed files with 260 additions and 10 deletions
--- 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.
--- 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;
 }
--- 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 */
--- 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;
--- 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
--- a/testdata/val_scrub_rr_length.rpl
+++ 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
--- 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];
--- 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.