refuse unsigned authority section. clean additional section as option.

git-svn-id: file:///svn/unbound/trunk@543 be551aaa-1e26-0410-a405-d3ace91eadb9

doc/Changelog

@@ -1,3 +1,7 @@
+24 August 2007: Wouter
+	- message is bogus if unsecure authority rrsets are present.
+	- val-clean-additional option, so you can turn it off.
+
 23 August 2007: Wouter
 	- CNAME handling - move needs_validation to before val_new().
 	  val_new() setups the chase-reply to be an edited copy of the msg.

doc/example.conf

@@ -172,6 +172,12 @@ server:
 	# some of the revalidation, until the time interval expires. in secs.
 	# val-bogus-ttl: 900
 	
+	# Should additional section of secure message also be kept clean of
+	# unsecure data. Useful to shield the users of this validator from
+	# potential bogus data in the additional section. All unsigned data 
+	# in the additional section is removed from secure messages.
+	# val-clean-additional: yes
+
 # Stub zones.
 # Create entries like below, to make all queries for 'example.com' and 
 # 'example.org' go to the given list of nameservers. list zero or more 

doc/unbound.conf.5

@@ -209,6 +209,12 @@ The time to live for bogus data. This is data that has failed validation;
 due to invalid signatures or other checks. The TTL from that data cannot be
 trusted, and this value is used instead. The value is in seconds, default 900.
 The time interval prevents repeated revalidation of bogus data.
+.It \fBval-clean-additional:\fR <yes or no>
+Instruct the validator to remove data from the additional section of secure
+messages that are not signed properly. Messages that are insecure, bogus,
+indeterminate or unchecked are not affected. Default is yes. Use this setting
+to protect the users that rely on this validator for authentication from 
+protentially bad data in the additional section.
 .El
 
 .Ss Stub Zone Options

util/config_file.c

@@ -89,6 +89,7 @@ config_create()
 	cfg->infra_cache_slabs = 4;
 	cfg->infra_cache_numhosts = 1000;
 	cfg->infra_cache_numlame = 1000;
+	cfg->val_clean_additional = 1;
 	if(!(cfg->username = strdup(""))) goto error_exit;
 	if(!(cfg->chrootdir = strdup(""))) goto error_exit;
 	if(!(cfg->directory = strdup("/etc/unbound"))) goto error_exit;

util/config_file.h

@@ -148,6 +148,8 @@ struct config_file {
 	int32_t val_date_override;
 	/** this value sets the number of seconds before revalidating bogus */
 	int bogus_ttl; 
+	/** should validator clean additional section for secure msgs */
+	int val_clean_additional;
 
 	/** daemonize, i.e. fork into the background. */
 	int do_daemonize;

util/configlexer.lex

@@ -146,6 +146,7 @@ trust-anchor-file{COLON}	{ YDOUT; return VAR_TRUST_ANCHOR_FILE;}
 trust-anchor{COLON}	{ YDOUT; return VAR_TRUST_ANCHOR;}
 val-override-date{COLON}	{ YDOUT; return VAR_VAL_OVERRIDE_DATE;}
 val-bogus-ttl{COLON}	{ YDOUT; return VAR_BOGUS_TTL;}
+val-clean-additional{COLON}	{ YDOUT; return VAR_VAL_CLEAN_ADDITIONAL;}
 {NEWLINE}		{ LEXOUT(("NL\n")); cfg_parser->line++;}
 
 	/* Quoted strings. Strip leading and ending quotes */

util/configparser.y

@@ -81,7 +81,7 @@ extern struct config_parser_state* cfg_parser;
 %token VAR_DO_NOT_QUERY_ADDRESS VAR_HIDE_IDENTITY VAR_HIDE_VERSION
 %token VAR_IDENTITY VAR_VERSION VAR_HARDEN_GLUE VAR_MODULE_CONF
 %token VAR_TRUST_ANCHOR_FILE VAR_TRUST_ANCHOR VAR_VAL_OVERRIDE_DATE
-%token VAR_BOGUS_TTL
+%token VAR_BOGUS_TTL VAR_VAL_CLEAN_ADDITIONAL
 
 %%
 toplevelvars: /* empty */ | toplevelvars toplevelvar ;
@@ -115,7 +115,8 @@ content_server: server_num_threads | server_verbosity | server_port |
 	server_do_not_query_address | server_hide_identity |
 	server_hide_version | server_identity | server_version |
 	server_harden_glue | server_module_conf | server_trust_anchor_file |
-	server_trust_anchor | server_val_override_date | server_bogus_ttl
+	server_trust_anchor | server_val_override_date | server_bogus_ttl |
+	server_val_clean_additional
 	;
 stubstart: VAR_STUB_ZONE
 	{
@@ -514,6 +515,16 @@ server_bogus_ttl: VAR_BOGUS_TTL STRING
 		free($2);
 	}
 	;
+server_val_clean_additional: VAR_VAL_CLEAN_ADDITIONAL STRING
+	{
+		OUTYY(("P(server_val_clean_additional:%s)\n", $2));
+		if(strcmp($2, "yes") != 0 && strcmp($2, "no") != 0)
+			yyerror("expected yes or no.");
+		else cfg_parser->cfg->val_clean_additional = 
+			(strcmp($2, "yes")==0);
+		free($2);
+	}
+	;
 
 stub_name: VAR_NAME STRING
 	{

validator/val_utils.c

@@ -557,26 +557,41 @@ val_fill_reply(struct reply_info* chase, struct reply_info* orig,
 }
 
 void
-val_dump_nonsecure(struct reply_info* rep) 
+val_check_nonsecure(struct val_env* ve, struct reply_info* rep) 
 {
 	size_t i;
 	/* authority */
 	for(i=rep->an_numrrsets; i<rep->an_numrrsets+rep->ns_numrrsets; i++) {
 		if(((struct packed_rrset_data*)rep->rrsets[i]->entry.data)
 			->security != sec_status_secure) {
-			/* remove this unsigned/bogus/unneeded rrset */
-			memmove(rep->rrsets+i, rep->rrsets+i+1, 
-				sizeof(struct ub_packed_rrset_key*)*
-				(rep->rrset_count - i - 1));
-			rep->ns_numrrsets--;
-			rep->rrset_count--;
+			/* because we want to return the authentic original
+			 * message when presented with CD-flagged queries,
+			 * we need to preserve AUTHORITY section data.
+			 * However, this rrset is not signed or signed
+			 * with the wrong keys. Validation has tried to
+			 * verify this rrset with the keysets of import.
+			 * But this rrset did not verify.
+			 * Therefore the message is bogus.
+			 */
+			rep->security = sec_status_bogus;
+			return;
 		}
 	}
 	/* additional */
+	if(!ve->clean_additional)
+		return;
 	for(i=rep->an_numrrsets+rep->ns_numrrsets; i<rep->rrset_count; i++) {
 		if(((struct packed_rrset_data*)rep->rrsets[i]->entry.data)
 			->security != sec_status_secure) {
-			/* remove this unsigned/bogus/unneeded rrset */
+			/* This does not cause message invalidation. It was
+			 * simply unsigned data in the additional. The
+			 * RRSIG must have been truncated off the message.
+			 *
+			 * However, we do not want to return possible bogus
+			 * data to clients that rely on this service for
+			 * their authentication.
+			 */
+			/* remove this unneeded additional rrset */
 			memmove(rep->rrsets+i, rep->rrsets+i+1, 
 				sizeof(struct ub_packed_rrset_key*)*
 				(rep->rrset_count - i - 1));

validator/val_utils.h

@@ -210,8 +210,9 @@ void val_fill_reply(struct reply_info* chase, struct reply_info* orig,
  * So that unsigned data does not get let through to clients, when we have
  * found the data to be secure.
  *
+ * @param ve: validator environment with cleaning options.
  * @param rep: reply to dump all nonsecure stuff out of.
  */
-void val_dump_nonsecure(struct reply_info* rep);
+void val_check_nonsecure(struct val_env* ve, struct reply_info* rep);
 
 #endif /* VALIDATOR_VAL_UTILS_H */

validator/validator.c

@@ -59,6 +59,7 @@ static int
 val_apply_cfg(struct val_env* val_env, struct config_file* cfg)
 {
 	val_env->bogus_ttl = (uint32_t)cfg->bogus_ttl;
+	val_env->clean_additional = cfg->val_clean_additional;
 	if(!val_env->anchors)
 		val_env->anchors = anchors_create();
 	if(!val_env->anchors) {
@@ -1267,8 +1268,9 @@ processFinished(struct module_qstate* qstate, struct val_qstate* vq,
 	if(vq->orig_msg->rep->security == sec_status_secure) {
 		/* Do not store the validated status of the dropped RRsets.
 		 * (only secure is reused). These rrsets are apparantly
-		 * added on maliciously, or are unsigned additional data */
-		val_dump_nonsecure(vq->orig_msg->rep);
+		 * added on maliciously, or are unsigned additional data 
+		 * This may cause the message to become bogus. */
+		val_check_nonsecure(ve, vq->orig_msg->rep);
 	}
 
 	/* if the result is bogus - set message ttl to bogus ttl to avoid

validator/validator.h

@@ -74,6 +74,11 @@ struct val_env {
 	 * Bogus data will not be verified more often than this interval. 
 	 * seconds. */
 	uint32_t bogus_ttl;
+
+	/** If set, the validator should clean the additional section of
+	 * secure messages.
+	 */
+	int clean_additional;
 };
 
 /**