From 2708d5a06ad2ef872ea89aca822328f98a86e7cb Mon Sep 17 00:00:00 2001
From: Jonathan McDowell <noodles@earth.li>
Date: Sun, 4 Aug 2019 19:19:31 +0100
Subject: [PATCH] Drop v3 keys by default when cleaning keys

v3 keys have long been considered insecure. While we want to retain
support for them there's no reason most keyservers should actually store
them these days. So drop them by default when running cleankeys()
---
 cleankey.c  | 27 +++++++++++++++++++--------
 cleankey.h  |  1 +
 onak-conf.c | 11 ++++++++++-
 onak.ini.in |  3 +++
 4 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/cleankey.c b/cleankey.c
index fe24c3b..c7a69be 100644
--- a/cleankey.c
+++ b/cleankey.c
@@ -239,26 +239,37 @@ int clean_large_packets(struct openpgp_publickey *key)
  */
 int cleankeys(struct openpgp_publickey **keys, uint64_t policies)
 {
-	struct openpgp_publickey *curkey;
+	struct openpgp_publickey **curkey, *tmp;
 	int changed = 0, count = 0;
 
 	if (keys == NULL)
 		return 0;
 
-	curkey = *keys;
-	while (curkey != NULL) {
+	curkey = keys;
+	while (*curkey != NULL) {
+		if (policies & ONAK_CLEAN_DROP_V3_KEYS) {
+			if ((*curkey)->publickey->data[0] < 4) {
+				/* Remove the key from the list */
+				tmp = *curkey;
+				*curkey = tmp->next;
+				tmp->next = NULL;
+				free_publickey(tmp);
+				changed++;
+				continue;
+			}
+		}
 		if (policies & ONAK_CLEAN_LARGE_PACKETS) {
-			count += clean_large_packets(curkey);
+			count += clean_large_packets(*curkey);
 		}
-		count += dedupuids(curkey);
-		count += dedupsubkeys(curkey);
+		count += dedupuids(*curkey);
+		count += dedupsubkeys(*curkey);
 		if (policies & ONAK_CLEAN_CHECK_SIGHASH) {
-			count += clean_key_sighashes(curkey);
+			count += clean_key_sighashes(*curkey);
 		}
 		if (count > 0) {
 			changed++;
 		}
-		curkey = curkey->next;
+		curkey = &(*curkey)->next;
 	}
 
 	return changed;
diff --git a/cleankey.h b/cleankey.h
index 350e75d..834b622 100644
--- a/cleankey.h
+++ b/cleankey.h
@@ -23,6 +23,7 @@
 
 #define ONAK_CLEAN_CHECK_SIGHASH	(1 << 0)
 #define ONAK_CLEAN_LARGE_PACKETS	(1 << 1)
+#define ONAK_CLEAN_DROP_V3_KEYS		(1 << 2)
 #define ONAK_CLEAN_ALL			(uint64_t) -1
 
 /**
diff --git a/onak-conf.c b/onak-conf.c
index c30260f..8925dbc 100644
--- a/onak-conf.c
+++ b/onak-conf.c
@@ -58,7 +58,7 @@ struct onak_config config = {
 	.dbinit = NULL,
 #endif
 
-	.clean_policies = ONAK_CLEAN_CHECK_SIGHASH,
+	.clean_policies = ONAK_CLEAN_DROP_V3_KEYS | ONAK_CLEAN_CHECK_SIGHASH,
 
 	.bin_dir = NULL,
 	.mail_dir = NULL,
@@ -284,6 +284,15 @@ static bool parseconfigline(char *line)
 			config.syncsites = lladd(config.syncsites,
 				strdup(value));
 		/* [verification] section */
+		} else if (MATCH("verification", "drop_v3")) {
+			if (parsebool(value, config.clean_policies &
+					ONAK_CLEAN_DROP_V3_KEYS)) {
+				config.clean_policies |=
+					ONAK_CLEAN_DROP_V3_KEYS;
+			} else {
+				config.clean_policies &=
+					~ONAK_CLEAN_DROP_V3_KEYS;
+			}
 		} else if (MATCH("verification", "check_sighash")) {
 			if (parsebool(value, config.clean_policies &
 					ONAK_CLEAN_CHECK_SIGHASH)) {
diff --git a/onak.ini.in b/onak.ini.in
index 4dbf830..8989f07 100644
--- a/onak.ini.in
+++ b/onak.ini.in
@@ -19,6 +19,9 @@ max_reply_keys=128
 ; Verify signature hashes - verify that the hash a signature claims to be
 ; over matches the hash of the data. Does not actually verify the signature.
 check_sighash=true
+; Drop v3 (and older) keys. These are long considered insecure, so unless there
+; is a good reason you should accept this default.
+drop_v3=true
 
 ; Settings related to the email interface to onak.
 [mail]
-- 
2.39.5