From: Jonathan McDowell <noodles@earth.li>
Date: Thu, 3 Feb 2022 19:07:58 +0000 (+0000)
Subject: Fix handling of other signature requirement
X-Git-Tag: onak-0.6.2~7
X-Git-Url: http://the.earth.li/gitweb/?p=onak.git;a=commitdiff_plain;h=58ed9a0076feb9604154b99da6ed1907ca7df089

Fix handling of other signature requirement

Two fixes related to the check that a key has another signature on it.

Firstly, if any of the UIDs has a signature from another key then allow
all of them. Otherwise it's not possible to add a new UID to an existing
key. Our primary concern is that the key is linked into the WoT, rather
than policing individual UIDs.

Secondly, if a key is already present in the backend database then don't
perform the other signature check. If we've added to the backend and all
of the cross signatures are removed then it would be no longer possible
to update the key, which isn't what we want. If we've trusted it at some
point and added it then we should allow verifiable updates, even if
there are no valid cross signatures left.
---

diff --git a/cleankey.c b/cleankey.c
index 3a74098..10fb661 100644
--- a/cleankey.c
+++ b/cleankey.c
@@ -151,9 +151,6 @@ int clean_sighashes(struct onak_dbctx *dbctx,
 	bool remove;
 
 	get_keyid(key, &keyid);
-	if (othersig != NULL) {
-		*othersig = false;
-	}
 	if (selfsig != NULL) {
 		*selfsig = false;
 	}
@@ -249,17 +246,19 @@ int clean_list_sighashes(struct onak_dbctx *dbctx,
 			struct openpgp_signedpacket_list **siglist,
 			bool fullverify, bool needother)
 {
-	struct openpgp_signedpacket_list *tmp = NULL;
+	struct openpgp_signedpacket_list **orig, *tmp = NULL;
 	bool selfsig, othersig;
 	int removed = 0;
 
+	othersig = false;
+	orig = siglist;
 	while (siglist != NULL && *siglist != NULL) {
-		selfsig = othersig = false;
+		selfsig = false;
 
 		removed += clean_sighashes(dbctx, key, (*siglist)->packet,
 			&(*siglist)->sigs, fullverify, &selfsig, &othersig);
 
-		if (fullverify && (!selfsig || (needother && !othersig))) {
+		if (fullverify && !selfsig) {
 			/* Remove the UID/subkey if there's no selfsig */
 			tmp = *siglist;
 			*siglist = (*siglist)->next;
@@ -270,6 +269,20 @@ int clean_list_sighashes(struct onak_dbctx *dbctx,
 		}
 	}
 
+	/*
+	 * We need at least one UID to have a signature from another key,
+	 * otherwise we remove all of them if needother is set.
+	 */
+	if (needother && fullverify && !othersig) {
+		siglist = orig;
+		while (siglist != NULL && *siglist != NULL) {
+			tmp = *siglist;
+			*siglist = (*siglist)->next;
+			tmp->next = NULL;
+			free_signedpacket_list(tmp);
+		}
+	}
+
 	return removed;
 }
 
@@ -350,7 +363,9 @@ int cleankeys(struct onak_dbctx *dbctx, struct openpgp_publickey **keys,
 		uint64_t policies)
 {
 	struct openpgp_publickey **curkey, *tmp;
+	struct openpgp_fingerprint fp;
 	int changed = 0, count = 0;
+	bool needother;
 
 	if (keys == NULL)
 		return 0;
@@ -375,9 +390,24 @@ int cleankeys(struct onak_dbctx *dbctx, struct openpgp_publickey **keys,
 		count += dedupsubkeys(*curkey);
 		if (policies & (ONAK_CLEAN_CHECK_SIGHASH |
 					ONAK_CLEAN_VERIFY_SIGNATURES)) {
+
+			needother = policies & ONAK_CLEAN_NEED_OTHER_SIG;
+			if (needother) {
+				/*
+				 * Check if we already have the key; if we do
+				 * then we can skip the check to make sure we
+				 * have signatures from other keys.
+				 */
+				get_fingerprint((*curkey)->publickey, &fp);
+				tmp = NULL;
+				needother = dbctx->fetch_key(dbctx, &fp,
+						&tmp, false) == 0;
+				free_publickey(tmp);
+			}
+
 			count += clean_key_signatures(dbctx, *curkey,
 				policies & ONAK_CLEAN_VERIFY_SIGNATURES,
-				policies & ONAK_CLEAN_NEED_OTHER_SIG);
+				needother);
 		}
 		if (count > 0) {
 			changed++;