[pacman-dev] [PATCH] WIP: possible new siglevel layout
This is a proposed new siglevel enum that will allow configuration of all the various combinations of signature checking. It is done via bitflags. To get any checking at all, at least one of PM_CHECK_PACKAGE or PM_CHECK_DATABASE must be flipped on. These can be made optional on an independent basis. Additional flags exist for UNKNOWN and MARGINAL signatures. Proof of concept only; still missing pieces here to be implemented like the actual handling of UNKNOWN/MARGINAL. --- lib/libalpm/alpm.h | 37 ++++++++++++++++++++++++------------- lib/libalpm/be_package.c | 12 ++++++------ lib/libalpm/be_sync.c | 23 +++++++++++------------ lib/libalpm/db.c | 33 +++++++++++++-------------------- lib/libalpm/db.h | 4 ++-- lib/libalpm/dload.c | 5 ++--- lib/libalpm/handle.c | 14 +++++++------- lib/libalpm/handle.h | 2 +- lib/libalpm/package.h | 2 +- lib/libalpm/signing.c | 15 --------------- lib/libalpm/signing.h | 1 - lib/libalpm/sync.c | 6 +++--- src/pacman/conf.c | 38 ++++++++++++++++++++------------------ src/pacman/conf.h | 2 +- src/pacman/query.c | 2 +- src/pacman/sync.c | 2 +- src/pacman/upgrade.c | 4 ++-- src/util/cleanupdelta.c | 3 ++- src/util/testdb.c | 3 ++- src/util/testpkg.c | 3 ++- 20 files changed, 101 insertions(+), 110 deletions(-) diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index b62c62d..bdb2383 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -63,12 +63,16 @@ typedef enum _pmpkgreason_t { /** * GPG signature verification options */ -typedef enum _pgp_verify_t { - PM_PGP_VERIFY_UNKNOWN, - PM_PGP_VERIFY_NEVER, - PM_PGP_VERIFY_OPTIONAL, - PM_PGP_VERIFY_ALWAYS -} pgp_verify_t; +typedef enum _pmsiglevel_t { + PM_SIG_PACKAGE = (1 << 0), + PM_SIG_PACKAGE_OPTIONAL = (1 << 1), + PM_SIG_DATABASE = (1 << 2), + PM_SIG_DATABASE_OPTIONAL = (1 << 3), + PM_SIG_UNKNOWN_OK = (1 << 4), + PM_SIG_MARGINAL_OK = (1 << 5), + + PM_SIG_USE_DEFAULT = (1 << 31), +} pmsiglevel_t; /* * Structures @@ -247,8 +251,8 @@ int alpm_option_set_usedelta(pmhandle_t *handle, int usedelta); int alpm_option_get_checkspace(pmhandle_t *handle); int alpm_option_set_checkspace(pmhandle_t *handle, int checkspace); -pgp_verify_t alpm_option_get_default_sigverify(pmhandle_t *handle); -int alpm_option_set_default_sigverify(pmhandle_t *handle, pgp_verify_t level); +pmsiglevel_t alpm_option_get_default_siglevel(pmhandle_t *handle); +int alpm_option_set_default_siglevel(pmhandle_t *handle, pmsiglevel_t level); /** @} */ @@ -276,12 +280,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 + * @param level 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, - pgp_verify_t check_sig); + pmsiglevel_t level); /** Unregister a package database. * @param db pointer to the package database to unregister @@ -301,6 +305,14 @@ int alpm_db_unregister_all(pmhandle_t *handle); */ const char *alpm_db_get_name(const pmdb_t *db); +/** Get the signature verification level for a database. + * Will return the default verification level if this database is set up + * with PM_SIG_USE_DEFAULT. + * @param db pointer to the package database + * @return the signature verification level + */ +pmsiglevel_t alpm_db_get_siglevel(pmdb_t *db); + /** Check the validity of a database. * This is most useful for sync databases and verifying signature status. * If invalid, the handle error code will be set accordingly. @@ -377,13 +389,13 @@ int alpm_db_set_pkgreason(pmdb_t *db, const char *name, pmpkgreason_t reason); * @param filename location of the package tarball * @param full whether to stop the load after metadata is read or continue * through the full archive - * @param check_sig what level of package signature checking to perform on the + * @param level 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(pmhandle_t *handle, const char *filename, int full, - pgp_verify_t check_sig, pmpkg_t **pkg); + pmsiglevel_t level, pmpkg_t **pkg); /** Free a package. * @param pkg package pointer to free @@ -622,7 +634,6 @@ alpm_list_t *alpm_pkg_unused_deltas(pmpkg_t *pkg); int alpm_pkg_check_pgp_signature(pmpkg_t *pkg); int alpm_db_check_pgp_signature(pmdb_t *db); -int alpm_db_set_pgp_verify(pmdb_t *db, pgp_verify_t verify); /* * Deltas diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c index a0261d0..4e09d3a 100644 --- a/lib/libalpm/be_package.c +++ b/lib/libalpm/be_package.c @@ -230,7 +230,7 @@ static int parse_descfile(pmhandle_t *handle, struct archive *a, pmpkg_t *newpkg */ pmpkg_t *_alpm_pkg_load_internal(pmhandle_t *handle, const char *pkgfile, int full, const char *md5sum, const char *base64_sig, - pgp_verify_t check_sig) + pmsiglevel_t level) { int ret; int config = 0; @@ -267,11 +267,11 @@ pmpkg_t *_alpm_pkg_load_internal(pmhandle_t *handle, const char *pkgfile, } _alpm_log(handle, PM_LOG_DEBUG, "base64_sig: %s\n", base64_sig); - if(check_sig != PM_PGP_VERIFY_NEVER) { + if(level & PM_SIG_PACKAGE) { _alpm_log(handle, PM_LOG_DEBUG, "checking signature for %s\n", pkgfile); ret = _alpm_gpgme_checksig(handle, pkgfile, base64_sig); - if((check_sig == PM_PGP_VERIFY_ALWAYS && ret != 0) || - (check_sig == PM_PGP_VERIFY_OPTIONAL && ret == 1)) { + if((!(level & PM_SIG_PACKAGE_OPTIONAL) && ret != 0) || + ((level & PM_SIG_PACKAGE_OPTIONAL) && ret == 1)) { alpm_pkg_free(newpkg); RET_ERR(handle, PM_ERR_SIG_INVALID, NULL); } @@ -384,12 +384,12 @@ error: } int SYMEXPORT alpm_pkg_load(pmhandle_t *handle, const char *filename, int full, - pgp_verify_t check_sig, pmpkg_t **pkg) + pmsiglevel_t level, pmpkg_t **pkg) { CHECK_HANDLE(handle, return -1); ASSERT(pkg != NULL, RET_ERR(handle, PM_ERR_WRONG_ARGS, -1)); - *pkg = _alpm_pkg_load_internal(handle, filename, full, NULL, NULL, check_sig); + *pkg = _alpm_pkg_load_internal(handle, filename, full, NULL, NULL, level); if(*pkg == NULL) { /* pm_errno is set by pkg_load */ return -1; diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c index 3a8bde0..8602d5a 100644 --- a/lib/libalpm/be_sync.c +++ b/lib/libalpm/be_sync.c @@ -69,7 +69,7 @@ static char *get_sync_dir(pmhandle_t *handle) static int sync_db_validate(pmdb_t *db) { - pgp_verify_t check_sig; + pmsiglevel_t level; if(db->valid) { return 0; @@ -77,9 +77,9 @@ static int sync_db_validate(pmdb_t *db) /* this takes into account the default verification level if UNKNOWN * was assigned to this db */ - check_sig = _alpm_db_get_sigverify_level(db); + level = alpm_db_get_siglevel(db); - if(check_sig != PM_PGP_VERIFY_NEVER) { + if(level & PM_SIG_DATABASE) { int ret; const char *dbpath = _alpm_db_path(db); if(!dbpath) { @@ -97,8 +97,8 @@ static int sync_db_validate(pmdb_t *db) _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)) { + if((!(level & PM_SIG_DATABASE_OPTIONAL) && ret != 0) || + ((level & PM_SIG_DATABASE_OPTIONAL) && ret == 1)) { db->valid = 0; RET_ERR(db->handle, PM_ERR_SIG_INVALID, -1); } @@ -154,7 +154,7 @@ int SYMEXPORT alpm_db_update(int force, pmdb_t *db) int ret = -1; mode_t oldmask; pmhandle_t *handle; - pgp_verify_t check_sig; + pmsiglevel_t level; /* Sanity checks */ ASSERT(db != NULL, return -1); @@ -170,7 +170,7 @@ int SYMEXPORT alpm_db_update(int force, pmdb_t *db) if(!syncpath) { return -1; } - check_sig = _alpm_db_get_sigverify_level(db); + level = alpm_db_get_siglevel(db); for(i = db->servers; i; i = i->next) { const char *server = i->data; @@ -185,8 +185,7 @@ int SYMEXPORT alpm_db_update(int force, pmdb_t *db) ret = _alpm_download(handle, fileurl, syncpath, force, 0, 0); - if(ret == 0 && (check_sig == PM_PGP_VERIFY_ALWAYS || - check_sig == PM_PGP_VERIFY_OPTIONAL)) { + if(ret == 0 && (level & PM_SIG_DATABASE)) { /* an existing sig file is no good at this point */ char *sigpath = _alpm_db_sig_path(db); if(!sigpath) { @@ -196,7 +195,7 @@ int SYMEXPORT alpm_db_update(int force, pmdb_t *db) unlink(sigpath); free(sigpath); - int errors_ok = (check_sig == PM_PGP_VERIFY_OPTIONAL); + int errors_ok = (level & PM_SIG_DATABASE_OPTIONAL); /* if we downloaded a DB, we want the .sig from the same server */ snprintf(fileurl, len, "%s/%s.db.sig", server, db->treename); @@ -579,7 +578,7 @@ struct db_operations sync_db_ops = { }; pmdb_t *_alpm_db_register_sync(pmhandle_t *handle, const char *treename, - pgp_verify_t level) + pmsiglevel_t level) { pmdb_t *db; @@ -591,7 +590,7 @@ pmdb_t *_alpm_db_register_sync(pmhandle_t *handle, const char *treename, } db->ops = &sync_db_ops; db->handle = handle; - db->pgp_verify = level; + db->siglevel = level; sync_db_validate(db); diff --git a/lib/libalpm/db.c b/lib/libalpm/db.c index 55c5bd4..37a2a6d 100644 --- a/lib/libalpm/db.c +++ b/lib/libalpm/db.c @@ -46,7 +46,7 @@ /** Register a sync database of packages. */ pmdb_t SYMEXPORT *alpm_db_register_sync(pmhandle_t *handle, const char *treename, - pgp_verify_t check_sig) + pmsiglevel_t level) { /* Sanity checks */ CHECK_HANDLE(handle, return NULL); @@ -55,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, check_sig); + return _alpm_db_register_sync(handle, treename, level); } /* Helper function for alpm_db_unregister{_all} */ @@ -211,23 +211,6 @@ int SYMEXPORT alpm_db_remove_server(pmdb_t *db, const char *url) return 1; } -/** Set the verify gpg signature option for a database. - * @param db database pointer - * @param verify enum pgp_verify_t - * @return 0 on success, -1 on error (pm_errno is set accordingly) - */ -int SYMEXPORT alpm_db_set_pgp_verify(pmdb_t *db, pgp_verify_t verify) -{ - /* Sanity checks */ - ASSERT(db != NULL, return -1); - db->handle->pm_errno = 0; - - db->pgp_verify = verify; - _alpm_log(db->handle, PM_LOG_DEBUG, "adding VerifySig option to database '%s': %d\n", - db->treename, verify); - - return 0; -} /** Get the name of a package database. */ const char SYMEXPORT *alpm_db_get_name(const pmdb_t *db) @@ -236,6 +219,16 @@ const char SYMEXPORT *alpm_db_get_name(const pmdb_t *db) return db->treename; } +/** Get the signature verification level for a database. */ +pmsiglevel_t SYMEXPORT alpm_db_get_siglevel(pmdb_t *db) +{ + if(db->siglevel & PM_SIG_USE_DEFAULT) { + return alpm_option_get_default_siglevel(db->handle); + } else { + return db->siglevel; + } +} + /** Check the validity of a database. */ int SYMEXPORT alpm_db_valid(pmdb_t *db) { @@ -329,7 +322,7 @@ pmdb_t *_alpm_db_new(const char *treename, int is_local) CALLOC(db, 1, sizeof(pmdb_t), return NULL); STRDUP(db->treename, treename, return NULL); db->is_local = is_local; - db->pgp_verify = PM_PGP_VERIFY_UNKNOWN; + db->siglevel = 0; return db; } diff --git a/lib/libalpm/db.h b/lib/libalpm/db.h index 5e9c9c1..af57b87 100644 --- a/lib/libalpm/db.h +++ b/lib/libalpm/db.h @@ -63,7 +63,7 @@ struct __pmdb_t { pmpkghash_t *pkgcache; alpm_list_t *grpcache; alpm_list_t *servers; - pgp_verify_t pgp_verify; + pmsiglevel_t siglevel; struct db_operations *ops; }; @@ -78,7 +78,7 @@ 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, - pgp_verify_t level); + pmsiglevel_t level); void _alpm_db_unregister(pmdb_t *db); /* be_*.c, backend specific calls */ diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c index 9354562..3a5ceb7 100644 --- a/lib/libalpm/dload.c +++ b/lib/libalpm/dload.c @@ -353,11 +353,10 @@ char SYMEXPORT *alpm_fetch_pkgurl(pmhandle_t *handle, const char *url) _alpm_log(handle, PM_LOG_DEBUG, "successfully downloaded %s\n", url); /* attempt to download the signature */ - if(ret == 0 && (handle->sigverify == PM_PGP_VERIFY_ALWAYS || - handle->sigverify == PM_PGP_VERIFY_OPTIONAL)) { + if(ret == 0 && (handle->siglevel & PM_SIG_PACKAGE)) { char *sig_url; size_t len; - int errors_ok = (handle->sigverify == PM_PGP_VERIFY_OPTIONAL); + int errors_ok = (handle->siglevel & PM_SIG_PACKAGE_OPTIONAL); len = strlen(url) + 5; CALLOC(sig_url, len, sizeof(char), RET_ERR(handle, PM_ERR_MEMORY, NULL)); diff --git a/lib/libalpm/handle.c b/lib/libalpm/handle.c index d2bb4f6..8cb8c08 100644 --- a/lib/libalpm/handle.c +++ b/lib/libalpm/handle.c @@ -43,7 +43,8 @@ pmhandle_t *_alpm_handle_new() CALLOC(handle, 1, sizeof(pmhandle_t), return NULL); - handle->sigverify = PM_PGP_VERIFY_OPTIONAL; + handle->siglevel = PM_SIG_PACKAGE | PM_SIG_PACKAGE_OPTIONAL | + PM_SIG_DATABASE | PM_SIG_DATABASE_OPTIONAL; return handle; } @@ -520,18 +521,17 @@ int SYMEXPORT alpm_option_set_checkspace(pmhandle_t *handle, int checkspace) return 0; } -int SYMEXPORT alpm_option_set_default_sigverify(pmhandle_t *handle, pgp_verify_t level) +int SYMEXPORT alpm_option_set_default_siglevel(pmhandle_t *handle, pmsiglevel_t level) { CHECK_HANDLE(handle, return -1); - ASSERT(level != PM_PGP_VERIFY_UNKNOWN, RET_ERR(handle, PM_ERR_WRONG_ARGS, -1)); - handle->sigverify = level; + handle->siglevel = level; return 0; } -pgp_verify_t SYMEXPORT alpm_option_get_default_sigverify(pmhandle_t *handle) +pmsiglevel_t SYMEXPORT alpm_option_get_default_siglevel(pmhandle_t *handle) { - CHECK_HANDLE(handle, return PM_PGP_VERIFY_UNKNOWN); - return handle->sigverify; + CHECK_HANDLE(handle, return -1); + return handle->siglevel; } /* vim: set ts=2 sw=2 noet: */ diff --git a/lib/libalpm/handle.h b/lib/libalpm/handle.h index bace805..1f81c74 100644 --- a/lib/libalpm/handle.h +++ b/lib/libalpm/handle.h @@ -69,7 +69,7 @@ struct __pmhandle_t { char *arch; /* Architecture of packages we should allow */ int usedelta; /* Download deltas if possible */ int checkspace; /* Check disk space before installing */ - pgp_verify_t sigverify; /* Default signature verification level */ + pmsiglevel_t siglevel; /* Default signature verification level */ /* error code */ enum _pmerrno_t pm_errno; diff --git a/lib/libalpm/package.h b/lib/libalpm/package.h index bc5b267..19b958f 100644 --- a/lib/libalpm/package.h +++ b/lib/libalpm/package.h @@ -144,7 +144,7 @@ void _alpm_pkg_free_trans(pmpkg_t *pkg); pmpkg_t *_alpm_pkg_load_internal(pmhandle_t *handle, const char *pkgfile, int full, const char *md5sum, const char *base64_sig, - pgp_verify_t check_sig); + pmsiglevel_t level); int _alpm_pkg_cmp(const void *p1, const void *p2); int _alpm_pkg_compare_versions(pmpkg_t *local_pkg, pmpkg_t *pkg); diff --git a/lib/libalpm/signing.c b/lib/libalpm/signing.c index 11886ec..2765756 100644 --- a/lib/libalpm/signing.c +++ b/lib/libalpm/signing.c @@ -358,21 +358,6 @@ int _alpm_gpgme_checksig(pmhandle_t *handle, const char *path, #endif /** - * Determines the necessity of checking for a valid PGP signature - * @param db the sync database to query - * - * @return signature verification level - */ -pgp_verify_t _alpm_db_get_sigverify_level(pmdb_t *db) -{ - if(db->pgp_verify != PM_PGP_VERIFY_UNKNOWN) { - return db->pgp_verify; - } else { - return alpm_option_get_default_sigverify(db->handle); - } -} - -/** * Check the PGP signature for the given package file. * @param pkg the package to check * @return a int value : 0 (valid), 1 (invalid), -1 (an error occurred) diff --git a/lib/libalpm/signing.h b/lib/libalpm/signing.h index fdf81fc..14b2ad7 100644 --- a/lib/libalpm/signing.h +++ b/lib/libalpm/signing.h @@ -23,7 +23,6 @@ int _alpm_gpgme_checksig(pmhandle_t *handle, 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 0573573..4a52c5c 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -865,7 +865,7 @@ int _alpm_sync_commit(pmhandle_t *handle, alpm_list_t **data) int percent = (current * 100) / numtargs; const char *filename; char *filepath; - pgp_verify_t check_sig; + pmsiglevel_t level; PROGRESS(trans, PM_TRANS_PROGRESS_INTEGRITY_START, "", percent, numtargs, current); @@ -876,7 +876,7 @@ int _alpm_sync_commit(pmhandle_t *handle, alpm_list_t **data) filename = alpm_pkg_get_filename(spkg); filepath = _alpm_filecache_find(handle, filename); pmdb_t *sdb = alpm_pkg_get_db(spkg); - check_sig = _alpm_db_get_sigverify_level(sdb); + level = alpm_db_get_siglevel(sdb); /* 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 */ @@ -884,7 +884,7 @@ int _alpm_sync_commit(pmhandle_t *handle, alpm_list_t **data) "replacing pkgcache entry with package file for target %s\n", spkg->name); pmpkg_t *pkgfile =_alpm_pkg_load_internal(handle, filepath, 1, spkg->md5sum, - spkg->base64_sig, check_sig); + spkg->base64_sig, level); if(!pkgfile) { errors++; *data = alpm_list_add(*data, strdup(filename)); diff --git a/src/pacman/conf.c b/src/pacman/conf.c index 1ea9e11..3ebab43 100644 --- a/src/pacman/conf.c +++ b/src/pacman/conf.c @@ -52,7 +52,7 @@ config_t *config_new(void) newconfig->op = PM_OP_MAIN; newconfig->logmask = PM_LOG_ERROR | PM_LOG_WARNING; newconfig->configfile = strdup(CONFFILE); - newconfig->sigverify = PM_PGP_VERIFY_UNKNOWN; + newconfig->siglevel = PM_SIG_USE_DEFAULT; return newconfig; } @@ -222,17 +222,18 @@ int config_set_arch(const char *arch) return 0; } -static pgp_verify_t option_verifysig(const char *value) +static pmsiglevel_t option_verifysig(const char *value) { - pgp_verify_t level; + pmsiglevel_t level; if(strcmp(value, "Always") == 0) { - level = PM_PGP_VERIFY_ALWAYS; + level = PM_SIG_PACKAGE | PM_SIG_DATABASE; } else if(strcmp(value, "Optional") == 0) { - level = PM_PGP_VERIFY_OPTIONAL; + level = PM_SIG_PACKAGE | PM_SIG_PACKAGE_OPTIONAL | + PM_SIG_DATABASE | PM_SIG_DATABASE_OPTIONAL; } else if(strcmp(value, "Never") == 0) { - level = PM_PGP_VERIFY_NEVER; + level = 0; } else { - level = PM_PGP_VERIFY_UNKNOWN; + return -1; } pm_printf(PM_LOG_DEBUG, "config: VerifySig = %s (%d)\n", value, level); return level; @@ -359,9 +360,9 @@ static int _parse_options(const char *key, char *value, } FREELIST(methods); } else if(strcmp(key, "VerifySig") == 0) { - pgp_verify_t level = option_verifysig(value); - if(level != PM_PGP_VERIFY_UNKNOWN) { - config->sigverify = level; + pmsiglevel_t level = option_verifysig(value); + if(level != -1) { + config->siglevel = level; } else { pm_printf(PM_LOG_ERROR, _("config file %s, line %d: directive '%s' has invalid value '%s'\n"), @@ -484,8 +485,8 @@ static int setup_libalpm(void) alpm_option_set_cachedirs(handle, config->cachedirs); } - if(config->sigverify != PM_PGP_VERIFY_UNKNOWN) { - alpm_option_set_default_sigverify(handle, config->sigverify); + if(config->siglevel != PM_SIG_USE_DEFAULT) { + alpm_option_set_default_siglevel(handle, config->siglevel); } if(config->xfercommand) { @@ -518,7 +519,7 @@ struct section_t { char *name; int is_options; /* db section option gathering */ - pgp_verify_t sigverify; + pmsiglevel_t siglevel; alpm_list_t *servers; }; @@ -545,7 +546,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, section->sigverify); + db = alpm_db_register_sync(config->handle, section->name, section->siglevel); if(db == NULL) { pm_printf(PM_LOG_ERROR, _("could not register '%s' database (%s)\n"), section->name, alpm_strerror(alpm_errno(config->handle))); @@ -568,7 +569,7 @@ static int finish_section(struct section_t *section, int parse_options) cleanup: alpm_list_free(section->servers); section->servers = NULL; - section->sigverify = 0; + section->siglevel = PM_SIG_USE_DEFAULT; free(section->name); section->name = NULL; return ret; @@ -726,9 +727,9 @@ static int _parseconfig(const char *file, struct section_t *section, } section->servers = alpm_list_add(section->servers, strdup(value)); } else if(strcmp(key, "VerifySig") == 0) { - pgp_verify_t level = option_verifysig(value); - if(level != PM_PGP_VERIFY_UNKNOWN) { - section->sigverify = level; + pmsiglevel_t level = option_verifysig(value); + if(level != -1) { + section->siglevel = level; } else { pm_printf(PM_LOG_ERROR, _("config file %s, line %d: directive '%s' has invalid value '%s'\n"), @@ -763,6 +764,7 @@ int parseconfig(const char *file) int ret; struct section_t section; memset(§ion, 0, sizeof(struct section_t)); + section.siglevel = PM_SIG_USE_DEFAULT; /* the config parse is a two-pass affair. We first parse the entire thing for * the [options] section so we can get all default and path options set. * Next, we go back and parse everything but [options]. */ diff --git a/src/pacman/conf.h b/src/pacman/conf.h index 4c44bfd..5e1a5b1 100644 --- a/src/pacman/conf.h +++ b/src/pacman/conf.h @@ -71,7 +71,7 @@ typedef struct __config_t { unsigned short noask; unsigned int ask; pmtransflag_t flags; - pgp_verify_t sigverify; + pmsiglevel_t siglevel; /* conf file options */ /* I Love Candy! */ diff --git a/src/pacman/query.c b/src/pacman/query.c index 38b3752..6b0011c 100644 --- a/src/pacman/query.c +++ b/src/pacman/query.c @@ -549,7 +549,7 @@ int pacman_query(alpm_list_t *targets) char *strname = alpm_list_getdata(i); if(config->op_q_isfile) { - alpm_pkg_load(config->handle, strname, 1, PM_PGP_VERIFY_NEVER, &pkg); + alpm_pkg_load(config->handle, strname, 1, 0, &pkg); } else { pkg = alpm_db_get_pkg(db_local, strname); } diff --git a/src/pacman/sync.c b/src/pacman/sync.c index 539e651..24e5b24 100644 --- a/src/pacman/sync.c +++ b/src/pacman/sync.c @@ -220,7 +220,7 @@ 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(config->handle, path, 0, PM_PGP_VERIFY_NEVER, &localpkg) != 0 + if(alpm_pkg_load(config->handle, path, 0, 0, &localpkg) != 0 || localpkg == NULL) { if(yesno(_("File %s does not seem to be a valid package, remove it?"), path)) { diff --git a/src/pacman/upgrade.c b/src/pacman/upgrade.c index 35624eb..a08a9d6 100644 --- a/src/pacman/upgrade.c +++ b/src/pacman/upgrade.c @@ -42,7 +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(config->handle); + pmsiglevel_t level = alpm_option_get_default_siglevel(config->handle); int retval = 0; if(targets == NULL) { @@ -76,7 +76,7 @@ int pacman_upgrade(alpm_list_t *targets) char *targ = alpm_list_getdata(i); pmpkg_t *pkg; - if(alpm_pkg_load(config->handle, targ, 1, check_sig, &pkg) != 0) { + if(alpm_pkg_load(config->handle, targ, 1, level, &pkg) != 0) { pm_fprintf(stderr, PM_LOG_ERROR, "'%s': %s\n", targ, alpm_strerror(alpm_errno(config->handle))); trans_release(); diff --git a/src/util/cleanupdelta.c b/src/util/cleanupdelta.c index 5ee59db..3809525 100644 --- a/src/util/cleanupdelta.c +++ b/src/util/cleanupdelta.c @@ -71,11 +71,12 @@ static void checkdbs(const char *dbpath, alpm_list_t *dbnames) { char syncdbpath[PATH_MAX]; pmdb_t *db = NULL; alpm_list_t *i; + const pmsiglevel_t level = PM_SIG_DATABASE | PM_SIG_DATABASE_OPTIONAL; 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, PM_PGP_VERIFY_OPTIONAL); + db = alpm_db_register_sync(handle, dbname, level); 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 f45b4a9..54a7040 100644 --- a/src/util/testdb.c +++ b/src/util/testdb.c @@ -149,10 +149,11 @@ static int check_syncdbs(alpm_list_t *dbnames) { int ret = 0; pmdb_t *db = NULL; alpm_list_t *i, *pkglist, *syncpkglist = NULL; + const pmsiglevel_t level = PM_SIG_DATABASE | PM_SIG_DATABASE_OPTIONAL; for(i = dbnames; i; i = alpm_list_next(i)) { char *dbname = alpm_list_getdata(i); - db = alpm_db_register_sync(handle, dbname, PM_PGP_VERIFY_OPTIONAL); + db = alpm_db_register_sync(handle, dbname, level); if(db == NULL) { fprintf(stderr, "error: could not register sync database (%s)\n", alpm_strerror(alpm_errno(handle))); diff --git a/src/util/testpkg.c b/src/util/testpkg.c index c6f02e3..29effd9 100644 --- a/src/util/testpkg.c +++ b/src/util/testpkg.c @@ -43,6 +43,7 @@ int main(int argc, char *argv[]) pmhandle_t *handle; enum _pmerrno_t err; pmpkg_t *pkg = NULL; + const pmsiglevel_t level = PM_SIG_PACKAGE | PM_SIG_PACKAGE_OPTIONAL; if(argc != 2) { fprintf(stderr, "usage: %s <package file>\n", BASENAME); @@ -58,7 +59,7 @@ int main(int argc, char *argv[]) /* let us get log messages from libalpm */ alpm_option_set_logcb(handle, output_cb); - if(alpm_pkg_load(handle, argv[1], 1, PM_PGP_VERIFY_OPTIONAL, &pkg) == -1 + if(alpm_pkg_load(handle, argv[1], 1, level, &pkg) == -1 || pkg == NULL) { enum _pmerrno_t err = alpm_errno(handle); switch(err) { -- 1.7.5.4
OK, I like the way this works. There are a lot of problems on the current master that are squashed by this and other commits in Dan's git (the "working" branch), so to avoid duplication I'll try to focus on the revision currently in Dan's "working" tree (last commit is this patch, b3581d). I may have misread some things, so this could be wrong: 1) The signature checking in _alpm_pkg_load_internal is messed up. In pacman's master, setting the signature check level to "optional" causes checks to succeed even if: - GPGME fails to initalize - sig isn't valid base64 encoding or .sig file can't be opened - various GPGME operations fail - the encoded PGP information doesn't actually contain a signature(?) - missing pubkey 2) In Dan's git, it's the same as the above, but now "no signature" is an automatic failure, and "bad signature" is explicitly checked for (also an automatic failure). 3) In alpm_db_update, setting the signature check level to "optional" causes pacman to attempt to download a .sig for a database; if it can't download one, it will still accept the database (vs. continuing to next mirror for "always"). Other than that, there is no difference between "optional" and "always"; this isn't bad, but we might want to do the "check 3 servers and then give up" thing for PM_SIG_DATABASE_OPTIONAL. It's not really KISS (probably extra baggage in the documentation too), so IDK. On an unrelated note, Dan killed a bug related to stale signatures in the "optional" case with the patch "Do database signature checking at load time" (bd1cf5 in his "working" branch), but, as Allan mentions in his notes, the new database still overwrites the old one in the "always" case (because the check-for-signature-existence happens after the new database is downloaded). 4) Dan's git adds signature checks for databases, which aren't in master. Although, I don't know if _alpm_db_register_sync() and alpm_db_update() necessarily the best places to do the signature check: since we're already doing general integrity checking in sync_db_populate (using the internal checksums embedded in various archive formats), doing a signature check at the top of that function might be better, but I don't really grok the code flow yet. _alpm_db_register_sync() (and thus the signature check) seems to get called every time pacman is invoked, which I don't think is desirable. So, _alpm_pkg_load_internal needs some attention, but we should decide what "optional" means first. I thought it would encompass the "no signature" and "missing pubkey" cases, but it seems to be backwards. 5) As far as I can tell, there should be a bug (which I cannot produce) in alpm_pkg_load() because it passes base64_sig to _alpm_pkg_load_internal() as NULL, so _alpm_log(handle, PM_LOG_DEBUG, "base64_sig: %s\n", base64_sig); should segfault; also, _alpm_gpgme_checksig() should balk at the NULL base64_sig, but I was too lazy at the time of writing to throw it into gdb, and this line didn't even seem to get printed when I ran ./pacman --debug --conf ~/pacman.conf -Q blah |& grep -i base64_sig On Tue, Jun 14, 2011 at 3:26 PM, Dan McGee <dan@archlinux.org> wrote:
To get any checking at all, at least one of PM_CHECK_PACKAGE or PM_CHECK_DATABASE must be flipped on. These can be made optional on an independent basis.
I think these became PM_SIG_PACKAGE and PM_SIG_DATABASE in your patch, but I like PM_SIG_CHECK_PACKAGE and PM_SIG_CHECK_DATABASE better (they're clearer but a bit long; perhaps _PKG and _DB would work since those abbreviations are also used a lot elsewhere; corresponding PM_SIG_PKG_OPTIONAL and PM_SIG_DB_OPTIONAL would be also be good).
+int alpm_option_set_default_siglevel(pmhandle_t *handle, pmsiglevel_t level); ... pmdb_t *alpm_db_register_sync(pmhandle_t *handle, const char *treename, - pgp_verify_t check_sig); + pmsiglevel_t level);
I like "pmsiglevel_t level" for cases like the first where it's specifically dealing with signing, but "pmsiglevel_t siglevel" for cases like the second where it's only a small part of a bigger function. Not a huge issue either way. As a self-reminder, config flags/directives still need to be defined and handled. I wanted to respond earlier, but school's kept me busy, and there's been a bunch of patches/commits, so I wanted to make sure I'm understanding everything that's been happening recently. I'm not done looking this over, but it's giving me a headache right now; anyway, I think I'll just write a bunch of pactests instead of digging through the code. Can we let this patch sit for a couple of days, and then go ahead and apply it to master (possibly with the CHECK/PKG/DB/siglevel suggestions)? It's not functionally complete, but it serves to refactor the code: it (apparently) doesn't break anything, and allows future work to go forth without annoying merges due to mixed conventions. -Kerrick Staley
participants (2)
-
Dan McGee
-
Kerrick Staley