From 3742e47f04d1186b7f879dc9160254df90c31dcf Mon Sep 17 00:00:00 2001
From: "W.C.A. Wijngaards" <wouter@nlnetlabs.nl>
Date: Thu, 13 Jun 2024 15:39:31 +0200
Subject: [PATCH 1/7] - cookie-secret-file, define struct.

---
 daemon/daemon.h |  3 +++
 util/edns.h     | 26 ++++++++++++++++++++++++++
 2 files changed, 29 insertions(+)

diff --git a/daemon/daemon.h b/daemon/daemon.h
index 57665446d..49a5ae39c 100644
--- a/daemon/daemon.h
+++ b/daemon/daemon.h
@@ -58,6 +58,7 @@ struct ub_randstate;
 struct daemon_remote;
 struct respip_set;
 struct shm_main_info;
+struct cookie_secrets;
 
 #include "dnstap/dnstap_config.h"
 #ifdef USE_DNSTAP
@@ -146,6 +147,8 @@ struct daemon {
 #endif
 	/** reuse existing cache on reload if other conditions allow it. */
 	int reuse_cache;
+	/** the EDNS cookie secrets from the cookie-secret-file */
+	struct cookie_secrets* cookie_secrets;
 };
 
 /**
diff --git a/util/edns.h b/util/edns.h
index 5da0ecb29..768464b4b 100644
--- a/util/edns.h
+++ b/util/edns.h
@@ -43,6 +43,7 @@
 #define UTIL_EDNS_H
 
 #include "util/storage/dnstree.h"
+#include "util/locks.h"
 
 struct edns_data;
 struct config_file;
@@ -75,6 +76,31 @@ struct edns_string_addr {
 	size_t string_len;
 };
 
+#define UNBOUND_COOKIE_HISTORY_SIZE 2
+#define UNBOUND_COOKIE_SECRET_SIZE 16
+
+typedef struct cookie_secret cookie_secret_type;
+struct cookie_secret {
+	/** cookie secret */
+	uint8_t cookie_secret[UNBOUND_COOKIE_SECRET_SIZE];
+};
+
+/**
+ * The cookie secrets from the cookie-secret-file.
+ */
+struct cookie_secrets {
+	/** lock on the structure, in case there are modifications
+	 * from remote control, this avoids race conditions. */
+	lock_basic_type lock;
+
+	/** how many cookies are there in the cookies array */
+	size_t cookie_count;
+
+	/* keep track of the last `UNBOUND_COOKIE_HISTORY_SIZE`
+	 * cookies as per rfc requirement .*/
+	cookie_secret_type cookie_secrets[UNBOUND_COOKIE_HISTORY_SIZE];
+};
+
 enum edns_cookie_val_status {
 	COOKIE_STATUS_CLIENT_ONLY = -3,
 	COOKIE_STATUS_FUTURE = -2,

From b70f1a8f6040ea18e69a80a6a220bb7e42a38174 Mon Sep 17 00:00:00 2001
From: "W.C.A. Wijngaards" <wouter@nlnetlabs.nl>
Date: Thu, 13 Jun 2024 17:38:06 +0200
Subject: [PATCH 2/7] - cookie-secret-file, add config option, create, read and
 delete struct.

---
 daemon/daemon.c       |  9 +++++
 doc/example.conf.in   |  5 +++
 doc/unbound.conf.5.in | 13 +++++++
 util/config_file.c    |  6 +++
 util/config_file.h    |  2 +
 util/configlexer.lex  |  1 +
 util/configparser.y   | 86 +++++++++++++++++++++++--------------------
 util/edns.c           | 84 ++++++++++++++++++++++++++++++++++++++++++
 util/edns.h           | 21 +++++++++++
 util/net_help.c       | 19 ++++++++++
 util/net_help.h       |  3 ++
 11 files changed, 210 insertions(+), 39 deletions(-)

diff --git a/daemon/daemon.c b/daemon/daemon.c
index dbb6db060..4e2c51d5c 100644
--- a/daemon/daemon.c
+++ b/daemon/daemon.c
@@ -718,6 +718,14 @@ daemon_fork(struct daemon* daemon)
 				   "dnscrypt support");
 #endif
 	}
+	if(daemon->cfg->cookie_secret_file &&
+		daemon->cfg->cookie_secret_file[0]) {
+		if(!(daemon->cookie_secrets = cookie_secrets_create()))
+			fatal_exit("Could not create cookie_secrets: out of memory");
+		if(!cookie_secrets_apply_cfg(daemon->cookie_secrets,
+			daemon->cfg->cookie_secret_file))
+			fatal_exit("Could not setu up cookie_secrets");
+	}
 	/* create global local_zones */
 	if(!(daemon->local_zones = local_zones_create()))
 		fatal_exit("Could not create local zones: out of memory");
@@ -910,6 +918,7 @@ daemon_delete(struct daemon* daemon)
 	acl_list_delete(daemon->acl);
 	acl_list_delete(daemon->acl_interface);
 	tcl_list_delete(daemon->tcl);
+	cookie_secrets_delete(daemon->cookie_secrets);
 	listen_desetup_locks();
 	free(daemon->chroot);
 	free(daemon->pidfile);
diff --git a/doc/example.conf.in b/doc/example.conf.in
index e0aec8ec7..698716b56 100644
--- a/doc/example.conf.in
+++ b/doc/example.conf.in
@@ -1044,6 +1044,11 @@ server:
 	# example value "000102030405060708090a0b0c0d0e0f".
 	# cookie-secret: <128 bit random hex string>
 
+	# File with cookie secrets, the 'cookie-secret:' option is ignored
+	# and the file can be managed to have staging and active secrets
+	# with remote control commands. Disabled with "". Default is "".
+	# cookie-secret-file: "/usr/local/etc/unbound_cookiesecrets.txt"
+
 	# Enable to attach Extended DNS Error codes (RFC8914) to responses.
 	# ede: no
 
diff --git a/doc/unbound.conf.5.in b/doc/unbound.conf.5.in
index 34e61d69f..bf2e2d1ba 100644
--- a/doc/unbound.conf.5.in
+++ b/doc/unbound.conf.5.in
@@ -1978,6 +1978,19 @@ Useful to explicitly set for servers in an anycast deployment that need to
 share the secret in order to verify each other's Server Cookies.
 An example hex string would be "000102030405060708090a0b0c0d0e0f".
 Default is a 128 bits random secret generated at startup time.
+This option is ignored if a \fBcookie\-secret\-file\fR is
+present.  In that case the secrets from that file are used in DNS Cookie
+calculations.
+.TP 5
+.B cookie\-secret\-file: \fI<filename>
+File from which the secrets are read used in DNS Cookie calculations. When this
+file exists, the secrets in this file are used and the secret specified by the
+\fBcookie-secret\fR option is ignored. Default is "". With "" it is disabled.
+Enable it by setting a filename, like "/usr/local/etc/unbound_cookiesecrets.txt".
+The content of this file must be manipulated with the \fBadd_cookie_secret\fR,
+\fBdrop_cookie_secret\fR and \fBactivate_cookie_secret\fR commands to the
+\fIunbound\-control\fR(8) tool. Please see that manpage how to perform a safe
+cookie secret rollover.
 .TP 5
 .B edns\-client\-string: \fI<IP netblock> <string>
 Include an EDNS0 option containing configured ascii string in queries with
diff --git a/util/config_file.c b/util/config_file.c
index 2ac6c4680..2be7829dd 100644
--- a/util/config_file.c
+++ b/util/config_file.c
@@ -387,6 +387,7 @@ config_create(void)
 	memset(cfg->cookie_secret, 0, sizeof(cfg->cookie_secret));
 	cfg->cookie_secret_len = 16;
 	init_cookie_secret(cfg->cookie_secret, cfg->cookie_secret_len);
+	cfg->cookie_secret_file = NULL;
 #ifdef USE_CACHEDB
 	if(!(cfg->cachedb_backend = strdup("testframe"))) goto error_exit;
 	if(!(cfg->cachedb_secret = strdup("default"))) goto error_exit;
@@ -838,6 +839,8 @@ int config_set_option(struct config_file* cfg, const char* opt,
 	{ IS_NUMBER_OR_ZERO; cfg->ipsecmod_max_ttl = atoi(val); }
 	else S_YNO("ipsecmod-strict:", ipsecmod_strict)
 #endif
+	else S_YNO("answer-cookie:", do_answer_cookie)
+	else S_STR("cookie-secret-file:", cookie_secret_file)
 #ifdef USE_CACHEDB
 	else S_YNO("cachedb-no-store:", cachedb_no_store)
 	else S_YNO("cachedb-check-when-serve-expired:", cachedb_check_when_serve_expired)
@@ -1334,6 +1337,8 @@ config_get_option(struct config_file* cfg, const char* opt,
 	else O_LST(opt, "ipsecmod-whitelist", ipsecmod_whitelist)
 	else O_YNO(opt, "ipsecmod-strict", ipsecmod_strict)
 #endif
+	else O_YNO(opt, "answer-cookie", do_answer_cookie)
+	else O_STR(opt, "cookie-secret-file", cookie_secret_file)
 #ifdef USE_CACHEDB
 	else O_STR(opt, "backend", cachedb_backend)
 	else O_STR(opt, "secret-seed", cachedb_secret)
@@ -1719,6 +1724,7 @@ config_delete(struct config_file* cfg)
 	free(cfg->ipsecmod_hook);
 	config_delstrlist(cfg->ipsecmod_whitelist);
 #endif
+	free(cfg->cookie_secret_file);
 #ifdef USE_CACHEDB
 	free(cfg->cachedb_backend);
 	free(cfg->cachedb_secret);
diff --git a/util/config_file.h b/util/config_file.h
index d3a2e268c..cb833054d 100644
--- a/util/config_file.h
+++ b/util/config_file.h
@@ -748,6 +748,8 @@ struct config_file {
 	uint8_t cookie_secret[40];
 	/** cookie secret length */
 	size_t  cookie_secret_len;
+	/** path to cookie secret store */
+	char* cookie_secret_file;
 
 	/* ipset module */
 #ifdef USE_IPSET
diff --git a/util/configlexer.lex b/util/configlexer.lex
index 7ae1b8c38..9eba580b6 100644
--- a/util/configlexer.lex
+++ b/util/configlexer.lex
@@ -581,6 +581,7 @@ udp-upstream-without-downstream{COLON} { YDVAR(1, VAR_UDP_UPSTREAM_WITHOUT_DOWNS
 tcp-connection-limit{COLON}	{ YDVAR(2, VAR_TCP_CONNECTION_LIMIT) }
 answer-cookie{COLON}		{ YDVAR(1, VAR_ANSWER_COOKIE ) }
 cookie-secret{COLON}		{ YDVAR(1, VAR_COOKIE_SECRET) }
+cookie-secret-file{COLON}	{ YDVAR(1, VAR_COOKIE_SECRET_FILE) }
 edns-client-string{COLON}	{ YDVAR(2, VAR_EDNS_CLIENT_STRING) }
 edns-client-string-opcode{COLON} { YDVAR(1, VAR_EDNS_CLIENT_STRING_OPCODE) }
 nsid{COLON}			{ YDVAR(1, VAR_NSID ) }
diff --git a/util/configparser.y b/util/configparser.y
index 5f42126f7..7000d0d74 100644
--- a/util/configparser.y
+++ b/util/configparser.y
@@ -204,6 +204,7 @@ extern struct config_parser_state* cfg_parser;
 %token VAR_PROXY_PROTOCOL_PORT VAR_STATISTICS_INHIBIT_ZERO
 %token VAR_HARDEN_UNKNOWN_ADDITIONAL VAR_DISABLE_EDNS_DO VAR_CACHEDB_NO_STORE
 %token VAR_LOG_DESTADDR VAR_CACHEDB_CHECK_WHEN_SERVE_EXPIRED
+%token VAR_COOKIE_SECRET_FILE
 
 %%
 toplevelvars: /* empty */ | toplevelvars toplevelvar ;
@@ -341,7 +342,7 @@ content_server: server_num_threads | server_verbosity | server_port |
 	server_interface_automatic_ports | server_ede |
 	server_proxy_protocol_port | server_statistics_inhibit_zero |
 	server_harden_unknown_additional | server_disable_edns_do |
-	server_log_destaddr
+	server_log_destaddr | server_cookie_secret_file
 	;
 stubstart: VAR_STUB_ZONE
 	{
@@ -3985,45 +3986,52 @@ server_cookie_secret: VAR_COOKIE_SECRET STRING_ARG
 		free($2);
 	}
 	;
-	ipsetstart: VAR_IPSET
-		{
-			OUTYY(("\nP(ipset:)\n"));
-			cfg_parser->started_toplevel = 1;
-		}
-		;
-	contents_ipset: contents_ipset content_ipset
-		| ;
-	content_ipset: ipset_name_v4 | ipset_name_v6
-		;
-	ipset_name_v4: VAR_IPSET_NAME_V4 STRING_ARG
-		{
-		#ifdef USE_IPSET
-			OUTYY(("P(name-v4:%s)\n", $2));
-			if(cfg_parser->cfg->ipset_name_v4)
-				yyerror("ipset name v4 override, there must be one "
-					"name for ip v4");
-			free(cfg_parser->cfg->ipset_name_v4);
-			cfg_parser->cfg->ipset_name_v4 = $2;
-		#else
-			OUTYY(("P(Compiled without ipset, ignoring)\n"));
-			free($2);
-		#endif
-		}
+server_cookie_secret_file: VAR_COOKIE_SECRET_FILE STRING_ARG
+	{
+		OUTYY(("P(cookie_secret_file:%s)\n", $2));
+		free(cfg_parser->cfg->cookie_secret_file);
+		cfg_parser->cfg->cookie_secret_file = $2;
+	}
 	;
-	ipset_name_v6: VAR_IPSET_NAME_V6 STRING_ARG
-	{
-		#ifdef USE_IPSET
-			OUTYY(("P(name-v6:%s)\n", $2));
-			if(cfg_parser->cfg->ipset_name_v6)
-				yyerror("ipset name v6 override, there must be one "
-					"name for ip v6");
-			free(cfg_parser->cfg->ipset_name_v6);
-			cfg_parser->cfg->ipset_name_v6 = $2;
-		#else
-			OUTYY(("P(Compiled without ipset, ignoring)\n"));
-			free($2);
-		#endif
-		}
+ipsetstart: VAR_IPSET
+	{
+		OUTYY(("\nP(ipset:)\n"));
+		cfg_parser->started_toplevel = 1;
+	}
+	;
+contents_ipset: contents_ipset content_ipset
+	| ;
+content_ipset: ipset_name_v4 | ipset_name_v6
+	;
+ipset_name_v4: VAR_IPSET_NAME_V4 STRING_ARG
+	{
+	#ifdef USE_IPSET
+		OUTYY(("P(name-v4:%s)\n", $2));
+		if(cfg_parser->cfg->ipset_name_v4)
+			yyerror("ipset name v4 override, there must be one "
+				"name for ip v4");
+		free(cfg_parser->cfg->ipset_name_v4);
+		cfg_parser->cfg->ipset_name_v4 = $2;
+	#else
+		OUTYY(("P(Compiled without ipset, ignoring)\n"));
+		free($2);
+	#endif
+	}
+	;
+ipset_name_v6: VAR_IPSET_NAME_V6 STRING_ARG
+	{
+	#ifdef USE_IPSET
+		OUTYY(("P(name-v6:%s)\n", $2));
+		if(cfg_parser->cfg->ipset_name_v6)
+			yyerror("ipset name v6 override, there must be one "
+				"name for ip v6");
+		free(cfg_parser->cfg->ipset_name_v6);
+		cfg_parser->cfg->ipset_name_v6 = $2;
+	#else
+		OUTYY(("P(Compiled without ipset, ignoring)\n"));
+		free($2);
+	#endif
+	}
 	;
 %%
 
diff --git a/util/edns.c b/util/edns.c
index 2b4047f0b..3cda80d81 100644
--- a/util/edns.c
+++ b/util/edns.c
@@ -187,3 +187,87 @@ edns_cookie_server_validate(const uint8_t* cookie, size_t cookie_len,
 		return COOKIE_STATUS_VALID_RENEW;
 	return COOKIE_STATUS_VALID;
 }
+
+struct cookie_secrets*
+cookie_secrets_create(void)
+{
+	struct cookie_secrets* cookie_secrets = calloc(1,
+		sizeof(*cookie_secrets));
+	if(!cookie_secrets)
+		return NULL;
+	lock_basic_init(&cookie_secrets->lock);
+	lock_protect(&cookie_secrets->lock, &cookie_secrets->cookie_count,
+		sizeof(cookie_secrets->cookie_count));
+	lock_protect(&cookie_secrets->lock, cookie_secrets->cookie_secrets,
+		sizeof(cookie_secret_type)*UNBOUND_COOKIE_HISTORY_SIZE);
+	return cookie_secrets;
+}
+
+void
+cookie_secrets_delete(struct cookie_secrets* cookie_secrets)
+{
+	if(!cookie_secrets)
+		return;
+	lock_basic_destroy(&cookie_secrets->lock);
+	explicit_bzero(cookie_secrets->cookie_secrets,
+		sizeof(cookie_secret_type)*UNBOUND_COOKIE_HISTORY_SIZE);
+	free(cookie_secrets);
+}
+
+/** Read the cookie secret file */
+static int
+cookie_secret_file_read(struct cookie_secrets* cookie_secrets,
+	char* cookie_secret_file)
+{
+	char secret[UNBOUND_COOKIE_SECRET_SIZE * 2 + 2/*'\n' and '\0'*/];
+	FILE* f;
+	int corrupt = 0;
+	size_t count;
+
+	log_assert(cookie_secret_file != NULL);
+	cookie_secrets->cookie_count = 0;
+	f = fopen(cookie_secret_file, "r");
+	/* a non-existing cookie file is not an error */
+	if( f == NULL ) {
+		if(errno != EPERM) {
+			log_err("Could not read cookie-secret-file '%s': %s",
+				cookie_secret_file, strerror(errno));
+			return 0;
+		}
+		return 1;
+	}
+	/* cookie secret file exists and is readable */
+	for( count = 0; count < UNBOUND_COOKIE_HISTORY_SIZE; count++ ) {
+		size_t secret_len = 0;
+		ssize_t decoded_len = 0;
+		if( fgets(secret, sizeof(secret), f) == NULL ) { break; }
+		secret_len = strlen(secret);
+		if( secret_len == 0 ) { break; }
+		log_assert( secret_len <= sizeof(secret) );
+		secret_len = secret[secret_len - 1] == '\n' ? secret_len - 1 : secret_len;
+		if( secret_len != UNBOUND_COOKIE_SECRET_SIZE * 2 ) { corrupt++; break; }
+		/* needed for `hex_pton`; stripping potential `\n` */
+		secret[secret_len] = '\0';
+		decoded_len = hex_pton(secret, cookie_secrets->cookie_secrets[count].cookie_secret,
+		                       UNBOUND_COOKIE_SECRET_SIZE);
+		if( decoded_len != UNBOUND_COOKIE_SECRET_SIZE ) { corrupt++; break; }
+		cookie_secrets->cookie_count++;
+	}
+	fclose(f);
+	return corrupt == 0;
+}
+
+int
+cookie_secrets_apply_cfg(struct cookie_secrets* cookie_secrets,
+	char* cookie_secret_file)
+{
+	if(!cookie_secrets) {
+		if(!cookie_secret_file || !cookie_secret_file[0])
+			return 1; /* There is nothing to read anyway */
+		log_err("Could not read cookie secrets, no structure alloced");
+		return 0;
+	}
+	if(!cookie_secret_file_read(cookie_secrets, cookie_secret_file))
+		return 0;
+	return 1;
+}
diff --git a/util/edns.h b/util/edns.h
index 768464b4b..e24297076 100644
--- a/util/edns.h
+++ b/util/edns.h
@@ -191,4 +191,25 @@ enum edns_cookie_val_status edns_cookie_server_validate(const uint8_t* cookie,
 	size_t cookie_len, const uint8_t* secret, size_t secret_len, int v4,
 	const uint8_t* hash_input, uint32_t now);
 
+/**
+ * Create the cookie secrets structure.
+ * @return the structure or NULL on failure.
+ */
+struct cookie_secrets* cookie_secrets_create(void);
+
+/**
+ * Delete the cookie secrets.
+ * @param cookie_secrets: the cookie secrets.
+ */
+void cookie_secrets_delete(struct cookie_secrets* cookie_secrets);
+
+/**
+ * Apply configuration to cookie secrets, read them from file.
+ * @param cookie_secrets: the cookie secrets structure.
+ * @param cookie_secret_file: the file name, it is read.
+ * @return false on failure.
+ */
+int cookie_secrets_apply_cfg(struct cookie_secrets* cookie_secrets,
+	char* cookie_secret_file);
+
 #endif
diff --git a/util/net_help.c b/util/net_help.c
index a1bd71865..2e76def78 100644
--- a/util/net_help.c
+++ b/util/net_help.c
@@ -47,6 +47,7 @@
 #ifdef HAVE_NETIOAPI_H
 #include <netioapi.h>
 #endif
+#include <ctype.h>
 #include "util/net_help.h"
 #include "util/log.h"
 #include "util/data/dname.h"
@@ -1871,3 +1872,21 @@ sock_close(int socket)
 	closesocket(socket);
 }
 #  endif /* USE_WINSOCK */
+
+ssize_t
+hex_pton(const char* src, uint8_t* target, size_t targsize)
+{
+	uint8_t *t = target;
+	if(strlen(src) % 2 != 0 || strlen(src)/2 > targsize) {
+		return -1;
+	}
+	while(*src) {
+		if(!isxdigit((unsigned char)src[0]) ||
+			!isxdigit((unsigned char)src[1]))
+			return -1;
+		*t++ = sldns_hexdigit_to_int(src[0]) * 16 +
+			sldns_hexdigit_to_int(src[1]) ;
+		src += 2;
+	}
+	return t-target;
+}
diff --git a/util/net_help.h b/util/net_help.h
index 1c57b5b70..2a20a542a 100644
--- a/util/net_help.h
+++ b/util/net_help.h
@@ -572,4 +572,7 @@ char* sock_strerror(int errn);
 /** close the socket with close, or wsa closesocket */
 void sock_close(int socket);
 
+/** Convert hexadecimal data to binary. */
+ssize_t hex_pton(const char* src, uint8_t* target, size_t targsize);
+
 #endif /* NET_HELP_H */

From 6f7e8ccf0cf09492596febfe4514a4763d4d7d7a Mon Sep 17 00:00:00 2001
From: "W.C.A. Wijngaards" <wouter@nlnetlabs.nl>
Date: Fri, 14 Jun 2024 09:17:30 +0200
Subject: [PATCH 3/7] - cookie-secret-file, check cookie secrets for cookie
 validation.

---
 daemon/worker.c      |  3 ++-
 util/data/msgparse.c | 26 +++++++++++++++++++-------
 util/data/msgparse.h |  5 ++++-
 util/edns.c          | 25 +++++++++++++++++++++++++
 util/edns.h          | 17 +++++++++++++++++
 5 files changed, 67 insertions(+), 9 deletions(-)

diff --git a/daemon/worker.c b/daemon/worker.c
index b35fe65a3..7345ee0e5 100644
--- a/daemon/worker.c
+++ b/daemon/worker.c
@@ -1571,7 +1571,8 @@ worker_handle_request(struct comm_point* c, void* arg, int error,
 	if((ret=parse_edns_from_query_pkt(
 			c->buffer, &edns, worker->env.cfg, c, repinfo,
 			(worker->env.now ? *worker->env.now : time(NULL)),
-			worker->scratchpad)) != 0) {
+			worker->scratchpad,
+			worker->daemon->cookie_secrets)) != 0) {
 		struct edns_data reply_edns;
 		verbose(VERB_ALGO, "worker parse edns: formerror.");
 		log_addr(VERB_CLIENT, "from", &repinfo->client_addr,
diff --git a/util/data/msgparse.c b/util/data/msgparse.c
index d06b7bb25..4fb460e90 100644
--- a/util/data/msgparse.c
+++ b/util/data/msgparse.c
@@ -947,7 +947,8 @@ parse_packet(sldns_buffer* pkt, struct msg_parse* msg, struct regional* region)
 static int
 parse_edns_options_from_query(uint8_t* rdata_ptr, size_t rdata_len,
 	struct edns_data* edns, struct config_file* cfg, struct comm_point* c,
-	struct comm_reply* repinfo, uint32_t now, struct regional* region)
+	struct comm_reply* repinfo, uint32_t now, struct regional* region,
+	struct cookie_secrets* cookie_secrets)
 {
 	/* To respond with a Keepalive option, the client connection must have
 	 * received one message with a TCP Keepalive EDNS option, and that
@@ -1070,10 +1071,20 @@ parse_edns_options_from_query(uint8_t* rdata_ptr, size_t rdata_len,
 					&((struct sockaddr_in6*)&repinfo->remote_addr)->sin6_addr, 16);
 			}
 
-			cookie_val_status = edns_cookie_server_validate(
-				rdata_ptr, opt_len, cfg->cookie_secret,
-				cfg->cookie_secret_len, cookie_is_v4,
-				server_cookie, now);
+			if(cfg->cookie_secret_file &&
+				cfg->cookie_secret_file[0]) {
+				/* Loop over the active and staging cookies. */
+				cookie_val_status =
+					cookie_secrets_server_validate(
+					rdata_ptr, opt_len, cookie_secrets,
+					cookie_is_v4, server_cookie, now);
+			} else {
+				/* Use the cookie option value to validate. */
+				cookie_val_status = edns_cookie_server_validate(
+					rdata_ptr, opt_len, cfg->cookie_secret,
+					cfg->cookie_secret_len, cookie_is_v4,
+					server_cookie, now);
+			}
 			switch(cookie_val_status) {
 			case COOKIE_STATUS_VALID:
 			case COOKIE_STATUS_VALID_RENEW:
@@ -1239,7 +1250,8 @@ skip_pkt_rrs(sldns_buffer* pkt, int num)
 int 
 parse_edns_from_query_pkt(sldns_buffer* pkt, struct edns_data* edns,
 	struct config_file* cfg, struct comm_point* c,
-	struct comm_reply* repinfo, time_t now, struct regional* region)
+	struct comm_reply* repinfo, time_t now, struct regional* region,
+	struct cookie_secrets* cookie_secrets)
 {
 	size_t rdata_len;
 	uint8_t* rdata_ptr;
@@ -1285,7 +1297,7 @@ parse_edns_from_query_pkt(sldns_buffer* pkt, struct edns_data* edns,
 	rdata_ptr = sldns_buffer_current(pkt);
 	/* ignore rrsigs */
 	return parse_edns_options_from_query(rdata_ptr, rdata_len, edns, cfg,
-		c, repinfo, now, region);
+		c, repinfo, now, region, cookie_secrets);
 }
 
 void
diff --git a/util/data/msgparse.h b/util/data/msgparse.h
index 656e0d285..aebd48efa 100644
--- a/util/data/msgparse.h
+++ b/util/data/msgparse.h
@@ -73,6 +73,7 @@ struct edns_option;
 struct config_file;
 struct comm_point;
 struct comm_reply;
+struct cookie_secrets;
 
 /** number of buckets in parse rrset hash table. Must be power of 2. */
 #define PARSE_TABLE_SIZE 32
@@ -322,12 +323,14 @@ int skip_pkt_rrs(struct sldns_buffer* pkt, int num);
  * @param repinfo: commreply to determine the client address
  * @param now: current time
  * @param region: region to alloc results in (edns option contents)
+ * @param cookie_secrets: the cookie secrets for EDNS COOKIE validation.
  * @return: 0 on success, or an RCODE on error.
  *	RCODE formerr if OPT is badly formatted and so on.
  */
 int parse_edns_from_query_pkt(struct sldns_buffer* pkt, struct edns_data* edns,
 	struct config_file* cfg, struct comm_point* c,
-	struct comm_reply* repinfo, time_t now, struct regional* region);
+	struct comm_reply* repinfo, time_t now, struct regional* region,
+	struct cookie_secrets* cookie_secrets);
 
 /**
  * Calculate hash value for rrset in packet.
diff --git a/util/edns.c b/util/edns.c
index 3cda80d81..45d6a3849 100644
--- a/util/edns.c
+++ b/util/edns.c
@@ -271,3 +271,28 @@ cookie_secrets_apply_cfg(struct cookie_secrets* cookie_secrets,
 		return 0;
 	return 1;
 }
+
+enum edns_cookie_val_status
+cookie_secrets_server_validate(const uint8_t* cookie, size_t cookie_len,
+	struct cookie_secrets* cookie_secrets, int v4,
+	const uint8_t* hash_input, uint32_t now)
+{
+	size_t i;
+	enum edns_cookie_val_status cookie_val_status,
+		last = COOKIE_STATUS_INVALID;
+	if(!cookie_secrets || cookie_secrets->cookie_count == 0)
+		return COOKIE_STATUS_INVALID; /* There are no cookie secrets.*/
+	for(i=0; i<cookie_secrets->cookie_count; i++) {
+		cookie_val_status = edns_cookie_server_validate(cookie,
+			cookie_len,
+			cookie_secrets->cookie_secrets[i].cookie_secret,
+			UNBOUND_COOKIE_SECRET_SIZE, v4, hash_input, now);
+		if(cookie_val_status == COOKIE_STATUS_VALID ||
+			cookie_val_status == COOKIE_STATUS_VALID_RENEW)
+			return cookie_val_status;
+		if(last == COOKIE_STATUS_INVALID)
+			last = cookie_val_status; /* Store more interesting
+				failure to return. */
+	}
+	return last;
+}
diff --git a/util/edns.h b/util/edns.h
index e24297076..75f1e3771 100644
--- a/util/edns.h
+++ b/util/edns.h
@@ -212,4 +212,21 @@ void cookie_secrets_delete(struct cookie_secrets* cookie_secrets);
 int cookie_secrets_apply_cfg(struct cookie_secrets* cookie_secrets,
 	char* cookie_secret_file);
 
+/**
+ * Validate the cookie secrets, try all of them.
+ * @param cookie: pointer to the cookie data.
+ * @param cookie_len: the length of the cookie data.
+ * @param cookie_secrets: struct of cookie secrets.
+ * @param v4: if the client IP is v4 or v6.
+ * @param hash_input: pointer to the hash input for validation. It needs to be:
+ *	Client Cookie | Version | Reserved | Timestamp | Client-IP
+ * @param now: the current time.
+ * return edns_cookie_val_status with the cookie validation status i.e.,
+ *	<=0 for invalid, else valid.
+ */
+enum edns_cookie_val_status cookie_secrets_server_validate(
+	const uint8_t* cookie, size_t cookie_len,
+	struct cookie_secrets* cookie_secrets, int v4,
+	const uint8_t* hash_input, uint32_t now);
+
 #endif

From a29fc034ac820666d0dde1527aa9bf6038597f8b Mon Sep 17 00:00:00 2001
From: "W.C.A. Wijngaards" <wouter@nlnetlabs.nl>
Date: Fri, 14 Jun 2024 10:15:55 +0200
Subject: [PATCH 4/7] - cookie-secret-file, unbound-control add_cookie_secret,
 drop_cookie_secret,   activate_cookie_secret and print_cookie_secrets.

---
 Makefile.in                |   2 +-
 daemon/remote.c            | 216 +++++++++++++++++++++++++++++++++++++
 doc/unbound-control.8.in   |  35 ++++++
 smallapp/unbound-control.c |   4 +
 testcode/unitmain.c        |   2 +-
 util/edns.c                |  78 +++++++++++++-
 util/edns.h                |  21 ++++
 util/net_help.c            |  21 ++++
 util/net_help.h            |   6 ++
 9 files changed, 381 insertions(+), 4 deletions(-)

diff --git a/Makefile.in b/Makefile.in
index f30ca81a8..9644bbcc8 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -1297,7 +1297,7 @@ remote.lo remote.o: $(srcdir)/daemon/remote.c config.h $(srcdir)/daemon/remote.h
  $(srcdir)/validator/val_anchor.h $(srcdir)/iterator/iterator.h $(srcdir)/services/outbound_list.h \
  $(srcdir)/iterator/iter_fwd.h $(srcdir)/iterator/iter_hints.h $(srcdir)/iterator/iter_delegpt.h \
  $(srcdir)/services/outside_network.h $(srcdir)/sldns/str2wire.h $(srcdir)/sldns/parseutil.h \
- $(srcdir)/sldns/wire2str.h
+ $(srcdir)/sldns/wire2str.h $(srcdir)/util/edns.h
 stats.lo stats.o: $(srcdir)/daemon/stats.c config.h $(srcdir)/daemon/stats.h $(srcdir)/util/timehist.h \
  $(srcdir)/libunbound/unbound.h $(srcdir)/daemon/worker.h $(srcdir)/libunbound/worker.h $(srcdir)/sldns/sbuffer.h \
  $(srcdir)/util/data/packed_rrset.h $(srcdir)/util/storage/lruhash.h $(srcdir)/util/locks.h $(srcdir)/util/log.h \
diff --git a/daemon/remote.c b/daemon/remote.c
index 341e56054..5e84fd4ac 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -88,6 +88,7 @@
 #include "sldns/wire2str.h"
 #include "sldns/sbuffer.h"
 #include "util/timeval_func.h"
+#include "util/edns.h"
 #ifdef USE_CACHEDB
 #include "cachedb/cachedb.h"
 #endif
@@ -3193,6 +3194,212 @@ do_rpz_disable(RES* ssl, struct worker* worker, char* arg)
     do_rpz_enable_disable(ssl, worker, arg, 0);
 }
 
+/* write the cookie secrets to file, returns `0` on failure */
+static int
+cookie_secret_file_dump(RES* ssl, struct worker* worker) {
+	char const* secret_file = worker->env.cfg->cookie_secret_file;
+	struct cookie_secrets* cookie_secrets = worker->daemon->cookie_secrets;
+	char secret_hex[UNBOUND_COOKIE_SECRET_SIZE * 2 + 1];
+	FILE* f;
+	size_t i;
+	if(secret_file == NULL || secret_file[0]==0) {
+		(void)ssl_printf(ssl, "error: no cookie secret file configured\n");
+		return 0;
+	}
+	log_assert( secret_file != NULL );
+
+	/* open write only and truncate */
+	if((f = fopen(secret_file, "w")) == NULL ) {
+		(void)ssl_printf(ssl, "unable to open cookie secret file %s: %s",
+		                 secret_file, strerror(errno));
+		return 0;
+	}
+	if(cookie_secrets == NULL) {
+		/* nothing to write */
+		fclose(f);
+		return 1;
+	}
+	lock_basic_lock(&cookie_secrets->lock);
+
+	for(i = 0; i < cookie_secrets->cookie_count; i++) {
+		struct cookie_secret const* cs = &cookie_secrets->
+			cookie_secrets[i];
+		ssize_t const len = hex_ntop(cs->cookie_secret,
+			UNBOUND_COOKIE_SECRET_SIZE, secret_hex,
+			sizeof(secret_hex));
+		(void)len; /* silence unused variable warning with -DNDEBUG */
+		log_assert( len == UNBOUND_COOKIE_SECRET_SIZE * 2 );
+		secret_hex[UNBOUND_COOKIE_SECRET_SIZE * 2] = '\0';
+		fprintf(f, "%s\n", secret_hex);
+	}
+	lock_basic_unlock(&cookie_secrets->lock);
+	explicit_bzero(secret_hex, sizeof(secret_hex));
+	fclose(f);
+	return 1;
+}
+
+/** Activate cookie secret */
+static void
+do_activate_cookie_secret(RES* ssl, struct worker* worker) {
+	char const* secret_file = worker->env.cfg->cookie_secret_file;
+	struct cookie_secrets* cookie_secrets = worker->daemon->cookie_secrets;
+
+	if(secret_file == NULL || secret_file[0] == 0) {
+		(void)ssl_printf(ssl, "error: no cookie secret file configured\n");
+		return;
+	}
+	if(cookie_secrets == NULL) {
+		(void)ssl_printf(ssl, "error: there are no cookie_secrets.");
+		return;
+	}
+	lock_basic_lock(&cookie_secrets->lock);
+
+	if(cookie_secrets->cookie_count <= 1 ) {
+		lock_basic_unlock(&cookie_secrets->lock);
+		(void)ssl_printf(ssl, "error: no staging cookie secret to activate\n");
+		return;
+	}
+	/* Only the worker 0 writes to file, the others update state. */
+	if(worker->thread_num == 0 && !cookie_secret_file_dump(ssl, worker)) {
+		lock_basic_unlock(&cookie_secrets->lock);
+		(void)ssl_printf(ssl, "error: writing to cookie secret file: \"%s\"\n",
+				secret_file);
+		return;
+	}
+	activate_cookie_secret(cookie_secrets);
+	if(worker->thread_num == 0)
+		(void)cookie_secret_file_dump(ssl, worker);
+	lock_basic_unlock(&cookie_secrets->lock);
+	send_ok(ssl);
+}
+
+/** Drop cookie secret */
+static void
+do_drop_cookie_secret(RES* ssl, struct worker* worker) {
+	char const* secret_file = worker->env.cfg->cookie_secret_file;
+	struct cookie_secrets* cookie_secrets = worker->daemon->cookie_secrets;
+
+	if(secret_file == NULL || secret_file[0] == 0) {
+		(void)ssl_printf(ssl, "error: no cookie secret file configured\n");
+		return;
+	}
+	if(cookie_secrets == NULL) {
+		(void)ssl_printf(ssl, "error: there are no cookie_secrets.");
+		return;
+	}
+	lock_basic_lock(&cookie_secrets->lock);
+
+	if(cookie_secrets->cookie_count <= 1 ) {
+		lock_basic_unlock(&cookie_secrets->lock);
+		(void)ssl_printf(ssl, "error: can not drop the currently active cookie secret\n");
+		return;
+	}
+	/* Only the worker 0 writes to file, the others update state. */
+	if(worker->thread_num == 0 && !cookie_secret_file_dump(ssl, worker)) {
+		lock_basic_unlock(&cookie_secrets->lock);
+		(void)ssl_printf(ssl, "error: writing to cookie secret file: \"%s\"\n",
+				secret_file);
+		return;
+	}
+	drop_cookie_secret(cookie_secrets);
+	if(worker->thread_num == 0)
+		(void)cookie_secret_file_dump(ssl, worker);
+	lock_basic_unlock(&cookie_secrets->lock);
+	send_ok(ssl);
+}
+
+/** Add cookie secret */
+static void
+do_add_cookie_secret(RES* ssl, struct worker* worker, char* arg) {
+	uint8_t secret[UNBOUND_COOKIE_SECRET_SIZE];
+	char const* secret_file = worker->env.cfg->cookie_secret_file;
+	struct cookie_secrets* cookie_secrets = worker->daemon->cookie_secrets;
+
+	if(secret_file == NULL || secret_file[0] == 0) {
+		(void)ssl_printf(ssl, "error: no cookie secret file configured\n");
+		return;
+	}
+	if(cookie_secrets == NULL) {
+		worker->daemon->cookie_secrets = cookie_secrets_create();
+		if(!worker->daemon->cookie_secrets) {
+			(void)ssl_printf(ssl, "error: out of memory");
+			return;
+		}
+		cookie_secrets = worker->daemon->cookie_secrets;
+	}
+	lock_basic_lock(&cookie_secrets->lock);
+
+	if(*arg == '\0') {
+		lock_basic_unlock(&cookie_secrets->lock);
+		(void)ssl_printf(ssl, "error: missing argument (cookie_secret)\n");
+		return;
+	}
+	if(strlen(arg) != 32) {
+		lock_basic_unlock(&cookie_secrets->lock);
+		explicit_bzero(arg, strlen(arg));
+		(void)ssl_printf(ssl, "invalid cookie secret: invalid argument length\n");
+		(void)ssl_printf(ssl, "please provide a 128bit hex encoded secret\n");
+		return;
+	}
+	if(hex_pton(arg, secret, UNBOUND_COOKIE_SECRET_SIZE) !=
+		UNBOUND_COOKIE_SECRET_SIZE ) {
+		lock_basic_unlock(&cookie_secrets->lock);
+		explicit_bzero(secret, UNBOUND_COOKIE_SECRET_SIZE);
+		explicit_bzero(arg, strlen(arg));
+		(void)ssl_printf(ssl, "invalid cookie secret: parse error\n");
+		(void)ssl_printf(ssl, "please provide a 128bit hex encoded secret\n");
+		return;
+	}
+	/* Only the worker 0 writes to file, the others update state. */
+	if(worker->thread_num == 0 && !cookie_secret_file_dump(ssl, worker)) {
+		lock_basic_unlock(&cookie_secrets->lock);
+		explicit_bzero(secret, UNBOUND_COOKIE_SECRET_SIZE);
+		explicit_bzero(arg, strlen(arg));
+		(void)ssl_printf(ssl, "error: writing to cookie secret file: \"%s\"\n",
+				secret_file);
+		return;
+	}
+	add_cookie_secret(cookie_secrets, secret, UNBOUND_COOKIE_SECRET_SIZE);
+	explicit_bzero(secret, UNBOUND_COOKIE_SECRET_SIZE);
+	if(worker->thread_num == 0)
+		(void)cookie_secret_file_dump(ssl, worker);
+	lock_basic_unlock(&cookie_secrets->lock);
+	explicit_bzero(arg, strlen(arg));
+	send_ok(ssl);
+}
+
+/** Print cookie secrets */
+static void
+do_print_cookie_secrets(RES* ssl, struct worker* worker) {
+	struct cookie_secrets* cookie_secrets = worker->daemon->cookie_secrets;
+	char secret_hex[UNBOUND_COOKIE_SECRET_SIZE * 2 + 1];
+	int i;
+
+	if(!cookie_secrets)
+		return; /* Output is empty. */
+	lock_basic_lock(&cookie_secrets->lock);
+	/* (void)ssl_printf(ssl, "cookie_secret_count=%zu\n", cookie_secrets->cookie_count); */
+	for(i = 0; (size_t)i < cookie_secrets->cookie_count; i++) {
+		struct cookie_secret const* cs = &cookie_secrets->
+			cookie_secrets[i];
+		ssize_t const len = hex_ntop(cs->cookie_secret,
+			UNBOUND_COOKIE_SECRET_SIZE, secret_hex,
+			sizeof(secret_hex));
+		(void)len; /* silence unused variable warning with -DNDEBUG */
+		log_assert( len == UNBOUND_COOKIE_SECRET_SIZE * 2 );
+		secret_hex[UNBOUND_COOKIE_SECRET_SIZE * 2] = '\0';
+		if (i == 0)
+			(void)ssl_printf(ssl, "active : %s\n",  secret_hex);
+		else if (cookie_secrets->cookie_count == 2)
+			(void)ssl_printf(ssl, "staging: %s\n",  secret_hex);
+		else
+			(void)ssl_printf(ssl, "staging[%d]: %s\n", i,
+				secret_hex);
+	}
+	lock_basic_unlock(&cookie_secrets->lock);
+	explicit_bzero(secret_hex, sizeof(secret_hex));
+}
+
 /** check for name with end-of-string, space or tab after it */
 static int
 cmdcmp(char* p, const char* cmd, size_t len)
@@ -3325,6 +3532,9 @@ execute_cmd(struct daemon_remote* rc, RES* ssl, char* cmd,
 	} else if(cmdcmp(p, "view_local_datas", 16)) {
 		do_view_datas_add(rc, ssl, worker, skipwhite(p+16));
 		return;
+	} else if(cmdcmp(p, "print_cookie_secrets", 20)) {
+		do_print_cookie_secrets(ssl, worker);
+		return;
 	}
 
 #ifdef THREADS_DISABLED
@@ -3389,6 +3599,12 @@ execute_cmd(struct daemon_remote* rc, RES* ssl, char* cmd,
 		do_rpz_enable(ssl, worker, skipwhite(p+10));
 	} else if(cmdcmp(p, "rpz_disable", 11)) {
 		do_rpz_disable(ssl, worker, skipwhite(p+11));
+	} else if(cmdcmp(p, "add_cookie_secret", 17)) {
+		do_add_cookie_secret(ssl, worker, skipwhite(p+17));
+	} else if(cmdcmp(p, "drop_cookie_secret", 18)) {
+		do_drop_cookie_secret(ssl, worker);
+	} else if(cmdcmp(p, "activate_cookie_secret", 22)) {
+		do_activate_cookie_secret(ssl, worker);
 	} else {
 		(void)ssl_printf(ssl, "error unknown command '%s'\n", p);
 	}
diff --git a/doc/unbound-control.8.in b/doc/unbound-control.8.in
index 8d98d05c8..862a2ddd1 100644
--- a/doc/unbound-control.8.in
+++ b/doc/unbound-control.8.in
@@ -350,6 +350,41 @@ Remove a list of \fIlocal_data\fR for given view from stdin. Like local_datas_re
 .TP
 .B view_local_datas \fIview\fR
 Add a list of \fIlocal_data\fR for given view from stdin.  Like local_datas.
+.TP
+.B add_cookie_secret <secret>
+Add or replace a cookie secret persistently. <secret> needs to be an 128 bit
+hex string.
+.IP
+Cookie secrets can be either \fIactive\fR or \fIstaging\fR. \fIActive\fR cookie
+secrets are used to create DNS Cookies, but verification of a DNS Cookie
+succeeds with any of the \fIactive\fR or \fIstaging\fR cookie secrets. The
+state of the current cookie secrets can be printed with the
+\fBprint_cookie_secrets\fR command.
+.IP
+When there are no cookie secrets configured yet, the <secret> is added as
+\fIactive\fR. If there is already an \fIactive\fR cookie secret, the <secret>
+is added as \fIstaging\fR or replacing an existing \fIstaging\fR secret.
+.IP
+To "roll" a cookie secret used in an anycast set. The new secret has to be
+added as staging secret to \fBall\fR nodes in the anycast set. When \fBall\fR
+nodes can verify DNS Cookies with the new secret, the new secret can be
+activated with the \fBactivate_cookie_secret\fR command. After \fBall\fR nodes
+have the new secret \fIactive\fR for at least one hour, the previous secret can
+be dropped with the \fBdrop_cookie_secret\fR command.
+.IP
+Persistence is accomplished by writing to a file which if configured with the
+\fBcookie\-secret\-file\fR option in the server section of the config file.
+This is default disabled, "".
+.TP
+.B drop_cookie_secret
+Drop the \fIstaging\fR cookie secret.
+.TP
+.B activate_cookie_secret
+Make the current \fIstaging\fR cookie secret \fIactive\fR, and the current
+\fIactive\fR cookie secret \fIstaging\fR.
+.TP
+.B print_cookie_secrets
+Show the current configured cookie secrets with their status.
 .SH "EXIT CODE"
 The unbound\-control program exits with status code 1 on error, 0 on success.
 .SH "SET UP"
diff --git a/smallapp/unbound-control.c b/smallapp/unbound-control.c
index 2a0cd688e..20a4e1d2b 100644
--- a/smallapp/unbound-control.c
+++ b/smallapp/unbound-control.c
@@ -186,6 +186,10 @@ usage(void)
 	printf("  rpz_enable zone		Enable the RPZ zone if it had previously\n");
 	printf("  				been disabled\n");
 	printf("  rpz_disable zone		Disable the RPZ zone\n");
+	printf("  add_cookie_secret <secret>	add (or replace) a new cookie secret <secret>\n");
+	printf("  drop_cookie_secret		drop a staging cookie secret\n");
+	printf("  activate_cookie_secret	make a staging cookie secret active\n");
+	printf("  print_cookie_secrets		show all cookie secrets with their status\n");
 	printf("Version %s\n", PACKAGE_VERSION);
 	printf("BSD licensed, see LICENSE in source package for details.\n");
 	printf("Report bugs to %s\n", PACKAGE_BUGREPORT);
diff --git a/testcode/unitmain.c b/testcode/unitmain.c
index 1ddc56750..084c12b93 100644
--- a/testcode/unitmain.c
+++ b/testcode/unitmain.c
@@ -1117,7 +1117,7 @@ static void edns_ede_encode_encodedecode(struct query_info* qinfo,
 	sldns_buffer_skip(pkt, 2 + 2);
 	/* decode */
 	unit_assert(parse_edns_from_query_pkt(pkt, edns, NULL, NULL, NULL, 0,
-		region) == 0);
+		region, NULL) == 0);
 }
 
 static void edns_ede_encode_check(struct edns_data* edns, int* found_ede,
diff --git a/util/edns.c b/util/edns.c
index 45d6a3849..fbbf25cfd 100644
--- a/util/edns.c
+++ b/util/edns.c
@@ -280,19 +280,93 @@ cookie_secrets_server_validate(const uint8_t* cookie, size_t cookie_len,
 	size_t i;
 	enum edns_cookie_val_status cookie_val_status,
 		last = COOKIE_STATUS_INVALID;
-	if(!cookie_secrets || cookie_secrets->cookie_count == 0)
+	if(!cookie_secrets)
+		return COOKIE_STATUS_INVALID; /* There are no cookie secrets.*/
+	lock_basic_lock(&cookie_secrets->lock);
+	if(cookie_secrets->cookie_count == 0) {
+		lock_basic_unlock(&cookie_secrets->lock);
 		return COOKIE_STATUS_INVALID; /* There are no cookie secrets.*/
+	}
 	for(i=0; i<cookie_secrets->cookie_count; i++) {
 		cookie_val_status = edns_cookie_server_validate(cookie,
 			cookie_len,
 			cookie_secrets->cookie_secrets[i].cookie_secret,
 			UNBOUND_COOKIE_SECRET_SIZE, v4, hash_input, now);
 		if(cookie_val_status == COOKIE_STATUS_VALID ||
-			cookie_val_status == COOKIE_STATUS_VALID_RENEW)
+			cookie_val_status == COOKIE_STATUS_VALID_RENEW) {
+			lock_basic_unlock(&cookie_secrets->lock);
 			return cookie_val_status;
+		}
 		if(last == COOKIE_STATUS_INVALID)
 			last = cookie_val_status; /* Store more interesting
 				failure to return. */
 	}
+	lock_basic_unlock(&cookie_secrets->lock);
 	return last;
 }
+
+void add_cookie_secret(struct cookie_secrets* cookie_secrets,
+	uint8_t* secret, size_t secret_len)
+{
+	log_assert(secret_len == UNBOUND_COOKIE_SECRET_SIZE);
+	(void)secret_len;
+	if(!cookie_secrets)
+		return;
+
+	/* New cookie secret becomes the staging secret (position 1)
+	 * unless there is no active cookie yet, then it becomes the active
+	 * secret.  If the UNBOUND_COOKIE_HISTORY_SIZE > 2 then all staging cookies
+	 * are moved one position down.
+	 */
+	if(cookie_secrets->cookie_count == 0) {
+		memcpy( cookie_secrets->cookie_secrets->cookie_secret
+		       , secret, UNBOUND_COOKIE_SECRET_SIZE);
+		cookie_secrets->cookie_count = 1;
+		explicit_bzero(secret, UNBOUND_COOKIE_SECRET_SIZE);
+		return;
+	}
+#if UNBOUND_COOKIE_HISTORY_SIZE > 2
+	memmove( &cookie_secrets->cookie_secrets[2], &cookie_secrets->cookie_secrets[1]
+	       , sizeof(struct cookie_secret) * (UNBOUND_COOKIE_HISTORY_SIZE - 2));
+#endif
+	memcpy( cookie_secrets->cookie_secrets[1].cookie_secret
+	      , secret, UNBOUND_COOKIE_SECRET_SIZE);
+	cookie_secrets->cookie_count = cookie_secrets->cookie_count     < UNBOUND_COOKIE_HISTORY_SIZE
+	                  ? cookie_secrets->cookie_count + 1 : UNBOUND_COOKIE_HISTORY_SIZE;
+	explicit_bzero(secret, UNBOUND_COOKIE_SECRET_SIZE);
+}
+
+void activate_cookie_secret(struct cookie_secrets* cookie_secrets)
+{
+	uint8_t active_secret[UNBOUND_COOKIE_SECRET_SIZE];
+	if(!cookie_secrets)
+		return;
+	/* The staging secret becomes the active secret.
+	 * The active secret becomes a staging secret.
+	 * If the UNBOUND_COOKIE_HISTORY_SIZE > 2 then all staging secrets are moved
+	 * one position up and the previously active secret becomes the last
+	 * staging secret.
+	 */
+	if(cookie_secrets->cookie_count < 2)
+		return;
+	memcpy( active_secret, cookie_secrets->cookie_secrets[0].cookie_secret
+	      , UNBOUND_COOKIE_SECRET_SIZE);
+	memmove( &cookie_secrets->cookie_secrets[0], &cookie_secrets->cookie_secrets[1]
+	       , sizeof(struct cookie_secret) * (UNBOUND_COOKIE_HISTORY_SIZE - 1));
+	memcpy( cookie_secrets->cookie_secrets[cookie_secrets->cookie_count - 1].cookie_secret
+	      , active_secret, UNBOUND_COOKIE_SECRET_SIZE);
+	explicit_bzero(active_secret, UNBOUND_COOKIE_SECRET_SIZE);
+}
+
+void drop_cookie_secret(struct cookie_secrets* cookie_secrets)
+{
+	if(!cookie_secrets)
+		return;
+	/* Drops a staging cookie secret. If there are more than one, it will
+	 * drop the last staging secret. */
+	if(cookie_secrets->cookie_count < 2)
+		return;
+	explicit_bzero( cookie_secrets->cookie_secrets[cookie_secrets->cookie_count - 1].cookie_secret
+	              , UNBOUND_COOKIE_SECRET_SIZE);
+	cookie_secrets->cookie_count -= 1;
+}
diff --git a/util/edns.h b/util/edns.h
index 75f1e3771..47ccb1ad2 100644
--- a/util/edns.h
+++ b/util/edns.h
@@ -229,4 +229,25 @@ enum edns_cookie_val_status cookie_secrets_server_validate(
 	struct cookie_secrets* cookie_secrets, int v4,
 	const uint8_t* hash_input, uint32_t now);
 
+/**
+ * Add a cookie secret. If there are no secrets yet, the secret will become
+ * the active secret. Otherwise it will become the staging secret.
+ * Active secrets are used to both verify and create new DNS Cookies.
+ * Staging secrets are only used to verify DNS Cookies. Caller has to lock.
+ */
+void add_cookie_secret(struct cookie_secrets* cookie_secrets, uint8_t* secret,
+	size_t secret_len);
+
+/**
+ * Makes the staging cookie secret active and the active secret staging.
+ * Caller has to lock.
+ */
+void activate_cookie_secret(struct cookie_secrets* cookie_secrets);
+
+/**
+ * Drop a cookie secret. Drops the staging secret. An active secret will not
+ * be dropped. Caller has to lock.
+ */
+void drop_cookie_secret(struct cookie_secrets* cookie_secrets);
+
 #endif
diff --git a/util/net_help.c b/util/net_help.c
index 2e76def78..3e3ac5bd3 100644
--- a/util/net_help.c
+++ b/util/net_help.c
@@ -1873,6 +1873,27 @@ sock_close(int socket)
 }
 #  endif /* USE_WINSOCK */
 
+ssize_t
+hex_ntop(uint8_t const *src, size_t srclength, char *target, size_t targsize)
+{
+	static char hexdigits[] = {
+		'0', '1', '2', '3', '4', '5', '6', '7',
+		'8', '9', 'a', 'b', 'c', 'd', 'e', 'f'
+	};
+	size_t i;
+
+	if (targsize < srclength * 2 + 1) {
+		return -1;
+	}
+
+	for (i = 0; i < srclength; ++i) {
+		*target++ = hexdigits[src[i] >> 4U];
+		*target++ = hexdigits[src[i] & 0xfU];
+	}
+	*target = '\0';
+	return 2 * srclength;
+}
+
 ssize_t
 hex_pton(const char* src, uint8_t* target, size_t targsize)
 {
diff --git a/util/net_help.h b/util/net_help.h
index 2a20a542a..28245ea0c 100644
--- a/util/net_help.h
+++ b/util/net_help.h
@@ -572,6 +572,12 @@ char* sock_strerror(int errn);
 /** close the socket with close, or wsa closesocket */
 void sock_close(int socket);
 
+/**
+ * Convert binary data to a string of hexadecimal characters.
+ */
+ssize_t hex_ntop(uint8_t const *src, size_t srclength, char *target,
+		 size_t targsize);
+
 /** Convert hexadecimal data to binary. */
 ssize_t hex_pton(const char* src, uint8_t* target, size_t targsize);
 

From f6050298692c431e9ddb57fbdce45751cda9c748 Mon Sep 17 00:00:00 2001
From: "W.C.A. Wijngaards" <wouter@nlnetlabs.nl>
Date: Fri, 14 Jun 2024 12:47:32 +0200
Subject: [PATCH 5/7] - cookie-secret-file, test and fix locks, renew writes a
 fresh cookie,   staging cookies get a fresh cookie and spelling in error
 message.

---
 daemon/daemon.c                               |   2 +-
 daemon/remote.c                               |   5 +-
 testdata/cookie_file.tdir/cookie_file.conf    |  23 +++
 testdata/cookie_file.tdir/cookie_file.dsc     |  16 ++
 testdata/cookie_file.tdir/cookie_file.post    |  11 ++
 testdata/cookie_file.tdir/cookie_file.pre     |  32 ++++
 testdata/cookie_file.tdir/cookie_file.test    |  11 ++
 .../cookie_file.test.scenario                 | 138 ++++++++++++++++++
 util/data/msgparse.c                          |  23 ++-
 util/edns.c                                   |   3 +
 10 files changed, 257 insertions(+), 7 deletions(-)
 create mode 100644 testdata/cookie_file.tdir/cookie_file.conf
 create mode 100644 testdata/cookie_file.tdir/cookie_file.dsc
 create mode 100644 testdata/cookie_file.tdir/cookie_file.post
 create mode 100644 testdata/cookie_file.tdir/cookie_file.pre
 create mode 100644 testdata/cookie_file.tdir/cookie_file.test
 create mode 100644 testdata/cookie_file.tdir/cookie_file.test.scenario

diff --git a/daemon/daemon.c b/daemon/daemon.c
index 4e2c51d5c..0e0db1063 100644
--- a/daemon/daemon.c
+++ b/daemon/daemon.c
@@ -724,7 +724,7 @@ daemon_fork(struct daemon* daemon)
 			fatal_exit("Could not create cookie_secrets: out of memory");
 		if(!cookie_secrets_apply_cfg(daemon->cookie_secrets,
 			daemon->cfg->cookie_secret_file))
-			fatal_exit("Could not setu up cookie_secrets");
+			fatal_exit("Could not setup cookie_secrets");
 	}
 	/* create global local_zones */
 	if(!(daemon->local_zones = local_zones_create()))
diff --git a/daemon/remote.c b/daemon/remote.c
index 5e84fd4ac..367017baf 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -3194,7 +3194,8 @@ do_rpz_disable(RES* ssl, struct worker* worker, char* arg)
     do_rpz_enable_disable(ssl, worker, arg, 0);
 }
 
-/* write the cookie secrets to file, returns `0` on failure */
+/** Write the cookie secrets to file, returns `0` on failure.
+ * Caller has to hold the lock. */
 static int
 cookie_secret_file_dump(RES* ssl, struct worker* worker) {
 	char const* secret_file = worker->env.cfg->cookie_secret_file;
@@ -3219,7 +3220,6 @@ cookie_secret_file_dump(RES* ssl, struct worker* worker) {
 		fclose(f);
 		return 1;
 	}
-	lock_basic_lock(&cookie_secrets->lock);
 
 	for(i = 0; i < cookie_secrets->cookie_count; i++) {
 		struct cookie_secret const* cs = &cookie_secrets->
@@ -3232,7 +3232,6 @@ cookie_secret_file_dump(RES* ssl, struct worker* worker) {
 		secret_hex[UNBOUND_COOKIE_SECRET_SIZE * 2] = '\0';
 		fprintf(f, "%s\n", secret_hex);
 	}
-	lock_basic_unlock(&cookie_secrets->lock);
 	explicit_bzero(secret_hex, sizeof(secret_hex));
 	fclose(f);
 	return 1;
diff --git a/testdata/cookie_file.tdir/cookie_file.conf b/testdata/cookie_file.tdir/cookie_file.conf
new file mode 100644
index 000000000..63d2b8315
--- /dev/null
+++ b/testdata/cookie_file.tdir/cookie_file.conf
@@ -0,0 +1,23 @@
+server:
+	verbosity: 7
+	use-syslog: no
+	directory: ""
+	pidfile: "unbound.pid"
+	chroot: ""
+	username: ""
+	do-not-query-localhost: no
+	use-caps-for-id: no
+	#port: @SERVER_PORT@
+	interface: lo
+	access-control: ::0/0 allow
+	cookie-secret-file: "cookie_secrets.txt"
+	answer-cookie: yes
+
+auth-zone:
+	name: "."
+	for-downstream: yes
+
+remote-control:
+	control-enable: yes
+	control-interface: @CONTROL_PATH@/controlpipe.@CONTROL_PID@
+	control-use-cert: no
diff --git a/testdata/cookie_file.tdir/cookie_file.dsc b/testdata/cookie_file.tdir/cookie_file.dsc
new file mode 100644
index 000000000..4f321bd2e
--- /dev/null
+++ b/testdata/cookie_file.tdir/cookie_file.dsc
@@ -0,0 +1,16 @@
+BaseName: cookie_file
+Version: 1.0
+Description: Check the cookie rollover
+CreationDate: Fri 14 Jun 11:00:00 CEST 2024
+Maintainer:
+Category:
+Component:
+CmdDepends:
+Depends:
+Help:
+Pre: cookie_file.pre
+Post: cookie_file.post
+Test: cookie_file.test
+AuxFiles:
+Passed:
+Failure:
diff --git a/testdata/cookie_file.tdir/cookie_file.post b/testdata/cookie_file.tdir/cookie_file.post
new file mode 100644
index 000000000..2aed848b4
--- /dev/null
+++ b/testdata/cookie_file.tdir/cookie_file.post
@@ -0,0 +1,11 @@
+# #-- cookie_file.post --#
+# source the master var file when it's there
+[ -f ../.tpkg.var.master ] && source ../.tpkg.var.master
+# source the test var file when it's there
+[ -f .tpkg.var.test ] && source .tpkg.var.test
+#
+# do your teardown here
+. ../common.sh
+kill_from_pidfile "unbound.pid"
+cat unbound.log
+rm -f $CONTROL_PATH/controlpipe.$CONTROL_PID
diff --git a/testdata/cookie_file.tdir/cookie_file.pre b/testdata/cookie_file.tdir/cookie_file.pre
new file mode 100644
index 000000000..d4f99da26
--- /dev/null
+++ b/testdata/cookie_file.tdir/cookie_file.pre
@@ -0,0 +1,32 @@
+# #-- cookie_file.pre--#
+PRE="../.."
+. ../common.sh
+
+# This test uses the unshare utility
+if test ! -x "`which unshare 2>&1`"; then
+	skip_test "no unshare (from util-linux package) available, skip test"
+fi
+
+get_random_port 1
+SERVER_PORT=$RND_PORT
+CONTROL_PATH=/tmp
+CONTROL_PID=$$
+
+# make config file
+sed \
+	-e 's/@SERVER_PORT\@/'$SERVER_PORT'/' \
+	-e 's?@CONTROL_PATH\@?'$CONTROL_PATH'?' \
+	-e 's/@CONTROL_PID\@/'$CONTROL_PID'/' \
+	< cookie_file.conf > ub.conf
+
+if test -x "`which bash`"; then
+	shell="bash"
+else
+	shell="sh"
+fi
+
+echo "SERVER_PORT=$SERVER_PORT" >> .tpkg.var.test
+echo "CONTROL_PORT=$CONTROL_PORT" >> .tpkg.var.test
+echo "CONTROL_PATH=$CONTROL_PATH" >> .tpkg.var.test
+echo "CONTROL_PID=$CONTROL_PID" >> .tpkg.var.test
+echo "shell=$shell" >> .tpkg.var.test
diff --git a/testdata/cookie_file.tdir/cookie_file.test b/testdata/cookie_file.tdir/cookie_file.test
new file mode 100644
index 000000000..8ae591d34
--- /dev/null
+++ b/testdata/cookie_file.tdir/cookie_file.test
@@ -0,0 +1,11 @@
+# #-- cookie_file.test --#
+# source the master var file when it's there
+[ -f ../.tpkg.var.master ] && source ../.tpkg.var.master
+# use .tpkg.var.test for in test variable passing
+[ -f .tpkg.var.test ] && source .tpkg.var.test
+PRE="../.."
+. ../common.sh
+
+# Run the scenario in an unshared namespace
+unshare -rUn $shell cookie_file.test.scenario
+exit $?
diff --git a/testdata/cookie_file.tdir/cookie_file.test.scenario b/testdata/cookie_file.tdir/cookie_file.test.scenario
new file mode 100644
index 000000000..4746133d5
--- /dev/null
+++ b/testdata/cookie_file.tdir/cookie_file.test.scenario
@@ -0,0 +1,138 @@
+# #-- cookie_file.test.scenario --#
+# source the master var file when it's there
+[ -f ../.tpkg.var.master ] && source ../.tpkg.var.master
+# use .tpkg.var.test for in test variable passing
+[ -f .tpkg.var.test ] && source .tpkg.var.test
+PRE="../.."
+. ../common.sh
+
+# set environment interfaces
+ip address add 2001:db8:220:1:59de:d0f4:8769:82b8 dev lo
+ip link set dev lo up
+
+# empty cookie file
+touch cookie_secrets.txt
+
+# start unbound in the background
+# start with faketime
+TZ=UTC faketime -f '2019-06-05 13:39:21' $PRE/unbound -d -c ub.conf >unbound.log 2>&1 &
+
+cat .tpkg.var.test
+wait_unbound_up unbound.log
+
+# add secret
+echo ">> add_cookie_secret dd3bdf9344b678b185a6f5cb60fca715"
+$PRE/unbound-control -c ub.conf add_cookie_secret dd3bdf9344b678b185a6f5cb60fca715
+echo ">> add_cookie_secret 445536bcd2513298075a5d379663c962"
+$PRE/unbound-control -c ub.conf add_cookie_secret 445536bcd2513298075a5d379663c962
+
+$PRE/unbound-control -c ub.conf print_cookie_secrets > cookie_secrets.out
+if ! grep -q "445536bcd2513298075a5d379663c962" cookie_secrets.out; then
+	sleep 1;
+	$PRE/unbound-control -c ub.conf print_cookie_secrets > cookie_secrets.out
+fi
+if ! grep -q "445536bcd2513298075a5d379663c962" cookie_secrets.out; then
+	sleep 1;
+	$PRE/unbound-control -c ub.conf print_cookie_secrets > cookie_secrets.out
+fi
+if ! grep -q "445536bcd2513298075a5d379663c962" cookie_secrets.out; then
+	sleep 1;
+	$PRE/unbound-control -c ub.conf print_cookie_secrets > cookie_secrets.out
+fi
+if ! grep -q "445536bcd2513298075a5d379663c962" cookie_secrets.out \
+	|| ! grep -q "dd3bdf9344b678b185a6f5cb60fca715" cookie_secrets.out
+	then
+		cat cookie_secrets.out
+		echo "Cookies did not get provisioned"
+		exit 1
+fi
+echo ">> print_cookie_secrets"
+cat cookie_secrets.out
+
+echo ">> cookie_secrets.txt"
+cat cookie_secrets.txt
+
+# check this valid cookie before rollover
+dig -b 2001:db8:220:1:59de:d0f4:8769:82b8 @2001:db8:220:1:59de:d0f4:8769:82b8 +cookie=22681ab97d52c298010000005cf7c57926556bd0934c72f8 > dig.output.1
+cat dig.output.1
+
+if grep -q "22681ab97d52c298010000005cf7c57926556bd0934c72f8" dig.output.1
+then
+	echo "IPv6 Query with Rolled Over Secret: Cookie matches pre-rollover"
+else
+	echo "IPv6 Query with Rolled Over Secret: Cookie does not match pre-rollover"
+	exit 1
+fi
+
+# secret rollover
+$PRE/unbound-control -c ub.conf activate_cookie_secret
+
+$PRE/unbound-control -c ub.conf print_cookie_secrets > cookie_secrets.2
+if ! grep -q "^active.*445536bcd2513298075a5d379663c962" cookie_secrets.2
+then
+	sleep 1
+	$PRE/unbound-control -c ub.conf print_cookie_secrets > cookie_secrets.2
+fi
+if ! grep -q "^active.*445536bcd2513298075a5d379663c962" cookie_secrets.2
+then
+	sleep 1
+	$PRE/unbound-control -c ub.conf print_cookie_secrets > cookie_secrets.2
+fi
+if ! grep -q "^active.*445536bcd2513298075a5d379663c962" cookie_secrets.2
+then
+	sleep 1
+	$PRE/unbound-control -c ub.conf print_cookie_secrets > cookie_secrets.2
+fi
+if ! grep -q "^active.*445536bcd2513298075a5d379663c962" cookie_secrets.2 
+then
+	cat cookie_secrets.2
+	echo "Cookie did not get activated"
+	exit 1
+fi
+echo ">> activate cookie secret, printout"
+cat cookie_secrets.2
+echo ">> cookie_secrets.txt"
+cat cookie_secrets.txt
+
+dig -b 2001:db8:220:1:59de:d0f4:8769:82b8 @2001:db8:220:1:59de:d0f4:8769:82b8 +cookie=22681ab97d52c298010000005cf7c57926556bd0934c72f8 > dig.output.2
+cat dig.output.2
+
+if grep -q "22681ab97d52c298010000005cf7c609a6bb79d16625507a" dig.output.2
+then
+	echo "IPv6 Query with Rolled Over Secret: Cookie matches after rollover"
+else
+	echo "IPv6 Query with Rolled Over Secret: Cookie does not match after rollover"
+	exit 1
+fi
+
+echo ">> drop cookie secret"
+$PRE/unbound-control -c ub.conf drop_cookie_secret
+
+$PRE/unbound-control -c ub.conf print_cookie_secrets > cookie_secrets.3
+if grep -q "^staging.*dd3bdf9344b678b185a6f5cb60fca715" cookie_secrets.3
+then
+	sleep 1
+	$PRE/unbound-control -c ub.conf print_cookie_secrets > cookie_secrets.3
+fi
+if grep -q "^staging.*dd3bdf9344b678b185a6f5cb60fca715" cookie_secrets.3
+then
+	sleep 1
+	$PRE/unbound-control -c ub.conf print_cookie_secrets > cookie_secrets.3
+fi
+if grep -q "^staging.*dd3bdf9344b678b185a6f5cb60fca715" cookie_secrets.3
+then
+	sleep 1
+	$PRE/unbound-control -c ub.conf print_cookie_secrets > cookie_secrets.3
+fi
+if grep -q "^staging.*dd3bdf9344b678b185a6f5cb60fca715" cookie_secrets.3
+then
+	cat cookie_secrets.3
+	echo "Cookie did not get dropped"
+	exit 1
+fi
+echo ">> drop cookie secret, printout"
+cat cookie_secrets.3
+echo ">> cookie_secrets.txt"
+cat cookie_secrets.txt
+
+exit 0
diff --git a/util/data/msgparse.c b/util/data/msgparse.c
index 4fb460e90..8a7c87ef6 100644
--- a/util/data/msgparse.c
+++ b/util/data/msgparse.c
@@ -1085,9 +1085,10 @@ parse_edns_options_from_query(uint8_t* rdata_ptr, size_t rdata_len,
 					cfg->cookie_secret_len, cookie_is_v4,
 					server_cookie, now);
 			}
+			if(cookie_val_status == COOKIE_STATUS_VALID_RENEW)
+				edns->cookie_valid = 1;
 			switch(cookie_val_status) {
 			case COOKIE_STATUS_VALID:
-			case COOKIE_STATUS_VALID_RENEW:
 				edns->cookie_valid = 1;
 				/* Reuse cookie */
 				if(!edns_opt_list_append(
@@ -1103,12 +1104,28 @@ parse_edns_options_from_query(uint8_t* rdata_ptr, size_t rdata_len,
 			case COOKIE_STATUS_CLIENT_ONLY:
 				edns->cookie_client = 1;
 				/* fallthrough */
+			case COOKIE_STATUS_VALID_RENEW:
 			case COOKIE_STATUS_FUTURE:
 			case COOKIE_STATUS_EXPIRED:
 			case COOKIE_STATUS_INVALID:
 			default:
-				edns_cookie_server_write(server_cookie,
-					cfg->cookie_secret, cookie_is_v4, now);
+				if(cfg->cookie_secret_file &&
+					cfg->cookie_secret_file[0]) {
+					if(!cookie_secrets)
+						break;
+					lock_basic_lock(&cookie_secrets->lock);
+					if(cookie_secrets->cookie_count < 1) {
+						lock_basic_unlock(&cookie_secrets->lock);
+						break;
+					}
+					edns_cookie_server_write(server_cookie,
+						cookie_secrets->cookie_secrets[0].cookie_secret,
+						cookie_is_v4, now);
+					lock_basic_unlock(&cookie_secrets->lock);
+				} else {
+					edns_cookie_server_write(server_cookie,
+						cfg->cookie_secret, cookie_is_v4, now);
+				}
 				if(!edns_opt_list_append(&edns->opt_list_out,
 					LDNS_EDNS_COOKIE, 24, server_cookie,
 					region)) {
diff --git a/util/edns.c b/util/edns.c
index fbbf25cfd..ee95a6912 100644
--- a/util/edns.c
+++ b/util/edns.c
@@ -295,6 +295,9 @@ cookie_secrets_server_validate(const uint8_t* cookie, size_t cookie_len,
 		if(cookie_val_status == COOKIE_STATUS_VALID ||
 			cookie_val_status == COOKIE_STATUS_VALID_RENEW) {
 			lock_basic_unlock(&cookie_secrets->lock);
+			/* For staging cookies, write a fresh cookie. */
+			if(i != 0)
+				return COOKIE_STATUS_VALID_RENEW;
 			return cookie_val_status;
 		}
 		if(last == COOKIE_STATUS_INVALID)

From 57bb0b2ae3bbea2aaa96a93de307396e087337db Mon Sep 17 00:00:00 2001
From: "W.C.A. Wijngaards" <wouter@nlnetlabs.nl>
Date: Fri, 14 Jun 2024 12:49:41 +0200
Subject: [PATCH 6/7] - cookie-secret-file, remove unused variable from cookie
 file unit test.

---
 testdata/cookie_file.tdir/cookie_file.pre | 1 -
 1 file changed, 1 deletion(-)

diff --git a/testdata/cookie_file.tdir/cookie_file.pre b/testdata/cookie_file.tdir/cookie_file.pre
index d4f99da26..c5104d71c 100644
--- a/testdata/cookie_file.tdir/cookie_file.pre
+++ b/testdata/cookie_file.tdir/cookie_file.pre
@@ -26,7 +26,6 @@ else
 fi
 
 echo "SERVER_PORT=$SERVER_PORT" >> .tpkg.var.test
-echo "CONTROL_PORT=$CONTROL_PORT" >> .tpkg.var.test
 echo "CONTROL_PATH=$CONTROL_PATH" >> .tpkg.var.test
 echo "CONTROL_PID=$CONTROL_PID" >> .tpkg.var.test
 echo "shell=$shell" >> .tpkg.var.test

From 2e9e9f8201d2bed9e0f653a2ba313aed0ba7dcbc Mon Sep 17 00:00:00 2001
From: Yorgos Thessalonikefs <yorgos@nlnetlabs.nl>
Date: Thu, 1 Aug 2024 21:59:31 +0200
Subject: [PATCH 7/7] Remove unshare and faketime dependencies for cookie_file
 test; documentation nits.

---
 daemon/remote.c                               |   1 -
 doc/unbound-control.8.in                      |   2 +-
 doc/unbound.conf.5.in                         |   7 +-
 testdata/cookie_file.tdir/cookie_file.conf    |  12 +-
 testdata/cookie_file.tdir/cookie_file.post    |   1 -
 testdata/cookie_file.tdir/cookie_file.pre     |  31 +--
 testdata/cookie_file.tdir/cookie_file.test    | 243 +++++++++++++++++-
 .../cookie_file.test.scenario                 | 138 ----------
 8 files changed, 261 insertions(+), 174 deletions(-)
 delete mode 100644 testdata/cookie_file.tdir/cookie_file.test.scenario

diff --git a/daemon/remote.c b/daemon/remote.c
index 367017baf..d025122df 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -3377,7 +3377,6 @@ do_print_cookie_secrets(RES* ssl, struct worker* worker) {
 	if(!cookie_secrets)
 		return; /* Output is empty. */
 	lock_basic_lock(&cookie_secrets->lock);
-	/* (void)ssl_printf(ssl, "cookie_secret_count=%zu\n", cookie_secrets->cookie_count); */
 	for(i = 0; (size_t)i < cookie_secrets->cookie_count; i++) {
 		struct cookie_secret const* cs = &cookie_secrets->
 			cookie_secrets[i];
diff --git a/doc/unbound-control.8.in b/doc/unbound-control.8.in
index 862a2ddd1..17073f938 100644
--- a/doc/unbound-control.8.in
+++ b/doc/unbound-control.8.in
@@ -374,7 +374,7 @@ be dropped with the \fBdrop_cookie_secret\fR command.
 .IP
 Persistence is accomplished by writing to a file which if configured with the
 \fBcookie\-secret\-file\fR option in the server section of the config file.
-This is default disabled, "".
+This is disabled by default, "".
 .TP
 .B drop_cookie_secret
 Drop the \fIstaging\fR cookie secret.
diff --git a/doc/unbound.conf.5.in b/doc/unbound.conf.5.in
index bf2e2d1ba..1e26f9eea 100644
--- a/doc/unbound.conf.5.in
+++ b/doc/unbound.conf.5.in
@@ -1985,12 +1985,13 @@ calculations.
 .B cookie\-secret\-file: \fI<filename>
 File from which the secrets are read used in DNS Cookie calculations. When this
 file exists, the secrets in this file are used and the secret specified by the
-\fBcookie-secret\fR option is ignored. Default is "". With "" it is disabled.
+\fBcookie-secret\fR option is ignored.
 Enable it by setting a filename, like "/usr/local/etc/unbound_cookiesecrets.txt".
 The content of this file must be manipulated with the \fBadd_cookie_secret\fR,
 \fBdrop_cookie_secret\fR and \fBactivate_cookie_secret\fR commands to the
-\fIunbound\-control\fR(8) tool. Please see that manpage how to perform a safe
-cookie secret rollover.
+\fIunbound\-control\fR(8) tool. Please see that manpage on how to perform a
+safe cookie secret rollover.
+Default is "" (disabled).
 .TP 5
 .B edns\-client\-string: \fI<IP netblock> <string>
 Include an EDNS0 option containing configured ascii string in queries with
diff --git a/testdata/cookie_file.tdir/cookie_file.conf b/testdata/cookie_file.tdir/cookie_file.conf
index 63d2b8315..25dd93f52 100644
--- a/testdata/cookie_file.tdir/cookie_file.conf
+++ b/testdata/cookie_file.tdir/cookie_file.conf
@@ -7,17 +7,13 @@ server:
 	username: ""
 	do-not-query-localhost: no
 	use-caps-for-id: no
-	#port: @SERVER_PORT@
-	interface: lo
-	access-control: ::0/0 allow
+	port: @SERVER_PORT@
+	interface: 127.0.0.1
 	cookie-secret-file: "cookie_secrets.txt"
 	answer-cookie: yes
-
-auth-zone:
-	name: "."
-	for-downstream: yes
+	access-control: 127.0.0.0/8 allow_cookie  # BADCOOKIE for incomplete/invalid cookies
 
 remote-control:
 	control-enable: yes
-	control-interface: @CONTROL_PATH@/controlpipe.@CONTROL_PID@
+	control-port: @CONTROL_PORT@
 	control-use-cert: no
diff --git a/testdata/cookie_file.tdir/cookie_file.post b/testdata/cookie_file.tdir/cookie_file.post
index 2aed848b4..b64af9cbd 100644
--- a/testdata/cookie_file.tdir/cookie_file.post
+++ b/testdata/cookie_file.tdir/cookie_file.post
@@ -8,4 +8,3 @@
 . ../common.sh
 kill_from_pidfile "unbound.pid"
 cat unbound.log
-rm -f $CONTROL_PATH/controlpipe.$CONTROL_PID
diff --git a/testdata/cookie_file.tdir/cookie_file.pre b/testdata/cookie_file.tdir/cookie_file.pre
index c5104d71c..61da5425a 100644
--- a/testdata/cookie_file.tdir/cookie_file.pre
+++ b/testdata/cookie_file.tdir/cookie_file.pre
@@ -2,30 +2,23 @@
 PRE="../.."
 . ../common.sh
 
-# This test uses the unshare utility
-if test ! -x "`which unshare 2>&1`"; then
-	skip_test "no unshare (from util-linux package) available, skip test"
-fi
-
-get_random_port 1
+get_random_port 2
 SERVER_PORT=$RND_PORT
-CONTROL_PATH=/tmp
-CONTROL_PID=$$
+CONTROL_PORT=$(($RND_PORT + 1))
+echo "SERVER_PORT=$SERVER_PORT" >> .tpkg.var.test
+echo "CONTROL_PORT=$CONTROL_PORT" >> .tpkg.var.test
 
 # make config file
 sed \
 	-e 's/@SERVER_PORT\@/'$SERVER_PORT'/' \
-	-e 's?@CONTROL_PATH\@?'$CONTROL_PATH'?' \
-	-e 's/@CONTROL_PID\@/'$CONTROL_PID'/' \
+	-e 's/@CONTROL_PORT\@/'$CONTROL_PORT'/' \
 	< cookie_file.conf > ub.conf
 
-if test -x "`which bash`"; then
-	shell="bash"
-else
-	shell="sh"
-fi
+# empty cookie file
+touch cookie_secrets.txt
 
-echo "SERVER_PORT=$SERVER_PORT" >> .tpkg.var.test
-echo "CONTROL_PATH=$CONTROL_PATH" >> .tpkg.var.test
-echo "CONTROL_PID=$CONTROL_PID" >> .tpkg.var.test
-echo "shell=$shell" >> .tpkg.var.test
+# start unbound in the background
+$PRE/unbound -d -c ub.conf > unbound.log 2>&1 &
+
+cat .tpkg.var.test
+wait_unbound_up unbound.log
diff --git a/testdata/cookie_file.tdir/cookie_file.test b/testdata/cookie_file.tdir/cookie_file.test
index 8ae591d34..7da4fa657 100644
--- a/testdata/cookie_file.tdir/cookie_file.test
+++ b/testdata/cookie_file.tdir/cookie_file.test
@@ -6,6 +6,243 @@
 PRE="../.."
 . ../common.sh
 
-# Run the scenario in an unshared namespace
-unshare -rUn $shell cookie_file.test.scenario
-exit $?
+first_secret=dd3bdf9344b678b185a6f5cb60fca715
+second_secret=445536bcd2513298075a5d379663c962
+
+
+teststep "Add first secret"
+echo ">> add_cookie_secret $first_secret"
+$PRE/unbound-control -c ub.conf add_cookie_secret $first_secret
+# check secret is persisted
+outfile=cookie_secrets.1
+$PRE/unbound-control -c ub.conf print_cookie_secrets > $outfile
+if ! grep -q "$first_secret" $outfile
+then
+	sleep 1
+	$PRE/unbound-control -c ub.conf print_cookie_secrets > $outfile
+fi
+if ! grep -q "$first_secret" $outfile
+then
+	sleep 1
+	$PRE/unbound-control -c ub.conf print_cookie_secrets > $outfile
+fi
+if ! grep -q "$first_secret" $outfile
+then
+	sleep 1
+	$PRE/unbound-control -c ub.conf print_cookie_secrets > $outfile
+fi
+if ! grep -q "^active.*$first_secret" $outfile
+then
+	cat $outfile
+	echo "First secret was not provisioned"
+	exit 1
+fi
+echo ">> print_cookie_secrets"
+cat $outfile
+
+
+teststep "Get a valid cookie for this secret"
+outfile=dig.output.1
+dig version.server ch txt @127.0.0.1 -p $SERVER_PORT +cookie=3132333435363738 > $outfile
+if ! grep -q "BADCOOKIE" $outfile
+then
+	cat $outfile
+	echo "Did not get a BADCOOKIE response for a client-only cookie"
+	exit 1
+fi
+if ! grep -q "COOKIE: 3132333435363738" $outfile
+then
+	cat $outfile
+	echo "Did not get a cookie in the response"
+	exit 1
+fi
+first_cookie=$(grep "; COOKIE:" $outfile | cut -d ' ' -f 3)
+cat $outfile
+echo "first cookie: $first_cookie"
+
+
+teststep "Verify the first cookie can be reused"
+outfile=dig.output.2
+dig version.server ch txt @127.0.0.1 -p $SERVER_PORT +cookie=$first_cookie > $outfile
+if grep -q "BADCOOKIE" $outfile
+then
+	cat $outfile
+	echo "Got BADCOOKIE response for a valid cookie"
+	exit 1
+fi
+if ! grep -q "COOKIE: $first_cookie" $outfile
+then
+	cat $outfile
+	echo "Did not get the same first cookie in the response"
+	exit 1
+fi
+
+
+teststep "Add second secret"
+outfile=cookie_secrets.2
+echo ">> add_cookie_secret $second_secret"
+$PRE/unbound-control -c ub.conf add_cookie_secret $second_secret
+$PRE/unbound-control -c ub.conf print_cookie_secrets > $outfile
+if ! grep -q "$second_secret" $outfile
+then
+	sleep 1
+	$PRE/unbound-control -c ub.conf print_cookie_secrets > $outfile
+fi
+if ! grep -q "$second_secret" $outfile
+then
+	sleep 1
+	$PRE/unbound-control -c ub.conf print_cookie_secrets > $outfile
+fi
+if ! grep -q "$second_secret" $outfile
+then
+	sleep 1
+	$PRE/unbound-control -c ub.conf print_cookie_secrets > $outfile
+fi
+if ! grep -q "^staging.*$second_secret" $outfile \
+	|| ! grep -q "^active.*$first_secret" $outfile
+then
+		cat $outfile
+		echo "Secrets were not provisioned"
+		exit 1
+fi
+echo ">> print_cookie_secrets"
+cat $outfile
+echo ">> cookie_secrets.txt"
+cat cookie_secrets.txt
+
+
+teststep "Verify the first cookie can be reused"
+outfile=dig.output.3
+dig version.server ch txt @127.0.0.1 -p $SERVER_PORT +cookie=$first_cookie > $outfile
+if grep -q "BADCOOKIE" $outfile
+then
+	cat $outfile
+	echo "Got BADCOOKIE response for a valid cookie"
+	exit 1
+fi
+if ! grep -q "COOKIE: $first_cookie" $outfile
+then
+	cat $outfile
+	echo "Did not get the same first cookie in the response"
+	exit 1
+fi
+
+
+teststep "Secret rollover"
+outfile=cookie_secrets.3
+$PRE/unbound-control -c ub.conf activate_cookie_secret
+$PRE/unbound-control -c ub.conf print_cookie_secrets > $outfile
+if ! grep -q "^active.*$second_secret" $outfile
+then
+	sleep 1
+	$PRE/unbound-control -c ub.conf print_cookie_secrets > $outfile
+fi
+if ! grep -q "^active.*$second_secret" $outfile
+then
+	sleep 1
+	$PRE/unbound-control -c ub.conf print_cookie_secrets > $outfile
+fi
+if ! grep -q "^active.*$second_secret" $outfile
+then
+	sleep 1
+	$PRE/unbound-control -c ub.conf print_cookie_secrets > $outfile
+fi
+if ! grep -q "^active.*$second_secret" $outfile \
+	|| ! grep -q "^staging.*$first_secret" $outfile
+then
+	cat $outfile
+	echo "Second secret was not activated"
+	exit 1
+fi
+echo ">> activate cookie secret, printout"
+cat $outfile
+echo ">> cookie_secrets.txt"
+cat cookie_secrets.txt
+
+
+teststep "Verify the first cookie can be reused but a new cookie is returned from the second secret"
+outfile=dig.output.4
+dig version.server ch txt @127.0.0.1 -p $SERVER_PORT +cookie=$first_cookie > $outfile
+if grep -q "BADCOOKIE" $outfile
+then
+	cat $outfile
+	echo "Got BADCOOKIE response for a valid cookie"
+	exit 1
+fi
+if ! grep -q "COOKIE: 3132333435363738" $outfile
+then
+	cat $outfile
+	echo "Did not get a cookie in the response"
+	exit 1
+fi
+if grep -q "COOKIE: $first_cookie" $outfile
+then
+	cat $outfile
+	echo "Got the same first cookie in the response while the second secret is active"
+	exit 1
+fi
+second_cookie=$(grep "; COOKIE:" $outfile | cut -d ' ' -f 3)
+cat $outfile
+echo "second cookie: $second_cookie"
+
+
+teststep "Drop cookie secret"
+outfile=cookie_secrets.4
+$PRE/unbound-control -c ub.conf drop_cookie_secret
+$PRE/unbound-control -c ub.conf print_cookie_secrets > $outfile
+if grep -q "^staging.*$first_secret" $outfile
+then
+	sleep 1
+	$PRE/unbound-control -c ub.conf print_cookie_secrets > $outfile
+fi
+if grep -q "^staging.*$first_secret" $outfile
+then
+	sleep 1
+	$PRE/unbound-control -c ub.conf print_cookie_secrets > $outfile
+fi
+if grep -q "^staging.*$first_secret" $outfile
+then
+	sleep 1
+	$PRE/unbound-control -c ub.conf print_cookie_secrets > $outfile
+fi
+if grep -q "^staging.*$first_secret" $outfile
+then
+	cat $outfile
+	echo "First secret was not dropped"
+	exit 1
+fi
+echo ">> drop cookie secret, printout"
+cat $outfile
+echo ">> cookie_secrets.txt"
+cat cookie_secrets.txt
+
+
+teststep "Verify the first cookie can not be reused and the second cookie is returned instead"
+outfile=dig.output.4
+dig version.server ch txt @127.0.0.1 -p $SERVER_PORT +cookie=$first_cookie > $outfile
+if ! grep -q "BADCOOKIE" $outfile
+then
+	cat $outfile
+	echo "Did not get BADCOOKIE response for an invalid cookie"
+	exit 1
+fi
+if ! grep -q "COOKIE: 3132333435363738" $outfile
+then
+	cat $outfile
+	echo "Did not get a cookie in the response"
+	exit 1
+fi
+if grep -q "COOKIE: $first_cookie" $outfile
+then
+	cat $outfile
+	echo "Got the same first cookie in the response while the second secret is active"
+	exit 1
+fi
+if ! grep -q "COOKIE: $second_cookie" $outfile
+then
+	cat $outfile
+	echo "Did not get the same second cookie in the response"
+	exit 1
+fi
+
+exit 0
diff --git a/testdata/cookie_file.tdir/cookie_file.test.scenario b/testdata/cookie_file.tdir/cookie_file.test.scenario
deleted file mode 100644
index 4746133d5..000000000
--- a/testdata/cookie_file.tdir/cookie_file.test.scenario
+++ /dev/null
@@ -1,138 +0,0 @@
-# #-- cookie_file.test.scenario --#
-# source the master var file when it's there
-[ -f ../.tpkg.var.master ] && source ../.tpkg.var.master
-# use .tpkg.var.test for in test variable passing
-[ -f .tpkg.var.test ] && source .tpkg.var.test
-PRE="../.."
-. ../common.sh
-
-# set environment interfaces
-ip address add 2001:db8:220:1:59de:d0f4:8769:82b8 dev lo
-ip link set dev lo up
-
-# empty cookie file
-touch cookie_secrets.txt
-
-# start unbound in the background
-# start with faketime
-TZ=UTC faketime -f '2019-06-05 13:39:21' $PRE/unbound -d -c ub.conf >unbound.log 2>&1 &
-
-cat .tpkg.var.test
-wait_unbound_up unbound.log
-
-# add secret
-echo ">> add_cookie_secret dd3bdf9344b678b185a6f5cb60fca715"
-$PRE/unbound-control -c ub.conf add_cookie_secret dd3bdf9344b678b185a6f5cb60fca715
-echo ">> add_cookie_secret 445536bcd2513298075a5d379663c962"
-$PRE/unbound-control -c ub.conf add_cookie_secret 445536bcd2513298075a5d379663c962
-
-$PRE/unbound-control -c ub.conf print_cookie_secrets > cookie_secrets.out
-if ! grep -q "445536bcd2513298075a5d379663c962" cookie_secrets.out; then
-	sleep 1;
-	$PRE/unbound-control -c ub.conf print_cookie_secrets > cookie_secrets.out
-fi
-if ! grep -q "445536bcd2513298075a5d379663c962" cookie_secrets.out; then
-	sleep 1;
-	$PRE/unbound-control -c ub.conf print_cookie_secrets > cookie_secrets.out
-fi
-if ! grep -q "445536bcd2513298075a5d379663c962" cookie_secrets.out; then
-	sleep 1;
-	$PRE/unbound-control -c ub.conf print_cookie_secrets > cookie_secrets.out
-fi
-if ! grep -q "445536bcd2513298075a5d379663c962" cookie_secrets.out \
-	|| ! grep -q "dd3bdf9344b678b185a6f5cb60fca715" cookie_secrets.out
-	then
-		cat cookie_secrets.out
-		echo "Cookies did not get provisioned"
-		exit 1
-fi
-echo ">> print_cookie_secrets"
-cat cookie_secrets.out
-
-echo ">> cookie_secrets.txt"
-cat cookie_secrets.txt
-
-# check this valid cookie before rollover
-dig -b 2001:db8:220:1:59de:d0f4:8769:82b8 @2001:db8:220:1:59de:d0f4:8769:82b8 +cookie=22681ab97d52c298010000005cf7c57926556bd0934c72f8 > dig.output.1
-cat dig.output.1
-
-if grep -q "22681ab97d52c298010000005cf7c57926556bd0934c72f8" dig.output.1
-then
-	echo "IPv6 Query with Rolled Over Secret: Cookie matches pre-rollover"
-else
-	echo "IPv6 Query with Rolled Over Secret: Cookie does not match pre-rollover"
-	exit 1
-fi
-
-# secret rollover
-$PRE/unbound-control -c ub.conf activate_cookie_secret
-
-$PRE/unbound-control -c ub.conf print_cookie_secrets > cookie_secrets.2
-if ! grep -q "^active.*445536bcd2513298075a5d379663c962" cookie_secrets.2
-then
-	sleep 1
-	$PRE/unbound-control -c ub.conf print_cookie_secrets > cookie_secrets.2
-fi
-if ! grep -q "^active.*445536bcd2513298075a5d379663c962" cookie_secrets.2
-then
-	sleep 1
-	$PRE/unbound-control -c ub.conf print_cookie_secrets > cookie_secrets.2
-fi
-if ! grep -q "^active.*445536bcd2513298075a5d379663c962" cookie_secrets.2
-then
-	sleep 1
-	$PRE/unbound-control -c ub.conf print_cookie_secrets > cookie_secrets.2
-fi
-if ! grep -q "^active.*445536bcd2513298075a5d379663c962" cookie_secrets.2 
-then
-	cat cookie_secrets.2
-	echo "Cookie did not get activated"
-	exit 1
-fi
-echo ">> activate cookie secret, printout"
-cat cookie_secrets.2
-echo ">> cookie_secrets.txt"
-cat cookie_secrets.txt
-
-dig -b 2001:db8:220:1:59de:d0f4:8769:82b8 @2001:db8:220:1:59de:d0f4:8769:82b8 +cookie=22681ab97d52c298010000005cf7c57926556bd0934c72f8 > dig.output.2
-cat dig.output.2
-
-if grep -q "22681ab97d52c298010000005cf7c609a6bb79d16625507a" dig.output.2
-then
-	echo "IPv6 Query with Rolled Over Secret: Cookie matches after rollover"
-else
-	echo "IPv6 Query with Rolled Over Secret: Cookie does not match after rollover"
-	exit 1
-fi
-
-echo ">> drop cookie secret"
-$PRE/unbound-control -c ub.conf drop_cookie_secret
-
-$PRE/unbound-control -c ub.conf print_cookie_secrets > cookie_secrets.3
-if grep -q "^staging.*dd3bdf9344b678b185a6f5cb60fca715" cookie_secrets.3
-then
-	sleep 1
-	$PRE/unbound-control -c ub.conf print_cookie_secrets > cookie_secrets.3
-fi
-if grep -q "^staging.*dd3bdf9344b678b185a6f5cb60fca715" cookie_secrets.3
-then
-	sleep 1
-	$PRE/unbound-control -c ub.conf print_cookie_secrets > cookie_secrets.3
-fi
-if grep -q "^staging.*dd3bdf9344b678b185a6f5cb60fca715" cookie_secrets.3
-then
-	sleep 1
-	$PRE/unbound-control -c ub.conf print_cookie_secrets > cookie_secrets.3
-fi
-if grep -q "^staging.*dd3bdf9344b678b185a6f5cb60fca715" cookie_secrets.3
-then
-	cat cookie_secrets.3
-	echo "Cookie did not get dropped"
-	exit 1
-fi
-echo ">> drop cookie secret, printout"
-cat cookie_secrets.3
-echo ">> cookie_secrets.txt"
-cat cookie_secrets.txt
-
-exit 0