certctl: Unstickify (un)trusted certificates

Ever since certctl was rewritten in C, the rehash command has reingested
TRUSTDESTDIR / UNTRUSTDESTDIR in addition to TRUSTPATH / UNTRUSTPATH.
This seemed like a good idea at the time but was, in retrospect, a
mistake, as it means a (un)trusted certificate remains (un)trusted
forever (or at least until it expires) even if it is removed from
(UN)TRUSTPATH.  Among other issues, it causes ports QA to fail for any
port that either installs certificates or depends on a port that does.

Although this behavior was undocumented, the change may surprise users
who have added certificates manually, so update the manual page to point
it out and add prominent warnings to the trust and untrust commands.

PR:		290078
MFC after:	1 week
Reviewed by:	kevans, bcr
Differential Revision:	https://reviews.freebsd.org/D56617

usr.sbin/certctl/certctl.8

@@ -24,7 +24,7 @@
 .\" IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 .\" POSSIBILITY OF SUCH DAMAGE.
 .\"
-.Dd December 3, 2025
+.Dd April 24, 2026
 .Dt CERTCTL 8
 .Os
 .Sh NAME
@@ -113,8 +113,19 @@ In addition, a bundle containing the trusted certificates is placed in
 .Ev BUNDLE .
 .It Ic untrust
 Add the specified file to the untrusted list.
+Note that the next
+.Ic rehash
+will remove it unless a copy of it is also placed somewhere in a
+directory included in
+.Ev UNTRUSTPATH .
 .It Ic trust
-Remove the specified file from the untrusted list.
+Add the specified file to the trusted list, unless it is already
+untrusted.
+Note that the next
+.Ic rehash
+will remove it unless a copy of it is also placed somewhere in a
+directory included in
+.Ev TRUSTPATH .
 .El
 .Sh ENVIRONMENT
 .Bl -tag -width UNTRUSTDESTDIR

usr.sbin/certctl/certctl.c

@@ -636,23 +636,18 @@ write_bundle(const char *dir, const char *file, struct cert_tree *tree)
  * Returns the number of certificates loaded.
  */
 static unsigned int
-load_trusted(bool all, struct cert_tree *exclude)
+load_trusted(void)
 {
 	unsigned int i, n;
 	int ret;
 
 	/* load external trusted certs */
-	for (i = n = 0; all && trusted_paths[i] != NULL; i++) {
-		ret = read_certs(trusted_paths[i], &trusted, exclude);
+	for (i = n = 0; trusted_paths[i] != NULL; i++) {
+		ret = read_certs(trusted_paths[i], &trusted, &untrusted);
 		if (ret > 0)
 			n += ret;
 	}
 
-	/* load installed trusted certs */
-	ret = read_certs(trusted_dest, &trusted, exclude);
-	if (ret > 0)
-		n += ret;
-
 	info("%d trusted certificates found", n);
 	return (n);
 }
@@ -663,24 +658,19 @@ load_trusted(bool all, struct cert_tree *exclude)
  * Returns the number of certificates loaded.
  */
 static unsigned int
-load_untrusted(bool all)
+load_untrusted(void)
 {
 	char *path;
 	unsigned int i, n;
 	int ret;
 
 	/* load external untrusted certs */
-	for (i = n = 0; all && untrusted_paths[i] != NULL; i++) {
+	for (i = n = 0; untrusted_paths[i] != NULL; i++) {
 		ret = read_certs(untrusted_paths[i], &untrusted, NULL);
 		if (ret > 0)
 			n += ret;
 	}
 
-	/* load installed untrusted certs */
-	ret = read_certs(untrusted_dest, &untrusted, NULL);
-	if (ret > 0)
-		n += ret;
-
 	/* load legacy untrusted certs */
 	path = expand_path(LEGACY_PATH);
 	ret = read_certs(path, &untrusted, NULL);
@@ -796,8 +786,8 @@ certctl_list(int argc, char **argv __unused)
 {
 	if (argc > 1)
 		usage();
-	/* load trusted certificates */
-	load_trusted(false, NULL);
+	/* load installed trusted certificates */
+	read_certs(trusted_dest, &trusted, NULL);
 	/* list them */
 	list_certs(&trusted);
 	free_certs(&trusted);
@@ -814,8 +804,8 @@ certctl_untrusted(int argc, char **argv __unused)
 {
 	if (argc > 1)
 		usage();
-	/* load untrusted certificates */
-	load_untrusted(false);
+	/* load installed untrusted certificates */
+	read_certs(untrusted_dest, &untrusted, NULL);
 	/* list them */
 	list_certs(&untrusted);
 	free_certs(&untrusted);
@@ -842,10 +832,10 @@ certctl_rehash(int argc, char **argv __unused)
 	}
 
 	/* load untrusted certs first */
-	load_untrusted(true);
+	load_untrusted();
 
 	/* load trusted certs, excluding any that are already untrusted */
-	load_trusted(true, &untrusted);
+	load_trusted();
 
 	/* save everything */
 	ret = save_all();
@@ -859,7 +849,8 @@ certctl_rehash(int argc, char **argv __unused)
 }
 
 /*
- * Manually add one or more certificates to the list of trusted certificates.
+ * Manually add one or more certificates to the list of trusted
+ * certificates.
  *
  * Returns 0 on success and -1 on failure.
  */
@@ -875,10 +866,10 @@ certctl_trust(int argc, char **argv)
 		usage();
 
 	/* load untrusted certs first */
-	load_untrusted(true);
+	load_untrusted();
 
 	/* load trusted certs, excluding any that are already untrusted */
-	load_trusted(true, &untrusted);
+	load_trusted();
 
 	/* now load the additional trusted certificates */
 	n = 0;
@@ -891,9 +882,9 @@ certctl_trust(int argc, char **argv)
 		warnx("no new trusted certificates found");
 		free_certs(&untrusted);
 		free_certs(&trusted);
-		free_certs(&extra);
 		return (0);
 	}
+	warnx("%u new trusted certificate%s found", n, n > 1 ? "s" : "");
 
 	/*
 	 * For each new trusted cert, move it from the extra list to the
@@ -906,10 +897,16 @@ certctl_trust(int argc, char **argv)
 		RB_INSERT(cert_tree, &trusted, cert);
 		if ((other = RB_FIND(cert_tree, &untrusted, cert)) != NULL) {
 			warnx("%s was previously untrusted", cert->name);
+			warnx("source of untrust: %s", other->path);
 			RB_REMOVE(cert_tree, &untrusted, other);
 			free_cert(other);
 		}
 	}
+	warnx("This operation is not persistent.  To persistently add");
+	warnx("trusted certificates to the system store, copy them to");
+	warnx("one of these directories, then run `certctl rehash`:");
+	for (i = 0; trusted_paths[i] != NULL; i++)
+		warnx("    %s", trusted_paths[i]);
 
 	/* save everything */
 	ret = save_all();
@@ -929,6 +926,8 @@ certctl_trust(int argc, char **argv)
 static int
 certctl_untrust(int argc, char **argv)
 {
+	struct cert_tree extra = RB_INITIALIZER(&extra);
+	struct cert *cert, *other, *tmp;
 	unsigned int n;
 	int i, ret;
 
@@ -936,23 +935,47 @@ certctl_untrust(int argc, char **argv)
 		usage();
 
 	/* load untrusted certs first */
-	load_untrusted(true);
+	load_untrusted();
+
+	/* load trusted certs, excluding any that are already untrusted */
+	load_trusted();
 
 	/* now load the additional untrusted certificates */
 	n = 0;
 	for (i = 1; i < argc; i++) {
-		ret = read_cert(argv[i], &untrusted, NULL);
+		ret = read_cert(argv[i], &extra, NULL);
 		if (ret > 0)
 			n += ret;
 	}
 	if (n == 0) {
 		warnx("no new untrusted certificates found");
 		free_certs(&untrusted);
+		free_certs(&trusted);
 		return (0);
 	}
+	warnx("%u new untrusted certificate%s found", n, n > 1 ? "s" : "");
 
-	/* load trusted certs, excluding any that are already untrusted */
-	load_trusted(true, &untrusted);
+	/*
+	 * For each new untrusted cert, move it from the extra list to the
+	 * untrusted list, then check if a matching certificate exists on
+	 * the trusted list.  If that is the case, warn the user, then
+	 * remove the matching certificate from the trusted list.
+	 */
+	RB_FOREACH_SAFE(cert, cert_tree, &extra, tmp) {
+		RB_REMOVE(cert_tree, &extra, cert);
+		RB_INSERT(cert_tree, &untrusted, cert);
+		if ((other = RB_FIND(cert_tree, &trusted, cert)) != NULL) {
+			warnx("%s was previously trusted", cert->name);
+			warnx("source of trust: %s", other->path);
+			RB_REMOVE(cert_tree, &trusted, other);
+			free_cert(other);
+		}
+	}
+	warnx("This operation is not persistent.  To persistently add");
+	warnx("untrusted certificates to the system store, copy them to");
+	warnx("one of these directories, then run `certctl rehash`:");
+	for (i = 0; untrusted_paths[i] != NULL; i++)
+		warnx("    %s", untrusted_paths[i]);
 
 	/* save everything */
 	ret = save_all();

usr.sbin/certctl/tests/certctl_test.sh

@@ -271,7 +271,8 @@ untrust_body()
 	crtfile=usr/share/certs/trusted/${crtname}.crt
 	check_trusted "${crtname}"
 	check_in_bundle ${crtfile}
-	atf_check certctl untrust "${crtfile}"
+	atf_check -e match:"1 new untrusted" \
+	    certctl untrust "${crtfile}"
 	check_untrusted "${crtname}"
 	check_not_in_bundle ${crtfile}
 }