[pacman-dev] [PATCH 5/9] Do database signature checking at load time

Dan McGee dan at archlinux.org
Wed Jun 8 03:51:48 EDT 2011


This is the ideal place to do it as all clients should be checking the
return value and ensuring there are no errors. This is similar to
pkg_load().

We also add an additional step of validation after we download a new
database; a subsequent '-y' operation can potentially invalidate the
original check at registration time.

Signed-off-by: Dan McGee <dan at archlinux.org>
---
 lib/libalpm/alpm.h      |    5 +++-
 lib/libalpm/be_sync.c   |   56 ++++++++++++++++++++++++++++++++++++++++++++++-
 lib/libalpm/db.c        |    5 ++-
 lib/libalpm/db.h        |    3 +-
 src/pacman/conf.c       |   16 ++-----------
 src/util/cleanupdelta.c |    2 +-
 src/util/testdb.c       |    2 +-
 test/pacman/pactest.py  |    7 ++---
 8 files changed, 72 insertions(+), 24 deletions(-)

diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h
index d7ee7d1..1e1bfdd 100644
--- a/lib/libalpm/alpm.h
+++ b/lib/libalpm/alpm.h
@@ -276,9 +276,12 @@ alpm_list_t *alpm_option_get_syncdbs(pmhandle_t *handle);
 /** Register a sync database of packages.
  * @param handle the context handle
  * @param treename the name of the sync repository
+ * @param check_sig what level of signature checking to perform on the
+ * database; note that this must be a '.sig' file type verification
  * @return a pmdb_t* on success (the value), NULL on error
  */
-pmdb_t *alpm_db_register_sync(pmhandle_t *handle, const char *treename);
+pmdb_t *alpm_db_register_sync(pmhandle_t *handle, const char *treename,
+		pgp_verify_t check_sig);
 
 /** Unregister a package database.
  * @param db pointer to the package database to unregister
diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c
index ccc8fdb..c6fee84 100644
--- a/lib/libalpm/be_sync.c
+++ b/lib/libalpm/be_sync.c
@@ -20,7 +20,9 @@
 
 #include "config.h"
 
+#include <errno.h>
 #include <sys/stat.h>
+#include <unistd.h>
 
 /* libarchive */
 #include <archive.h>
@@ -65,6 +67,37 @@ static char *get_sync_dir(pmhandle_t *handle)
 	return syncpath;
 }
 
+static int sync_db_validate(pmdb_t *db)
+{
+	/* this takes into account the default verification level if UNKNOWN
+	 * was assigned to this db */
+	pgp_verify_t check_sig = _alpm_db_get_sigverify_level(db);
+
+	if(check_sig != PM_PGP_VERIFY_NEVER) {
+		int ret;
+		const char *dbpath = _alpm_db_path(db);
+		if(!dbpath) {
+			/* pm_errno set in _alpm_db_path() */
+			return -1;
+		}
+
+		/* we can skip any validation if the database doesn't exist */
+		if(access(dbpath, R_OK) != 0 && errno == ENOENT) {
+			return 0;
+		}
+
+		_alpm_log(db->handle, PM_LOG_DEBUG, "checking signature for %s\n",
+				db->treename);
+		ret = _alpm_gpgme_checksig(db->handle, dbpath, NULL);
+		if((check_sig == PM_PGP_VERIFY_ALWAYS && ret != 0) ||
+				(check_sig == PM_PGP_VERIFY_OPTIONAL && ret == 1)) {
+			RET_ERR(db->handle, PM_ERR_SIG_INVALID, -1);
+		}
+	}
+
+	return 0;
+}
+
 /** Update a package database
  *
  * An update of the package database \a db will be attempted. Unless
@@ -143,6 +176,15 @@ int SYMEXPORT alpm_db_update(int force, pmdb_t *db)
 
 		if(ret == 0 && (check_sig == PM_PGP_VERIFY_ALWAYS ||
 					check_sig == PM_PGP_VERIFY_OPTIONAL)) {
+			/* an existing sig file is no good at this point */
+			char *sigpath = _alpm_db_sig_path(db);
+			if(!sigpath) {
+				ret = -1;
+				break;
+			}
+			unlink(sigpath);
+			free(sigpath);
+
 			int errors_ok = (check_sig == PM_PGP_VERIFY_OPTIONAL);
 			/* if we downloaded a DB, we want the .sig from the same server */
 			snprintf(fileurl, len, "%s/%s.db.sig", server, db->treename);
@@ -172,6 +214,11 @@ int SYMEXPORT alpm_db_update(int force, pmdb_t *db)
 	/* Cache needs to be rebuilt */
 	_alpm_db_free_pkgcache(db);
 
+	if(sync_db_validate(db)) {
+		/* pm_errno should be set */
+		ret = -1;
+	}
+
 cleanup:
 
 	free(syncpath);
@@ -524,7 +571,8 @@ struct db_operations sync_db_ops = {
 	.version          = sync_db_version,
 };
 
-pmdb_t *_alpm_db_register_sync(pmhandle_t *handle, const char *treename)
+pmdb_t *_alpm_db_register_sync(pmhandle_t *handle, const char *treename,
+		pgp_verify_t level)
 {
 	pmdb_t *db;
 
@@ -536,6 +584,12 @@ pmdb_t *_alpm_db_register_sync(pmhandle_t *handle, const char *treename)
 	}
 	db->ops = &sync_db_ops;
 	db->handle = handle;
+	db->pgp_verify = level;
+
+	if(sync_db_validate(db)) {
+		_alpm_db_free(db);
+		return NULL;
+	}
 
 	handle->dbs_sync = alpm_list_add(handle->dbs_sync, db);
 	return db;
diff --git a/lib/libalpm/db.c b/lib/libalpm/db.c
index f26eaae..9c974f5 100644
--- a/lib/libalpm/db.c
+++ b/lib/libalpm/db.c
@@ -45,7 +45,8 @@
  */
 
 /** Register a sync database of packages. */
-pmdb_t SYMEXPORT *alpm_db_register_sync(pmhandle_t *handle, const char *treename)
+pmdb_t SYMEXPORT *alpm_db_register_sync(pmhandle_t *handle, const char *treename,
+		pgp_verify_t check_sig)
 {
 	/* Sanity checks */
 	ASSERT(handle != NULL, return NULL);
@@ -54,7 +55,7 @@ pmdb_t SYMEXPORT *alpm_db_register_sync(pmhandle_t *handle, const char *treename
 	/* Do not register a database if a transaction is on-going */
 	ASSERT(handle->trans == NULL, RET_ERR(handle, PM_ERR_TRANS_NOT_NULL, NULL));
 
-	return _alpm_db_register_sync(handle, treename);
+	return _alpm_db_register_sync(handle, treename, check_sig);
 }
 
 /* Helper function for alpm_db_unregister{_all} */
diff --git a/lib/libalpm/db.h b/lib/libalpm/db.h
index e3faeeb..c5fcd5f 100644
--- a/lib/libalpm/db.h
+++ b/lib/libalpm/db.h
@@ -77,7 +77,8 @@ int _alpm_db_version(pmdb_t *db);
 int _alpm_db_cmp(const void *d1, const void *d2);
 alpm_list_t *_alpm_db_search(pmdb_t *db, const alpm_list_t *needles);
 pmdb_t *_alpm_db_register_local(pmhandle_t *handle);
-pmdb_t *_alpm_db_register_sync(pmhandle_t *handle, const char *treename);
+pmdb_t *_alpm_db_register_sync(pmhandle_t *handle, const char *treename,
+		pgp_verify_t level);
 void _alpm_db_unregister(pmdb_t *db);
 
 /* be_*.c, backend specific calls */
diff --git a/src/pacman/conf.c b/src/pacman/conf.c
index a5bc148..33ac0b8 100644
--- a/src/pacman/conf.c
+++ b/src/pacman/conf.c
@@ -460,7 +460,7 @@ static int setup_libalpm(void)
 	ret = alpm_option_set_logfile(handle, config->logfile);
 	if(ret != 0) {
 		pm_printf(PM_LOG_ERROR, _("problem setting logfile '%s' (%s)\n"),
-				config->logfile, alpm_strerror(alpm_errno(config->handle)));
+				config->logfile, alpm_strerror(alpm_errno(handle)));
 		return ret;
 	}
 
@@ -470,7 +470,7 @@ static int setup_libalpm(void)
 	ret = alpm_option_set_signaturedir(handle, config->gpgdir);
 	if(ret != 0) {
 		pm_printf(PM_LOG_ERROR, _("problem setting gpgdir '%s' (%s)\n"),
-				config->gpgdir, alpm_strerror(alpm_errno(config->handle)));
+				config->gpgdir, alpm_strerror(alpm_errno(handle)));
 		return ret;
 	}
 
@@ -543,7 +543,7 @@ static int finish_section(struct section_t *section, int parse_options)
 	}
 
 	/* if we are not looking at options sections only, register a db */
-	db = alpm_db_register_sync(config->handle, section->name);
+	db = alpm_db_register_sync(config->handle, section->name, section->sigverify);
 	if(db == NULL) {
 		pm_printf(PM_LOG_ERROR, _("could not register '%s' database (%s)\n"),
 				section->name, alpm_strerror(alpm_errno(config->handle)));
@@ -551,16 +551,6 @@ static int finish_section(struct section_t *section, int parse_options)
 		goto cleanup;
 	}
 
-	if(section->sigverify) {
-		if(alpm_db_set_pgp_verify(db, section->sigverify)) {
-			pm_printf(PM_LOG_ERROR,
-					_("could not set verify option for database '%s' (%s)\n"),
-					section->name, alpm_strerror(alpm_errno(config->handle)));
-			ret = 1;
-			goto cleanup;
-		}
-	}
-
 	for(i = section->servers; i; i = alpm_list_next(i)) {
 		char *value = alpm_list_getdata(i);
 		if(_add_mirror(db, value) != 0) {
diff --git a/src/util/cleanupdelta.c b/src/util/cleanupdelta.c
index 9829170..5ee59db 100644
--- a/src/util/cleanupdelta.c
+++ b/src/util/cleanupdelta.c
@@ -75,7 +75,7 @@ static void checkdbs(const char *dbpath, alpm_list_t *dbnames) {
 	for(i = dbnames; i; i = alpm_list_next(i)) {
 		char *dbname = alpm_list_getdata(i);
 		snprintf(syncdbpath, PATH_MAX, "%s/sync/%s", dbpath, dbname);
-		db = alpm_db_register_sync(handle, dbname);
+		db = alpm_db_register_sync(handle, dbname, PM_PGP_VERIFY_OPTIONAL);
 		if(db == NULL) {
 			fprintf(stderr, "error: could not register sync database (%s)\n",
 					alpm_strerror(alpm_errno(handle)));
diff --git a/src/util/testdb.c b/src/util/testdb.c
index 0bd7820..f45b4a9 100644
--- a/src/util/testdb.c
+++ b/src/util/testdb.c
@@ -152,7 +152,7 @@ static int check_syncdbs(alpm_list_t *dbnames) {
 
 	for(i = dbnames; i; i = alpm_list_next(i)) {
 		char *dbname = alpm_list_getdata(i);
-		db = alpm_db_register_sync(handle, dbname);
+		db = alpm_db_register_sync(handle, dbname, PM_PGP_VERIFY_OPTIONAL);
 		if(db == NULL) {
 			fprintf(stderr, "error: could not register sync database (%s)\n",
 					alpm_strerror(alpm_errno(handle)));
diff --git a/test/pacman/pactest.py b/test/pacman/pactest.py
index 77f87da..deb338f 100755
--- a/test/pacman/pactest.py
+++ b/test/pacman/pactest.py
@@ -113,13 +113,12 @@ def createOptParser():
     env.run()
     env.results()
 
-    if env.failed > 0:
-        print "pacman testing root saved: %s" % root_path
-        sys.exit(1)
-
     if not opts.keeproot:
         shutil.rmtree(root_path)
     else:
         print "pacman testing root saved: %s" % root_path
 
+    if env.failed > 0:
+        sys.exit(1)
+
 # vim: set ts=4 sw=4 et:
-- 
1.7.5.2



More information about the pacman-dev mailing list