From afdbfc05f771f2c684ee195b46d26b3d08a67085 Mon Sep 17 00:00:00 2001 From: Dan McGee Date: Mon, 19 Sep 2011 20:40:32 -0500 Subject: [PATCH] Extract an _alpm_pkg_validate_internal() method _alpm_pkg_load_internal() was becoming a monster. Extract the top bit of the method that dealt with checksum and signature validation into a separate method that should be called before one loads a package to ensure it is valid. Signed-off-by: Dan McGee --- lib/libalpm/be_package.c | 133 +++++++++++++++++++++++---------------- lib/libalpm/package.h | 6 +- lib/libalpm/sync.c | 10 ++- 3 files changed, 90 insertions(+), 59 deletions(-) diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c index ff820b80..b6cb8c4e 100644 --- a/lib/libalpm/be_package.c +++ b/lib/libalpm/be_package.c @@ -273,20 +273,86 @@ static alpm_file_t *files_msort(alpm_file_t *files, size_t n) } /** - * Load a package and create the corresponding alpm_pkg_t struct. + * Validate a package. * @param handle the context handle * @param pkgfile path to the package file * @param syncpkg package object to load verification data from (md5sum, * sha256sum, and/or base64 signature) + * @param level the required level of signature verification + * @return 0 if package is fully valid, -1 and pm_errno otherwise + */ +int _alpm_pkg_validate_internal(alpm_handle_t *handle, + const char *pkgfile, alpm_pkg_t *syncpkg, alpm_siglevel_t level) +{ + int has_sig; + + if(pkgfile == NULL || strlen(pkgfile) == 0) { + RET_ERR(handle, ALPM_ERR_WRONG_ARGS, -1); + } + + /* attempt to access the package file, ensure it exists */ + if(access(pkgfile, R_OK) != 0) { + RET_ERR(handle, ALPM_ERR_PKG_NOT_FOUND, -1); + } + + /* can we get away with skipping checksums? */ + has_sig = 0; + if(level & ALPM_SIG_PACKAGE) { + if(syncpkg && syncpkg->base64_sig) { + has_sig = 1; + } else { + char *sigpath = _alpm_sigpath(handle, pkgfile); + if(sigpath && !_alpm_access(handle, NULL, sigpath, R_OK)) { + has_sig = 1; + } + free(sigpath); + } + } + + if(syncpkg && !has_sig) { + if(syncpkg->md5sum && !syncpkg->sha256sum) { + _alpm_log(handle, ALPM_LOG_DEBUG, "md5sum: %s\n", syncpkg->md5sum); + _alpm_log(handle, ALPM_LOG_DEBUG, "checking md5sum for %s\n", pkgfile); + if(_alpm_test_checksum(pkgfile, syncpkg->md5sum, ALPM_CSUM_MD5) != 0) { + RET_ERR(handle, ALPM_ERR_PKG_INVALID_CHECKSUM, -1); + } + } + + if(syncpkg->sha256sum) { + _alpm_log(handle, ALPM_LOG_DEBUG, "sha256sum: %s\n", syncpkg->sha256sum); + _alpm_log(handle, ALPM_LOG_DEBUG, "checking sha256sum for %s\n", pkgfile); + if(_alpm_test_checksum(pkgfile, syncpkg->sha256sum, ALPM_CSUM_SHA256) != 0) { + RET_ERR(handle, ALPM_ERR_PKG_INVALID_CHECKSUM, -1); + } + } + } + + /* even if we don't have a sig, run the check code if level tells us to */ + if(has_sig || level & ALPM_SIG_PACKAGE) { + const char *sig = syncpkg ? syncpkg->base64_sig : NULL; + _alpm_log(handle, ALPM_LOG_DEBUG, "sig data: %s\n", sig ? sig : ""); + if(_alpm_check_pgp_helper(handle, pkgfile, sig, + level & ALPM_SIG_PACKAGE_OPTIONAL, level & ALPM_SIG_PACKAGE_MARGINAL_OK, + level & ALPM_SIG_PACKAGE_UNKNOWN_OK)) { + handle->pm_errno = ALPM_ERR_PKG_INVALID_SIG; + return -1; + } + } + + return 0; +} + +/** + * Load a package and create the corresponding alpm_pkg_t struct. + * @param handle the context handle + * @param pkgfile path to the package file * @param full whether to stop the load after metadata is read or continue * through the full archive - * @param level the required level of signature verification - * @return An information filled alpm_pkg_t struct */ -alpm_pkg_t *_alpm_pkg_load_internal(alpm_handle_t *handle, const char *pkgfile, - alpm_pkg_t *syncpkg, int full, alpm_siglevel_t level) +alpm_pkg_t *_alpm_pkg_load_internal(alpm_handle_t *handle, + const char *pkgfile, int full) { - int ret, has_sig, config = 0; + int ret, config = 0; struct archive *archive; struct archive_entry *entry; alpm_pkg_t *newpkg = NULL; @@ -311,54 +377,7 @@ alpm_pkg_t *_alpm_pkg_load_internal(alpm_handle_t *handle, const char *pkgfile, RET_ERR(handle, ALPM_ERR_PKG_NOT_FOUND, NULL); } - /* can we get away with skipping checksums? */ - has_sig = 0; - if(level & ALPM_SIG_PACKAGE) { - if(syncpkg && syncpkg->base64_sig) { - has_sig = 1; - } else { - char *sigpath = _alpm_sigpath(handle, pkgfile); - if(sigpath && !_alpm_access(handle, NULL, sigpath, R_OK)) { - has_sig = 1; - } - free(sigpath); - } - } - - if(syncpkg && !has_sig) { - if(syncpkg->md5sum && !syncpkg->sha256sum) { - _alpm_log(handle, ALPM_LOG_DEBUG, "md5sum: %s\n", syncpkg->md5sum); - _alpm_log(handle, ALPM_LOG_DEBUG, "checking md5sum for %s\n", pkgfile); - if(_alpm_test_checksum(pkgfile, syncpkg->md5sum, ALPM_CSUM_MD5) != 0) { - alpm_pkg_free(newpkg); - RET_ERR(handle, ALPM_ERR_PKG_INVALID_CHECKSUM, NULL); - } - } - - if(syncpkg->sha256sum) { - _alpm_log(handle, ALPM_LOG_DEBUG, "sha256sum: %s\n", syncpkg->sha256sum); - _alpm_log(handle, ALPM_LOG_DEBUG, "checking sha256sum for %s\n", pkgfile); - if(_alpm_test_checksum(pkgfile, syncpkg->sha256sum, ALPM_CSUM_SHA256) != 0) { - alpm_pkg_free(newpkg); - RET_ERR(handle, ALPM_ERR_PKG_INVALID_CHECKSUM, NULL); - } - } - } - - /* even if we don't have a sig, run the check code if level tells us to */ - if(has_sig || level & ALPM_SIG_PACKAGE) { - const char *sig = syncpkg ? syncpkg->base64_sig : NULL; - _alpm_log(handle, ALPM_LOG_DEBUG, "sig data: %s\n", sig ? sig : ""); - if(_alpm_check_pgp_helper(handle, pkgfile, sig, - level & ALPM_SIG_PACKAGE_OPTIONAL, level & ALPM_SIG_PACKAGE_MARGINAL_OK, - level & ALPM_SIG_PACKAGE_UNKNOWN_OK)) { - handle->pm_errno = ALPM_ERR_PKG_INVALID_SIG; - _alpm_pkg_free(newpkg); - return NULL; - } - } - - /* next- try to create an archive object to read in the package */ + /* try to create an archive object to read in the package */ if((archive = archive_read_new()) == NULL) { alpm_pkg_free(newpkg); RET_ERR(handle, ALPM_ERR_LIBARCHIVE, NULL); @@ -490,7 +509,11 @@ int SYMEXPORT alpm_pkg_load(alpm_handle_t *handle, const char *filename, int ful CHECK_HANDLE(handle, return -1); ASSERT(pkg != NULL, RET_ERR(handle, ALPM_ERR_WRONG_ARGS, -1)); - *pkg = _alpm_pkg_load_internal(handle, filename, NULL, full, level); + if(_alpm_pkg_validate_internal(handle, filename, NULL, level) == -1) { + /* pm_errno is set by pkg_validate */ + return -1; + } + *pkg = _alpm_pkg_load_internal(handle, filename, full); 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 c19625c1..fbae766c 100644 --- a/lib/libalpm/package.h +++ b/lib/libalpm/package.h @@ -135,8 +135,10 @@ void _alpm_pkg_free(alpm_pkg_t *pkg); void _alpm_pkg_free_trans(alpm_pkg_t *pkg); -alpm_pkg_t *_alpm_pkg_load_internal(alpm_handle_t *handle, const char *pkgfile, - alpm_pkg_t *syncpkg, int full, alpm_siglevel_t level); +int _alpm_pkg_validate_internal(alpm_handle_t *handle, + const char *pkgfile, alpm_pkg_t *syncpkg, alpm_siglevel_t level); +alpm_pkg_t *_alpm_pkg_load_internal(alpm_handle_t *handle, + const char *pkgfile, int full); int _alpm_pkg_cmp(const void *p1, const void *p2); int _alpm_pkg_compare_versions(alpm_pkg_t *local_pkg, alpm_pkg_t *pkg); diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index 732e786f..a5281078 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -971,14 +971,20 @@ int _alpm_sync_commit(alpm_handle_t *handle, alpm_list_t **data) _alpm_log(handle, ALPM_LOG_DEBUG, "replacing pkgcache entry with package file for target %s\n", spkg->name); - alpm_pkg_t *pkgfile =_alpm_pkg_load_internal(handle, filepath, spkg, 1, level); - if(!pkgfile) { + if(_alpm_pkg_validate_internal(handle, filepath, spkg, level) == -1) { prompt_to_delete(handle, filepath, handle->pm_errno); errors++; *data = alpm_list_add(*data, strdup(spkg->filename)); FREE(filepath); continue; } + alpm_pkg_t *pkgfile =_alpm_pkg_load_internal(handle, filepath, 1); + if(!pkgfile) { + errors++; + *data = alpm_list_add(*data, strdup(spkg->filename)); + FREE(filepath); + continue; + } FREE(filepath); pkgfile->reason = spkg->reason; /* copy over install reason */ i->data = pkgfile;