From 5cb163e87c7f0717aa94ca281a56e572c2a6c8f3 Mon Sep 17 00:00:00 2001
From: Jonathan McDowell <noodles@earth.li>
Date: Thu, 1 Aug 2019 22:18:24 +0100
Subject: [PATCH] Change delete_key to use a full fingerprint

delete_key was deleting a key based on the keyid, which is
insufficiently precise. Move over to the full fingerprint (even though
with some backends this is an effective no-op for now).
---
 keyd.c          |  7 ++++---
 keydb.h         |  5 +++--
 keydb_db4.c     | 55 +++++++++++++++++++++----------------------------
 keydb_dynamic.c |  5 +++--
 keydb_file.c    |  6 +++---
 keydb_fs.c      | 11 ++++++----
 keydb_hkp.c     |  4 ++--
 keydb_keyd.c    |  6 +++---
 keydb_keyring.c |  6 +++---
 keydb_pg.c      | 12 ++++++++---
 keydb_stacked.c |  5 +++--
 onak.c          | 16 ++++++++++----
 12 files changed, 76 insertions(+), 62 deletions(-)

diff --git a/keyd.c b/keyd.c
index 9994dea..4520b14 100644
--- a/keyd.c
+++ b/keyd.c
@@ -444,8 +444,9 @@ static int sock_do(struct onak_dbctx *dbctx, int fd)
 				ret = 1;
 			}
 			if (ret == 0) {
-				bytes = read(fd, &keyid, sizeof(keyid));
-				if (bytes != sizeof(keyid)) {
+				bytes = read(fd, &fingerprint,
+						sizeof(fingerprint));
+				if (bytes != sizeof(fingerprint)) {
 					ret = 1;
 				}
 			}
@@ -455,7 +456,7 @@ static int sock_do(struct onak_dbctx *dbctx, int fd)
 						", result: %d",
 						keyid,
 						dbctx->delete_key(dbctx,
-							keyid, false));
+							&fingerprint, false));
 			}
 			break;
 		case KEYD_CMD_GETFULLKEYID:
diff --git a/keydb.h b/keydb.h
index 05405c3..f448215 100644
--- a/keydb.h
+++ b/keydb.h
@@ -104,13 +104,14 @@ struct onak_dbctx {
 
 /**
  * @brief Given a keyid delete the key from storage.
- * @param keyid The keyid to delete.
+ * @param fp The fingerprint of the key to delete.
  * @param intrans If we're already in a transaction.
  *
  * This function deletes a public key from whatever storage mechanism we
  * are using. Returns 0 if the key existed.
  */
-	int (*delete_key)(struct onak_dbctx *, uint64_t keyid, bool intrans);
+	int (*delete_key)(struct onak_dbctx *, struct openpgp_fingerprint *fp,
+			bool intrans);
 
 /**
  * @brief Trys to find the keys that contain the supplied text.
diff --git a/keydb_db4.c b/keydb_db4.c
index aa2f229..d2db2c5 100644
--- a/keydb_db4.c
+++ b/keydb_db4.c
@@ -703,14 +703,15 @@ static int db4_fetch_key_skshash(struct onak_dbctx *dbctx,
 
 /**
  *	delete_key - Given a keyid delete the key from storage.
- *	@keyid: The keyid to delete.
+ *	@fp: The fingerprint of the key to delete.
  *	@intrans: If we're already in a transaction.
  *
  *	This function deletes a public key from whatever storage mechanism we
  *	are using. Returns 0 if the key existed.
  */
 static int db4_delete_key(struct onak_dbctx *dbctx,
-		uint64_t keyid, bool intrans)
+		struct openpgp_fingerprint *fp,
+		bool intrans)
 {
 	struct onak_db4_dbctx *privctx = (struct onak_db4_dbctx *) dbctx->priv;
 	struct openpgp_publickey *publickey = NULL;
@@ -729,20 +730,22 @@ static int db4_delete_key(struct onak_dbctx *dbctx,
 	struct ll *curword  = NULL;
 	bool deadlock = false;
 	struct skshash hash;
-	struct openpgp_fingerprint fingerprint;
+	uint64_t keyid;
 
 	if (!intrans) {
 		db4_starttrans(dbctx);
 	}
 
-	if (db4_fetch_key_id(dbctx, keyid, &publickey, true) == 0) {
+	if (db4_fetch_key_fp(dbctx, fp, &publickey, true) == 0) {
 		if (!intrans) {
 			db4_endtrans(dbctx);
 		}
 		return 1;
 	}
 
-	get_fingerprint(publickey->publickey, &fingerprint);
+	if (get_keyid(publickey, &keyid) != ONAK_E_OK) {
+		return 1;
+	}
 
 	/*
 	 * Walk through the uids removing the words from the worddb.
@@ -777,8 +780,8 @@ static int db4_delete_key(struct onak_dbctx *dbctx,
 			memset(&data, 0, sizeof(data));
 			key.data = curword->object;
 			key.size = strlen(key.data);
-			data.data = fingerprint.fp;
-			data.size = fingerprint.length;
+			data.data = fp->fp;
+			data.size = fp->length;
 
 			ret = cursor->c_get(cursor,
 				&key,
@@ -832,8 +835,8 @@ static int db4_delete_key(struct onak_dbctx *dbctx,
 		memset(&data, 0, sizeof(data));
 		key.data = &shortkeyid;
 		key.size = sizeof(shortkeyid);
-		data.data = fingerprint.fp;
-		data.size = fingerprint.length;
+		data.data = fp->fp;
+		data.size = fp->length;
 
 		ret = cursor->c_get(cursor,
 			&key,
@@ -860,8 +863,8 @@ static int db4_delete_key(struct onak_dbctx *dbctx,
 		memset(&data, 0, sizeof(data));
 		key.data = &keyid;
 		key.size = sizeof(keyid);
-		data.data = fingerprint.fp;
-		data.size = fingerprint.length;
+		data.data = fp->fp;
+		data.size = fp->length;
 
 		ret = cursor64->c_get(cursor64,
 			&key,
@@ -910,8 +913,8 @@ static int db4_delete_key(struct onak_dbctx *dbctx,
 			memset(&data, 0, sizeof(data));
 			key.data = &shortkeyid;
 			key.size = sizeof(shortkeyid);
-			data.data = fingerprint.fp;
-			data.size = fingerprint.length;
+			data.data = fp->fp;
+			data.size = fp->length;
 
 			ret = cursor->c_get(cursor,
 				&key,
@@ -938,8 +941,8 @@ static int db4_delete_key(struct onak_dbctx *dbctx,
 			memset(&data, 0, sizeof(data));
 			key.data = &subkeyid;
 			key.size = sizeof(subkeyid);
-			data.data = fingerprint.fp;
-			data.size = fingerprint.length;
+			data.data = fp->fp;
+			data.size = fp->length;
 
 			ret = cursor64->c_get(cursor64,
 				&key,
@@ -985,8 +988,8 @@ static int db4_delete_key(struct onak_dbctx *dbctx,
 			memset(&data, 0, sizeof(data));
 			key.data = hash.hash;
 			key.size = sizeof(hash.hash);
-			data.data = fingerprint.fp;
-			data.size = fingerprint.length;
+			data.data = fp->fp;
+			data.size = fp->length;
 
 			ret = cursor->c_get(cursor,
 				&key,
@@ -1016,20 +1019,10 @@ static int db4_delete_key(struct onak_dbctx *dbctx,
 	publickey = NULL;
 
 	if (!deadlock) {
-		key.data = fingerprint.fp;
-		key.size = fingerprint.length;
-
-		keydb_fp(privctx, &fingerprint)->del(keydb_fp(privctx,
-					&fingerprint),
-				privctx->txn,
-				&key,
-				0); /* flags */
-
-		/* Delete old style 64 bit keyid */
-		key.data = &keyid;
-		key.size = sizeof(keyid);
+		key.data = fp->fp;
+		key.size = fp->length;
 
-		keydb_id(privctx, keyid)->del(keydb_id(privctx, keyid),
+		keydb_fp(privctx, fp)->del(keydb_fp(privctx, fp),
 				privctx->txn,
 				&key,
 				0); /* flags */
@@ -1101,7 +1094,7 @@ static int db4_store_key(struct onak_dbctx *dbctx,
 	 * it definitely needs updated.
 	 */
 	if (update) {
-		deadlock = (db4_delete_key(dbctx, keyid, true) == -1);
+		deadlock = (db4_delete_key(dbctx, &fingerprint, true) == -1);
 	}
 
 	/*
diff --git a/keydb_dynamic.c b/keydb_dynamic.c
index ef42fbe..b302efb 100644
--- a/keydb_dynamic.c
+++ b/keydb_dynamic.c
@@ -108,14 +108,15 @@ static int dynamic_store_key(struct onak_dbctx *dbctx,
 			publickey, intrans, update);
 }
 
-static int dynamic_delete_key(struct onak_dbctx *dbctx, uint64_t keyid,
+static int dynamic_delete_key(struct onak_dbctx *dbctx,
+		struct openpgp_fingerprint *fp,
 		bool intrans)
 {
 	struct onak_dynamic_dbctx *privctx =
 			(struct onak_dynamic_dbctx *) dbctx->priv;
 
 	return privctx->loadeddbctx->delete_key(privctx->loadeddbctx,
-			keyid, intrans);
+			fp, intrans);
 }
 
 static int dynamic_update_keys(struct onak_dbctx *dbctx,
diff --git a/keydb_file.c b/keydb_file.c
index bc3c578..c5d3ece 100644
--- a/keydb_file.c
+++ b/keydb_file.c
@@ -143,20 +143,20 @@ static int file_store_key(struct onak_dbctx *dbctx,
 
 /**
  *	delete_key - Given a keyid delete the key from storage.
- *	@keyid: The keyid to delete.
+ *	@fp: The fingerprint of the key to delete.
  *	@intrans: If we're already in a transaction.
  *
  *	This function deletes a public key from whatever storage mechanism we
  *	are using. Returns 0 if the key existed.
  */
 static int file_delete_key(struct onak_dbctx *dbctx,
-		uint64_t keyid, bool intrans)
+		struct openpgp_fingerprint *fp, bool intrans)
 {
 	char *db_dir = (char *) dbctx->priv;
 	char keyfile[1024];
 
 	snprintf(keyfile, 1023, "%s/0x%" PRIX64, db_dir,
-			keyid & 0xFFFFFFFF);
+			fingerprint2keyid(fp) & 0xFFFFFFFF);
 
 	return unlink(keyfile);
 }
diff --git a/keydb_fs.c b/keydb_fs.c
index 0bf922f..926960f 100644
--- a/keydb_fs.c
+++ b/keydb_fs.c
@@ -394,10 +394,11 @@ static int fs_store_key(struct onak_dbctx *dbctx,
 
 /**
  *	delete_key - Given a keyid delete the key from storage.
- *	@keyid: The keyid to delete.
+ *	@fp: The fingerprint of the key to delete.
  *	@intrans: If we're already in a transaction.
  */
-static int fs_delete_key(struct onak_dbctx *dbctx, uint64_t keyid, bool intrans)
+static int fs_delete_key(struct onak_dbctx *dbctx,
+		struct openpgp_fingerprint *fp, bool intrans)
 {
 	static char buffer[PATH_MAX];
 	int ret;
@@ -407,9 +408,11 @@ static int fs_delete_key(struct onak_dbctx *dbctx, uint64_t keyid, bool intrans)
 	struct openpgp_fingerprint *subkeyids = NULL;
 	uint64_t subkeyid;
 	int i = 0;
+	uint64_t keyid;
 
-	if ((keyid >> 32) == 0)
-		keyid = fs_getfullkeyid(dbctx, keyid);
+	keyid = fingerprint2keyid(fp);
+	if (keyid == 0)
+		return 1;
 
 	if (!intrans)
 		fs_starttrans(dbctx);
diff --git a/keydb_hkp.c b/keydb_hkp.c
index 3ec46a4..46fceb8 100644
--- a/keydb_hkp.c
+++ b/keydb_hkp.c
@@ -271,13 +271,13 @@ static int hkp_store_key(struct onak_dbctx *dbctx,
 
 /**
  *	delete_key - Given a keyid delete the key from storage.
- *	@keyid: The keyid to delete.
+ *	@fp: The fingerprint of the key to delete.
  *	@intrans: If we're already in a transaction.
  *
  *	No op for HKP.
  */
 static int hkp_delete_key(struct onak_dbctx *dbctx,
-		uint64_t keyid, bool intrans)
+		struct openpgp_fingerprint *fp, bool intrans)
 {
 	return -1;
 }
diff --git a/keydb_keyd.c b/keydb_keyd.c
index cd1a281..597507e 100644
--- a/keydb_keyd.c
+++ b/keydb_keyd.c
@@ -187,19 +187,19 @@ static int keyd_fetch_key_fp(struct onak_dbctx *dbctx,
 
 /**
 *	delete_key - Given a keyid delete the key from storage.
-*	@keyid: The keyid to delete.
+ *	@fp: The fingerprint of the key to delete.
 *	@intrans: If we're already in a transaction.
 *
 *	This function deletes a public key from whatever storage mechanism we
 *	are using. Returns 0 if the key existed.
 */
 static int keyd_delete_key(struct onak_dbctx *dbctx,
-		uint64_t keyid, bool intrans)
+		struct openpgp_fingerprint *fp, bool intrans)
 {
 	int keyd_fd = (intptr_t) dbctx->priv;
 
 	if (keyd_send_cmd(keyd_fd, KEYD_CMD_DELETE)) {
-		write(keyd_fd, &keyid, sizeof(keyid));
+		write(keyd_fd, fp, sizeof(*fp));
 	}
 
 	return 0;
diff --git a/keydb_keyring.c b/keydb_keyring.c
index 62371ae..e8c056b 100644
--- a/keydb_keyring.c
+++ b/keydb_keyring.c
@@ -161,13 +161,13 @@ static int keyring_store_key(struct onak_dbctx *dbctx,
 
 /**
  *	delete_key - Given a keyid delete the key from storage.
- *	@keyid: The keyid to delete.
+ *	@fp: The fingerprint of the key to delete.
  *	@intrans: If we're already in a transaction.
  *
  *	We don't support removing keys from a keyring file.
  */
 static int keyring_delete_key(struct onak_dbctx *dbctx,
-		uint64_t keyid, bool intrans)
+		struct openpgp_fingerprint *fp, bool intrans)
 {
 	return 1;
 }
@@ -254,7 +254,7 @@ static int keyring_parse_keys(struct onak_keyring_dbctx *privctx)
 	 * Walk the keyring file, noting the start of each public key and the
 	 * total length of packets associated with it.
 	 */
-	pos = start = totlen = 0;
+	len = pos = start = totlen = 0;
 	while (((privctx->length - pos) > 5) && (privctx->file[pos] & 0x80)) {
 		if (privctx->file[pos] & 0x40) {
 			tag = privctx->file[pos] & 0x3F;
diff --git a/keydb_pg.c b/keydb_pg.c
index 0b58d28..7663c3d 100644
--- a/keydb_pg.c
+++ b/keydb_pg.c
@@ -247,13 +247,14 @@ static int pg_fetch_key_text(struct onak_dbctx *dbctx,
 
 /**
  *	delete_key - Given a keyid delete the key from storage.
- *	@keyid: The keyid to delete.
+ *	@fp: The fingerprint of the key to delete.
  *	@intrans: If we're already in a transaction.
  *
  *	This function deletes a public key from whatever storage mechanism we
  *	are using. Returns 0 if the key existed.
  */
-static int pg_delete_key(struct onak_dbctx *dbctx, uint64_t keyid, bool intrans)
+static int pg_delete_key(struct onak_dbctx *dbctx,
+		struct openpgp_fingerprint *fp, bool intrans)
 {
 	PGconn *dbconn = (PGconn *) dbctx->priv;
 	PGresult *result = NULL;
@@ -262,11 +263,14 @@ static int pg_delete_key(struct onak_dbctx *dbctx, uint64_t keyid, bool intrans)
 	int found = 1;
 	int i;
 	Oid key_oid;
+	uint64_t keyid;
 
 	if (!intrans) {
 		result = PQexec(dbconn, "BEGIN");
 		PQclear(result);
 	}
+
+	keyid = fingerprint2keyid(fp);
 	
 	snprintf(statement, 1023,
 			"SELECT keydata FROM onak_keys WHERE keyid = '%"
@@ -347,6 +351,7 @@ static int pg_store_key(struct onak_dbctx *dbctx,
 	int i;
 	uint64_t keyid;
 	struct pg_fc_ctx fcctx;
+	struct openpgp_fingerprint fp;
 
 	if (!intrans) {
 		result = PQexec(dbconn, "BEGIN");
@@ -367,7 +372,8 @@ static int pg_store_key(struct onak_dbctx *dbctx,
 	 * it definitely needs updated.
 	 */
 	if (update) {
-		pg_delete_key(dbctx, keyid, true);
+		get_fingerprint(publickey->publickey, &fp);
+		pg_delete_key(dbctx, &fp, true);
 	}
 
 	next = publickey->next;
diff --git a/keydb_stacked.c b/keydb_stacked.c
index 59c988d..c715c63 100644
--- a/keydb_stacked.c
+++ b/keydb_stacked.c
@@ -71,7 +71,8 @@ static int stacked_store_key(struct onak_dbctx *dbctx,
 			publickey, intrans, update);
 }
 
-static int stacked_delete_key(struct onak_dbctx *dbctx, uint64_t keyid,
+static int stacked_delete_key(struct onak_dbctx *dbctx,
+		struct openpgp_fingerprint *fp,
 		bool intrans)
 {
 	struct onak_stacked_dbctx *privctx =
@@ -80,7 +81,7 @@ static int stacked_delete_key(struct onak_dbctx *dbctx, uint64_t keyid,
 			(struct onak_dbctx *) privctx->backends->object;
 
 	return backend->delete_key(backend,
-			keyid, intrans);
+			fp, intrans);
 }
 
 static int stacked_update_keys(struct onak_dbctx *dbctx,
diff --git a/onak.c b/onak.c
index b7cfc80..384b674 100644
--- a/onak.c
+++ b/onak.c
@@ -387,9 +387,16 @@ int main(int argc, char *argv[])
 				puts("Key not found");
 			}
 		} else if (!strcmp("delete", argv[optind])) {
-			dbctx->delete_key(dbctx,
-					dbctx->getfullkeyid(dbctx, keyid),
-					false);
+			if (!isfp) {
+				if (dbctx->fetch_key_id(dbctx, keyid, &keys,
+							true)) {
+					get_fingerprint(keys->publickey,
+							&fingerprint);
+					dbctx->delete_key(dbctx, &fingerprint,
+							true);
+				}
+			} else
+				dbctx->delete_key(dbctx, &fingerprint, false);
 		} else if (!strcmp("get", argv[optind])) {
 			if (!(ishex || isfp)) {
 				puts("Can't get a key on uid text."
@@ -448,7 +455,8 @@ int main(int argc, char *argv[])
 		} else if (!strcmp("reindex", argv[optind])) {
 			dbctx->starttrans(dbctx);
 			if (dbctx->fetch_key_id(dbctx, keyid, &keys, true)) {
-				dbctx->delete_key(dbctx, keyid, true);
+				get_fingerprint(keys->publickey, &fingerprint);
+				dbctx->delete_key(dbctx, &fingerprint, true);
 				cleankeys(&keys, config.clean_policies);
 				dbctx->store_key(dbctx, keys, true, false);
 			} else {
-- 
2.39.2