[pacman-dev] [PATCH 1/4] validate_deltas: split verify/check errors loops
This allows us to do all delta verification up front, followed by whatever needs to be done with any found errors. In this case, we call prompt_to_delete() for each error. Add back the missing EVENT(ALPM_EVENT_DELTA_INTEGRITY_DONE) that accidentally got removed in commit 062c391919e93f1d6. Remove use of *data; we never even look at the stuff in this array for the error code we were returning and this would be much better handled by one callback per error anyway, or at least some strongly typed return values. Signed-off-by: Dan McGee <dan@archlinux.org> --- lib/libalpm/sync.c | 24 ++++++++++++++---------- 1 files changed, 14 insertions(+), 10 deletions(-) diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index 2092912..dda4b24 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -753,11 +753,9 @@ static int prompt_to_delete(alpm_handle_t *handle, const char *filepath, return doremove; } -static int validate_deltas(alpm_handle_t *handle, alpm_list_t *deltas, - alpm_list_t **data) +static int validate_deltas(alpm_handle_t *handle, alpm_list_t *deltas) { - int errors = 0; - alpm_list_t *i; + alpm_list_t *i, *errors = NULL; if(!deltas) { return 0; @@ -765,19 +763,25 @@ static int validate_deltas(alpm_handle_t *handle, alpm_list_t *deltas, /* Check integrity of deltas */ EVENT(handle, ALPM_EVENT_DELTA_INTEGRITY_START, NULL, NULL); - for(i = deltas; i; i = i->next) { alpm_delta_t *d = i->data; char *filepath = _alpm_filecache_find(handle, d->delta); if(_alpm_test_checksum(filepath, d->delta_md5, ALPM_CSUM_MD5)) { - prompt_to_delete(handle, filepath, ALPM_ERR_DLT_INVALID); - errors++; - *data = alpm_list_add(*data, strdup(d->delta)); + errors = alpm_list_add(errors, filepath); + } else { + FREE(filepath); } - FREE(filepath); } + EVENT(handle, ALPM_EVENT_DELTA_INTEGRITY_DONE, NULL, NULL); + if(errors) { + for(i = errors; i; i = i->next) { + char *filepath = i->data; + prompt_to_delete(handle, filepath, ALPM_ERR_DLT_INVALID); + FREE(filepath); + } + alpm_list_free(errors); handle->pm_errno = ALPM_ERR_DLT_INVALID; return -1; } @@ -1024,7 +1028,7 @@ int _alpm_sync_commit(alpm_handle_t *handle, alpm_list_t **data) return -1; } - if(validate_deltas(handle, deltas, data)) { + if(validate_deltas(handle, deltas)) { alpm_list_free(deltas); return -1; } -- 1.7.6.3
This is for eventual use by the PGP key import code. Breaking this into a separate commit now makes the following patches a bit easier to understand. Signed-off-by: Dan McGee <dan@archlinux.org> --- lib/libalpm/alpm.h | 1 + src/pacman/callback.c | 9 +++++++++ 2 files changed, 10 insertions(+), 0 deletions(-) diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index 9fe8034..853cb8b 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -352,6 +352,7 @@ typedef enum _alpm_question_t { ALPM_QUESTION_LOCAL_NEWER = (1 << 4), ALPM_QUESTION_REMOVE_PKGS = (1 << 5), ALPM_QUESTION_SELECT_PROVIDER = (1 << 6), + ALPM_QUESTION_IMPORT_KEY = (1 << 7) } alpm_question_t; /** Question callback */ diff --git a/src/pacman/callback.c b/src/pacman/callback.c index a01fc07..9d4663a 100644 --- a/src/pacman/callback.c +++ b/src/pacman/callback.c @@ -342,6 +342,15 @@ void cb_question(alpm_question_t event, void *data1, void *data2, (char *)data1, alpm_strerror(*(enum _alpm_errno_t *)data2)); break; + case ALPM_QUESTION_IMPORT_KEY: + { + alpm_pgpkey_t *key = data1; + char created[12]; + strftime(created, 12, "%Y-%m-%d", localtime(&(key->created))); + *response = yesno(_(":: Import PGP key %s, \"%s\", created %s?"), + key->fingerprint, key->uid, created); + } + break; } if(config->noask) { if(config->ask & event) { -- 1.7.6.3
This moves the result processing out of the validation check loop itself and into a new loop. Errors will be presented to the user one-by-one after we fully complete the validation loop, so they no longer overlap the progress bar. Unlike the database validation, we may have several errors to process in sequence here, so we use a function-scoped struct to track all the necessary information between seeing an error and asking the user about it. The older prompt_to_delete() callback logic is still kept, but only for checksum failures. It is debatable whether we should do this at all or just delegate said actions to the user. Signed-off-by: Dan McGee <dan@archlinux.org> --- lib/libalpm/be_package.c | 1 + lib/libalpm/sync.c | 82 ++++++++++++++++++++++++++++++++------------- 2 files changed, 59 insertions(+), 24 deletions(-) diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c index a0534b0..c20c703 100644 --- a/lib/libalpm/be_package.c +++ b/lib/libalpm/be_package.c @@ -287,6 +287,7 @@ int _alpm_pkg_validate_internal(alpm_handle_t *handle, alpm_siglist_t **sigdata) { int has_sig; + handle->pm_errno = 0; if(pkgfile == NULL || strlen(pkgfile) == 0) { RET_ERR(handle, ALPM_ERR_WRONG_ARGS, -1); diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index dda4b24..abd7cdb 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -909,42 +909,47 @@ static int download_files(alpm_handle_t *handle, alpm_list_t **deltas) return errors; } -static int check_validity(alpm_handle_t *handle, alpm_list_t **data, +static int check_validity(alpm_handle_t *handle, size_t total, size_t total_bytes) { + struct validity { + alpm_pkg_t *pkg; + char *path; + alpm_siglist_t *siglist; + alpm_siglevel_t level; + enum _alpm_errno_t error; + }; size_t current = 0, current_bytes = 0; - int errors = 0; - alpm_list_t *i; + alpm_list_t *i, *errors = NULL; /* Check integrity of packages */ EVENT(handle, ALPM_EVENT_INTEGRITY_START, NULL, NULL); for(i = handle->trans->add; i; i = i->next, current++) { - alpm_pkg_t *spkg = i->data; - char *filepath; - alpm_siglevel_t level; - alpm_siglist_t *siglist = NULL; + struct validity v = { i->data, NULL, NULL, 0, 0 }; int percent = (int)(((double)current_bytes / total_bytes) * 100); PROGRESS(handle, ALPM_PROGRESS_INTEGRITY_START, "", percent, total, current); - if(spkg->origin == PKG_FROM_FILE) { + if(v.pkg->origin == PKG_FROM_FILE) { continue; /* pkg_load() has been already called, this package is valid */ } - current_bytes += spkg->size; - filepath = _alpm_filecache_find(handle, spkg->filename); - alpm_db_t *sdb = alpm_pkg_get_db(spkg); - level = alpm_db_get_siglevel(sdb); + current_bytes += v.pkg->size; + v.path = _alpm_filecache_find(handle, v.pkg->filename); + v.level = alpm_db_get_siglevel(alpm_pkg_get_db(v.pkg)); - if(_alpm_pkg_validate_internal(handle, filepath, spkg, level, &siglist) == -1) { - prompt_to_delete(handle, filepath, handle->pm_errno); - errors++; - *data = alpm_list_add(*data, strdup(spkg->filename)); + if(_alpm_pkg_validate_internal(handle, v.path, v.pkg, + v.level, &v.siglist) == -1) { + v.error = handle->pm_errno; + struct validity *invalid = malloc(sizeof(struct validity)); + memcpy(invalid, &v, sizeof(struct validity)); + errors = alpm_list_add(errors, invalid); + } else { + alpm_siglist_cleanup(v.siglist); + free(v.siglist); + free(v.path); } - alpm_siglist_cleanup(siglist); - free(siglist); - free(filepath); } PROGRESS(handle, ALPM_PROGRESS_INTEGRITY_START, "", 100, @@ -952,10 +957,33 @@ static int check_validity(alpm_handle_t *handle, alpm_list_t **data, EVENT(handle, ALPM_EVENT_INTEGRITY_DONE, NULL, NULL); if(errors) { - if(!handle->pm_errno) { - RET_ERR(handle, ALPM_ERR_PKG_INVALID, -1); + int tryagain = 0; + for(i = errors; i; i = i->next) { + struct validity *v = i->data; + if(v->error == ALPM_ERR_PKG_INVALID_SIG) { + int retry = _alpm_process_siglist(handle, v->pkg->name, v->siglist, + v->level & ALPM_SIG_PACKAGE_OPTIONAL, + v->level & ALPM_SIG_PACKAGE_MARGINAL_OK, + v->level & ALPM_SIG_PACKAGE_UNKNOWN_OK); + tryagain += retry; + } else if(v->error == ALPM_ERR_PKG_INVALID_CHECKSUM) { + prompt_to_delete(handle, v->path, v->error); + } + alpm_siglist_cleanup(v->siglist); + free(v->siglist); + free(v->path); + free(v); } - return -1; + alpm_list_free(errors); + + if(tryagain == 0) { + if(!handle->pm_errno) { + RET_ERR(handle, ALPM_ERR_PKG_INVALID, -1); + } + return -1; + } + /* we were told at least once we can try again */ + return 1; } return 0; @@ -1051,8 +1079,14 @@ int _alpm_sync_commit(alpm_handle_t *handle, alpm_list_t **data) /* this can only happen maliciously */ total_bytes = total_bytes ? total_bytes : 1; - if(check_validity(handle, data, total, total_bytes)) { - return -1; + /* this one is special: -1 is failure, 1 is retry, 0 is success */ + while(1) { + int ret = check_validity(handle, total, total_bytes); + if(ret == 0) { + break; + } else if(ret < 0) { + return -1; + } } if(trans->flags & ALPM_TRANS_FLAG_DOWNLOADONLY) { -- 1.7.6.3
Add two new static methods, key_search() and key_import(), to our growing list of signing code. If we come across a key we do not have, attempt to look it up remotely and ask the user if they wish to import said key. If they do, flag the validation process as a potential 'retry', meaning it might succeed the next time it is ran. These depend on you having a 'keyserver hkp://foo.example.com' line in your gpg.conf file in your gnupg home directory to function. Signed-off-by: Dan McGee <dan@archlinux.org> --- lib/libalpm/signing.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++--- 1 files changed, 92 insertions(+), 5 deletions(-) diff --git a/lib/libalpm/signing.c b/lib/libalpm/signing.c index e1b6452..4554d13 100644 --- a/lib/libalpm/signing.c +++ b/lib/libalpm/signing.c @@ -160,6 +160,74 @@ error: RET_ERR(handle, ALPM_ERR_GPGME, 1); } +static int key_search(alpm_handle_t *handle, const char *fpr, + alpm_pgpkey_t *pgpkey) +{ + gpgme_error_t err; + gpgme_ctx_t ctx; + gpgme_keylist_mode_t mode; + gpgme_key_t key; + + memset(&ctx, 0, sizeof(ctx)); + err = gpgme_new(&ctx); + CHECK_ERR(); + + mode = gpgme_get_keylist_mode(ctx); + /* using LOCAL and EXTERN together doesn't work for GPG 1.X. Ugh. */ + mode &= ~GPGME_KEYLIST_MODE_LOCAL; + mode |= GPGME_KEYLIST_MODE_EXTERN; + err = gpgme_set_keylist_mode(ctx, mode); + CHECK_ERR(); + + _alpm_log(handle, ALPM_LOG_DEBUG, "looking up key %s\n", fpr); + + err = gpgme_get_key(ctx, fpr, &key, 0); + if(gpg_err_code(err) == GPG_ERR_EOF) { + _alpm_log(handle, ALPM_LOG_DEBUG, "key lookup failed, unknown key\n"); + } else if(gpg_err_code(err) != GPG_ERR_NO_ERROR) { + _alpm_log(handle, ALPM_LOG_DEBUG, + "gpg error: %s\n", gpgme_strerror(err)); + CHECK_ERR(); + } + + /* should only get here if key actually exists */ + pgpkey->data = key; + if(key->subkeys->fpr) { + pgpkey->fingerprint = key->subkeys->fpr; + } else if(key->subkeys->keyid) { + pgpkey->fingerprint = key->subkeys->keyid; + } + pgpkey->uid = key->uids->uid; + pgpkey->name = key->uids->name; + pgpkey->email = key->uids->email; + pgpkey->created = key->subkeys->timestamp; + pgpkey->expires = key->subkeys->expires; + +error: + gpgme_release(ctx); + return gpg_err_code(err) == GPG_ERR_NO_ERROR; +} + +static int key_import(alpm_handle_t *handle, alpm_pgpkey_t *key) +{ + gpgme_error_t err; + gpgme_ctx_t ctx; + gpgme_key_t keys[2]; + + memset(&ctx, 0, sizeof(ctx)); + err = gpgme_new(&ctx); + CHECK_ERR(); + + keys[0] = key->data; + keys[1] = NULL; + err = gpgme_op_import_keys(ctx, keys); + CHECK_ERR(); + +error: + gpgme_release(ctx); + return gpg_err_code(err) != GPG_ERR_NO_ERROR; +} + /** * Decode a loaded signature in base64 form. * @param base64_data the signature to attempt to decode @@ -521,6 +589,7 @@ int _alpm_process_siglist(alpm_handle_t *handle, const char *identifier, for(i = 0; i < siglist->count; i++) { alpm_sigresult_t *result = siglist->results + i; const char *name = result->key.uid ? result->key.uid : result->key.fingerprint; + int answer; switch(result->status) { case ALPM_SIGSTATUS_VALID: case ALPM_SIGSTATUS_KEY_EXPIRED: @@ -532,6 +601,7 @@ int _alpm_process_siglist(alpm_handle_t *handle, const char *identifier, _alpm_log(handle, ALPM_LOG_ERROR, _("%s: signature from \"%s\" is marginal trust\n"), identifier, name); + /* QUESTION(handle, ALPM_QUESTION_EDIT_KEY_TRUST, &result->key, NULL, NULL, &answer); */ } break; case ALPM_SIGVALIDITY_UNKNOWN: @@ -539,6 +609,7 @@ int _alpm_process_siglist(alpm_handle_t *handle, const char *identifier, _alpm_log(handle, ALPM_LOG_ERROR, _("%s: signature from \"%s\" is unknown trust\n"), identifier, name); + /* QUESTION(handle, ALPM_QUESTION_EDIT_KEY_TRUST, &result->key, NULL, NULL, &answer); */ } break; case ALPM_SIGVALIDITY_NEVER: @@ -549,15 +620,31 @@ int _alpm_process_siglist(alpm_handle_t *handle, const char *identifier, } break; case ALPM_SIGSTATUS_KEY_UNKNOWN: - /* TODO import key here */ _alpm_log(handle, ALPM_LOG_ERROR, - _("%s: key \"%s\" is unknown\n"), - identifier, name); + _("%s: key \"%s\" is unknown\n"), identifier, name); + { + alpm_pgpkey_t fetch_key; + memset(&fetch_key, 0, sizeof(fetch_key)); + + if(key_search(handle, result->key.fingerprint, &fetch_key)) { + _alpm_log(handle, ALPM_LOG_DEBUG, + "unknown key, found %s on keyserver\n", fetch_key.uid); + QUESTION(handle, ALPM_QUESTION_IMPORT_KEY, + &fetch_key, NULL, NULL, &answer); + if(answer && !key_import(handle, &fetch_key)) { + retry = 1; + } + } else { + _alpm_log(handle, ALPM_LOG_DEBUG, + "key could not be looked up remotely\n"); + } + gpgme_key_unref(fetch_key.data); + break; + } break; case ALPM_SIGSTATUS_SIG_EXPIRED: _alpm_log(handle, ALPM_LOG_ERROR, - _("%s: signature from \"%s\" is expired\n"), - identifier, name); + _("%s: signature from \"%s\" is expired\n"), identifier, name); break; case ALPM_SIGSTATUS_INVALID: _alpm_log(handle, ALPM_LOG_ERROR, -- 1.7.6.3
participants (1)
-
Dan McGee