[pacman-dev] Checking package validity
I was thinking of how we currently check package validity and had planned to do something like: 1) signature check 2) md5sum check _only_ if no signature to check with the intention of adding an sha256sum check in the middle in the future (perhaps only if pacman is built using openssl to save us having to provide the routines...). But as far as I can tell, _alpm_check_pgp_helper does not allow you to distinguish between a successful signature check and the case where no signature is available and signature checking is not required. Is that correct or am I missing something? Allan
On 31/07/11 11:15, Allan McRae wrote:
I was thinking of how we currently check package validity and had planned to do something like:
1) signature check 2) md5sum check _only_ if no signature to check
with the intention of adding an sha256sum check in the middle in the future (perhaps only if pacman is built using openssl to save us having to provide the routines...).
But as far as I can tell, _alpm_check_pgp_helper does not allow you to distinguish between a successful signature check and the case where no signature is available and signature checking is not required. Is that correct or am I missing something?
It appears that this is an area that needs work anyway...
pacman -Sw libcups resolving dependencies...
Targets (1): libcups-1.4.7-3 Total Download Size: 0.00 MiB Proceed with download? [Y/n] (1/1) checking package integrity [######################] 100% error: failed to commit transaction (invalid or corrupted package (checksum)) libcups-1.4.7-3-i686.pkg.tar.xz is invalid or corrupted Errors occurred, no packages were upgraded. This happened with a lot of packages so it definitely was not a checksum error... [12:11:35] debug: using cachedir: /home/arch/pkgcache/i686/ checking package integrity... [12:11:35] debug: found cached pkg: /home/arch/pkgcache/i686/libcups-1.4.7-3-i686.pkg.tar.xz [12:11:35] debug: replacing pkgcache entry with package file for target libcups [12:11:35] debug: md5sum: 772cf71cb8abb5afce923ae870130a51 [12:11:35] debug: checking md5sum for /home/arch/pkgcache/i686/libcups-1.4.7-3-i686.pkg.tar.xz [12:11:35] debug: base64_sig: (null) [12:11:35] debug: checking signatures for /home/arch/pkgcache/i686/libcups-1.4.7-3-i686.pkg.tar.xz [12:11:35] debug: checking signature for /home/arch/pkgcache/i686/libcups-1.4.7-3-i686.pkg.tar.xz [12:11:35] debug: 1 signatures returned [12:11:35] debug: fingerprint: 976AC6FA3B94FA10 [12:11:35] debug: summary: key missing [12:11:35] debug: status: No public key [12:11:35] debug: timestamp: 1311845034 [12:11:35] debug: exp_timestamp: 0 [12:11:35] debug: validity: unknown; reason: Success [12:11:35] debug: key lookup failed, unknown key [12:11:35] debug: signature is not valid [12:11:35] debug: returning error 33 from _alpm_sync_commit : invalid or corrupted package (checksum) error: failed to commit transaction (invalid or corrupted package (checksum)) My cache is essentially a mirror of the repo so has a bunch of signature files in it. So when I "download" a package from the repos and pacman finds its signature in the cache, that gets checked. So, that was quite unexpected for me (but I suppose it is a good thing?). We just need to fix that error message. Allan
This is a bit of a mess, due to the fact that we have a progress meter running. It is also ironic that we are in the midst of a method named "commit" when we haven't done a damn thing yet, and can still fail hard if either a checksum or signature is invalid or unrecognized. Adapt the former test_md5sum method to be invoked for any of the various failure types, which at least gives the user some indication of what packages are failing. A second patch will be needed to actually show worthwhile error codes, but this is going to involve modifying the actual data passed with the callback. Signed-off-by: Dan McGee <dan@archlinux.org> --- lib/libalpm/alpm.h | 1 + lib/libalpm/be_package.c | 2 +- lib/libalpm/error.c | 2 ++ lib/libalpm/sync.c | 32 ++++++++++++++++---------------- src/pacman/sync.c | 4 +++- 5 files changed, 23 insertions(+), 18 deletions(-) diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index a91b00f..8a95f26 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -1093,6 +1093,7 @@ enum _alpm_errno_t { ALPM_ERR_PKG_NOT_FOUND, ALPM_ERR_PKG_IGNORED, ALPM_ERR_PKG_INVALID, + ALPM_ERR_PKG_INVALID_CHECKSUM, ALPM_ERR_PKG_INVALID_SIG, ALPM_ERR_PKG_OPEN, ALPM_ERR_PKG_CANT_REMOVE, diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c index f80790c..41b1eb2 100644 --- a/lib/libalpm/be_package.c +++ b/lib/libalpm/be_package.c @@ -313,7 +313,7 @@ alpm_pkg_t *_alpm_pkg_load_internal(alpm_handle_t *handle, const char *pkgfile, _alpm_log(handle, ALPM_LOG_DEBUG, "checking md5sum for %s\n", pkgfile); if(_alpm_test_md5sum(pkgfile, md5sum) != 0) { alpm_pkg_free(newpkg); - RET_ERR(handle, ALPM_ERR_PKG_INVALID, NULL); + RET_ERR(handle, ALPM_ERR_PKG_INVALID_CHECKSUM, NULL); } } diff --git a/lib/libalpm/error.c b/lib/libalpm/error.c index 7e0e5c8..b3f5681 100644 --- a/lib/libalpm/error.c +++ b/lib/libalpm/error.c @@ -108,6 +108,8 @@ const char SYMEXPORT *alpm_strerror(enum _alpm_errno_t err) case ALPM_ERR_PKG_IGNORED: return _("operation cancelled due to ignorepkg"); case ALPM_ERR_PKG_INVALID: + return _("invalid or corrupted package"); + case ALPM_ERR_PKG_INVALID_CHECKSUM: return _("invalid or corrupted package (checksum)"); case ALPM_ERR_PKG_INVALID_SIG: return _("invalid or corrupted package (PGP signature)"); diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index ca7e657..663fca6 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -677,24 +677,18 @@ static int apply_deltas(alpm_handle_t *handle) * * @param trans the transaction * @param filename the absolute path of the file to test - * @param md5sum the expected md5sum of the file * - * @return 0 if the md5sum matched, 1 if not, -1 in case of errors + * @return 1 if file was removed, 0 otherwise */ -static int test_md5sum(alpm_trans_t *trans, const char *filepath, - const char *md5sum) +static int prompt_to_delete(alpm_trans_t *trans, const char *filepath) { - int ret = _alpm_test_md5sum(filepath, md5sum); - if(ret == 1) { - int doremove = 0; - QUESTION(trans, ALPM_TRANS_CONV_CORRUPTED_PKG, (char *)filepath, - NULL, NULL, &doremove); - if(doremove) { - unlink(filepath); - } + int doremove = 0; + QUESTION(trans, ALPM_TRANS_CONV_CORRUPTED_PKG, (char *)filepath, + NULL, NULL, &doremove); + if(doremove) { + unlink(filepath); } - - return ret; + return doremove; } static int validate_deltas(alpm_handle_t *handle, alpm_list_t *deltas, @@ -715,7 +709,9 @@ static int validate_deltas(alpm_handle_t *handle, alpm_list_t *deltas, alpm_delta_t *d = alpm_list_getdata(i); char *filepath = _alpm_filecache_find(handle, d->delta); - if(test_md5sum(trans, filepath, d->delta_md5) != 0) { + ret = _alpm_test_md5sum(filepath, d->delta_md5); + if(ret != 0) { + prompt_to_delete(trans, filepath); errors++; *data = alpm_list_add(*data, strdup(d->delta)); } @@ -903,6 +899,7 @@ int _alpm_sync_commit(alpm_handle_t *handle, alpm_list_t **data) alpm_pkg_t *pkgfile =_alpm_pkg_load_internal(handle, filepath, 1, spkg->md5sum, spkg->base64_sig, level); if(!pkgfile) { + prompt_to_delete(trans, filepath); errors++; *data = alpm_list_add(*data, strdup(filename)); FREE(filepath); @@ -920,7 +917,10 @@ int _alpm_sync_commit(alpm_handle_t *handle, alpm_list_t **data) if(errors) { - RET_ERR(handle, ALPM_ERR_PKG_INVALID, -1); + if(!handle->pm_errno) { + RET_ERR(handle, ALPM_ERR_PKG_INVALID, -1); + } + return -1; } if(trans->flags & ALPM_TRANS_FLAG_DOWNLOADONLY) { diff --git a/src/pacman/sync.c b/src/pacman/sync.c index 3846b30..f4f8fc4 100644 --- a/src/pacman/sync.c +++ b/src/pacman/sync.c @@ -844,9 +844,11 @@ int sync_prepare_execute(void) } break; case ALPM_ERR_PKG_INVALID: + case ALPM_ERR_PKG_INVALID_CHECKSUM: + case ALPM_ERR_PKG_INVALID_SIG: case ALPM_ERR_DLT_INVALID: for(i = data; i; i = alpm_list_next(i)) { - char *filename = alpm_list_getdata(i); + const char *filename = alpm_list_getdata(i); printf(_("%s is invalid or corrupted\n"), filename); } break; -- 1.7.6
Signed-off-by: Dan McGee <dan@archlinux.org> --- lib/libalpm/sync.c | 10 ++++++---- src/pacman/callback.c | 6 ++++-- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index 663fca6..e860591 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -677,14 +677,16 @@ static int apply_deltas(alpm_handle_t *handle) * * @param trans the transaction * @param filename the absolute path of the file to test + * @param reason an error code indicating the reason for package invalidity * * @return 1 if file was removed, 0 otherwise */ -static int prompt_to_delete(alpm_trans_t *trans, const char *filepath) +static int prompt_to_delete(alpm_trans_t *trans, const char *filepath, + enum _alpm_errno_t reason) { int doremove = 0; QUESTION(trans, ALPM_TRANS_CONV_CORRUPTED_PKG, (char *)filepath, - NULL, NULL, &doremove); + &reason, NULL, &doremove); if(doremove) { unlink(filepath); } @@ -711,7 +713,7 @@ static int validate_deltas(alpm_handle_t *handle, alpm_list_t *deltas, ret = _alpm_test_md5sum(filepath, d->delta_md5); if(ret != 0) { - prompt_to_delete(trans, filepath); + prompt_to_delete(trans, filepath, ALPM_ERR_DLT_INVALID); errors++; *data = alpm_list_add(*data, strdup(d->delta)); } @@ -899,7 +901,7 @@ int _alpm_sync_commit(alpm_handle_t *handle, alpm_list_t **data) alpm_pkg_t *pkgfile =_alpm_pkg_load_internal(handle, filepath, 1, spkg->md5sum, spkg->base64_sig, level); if(!pkgfile) { - prompt_to_delete(trans, filepath); + prompt_to_delete(trans, filepath, handle->pm_errno); errors++; *data = alpm_list_add(*data, strdup(filename)); FREE(filepath); diff --git a/src/pacman/callback.c b/src/pacman/callback.c index 237ccea..a8163d0 100644 --- a/src/pacman/callback.c +++ b/src/pacman/callback.c @@ -330,8 +330,10 @@ void cb_trans_conv(alpm_transconv_t event, void *data1, void *data2, } break; case ALPM_TRANS_CONV_CORRUPTED_PKG: - *response = yesno(_(":: File %s is corrupted. Do you want to delete it?"), - (char *)data1); + *response = yesno(_(":: File %s is corrupted (%s).\n" + "Do you want to delete it?"), + (char *)data1, + alpm_strerror(*(enum _alpm_errno_t *)data2)); break; } if(config->noask) { -- 1.7.6
participants (2)
-
Allan McRae
-
Dan McGee