[pacman-dev] [PATCH 1/3] Revamp signing checks

Dan McGee dan at archlinux.org
Thu Jul 28 20:04:57 EDT 2011


This ensures we are actually making correct use of the information gpgme
is returning to us. Marginal being allowed was obvious before, but
Unknown should deal with trust level, and not the presence or lack
thereof of a public key to validate the signature with.

Return status and validity information in two separate values so check
methods and the frontend can use them independently. For now, we treat
expired keys as valid, while expired signatures are invalid.

Signed-off-by: Dan McGee <dan at archlinux.org>
---
 lib/libalpm/alpm.h    |   22 +++++++--
 lib/libalpm/signing.c |  122 +++++++++++++++++++++++++++++++-----------------
 src/pacman/util.c     |   42 ++++++++++++----
 3 files changed, 127 insertions(+), 59 deletions(-)

diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h
index d9f3504..a91b00f 100644
--- a/lib/libalpm/alpm.h
+++ b/lib/libalpm/alpm.h
@@ -104,15 +104,26 @@ typedef enum _alpm_siglevel_t {
 } alpm_siglevel_t;
 
 /**
- * PGP signature verification return codes
+ * PGP signature verification status return codes
  */
 typedef enum _alpm_sigstatus_t {
-	ALPM_SIGSTATUS_VALID = 0,
-	ALPM_SIGSTATUS_MARGINAL,
-	ALPM_SIGSTATUS_UNKNOWN,
-	ALPM_SIGSTATUS_BAD
+	ALPM_SIGSTATUS_VALID,
+	ALPM_SIGSTATUS_KEY_EXPIRED,
+	ALPM_SIGSTATUS_SIG_EXPIRED,
+	ALPM_SIGSTATUS_KEY_UNKNOWN,
+	ALPM_SIGSTATUS_INVALID
 } alpm_sigstatus_t;
 
+/**
+ * PGP signature verification status return codes
+ */
+typedef enum _alpm_sigvalidity_t {
+	ALPM_SIGVALIDITY_FULL,
+	ALPM_SIGVALIDITY_MARGINAL,
+	ALPM_SIGVALIDITY_NEVER,
+	ALPM_SIGVALIDITY_UNKNOWN
+} alpm_sigvalidity_t;
+
 /*
  * Structures
  */
@@ -202,6 +213,7 @@ typedef struct _alpm_backup_t {
 typedef struct _alpm_sigresult_t {
 	int count;
 	alpm_sigstatus_t *status;
+	alpm_sigvalidity_t *validity;
 	char **uid;
 } alpm_sigresult_t;
 
diff --git a/lib/libalpm/signing.c b/lib/libalpm/signing.c
index 275851c..78e6264 100644
--- a/lib/libalpm/signing.c
+++ b/lib/libalpm/signing.c
@@ -305,8 +305,9 @@ int _alpm_gpgme_checksig(alpm_handle_t *handle, const char *path,
 	_alpm_log(handle, ALPM_LOG_DEBUG, "%d signatures returned\n", sigcount);
 
 	result->status = calloc(sigcount, sizeof(alpm_sigstatus_t));
+	result->validity = calloc(sigcount, sizeof(alpm_sigvalidity_t));
 	result->uid = calloc(sigcount, sizeof(char*));
-	if(!result->status || !result->uid) {
+	if(!result->status || !result->validity || !result->uid) {
 		handle->pm_errno = ALPM_ERR_MEMORY;
 		goto error;
 	}
@@ -316,6 +317,7 @@ int _alpm_gpgme_checksig(alpm_handle_t *handle, const char *path,
 			gpgsig = gpgsig->next, sigcount++) {
 		alpm_list_t *summary_list, *summary;
 		alpm_sigstatus_t status;
+		alpm_sigvalidity_t validity;
 		gpgme_key_t key;
 
 		_alpm_log(handle, ALPM_LOG_DEBUG, "fingerprint: %s\n", gpgsig->fpr);
@@ -335,6 +337,8 @@ int _alpm_gpgme_checksig(alpm_handle_t *handle, const char *path,
 		if(gpg_err_code(err) == GPG_ERR_EOF) {
 			_alpm_log(handle, ALPM_LOG_DEBUG, "key lookup failed, unknown key\n");
 			err = GPG_ERR_NO_ERROR;
+			STRDUP(result->uid[sigcount], gpgsig->fpr,
+					handle->pm_errno = ALPM_ERR_MEMORY; goto error);
 		} else {
 			CHECK_ERR();
 			if(key->uids) {
@@ -346,34 +350,52 @@ int _alpm_gpgme_checksig(alpm_handle_t *handle, const char *path,
 			gpgme_key_unref(key);
 		}
 
-		if(gpgsig->summary & GPGME_SIGSUM_VALID) {
-			/* definite good signature */
-			_alpm_log(handle, ALPM_LOG_DEBUG, "result: valid signature\n");
-			status = ALPM_SIGSTATUS_VALID;
-		} else if(gpgsig->summary & GPGME_SIGSUM_GREEN) {
-			/* good signature */
-			_alpm_log(handle, ALPM_LOG_DEBUG, "result: green signature\n");
-			status = ALPM_SIGSTATUS_VALID;
-		} else if(gpgsig->summary & GPGME_SIGSUM_RED) {
-			/* definite bad signature, error */
-			_alpm_log(handle, ALPM_LOG_DEBUG, "result: red signature\n");
-			status = ALPM_SIGSTATUS_BAD;
-		} else if(gpgsig->summary & GPGME_SIGSUM_KEY_MISSING) {
-			_alpm_log(handle, ALPM_LOG_DEBUG, "result: signature from unknown key\n");
-			status = ALPM_SIGSTATUS_UNKNOWN;
-		} else if(gpgsig->summary & GPGME_SIGSUM_KEY_EXPIRED) {
-			_alpm_log(handle, ALPM_LOG_DEBUG, "result: key expired\n");
-			status = ALPM_SIGSTATUS_BAD;
-		} else if(gpgsig->summary & GPGME_SIGSUM_SIG_EXPIRED) {
-			_alpm_log(handle, ALPM_LOG_DEBUG, "result: signature expired\n");
-			status = ALPM_SIGSTATUS_BAD;
+		switch(gpg_err_code(gpgsig->status)) {
+			/* good cases */
+			case GPG_ERR_NO_ERROR:
+				status = ALPM_SIGSTATUS_VALID;
+				break;
+			case GPG_ERR_KEY_EXPIRED:
+				status = ALPM_SIGSTATUS_KEY_EXPIRED;
+				break;
+			/* bad cases */
+			case GPG_ERR_SIG_EXPIRED:
+				status = ALPM_SIGSTATUS_SIG_EXPIRED;
+				break;
+			case GPG_ERR_NO_PUBKEY:
+				status = ALPM_SIGSTATUS_KEY_UNKNOWN;
+				break;
+			case GPG_ERR_BAD_SIGNATURE:
+			default:
+				status = ALPM_SIGSTATUS_INVALID;
+				break;
+		}
+
+		if(status == ALPM_SIGSTATUS_VALID
+				|| status == ALPM_SIGSTATUS_KEY_EXPIRED) {
+			switch(gpgsig->validity) {
+				case GPGME_VALIDITY_ULTIMATE:
+				case GPGME_VALIDITY_FULL:
+					validity = ALPM_SIGVALIDITY_FULL;
+					break;
+				case GPGME_VALIDITY_MARGINAL:
+					validity = ALPM_SIGVALIDITY_MARGINAL;
+					break;
+				case GPGME_VALIDITY_NEVER:
+					validity = ALPM_SIGVALIDITY_NEVER;
+					break;
+				case GPGME_VALIDITY_UNKNOWN:
+				case GPGME_VALIDITY_UNDEFINED:
+				default:
+					validity = ALPM_SIGVALIDITY_UNKNOWN;
+					break;
+			}
 		} else {
-			/* we'll capture everything else here */
-			_alpm_log(handle, ALPM_LOG_DEBUG, "result: invalid signature\n");
-			status = ALPM_SIGSTATUS_BAD;
+			validity = ALPM_SIGVALIDITY_NEVER;
 		}
 
 		result->status[sigcount] = status;
+		result->validity[sigcount] = validity;
 	}
 
 	ret = 0;
@@ -429,31 +451,45 @@ int _alpm_check_pgp_helper(alpm_handle_t *handle, const char *path,
 		/* ret will already be -1 */
 	} else {
 		int num;
-		for(num = 0; num < result.count; num++) {
-			/* fallthrough in this case block is on purpose. if one allows unknown
-			 * signatures, then a marginal signature should be allowed as well, and
-			 * if neither of these are allowed we fall all the way through to bad. */
+		for(num = 0; !ret && num < result.count; num++) {
 			switch(result.status[num]) {
 				case ALPM_SIGSTATUS_VALID:
+				case ALPM_SIGSTATUS_KEY_EXPIRED:
 					_alpm_log(handle, ALPM_LOG_DEBUG, "signature is valid\n");
-					break;
-				case ALPM_SIGSTATUS_MARGINAL:
-					if(marginal) {
-						_alpm_log(handle, ALPM_LOG_DEBUG, "allowing marginal signature\n");
-						break;
-					}
-				case ALPM_SIGSTATUS_UNKNOWN:
-					if(unknown) {
-						_alpm_log(handle, ALPM_LOG_DEBUG, "allowing unknown signature\n");
-						break;
+					switch(result.validity[num]) {
+						case ALPM_SIGVALIDITY_FULL:
+							_alpm_log(handle, ALPM_LOG_DEBUG, "signature is fully trusted\n");
+							break;
+						case ALPM_SIGVALIDITY_MARGINAL:
+							_alpm_log(handle, ALPM_LOG_DEBUG, "signature is marginal trust\n");
+							if(!marginal) {
+								ret = -1;
+							}
+							break;
+						case ALPM_SIGVALIDITY_UNKNOWN:
+							_alpm_log(handle, ALPM_LOG_DEBUG, "signature is unknown trust\n");
+							if(!unknown) {
+								ret = -1;
+							}
+							break;
+						case ALPM_SIGVALIDITY_NEVER:
+							_alpm_log(handle, ALPM_LOG_DEBUG, "signature should never be trusted\n");
+							ret = -1;
+							break;
 					}
-				case ALPM_SIGSTATUS_BAD:
-				default:
-					_alpm_log(handle, ALPM_LOG_DEBUG, "signature is invalid\n");
-					handle->pm_errno = invalid_err;
+					break;
+				case ALPM_SIGSTATUS_SIG_EXPIRED:
+				case ALPM_SIGSTATUS_KEY_UNKNOWN:
+				case ALPM_SIGSTATUS_INVALID:
+					_alpm_log(handle, ALPM_LOG_DEBUG, "signature is not valid\n");
 					ret = -1;
+					break;
 			}
 		}
+
+		if(ret) {
+			handle->pm_errno = invalid_err;
+		}
 	}
 
 	alpm_sigresult_cleanup(&result);
diff --git a/src/pacman/util.c b/src/pacman/util.c
index 7065abd..8765da7 100644
--- a/src/pacman/util.c
+++ b/src/pacman/util.c
@@ -678,7 +678,7 @@ void signature_display(const char *title, alpm_sigresult_t *result)
 		int i;
 		for(i = 0; i < result->count; i++) {
 			char sigline[PATH_MAX];
-			const char *validity, *name;
+			const char *status, *validity, *name;
 			/* Don't re-indent the first result */
 			if(i != 0) {
 				int j;
@@ -688,22 +688,42 @@ void signature_display(const char *title, alpm_sigresult_t *result)
 			}
 			switch(result->status[i]) {
 				case ALPM_SIGSTATUS_VALID:
-					validity = _("Valid signature");
+					status = _("Valid");
 					break;
-				case ALPM_SIGSTATUS_MARGINAL:
-					validity = _("Marginal signature");
+				case ALPM_SIGSTATUS_KEY_EXPIRED:
+					status = _("Key expired");
 					break;
-				case ALPM_SIGSTATUS_UNKNOWN:
-					validity = _("Unknown signature");
+				case ALPM_SIGSTATUS_SIG_EXPIRED:
+					status = _("Expired");
 					break;
-				case ALPM_SIGSTATUS_BAD:
-					validity = _("Invalid signature");
+				case ALPM_SIGSTATUS_INVALID:
+					status = _("Invalid");
 					break;
+				case ALPM_SIGSTATUS_KEY_UNKNOWN:
+					status = _("Key unknown");
+					break;
+				default:
+					status = _("Signature error");
+					break;
+			}
+			switch(result->validity[i]) {
+				case ALPM_SIGVALIDITY_FULL:
+					validity = _("fully trusted");
+					break;
+				case ALPM_SIGVALIDITY_MARGINAL:
+					validity = _("marginal trusted");
+					break;
+				case ALPM_SIGVALIDITY_NEVER:
+					validity = _("never trusted");
+					break;
+				case ALPM_SIGVALIDITY_UNKNOWN:
 				default:
-					validity = _("Signature error");
+					validity = _("unknown trust");
+					break;
 			}
-			name = result->uid[i] ? result->uid[i] : _("<Key Unknown>");
-			snprintf(sigline, PATH_MAX, _("%s from \"%s\""), validity, name);
+			name = result->uid[i] ? result->uid[i] : _("{Key Unknown}");
+			snprintf(sigline, PATH_MAX, _("%s, %s from \"%s\""),
+					status, validity, name);
 			indentprint(sigline, len);
 			printf("\n");
 		}
-- 
1.7.6



More information about the pacman-dev mailing list