[pacman-dev] [PATCH 2/6] Rein in the complexity of the signature type

Dan McGee dan at archlinux.org
Fri Apr 22 17:40:07 EDT 2011


Given that we offer no transparency into the pmpgpsig_t type, we don't
really need to expose it outside of the library, and at this point, we
don't need it at all. Don't decode anything except when checking
signatures. For packages/files not from a sync database, we now just
read the signature file directly anyway.

Also push the decoding logic down further into the check method so we
don't need this hanging out in a less than ideal place. This will make
it easier to conditionally compile things down the road.

Signed-off-by: Dan McGee <dan at archlinux.org>
---
 lib/libalpm/alpm.h    |    3 --
 lib/libalpm/be_sync.c |    2 +-
 lib/libalpm/package.c |   41 +-------------------------------
 lib/libalpm/package.h |    3 +-
 lib/libalpm/signing.c |   63 +++++++++++++++++++++++++++++++++++++++++-------
 lib/libalpm/signing.h |   12 +--------
 lib/libalpm/sync.c    |    3 +-
 7 files changed, 58 insertions(+), 69 deletions(-)

diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h
index 6818cb2..8d58b7c 100644
--- a/lib/libalpm/alpm.h
+++ b/lib/libalpm/alpm.h
@@ -76,7 +76,6 @@ typedef enum _pgp_verify_t {
 
 typedef struct __pmdb_t pmdb_t;
 typedef struct __pmpkg_t pmpkg_t;
-typedef struct __pmpgpsig_t pmpgpsig_t;
 typedef struct __pmdelta_t pmdelta_t;
 typedef struct __pmgrp_t pmgrp_t;
 typedef struct __pmtrans_t pmtrans_t;
@@ -476,8 +475,6 @@ const char *alpm_pkg_get_packager(pmpkg_t *pkg);
  */
 const char *alpm_pkg_get_md5sum(pmpkg_t *pkg);
 
-const pmpgpsig_t *alpm_pkg_get_pgpsig(pmpkg_t *pkg);
-
 /** Returns the architecture for which the package was built.
  * @param pkg a pointer to package
  * @return a reference to an internal string
diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c
index 3dfea14..591747d 100644
--- a/lib/libalpm/be_sync.c
+++ b/lib/libalpm/be_sync.c
@@ -456,7 +456,7 @@ static int sync_db_read(pmdb_t *db, struct archive *archive,
 				/* we don't do anything with this value right now */
 				READ_NEXT(line);
 			} else if(strcmp(line, "%PGPSIG%") == 0) {
-				READ_AND_STORE(pkg->pgpsig.base64_data);
+				READ_AND_STORE(pkg->base64_sig);
 			} else if(strcmp(line, "%REPLACES%") == 0) {
 				READ_AND_STORE_ALL(pkg->replaces);
 			} else if(strcmp(line, "%DEPENDS%") == 0) {
diff --git a/lib/libalpm/package.c b/lib/libalpm/package.c
index 8c927c1..393dae0 100644
--- a/lib/libalpm/package.c
+++ b/lib/libalpm/package.c
@@ -36,7 +36,6 @@
 #include "delta.h"
 #include "handle.h"
 #include "deps.h"
-#include "base64.h"
 
 /** \addtogroup alpm_packages Package Functions
  * @brief Functions to manipulate libalpm packages
@@ -197,43 +196,6 @@ const char SYMEXPORT *alpm_pkg_get_md5sum(pmpkg_t *pkg)
 	return pkg->ops->get_md5sum(pkg);
 }
 
-static int decode_pgpsig(pmpkg_t *pkg) {
-	const int len = strlen(pkg->pgpsig.base64_data);
-	const unsigned char *usline = (const unsigned char *)pkg->pgpsig.base64_data;
-	int ret, destlen = 0;
-	/* get the necessary size for the buffer by passing 0 */
-	ret = base64_decode(NULL, &destlen, usline, len);
-	/* alloc our memory and repeat the call to decode */
-	MALLOC(pkg->pgpsig.data, (size_t)destlen, goto error);
-	ret = base64_decode(pkg->pgpsig.data, &destlen, usline, len);
-	pkg->pgpsig.len = destlen;
-	if(ret != 0) {
-		goto error;
-	}
-
-	/* we no longer have a need for this */
-	FREE(pkg->pgpsig.base64_data);
-	return 0;
-
-error:
-	FREE(pkg->pgpsig.data);
-	pkg->pgpsig.len = 0;
-	return 1;
-}
-
-const pmpgpsig_t SYMEXPORT *alpm_pkg_get_pgpsig(pmpkg_t *pkg)
-{
-	ALPM_LOG_FUNC;
-
-	/* Sanity checks */
-	ASSERT(pkg != NULL, RET_ERR(PM_ERR_WRONG_ARGS, NULL));
-
-	if(pkg->pgpsig.data == NULL && pkg->pgpsig.base64_data != NULL) {
-		decode_pgpsig(pkg);
-	}
-	return &(pkg->pgpsig);
-}
-
 const char SYMEXPORT *alpm_pkg_get_arch(pmpkg_t *pkg)
 {
 	return pkg->ops->get_arch(pkg);
@@ -468,8 +430,7 @@ void _alpm_pkg_free(pmpkg_t *pkg)
 	FREE(pkg->url);
 	FREE(pkg->packager);
 	FREE(pkg->md5sum);
-	FREE(pkg->pgpsig.base64_data);
-	FREE(pkg->pgpsig.data);
+	FREE(pkg->base64_sig);
 	FREE(pkg->arch);
 	FREELIST(pkg->licenses);
 	FREELIST(pkg->replaces);
diff --git a/lib/libalpm/package.h b/lib/libalpm/package.h
index c84e222..390dcd9 100644
--- a/lib/libalpm/package.h
+++ b/lib/libalpm/package.h
@@ -97,10 +97,9 @@ struct __pmpkg_t {
 	char *url;
 	char *packager;
 	char *md5sum;
+	char *base64_sig;
 	char *arch;
 
-	pmpgpsig_t pgpsig;
-
 	time_t builddate;
 	time_t installdate;
 
diff --git a/lib/libalpm/signing.c b/lib/libalpm/signing.c
index 28b7ad9..d69fd52 100644
--- a/lib/libalpm/signing.c
+++ b/lib/libalpm/signing.c
@@ -28,6 +28,7 @@
 /* libalpm */
 #include "signing.h"
 #include "package.h"
+#include "base64.h"
 #include "util.h"
 #include "log.h"
 #include "alpm.h"
@@ -92,13 +93,48 @@ error:
 }
 
 /**
+ * Decode a loaded signature in base64 form.
+ * @param base64_data the signature to attempt to decode
+ * @param data the decoded data; must be freed by the caller
+ * @param data_len the length of the returned data
+ * @return 0 on success, 1 on failure to properly decode
+ */
+static int decode_signature(const char *base64_data,
+		unsigned char **data, int *data_len) {
+	unsigned char *usline;
+	int len;
+
+	len = strlen(base64_data);
+	usline = (unsigned char *)base64_data;
+	int ret, destlen = 0;
+	/* get the necessary size for the buffer by passing 0 */
+	ret = base64_decode(NULL, &destlen, usline, len);
+	if(ret != 0 || ret != POLARSSL_ERR_BASE64_BUFFER_TOO_SMALL) {
+		goto error;
+	}
+	/* alloc our memory and repeat the call to decode */
+	MALLOC(data, (size_t)destlen, goto error);
+	ret = base64_decode(*data, &destlen, usline, len);
+	if(ret != 0) {
+		goto error;
+	}
+	*data_len = destlen;
+	return 0;
+
+error:
+	*data = NULL;
+	*data_len = 0;
+	return 1;
+}
+
+/**
  * Check the PGP signature for the given file.
  * @param path the full path to a file
- * @param sig PGP signature data in raw form (already decoded); if NULL, expect
- * a signature file next to 'path'
+ * @param base64_sig PGP signature data in base64 encoding; if NULL, expect a
+ * signature file next to 'path'
  * @return a int value : 0 (valid), 1 (invalid), -1 (an error occured)
  */
-int _alpm_gpgme_checksig(const char *path, const pmpgpsig_t *sig)
+int _alpm_gpgme_checksig(const char *path, const char *base64_sig)
 {
 	int ret = 0;
 	gpgme_error_t err;
@@ -107,6 +143,7 @@ int _alpm_gpgme_checksig(const char *path, const pmpgpsig_t *sig)
 	gpgme_verify_result_t result;
 	gpgme_signature_t gpgsig;
 	char *sigpath = NULL;
+	unsigned char *decoded_sigdata = NULL;
 	FILE *file = NULL, *sigfile = NULL;
 
 	ALPM_LOG_FUNC;
@@ -115,7 +152,7 @@ int _alpm_gpgme_checksig(const char *path, const pmpgpsig_t *sig)
 		RET_ERR(PM_ERR_NOT_A_FILE, -1);
 	}
 
-	if(!sig) {
+	if(!base64_sig) {
 		size_t len = strlen(path) + 5;
 		CALLOC(sigpath, len, sizeof(char), RET_ERR(PM_ERR_MEMORY, -1));
 		snprintf(sigpath, len, "%s.sig", path);
@@ -124,8 +161,6 @@ int _alpm_gpgme_checksig(const char *path, const pmpgpsig_t *sig)
 			FREE(sigpath);
 			RET_ERR(PM_ERR_SIG_UNKNOWN, -1);
 		}
-	} else if(!sig->data) {
-		 RET_ERR(PM_ERR_SIG_UNKNOWN, -1);
 	}
 
 	if(gpgme_init()) {
@@ -153,9 +188,17 @@ int _alpm_gpgme_checksig(const char *path, const pmpgpsig_t *sig)
 	CHECK_ERR();
 
 	/* next create data object for the signature */
-	if(sig) {
+	if(base64_sig) {
 		/* memory-based, we loaded it from a sync DB */
-		err = gpgme_data_new_from_mem(&sigdata, (char *)sig->data, sig->len, 0);
+		int data_len;
+		int decode_ret = decode_signature(base64_sig,
+				&decoded_sigdata, &data_len);
+		if(decode_ret) {
+			ret = -1;
+			goto error;
+		}
+		err = gpgme_data_new_from_mem(&sigdata,
+				(char *)decoded_sigdata, data_len, 0);
 	} else {
 		/* file-based, it is on disk */
 		sigfile = fopen(sigpath, "rb");
@@ -222,6 +265,7 @@ error:
 		fclose(file);
 	}
 	FREE(sigpath);
+	FREE(decoded_sigdata);
 	if(err != GPG_ERR_NO_ERROR) {
 		_alpm_log(PM_LOG_ERROR, _("GPGME error: %s\n"), gpgme_strerror(err));
 		RET_ERR(PM_ERR_GPGME, -1);
@@ -257,8 +301,7 @@ int SYMEXPORT alpm_pkg_check_pgp_signature(pmpkg_t *pkg)
 	ALPM_LOG_FUNC;
 	ASSERT(pkg != NULL, return 0);
 
-	return _alpm_gpgme_checksig(alpm_pkg_get_filename(pkg),
-			alpm_pkg_get_pgpsig(pkg));
+	return _alpm_gpgme_checksig(alpm_pkg_get_filename(pkg), pkg->base64_sig);
 }
 
 /**
diff --git a/lib/libalpm/signing.h b/lib/libalpm/signing.h
index 5976cf2..8d8c164 100644
--- a/lib/libalpm/signing.h
+++ b/lib/libalpm/signing.h
@@ -21,17 +21,7 @@
 
 #include "alpm.h"
 
-struct __pmpgpsig_t {
-	/* we will either store the encoded data or the raw data-
-	 * this way we can decode on an as-needed basis since most
-	 * operations won't require the overhead of base64 decodes
-	 * on all packages in a sync repository. */
-	char *base64_data;
-	unsigned char *data;
-	size_t len;
-};
-
-int _alpm_gpgme_checksig(const char *path, const pmpgpsig_t *sig);
+int _alpm_gpgme_checksig(const char *path, const char *base64_sig);
 pgp_verify_t _alpm_db_get_sigverify_level(pmdb_t *db);
 
 #endif /* _ALPM_SIGNING_H */
diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c
index 4ce62e6..0ff0c79 100644
--- a/lib/libalpm/sync.c
+++ b/lib/libalpm/sync.c
@@ -856,7 +856,6 @@ int _alpm_sync_commit(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t **data)
 		const char *filename = alpm_pkg_get_filename(spkg);
 		char *filepath = _alpm_filecache_find(filename);
 		const char *md5sum = alpm_pkg_get_md5sum(spkg);
-		const pmpgpsig_t *pgpsig = alpm_pkg_get_pgpsig(spkg);
 		pgp_verify_t check_sig;
 
 		/* check md5sum first */
@@ -872,7 +871,7 @@ int _alpm_sync_commit(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t **data)
 		check_sig = _alpm_db_get_sigverify_level(sdb);
 
 		if(check_sig != PM_PGP_VERIFY_NEVER) {
-			int ret = _alpm_gpgme_checksig(filepath, pgpsig);
+			int ret = _alpm_gpgme_checksig(filepath, spkg->base64_sig);
 			if((check_sig == PM_PGP_VERIFY_ALWAYS && ret != 0) ||
 					(check_sig == PM_PGP_VERIFY_OPTIONAL && ret == 1)) {
 				errors++;
-- 
1.7.4.4



More information about the pacman-dev mailing list