From 2f4a9c8d504ffe0bfb85ea242ea48d695c7da2f3 Mon Sep 17 00:00:00 2001
From: Stephane Cance <stephane.cance@varnish-software.com>
Date: Fri, 5 Apr 2024 10:42:14 +0200
Subject: [PATCH] mgt: Always recreate secret file on startup

As both the varnish working directory and the secret file may
pre-exist, this ensures permissions remain restrictive on it.
---
 bin/varnishd/mgt/mgt_main.c      | 10 ++++++--
 bin/varnishtest/tests/b00084.vtc | 42 ++++++++++++++++++++++++++++++++
 bin/varnishtest/tests/u00000.vtc |  2 +-
 3 files changed, 51 insertions(+), 3 deletions(-)
 create mode 100644 bin/varnishtest/tests/b00084.vtc

diff --git a/bin/varnishd/mgt/mgt_main.c b/bin/varnishd/mgt/mgt_main.c
index c1bdaf812c3..dc2bce56b9b 100644
--- a/bin/varnishd/mgt/mgt_main.c
+++ b/bin/varnishd/mgt/mgt_main.c
@@ -272,10 +272,16 @@ make_secret(const char *dirname)
 	assert(asprintf(&fn, "%s/_.secret", dirname) > 0);
 
 	VJ_master(JAIL_MASTER_FILE);
-	fdo = open(fn, O_RDWR|O_CREAT|O_TRUNC, 0640);
-	if (fdo < 0)
+	if (unlink(fn) < 0 && errno != ENOENT) {
+		ARGV_ERR("Cannot remove pre-existing secret-file in %s (%s)\n",
+		    dirname, VAS_errtxt(errno));
+	}
+
+	fdo = open(fn, O_RDWR|O_CREAT|O_EXCL, 0640);
+	if (fdo < 0) {
 		ARGV_ERR("Cannot create secret-file in %s (%s)\n",
 		    dirname, VAS_errtxt(errno));
+	}
 
 	for (i = 0; i < 256; i++) {
 		AZ(VRND_RandomCrypto(&b, 1));
diff --git a/bin/varnishtest/tests/b00084.vtc b/bin/varnishtest/tests/b00084.vtc
new file mode 100644
index 00000000000..6797d28e61f
--- /dev/null
+++ b/bin/varnishtest/tests/b00084.vtc
@@ -0,0 +1,42 @@
+varnishtest "make sure an already setup secret file remains protected"
+
+varnish v1 -vcl { backend default none; } -start
+
+shell -match _.secret {
+	find "${tmpdir}"/v1/_.secret -perm 0640 -size 256c
+}
+
+varnish v1 -stop -wait
+
+shell {
+	test ! -f "${tmpdir}"/v1/_.secret
+}
+
+# since varnishtest destroys workdir silently before startup
+# this must fool varnishtest to not manage the workdir
+shell -match _.secret {
+	set -e
+	mkdir -p "${tmpdir}"/v2/
+	touch "${tmpdir}"/v2/_.secret
+	chmod 0666 "${tmpdir}"/v2/_.secret
+	find "${tmpdir}"/v2/_.secret -perm 0666 -size 0c
+}
+
+process p1 "exec varnishd -n ${tmpdir}/v2 -F -f '' -a :0" -start
+
+# wait for startup and check permissions have changed
+shell -match _.secret {
+	set -e
+	t=50
+	while [ "$t" -gt 0 ] && [ ! -d "${tmpdir}"/v2/_.vsm_mgt ]; do
+	    sleep 0.1
+	    t=$(($t - 1))
+	done
+	find "${tmpdir}"/v2/_.secret -perm 0640 -size 256c
+}
+
+process p1 -stop -wait
+
+shell {
+	test ! -f "${tmpdir}"/v2/_.secret
+}
diff --git a/bin/varnishtest/tests/u00000.vtc b/bin/varnishtest/tests/u00000.vtc
index 8ce91b74b04..090f2614750 100644
--- a/bin/varnishtest/tests/u00000.vtc
+++ b/bin/varnishtest/tests/u00000.vtc
@@ -44,7 +44,7 @@ shell -err -expect {Cannot open -S file} {
 	varnishd -S ${tmpdir}/nonexistent -n ${tmpdir}/v0 -f ''
 }
 
-shell -err -expect {Cannot create secret-file in} {
+shell -err -expect {Cannot remove pre-existing secret-file in} {
 	mkdir ${tmpdir}/is_a_dir ${tmpdir}/is_a_dir/_.secret
 	varnishd -n ${tmpdir}/is_a_dir -d -a :0
 }