[pacman-dev] [PATCH 5/6] Perform package verification at package load time

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


Both md5sum verification and PGP verification can and should be done at
package load time. This allows verification to happen as early as
possible for packages provided by filename and loaded in the frontend,
and moves more stuff out of sync_commit that doesn't really belong
there. This should also set the stage for simplified parallel loading of
packages later down the road.

Signed-off-by: Dan McGee <dan at archlinux.org>
---
 lib/libalpm/alpm.h       |    7 +++++--
 lib/libalpm/be_package.c |   34 +++++++++++++++++++++++++++-------
 lib/libalpm/package.h    |    4 ++++
 lib/libalpm/sync.c       |   40 +++++++++++-----------------------------
 src/pacman/query.c       |    2 +-
 src/pacman/sync.c        |    3 ++-
 src/pacman/upgrade.c     |    3 ++-
 src/util/testpkg.c       |    3 ++-
 8 files changed, 54 insertions(+), 42 deletions(-)

diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h
index 8d58b7c..c4f6cc0 100644
--- a/lib/libalpm/alpm.h
+++ b/lib/libalpm/alpm.h
@@ -382,11 +382,14 @@ int alpm_db_set_pkgreason(pmdb_t *db, const char *name, pmpkgreason_t reason);
  * The allocated structure should be freed using alpm_pkg_free().
  * @param filename location of the package tarball
  * @param full whether to stop the load after metadata is read or continue
- *             through the full archive
+ * through the full archive
+ * @param check_sig what level of package signature checking to perform on the
+ * package; note that this must be a '.sig' file type verification
  * @param pkg address of the package pointer
  * @return 0 on success, -1 on error (pm_errno is set accordingly)
  */
-int alpm_pkg_load(const char *filename, int full, pmpkg_t **pkg);
+int alpm_pkg_load(const char *filename, int full, pgp_verify_t check_sig,
+		pmpkg_t **pkg);
 
 /** Free a package.
  * @param pkg package pointer to free
diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c
index 6e65c7d..2044575 100644
--- a/lib/libalpm/be_package.c
+++ b/lib/libalpm/be_package.c
@@ -226,9 +226,10 @@ static int parse_descfile(struct archive *a, pmpkg_t *newpkg)
  *             through the full archive
  * @return An information filled pmpkg_t struct
  */
-static pmpkg_t *pkg_load(const char *pkgfile, int full)
+pmpkg_t *_alpm_pkg_load_internal(const char *pkgfile, int full,
+		const char *md5sum, const char *base64_sig, pgp_verify_t check_sig)
 {
-	int ret = ARCHIVE_OK;
+	int ret;
 	int config = 0;
 	struct archive *archive;
 	struct archive_entry *entry;
@@ -254,6 +255,27 @@ static pmpkg_t *pkg_load(const char *pkgfile, int full)
 		RET_ERR(PM_ERR_PKG_OPEN, NULL);
 	}
 
+	/* first steps- validate the package file */
+	_alpm_log(PM_LOG_DEBUG, "md5sum: %s\n", md5sum);
+	if(md5sum) {
+		_alpm_log(PM_LOG_DEBUG, "checking md5sum for %s\n", pkgfile);
+		if(_alpm_test_md5sum(pkgfile, md5sum) != 0) {
+			alpm_pkg_free(newpkg);
+			RET_ERR(PM_ERR_PKG_INVALID, NULL);
+		}
+	}
+
+	_alpm_log(PM_LOG_DEBUG, "base64_sig: %s\n", base64_sig);
+	if(check_sig != PM_PGP_VERIFY_NEVER) {
+		_alpm_log(PM_LOG_DEBUG, "checking signature for %s\n", pkgfile);
+		ret = _alpm_gpgme_checksig(pkgfile, base64_sig);
+		if((check_sig == PM_PGP_VERIFY_ALWAYS && ret != 0) ||
+				(check_sig == PM_PGP_VERIFY_OPTIONAL && ret == 1)) {
+			RET_ERR(PM_ERR_SIG_INVALID, NULL);
+		}
+	}
+
+	/* next- try to create an archive object to read in the package */
 	if((archive = archive_read_new()) == NULL) {
 		alpm_pkg_free(newpkg);
 		RET_ERR(PM_ERR_LIBARCHIVE, NULL);
@@ -332,7 +354,6 @@ static pmpkg_t *pkg_load(const char *pkgfile, int full)
 
 	/* internal fields for package struct */
 	newpkg->origin = PKG_FROM_FILE;
-	/* TODO eventually kill/move this? */
 	newpkg->origin_data.file = strdup(pkgfile);
 	newpkg->ops = get_file_pkg_ops();
 
@@ -359,16 +380,15 @@ error:
 	return NULL;
 }
 
-int SYMEXPORT alpm_pkg_load(const char *filename, int full, pmpkg_t **pkg)
+int SYMEXPORT alpm_pkg_load(const char *filename, int full,
+		pgp_verify_t check_sig, pmpkg_t **pkg)
 {
 	ALPM_LOG_FUNC;
 
 	/* Sanity checks */
-	ASSERT(filename != NULL && strlen(filename) != 0,
-			RET_ERR(PM_ERR_WRONG_ARGS, -1));
 	ASSERT(pkg != NULL, RET_ERR(PM_ERR_WRONG_ARGS, -1));
 
-	*pkg = pkg_load(filename, full);
+	*pkg = _alpm_pkg_load_internal(filename, full, NULL, NULL, check_sig);
 	if(*pkg == NULL) {
 		/* pm_errno is set by pkg_load */
 		return -1;
diff --git a/lib/libalpm/package.h b/lib/libalpm/package.h
index 390dcd9..debb239 100644
--- a/lib/libalpm/package.h
+++ b/lib/libalpm/package.h
@@ -139,6 +139,10 @@ pmpkg_t* _alpm_pkg_new(void);
 pmpkg_t *_alpm_pkg_dup(pmpkg_t *pkg);
 void _alpm_pkg_free(pmpkg_t *pkg);
 void _alpm_pkg_free_trans(pmpkg_t *pkg);
+
+pmpkg_t *_alpm_pkg_load_internal(const char *filename, int full,
+		const char *md5sum, const char *base64_sig, pgp_verify_t check_sig);
+
 int _alpm_pkg_cmp(const void *p1, const void *p2);
 int _alpm_pkg_compare_versions(pmpkg_t *local_pkg, pmpkg_t *pkg);
 pmpkg_t *_alpm_pkg_find(alpm_list_t *haystack, const char *needle);
diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c
index a702bac..4b42af4 100644
--- a/lib/libalpm/sync.c
+++ b/lib/libalpm/sync.c
@@ -862,45 +862,27 @@ int _alpm_sync_commit(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t **data)
 	for(i = trans->add; i; i = i->next, current++) {
 		pmpkg_t *spkg = i->data;
 		int percent = (current * 100) / numtargs;
+		const char *filename;
+		char *filepath;
+		pgp_verify_t check_sig;
+
+		PROGRESS(trans, PM_TRANS_PROGRESS_INTEGRITY_START, "", percent,
+				numtargs, current);
 		if(spkg->origin == PKG_FROM_FILE) {
 			continue; /* pkg_load() has been already called, this package is valid */
 		}
-		PROGRESS(trans, PM_TRANS_PROGRESS_INTEGRITY_START, "", percent,
-				numtargs, current);
 
-		const char *filename = alpm_pkg_get_filename(spkg);
-		char *filepath = _alpm_filecache_find(filename);
-		const char *md5sum = alpm_pkg_get_md5sum(spkg);
-		pgp_verify_t check_sig;
-
-		/* check md5sum first */
-		if(test_md5sum(trans, filepath, md5sum) != 0) {
-			errors++;
-			*data = alpm_list_add(*data, strdup(filename));
-			FREE(filepath);
-			continue;
-		}
-		/* check PGP signature next */
+		filename = alpm_pkg_get_filename(spkg);
+		filepath = _alpm_filecache_find(filename);
 		pmdb_t *sdb = alpm_pkg_get_db(spkg);
-
 		check_sig = _alpm_db_get_sigverify_level(sdb);
 
-		if(check_sig != PM_PGP_VERIFY_NEVER) {
-			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++;
-				*data = alpm_list_add(*data, strdup(filename));
-				FREE(filepath);
-				continue;
-			}
-		}
 		/* load the package file and replace pkgcache entry with it in the target list */
 		/* TODO: alpm_pkg_get_db() will not work on this target anymore */
 		_alpm_log(PM_LOG_DEBUG, "replacing pkgcache entry with package file for target %s\n", spkg->name);
-		pmpkg_t *pkgfile;
-		if(alpm_pkg_load(filepath, 1, &pkgfile) != 0) {
-			_alpm_pkg_free(pkgfile);
+		pmpkg_t *pkgfile =_alpm_pkg_load_internal(filepath, 1, spkg->md5sum,
+				spkg->base64_sig, check_sig);
+		if(!pkgfile) {
 			errors++;
 			*data = alpm_list_add(*data, strdup(filename));
 			FREE(filepath);
diff --git a/src/pacman/query.c b/src/pacman/query.c
index eaf3b9e..5ca52c3 100644
--- a/src/pacman/query.c
+++ b/src/pacman/query.c
@@ -547,7 +547,7 @@ int pacman_query(alpm_list_t *targets)
 		char *strname = alpm_list_getdata(i);
 
 		if(config->op_q_isfile) {
-			alpm_pkg_load(strname, 1, &pkg);
+			alpm_pkg_load(strname, 1, PM_PGP_VERIFY_OPTIONAL, &pkg);
 		} else {
 			pkg = alpm_db_get_pkg(db_local, strname);
 		}
diff --git a/src/pacman/sync.c b/src/pacman/sync.c
index 5529288..5fb8c34 100644
--- a/src/pacman/sync.c
+++ b/src/pacman/sync.c
@@ -211,7 +211,8 @@ static int sync_cleancache(int level)
 			/* attempt to load the package, prompt removal on failures as we may have
 			 * files here that aren't valid packages. we also don't need a full
 			 * load of the package, just the metadata. */
-			if(alpm_pkg_load(path, 0, &localpkg) != 0 || localpkg == NULL) {
+			if(alpm_pkg_load(path, 0, PM_PGP_VERIFY_NEVER, &localpkg) != 0
+					|| localpkg == NULL) {
 				if(yesno(_("File %s does not seem to be a valid package, remove it?"), path)) {
 					if(localpkg) {
 						alpm_pkg_free(localpkg);
diff --git a/src/pacman/upgrade.c b/src/pacman/upgrade.c
index 5b89400..0ffc94c 100644
--- a/src/pacman/upgrade.c
+++ b/src/pacman/upgrade.c
@@ -42,6 +42,7 @@
 int pacman_upgrade(alpm_list_t *targets)
 {
 	alpm_list_t *i, *data = NULL;
+	pgp_verify_t check_sig = alpm_option_get_default_sigverify();
 	int retval = 0;
 
 	if(targets == NULL) {
@@ -75,7 +76,7 @@ int pacman_upgrade(alpm_list_t *targets)
 		char *targ = alpm_list_getdata(i);
 		pmpkg_t *pkg;
 
-		if(alpm_pkg_load(targ, 1, &pkg) != 0) {
+		if(alpm_pkg_load(targ, 1, check_sig, &pkg) != 0) {
 			pm_fprintf(stderr, PM_LOG_ERROR, "'%s': %s\n",
 					targ, alpm_strerrorlast());
 			trans_release();
diff --git a/src/util/testpkg.c b/src/util/testpkg.c
index e562dde..ad6ec30 100644
--- a/src/util/testpkg.c
+++ b/src/util/testpkg.c
@@ -55,7 +55,8 @@ int main(int argc, char *argv[])
 	/* let us get log messages from libalpm */
 	alpm_option_set_logcb(output_cb);
 
-	if(alpm_pkg_load(argv[1], 1, &pkg) == -1 || pkg == NULL) {
+	if(alpm_pkg_load(argv[1], 1, PM_PGP_VERIFY_OPTIONAL, &pkg) == -1
+			|| pkg == NULL) {
 		switch(pm_errno) {
 			case PM_ERR_PKG_OPEN:
 				printf("Cannot open the given file.\n");
-- 
1.7.4.4



More information about the pacman-dev mailing list