I was trying to take a shortcut and not introduce a wrapper struct for the signature results, so packed it all into alpm_sigresult_t in the first iteration. However, this is painful when one wants to add new fields or only return information regarding a single signature.
Refactor the type into a few components which are exposed to the end user, and will allow a lot more future flexibility. This also exposes more information regarding the key to the frontend than was previously available. The "private" void *data pointer is used by the library to store the actual key object returned by gpgme; it is typed this way so the frontend has no expectations of what is there, and so we don't have any hard gpgme requirement in our public API. Signed-off-by: Dan McGee <[email protected]> --- lib/libalpm/alpm.h | 28 +++++++++++---- lib/libalpm/signing.c | 94 ++++++++++++++++++++++++------------------------ lib/libalpm/signing.h | 2 +- src/pacman/package.c | 8 ++-- src/pacman/util.c | 15 ++++---- src/pacman/util.h | 2 +- 6 files changed, 82 insertions(+), 67 deletions(-) diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index fc8f0bc..c94cdf7 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -209,16 +209,30 @@ typedef struct _alpm_backup_t { char *hash; } alpm_backup_t; +typedef struct _alpm_pgpkey_t { + void *data; + char *fingerprint; + char *uid; + char *name; + char *email; + time_t created; + time_t expires; +} alpm_pgpkey_t; + /** Signature result. Contains the number of signatures found and pointers to * arrays containing key and status info. All contained arrays have size * #count.*/ typedef struct _alpm_sigresult_t { - int count; - alpm_sigstatus_t *status; - alpm_sigvalidity_t *validity; - char **uid; + alpm_pgpkey_t key; + alpm_sigstatus_t status; + alpm_sigvalidity_t validity; } alpm_sigresult_t; +typedef struct _alpm_siglist_t { + size_t count; + alpm_sigresult_t *results; +} alpm_siglist_t; + /* * Logging facilities */ @@ -776,11 +790,11 @@ alpm_list_t *alpm_pkg_unused_deltas(alpm_pkg_t *pkg); * Signatures */ -int alpm_pkg_check_pgp_signature(alpm_pkg_t *pkg, alpm_sigresult_t *result); +int alpm_pkg_check_pgp_signature(alpm_pkg_t *pkg, alpm_siglist_t *siglist); -int alpm_db_check_pgp_signature(alpm_db_t *db, alpm_sigresult_t *result); +int alpm_db_check_pgp_signature(alpm_db_t *db, alpm_siglist_t *siglist); -int alpm_sigresult_cleanup(alpm_sigresult_t *result); +int alpm_siglist_cleanup(alpm_siglist_t *siglist); /* * Groups diff --git a/lib/libalpm/signing.c b/lib/libalpm/signing.c index 4428b54..8af8a99 100644 --- a/lib/libalpm/signing.c +++ b/lib/libalpm/signing.c @@ -195,16 +195,16 @@ error: * The return value will be 0 if nothing abnormal happened during the signature * check, and -1 if an error occurred while checking signatures or if a * signature could not be found; pm_errno will be set. Note that "abnormal" - * does not include a failed signature; the value in #result should be checked + * does not include a failed signature; the value in #siglist should be checked * to determine if the signature(s) are good. * @param handle the context handle * @param path the full path to a file * @param base64_sig optional PGP signature data in base64 encoding - * @result + * @param siglist a pointer to storage for signature results * @return 0 in normal cases, -1 if the something failed in the check process */ int _alpm_gpgme_checksig(alpm_handle_t *handle, const char *path, - const char *base64_sig, alpm_sigresult_t *result) + const char *base64_sig, alpm_siglist_t *siglist) { int ret = -1, sigcount; gpgme_error_t err; @@ -220,10 +220,10 @@ int _alpm_gpgme_checksig(alpm_handle_t *handle, const char *path, RET_ERR(handle, ALPM_ERR_NOT_A_FILE, -1); } - if(!result) { + if(!siglist) { RET_ERR(handle, ALPM_ERR_WRONG_ARGS, -1); } - result->count = 0; + siglist->count = 0; if(!base64_sig) { sigpath = _alpm_sigpath(handle, path); @@ -293,14 +293,9 @@ int _alpm_gpgme_checksig(alpm_handle_t *handle, const char *path, gpgsig; gpgsig = gpgsig->next, sigcount++); _alpm_log(handle, ALPM_LOG_DEBUG, "%d signatures returned\n", sigcount); - result->status = calloc(sigcount, sizeof(alpm_sigstatus_t)); - result->validity = calloc(sigcount, sizeof(alpm_sigvalidity_t)); - result->uid = calloc(sigcount, sizeof(char*)); - if(!result->status || !result->validity || !result->uid) { - handle->pm_errno = ALPM_ERR_MEMORY; - goto error; - } - result->count = sigcount; + CALLOC(siglist->results, sigcount, sizeof(alpm_sigresult_t), + handle->pm_errno = ALPM_ERR_MEMORY; goto error); + siglist->count = sigcount; for(gpgsig = verify_result->signatures, sigcount = 0; gpgsig; gpgsig = gpgsig->next, sigcount++) { @@ -308,6 +303,7 @@ int _alpm_gpgme_checksig(alpm_handle_t *handle, const char *path, alpm_sigstatus_t status; alpm_sigvalidity_t validity; gpgme_key_t key; + alpm_sigresult_t *result; _alpm_log(handle, ALPM_LOG_DEBUG, "fingerprint: %s\n", gpgsig->fpr); summary_list = list_sigsum(gpgsig->summary); @@ -322,21 +318,26 @@ int _alpm_gpgme_checksig(alpm_handle_t *handle, const char *path, string_validity(gpgsig->validity), gpgme_strerror(gpgsig->validity_reason)); + result = siglist->results + sigcount; err = gpgme_get_key(ctx, gpgsig->fpr, &key, 0); if(gpg_err_code(err) == GPG_ERR_EOF) { _alpm_log(handle, ALPM_LOG_DEBUG, "key lookup failed, unknown key\n"); err = GPG_ERR_NO_ERROR; - STRDUP(result->uid[sigcount], gpgsig->fpr, + /* we dupe the fpr in this case since we have no key to point at */ + STRDUP(result->key.fingerprint, gpgsig->fpr, handle->pm_errno = ALPM_ERR_MEMORY; goto error); } else { CHECK_ERR(); if(key->uids) { - const char *uid = key->uids->uid; - STRDUP(result->uid[sigcount], uid, - handle->pm_errno = ALPM_ERR_MEMORY; goto error); - _alpm_log(handle, ALPM_LOG_DEBUG, "key user: %s\n", uid); + result->key.data = key; + result->key.fingerprint = key->subkeys->fpr; + result->key.uid = key->uids->uid; + result->key.name = key->uids->name; + result->key.email = key->uids->email; + result->key.created = key->subkeys->timestamp; + result->key.expires = key->subkeys->expires; + _alpm_log(handle, ALPM_LOG_DEBUG, "key user: %s\n", key->uids->uid); } - gpgme_key_unref(key); } switch(gpg_err_code(gpgsig->status)) { @@ -383,8 +384,8 @@ int _alpm_gpgme_checksig(alpm_handle_t *handle, const char *path, validity = ALPM_SIGVALIDITY_NEVER; } - result->status[sigcount] = status; - result->validity[sigcount] = validity; + result->status = status; + result->validity = validity; } ret = 0; @@ -440,13 +441,13 @@ int _alpm_check_pgp_helper(alpm_handle_t *handle, const char *path, const char *base64_sig, int optional, int marginal, int unknown, enum _alpm_errno_t invalid_err) { - alpm_sigresult_t result; + alpm_siglist_t siglist; int ret; - memset(&result, 0, sizeof(result)); + memset(&siglist, 0, sizeof(alpm_siglist_t)); _alpm_log(handle, ALPM_LOG_DEBUG, "checking signatures for %s\n", path); - ret = _alpm_gpgme_checksig(handle, path, base64_sig, &result); + ret = _alpm_gpgme_checksig(handle, path, base64_sig, &siglist); if(ret && handle->pm_errno == ALPM_ERR_SIG_MISSING) { if(optional) { _alpm_log(handle, ALPM_LOG_DEBUG, "missing optional signature\n"); @@ -460,13 +461,13 @@ int _alpm_check_pgp_helper(alpm_handle_t *handle, const char *path, _alpm_log(handle, ALPM_LOG_DEBUG, "signature check failed\n"); /* ret will already be -1 */ } else { - int num; - for(num = 0; !ret && num < result.count; num++) { - switch(result.status[num]) { + size_t num; + for(num = 0; !ret && num < siglist.count; num++) { + switch(siglist.results[num].status) { case ALPM_SIGSTATUS_VALID: case ALPM_SIGSTATUS_KEY_EXPIRED: _alpm_log(handle, ALPM_LOG_DEBUG, "signature is valid\n"); - switch(result.validity[num]) { + switch(siglist.results[num].validity) { case ALPM_SIGVALIDITY_FULL: _alpm_log(handle, ALPM_LOG_DEBUG, "signature is fully trusted\n"); break; @@ -502,7 +503,7 @@ int _alpm_check_pgp_helper(alpm_handle_t *handle, const char *path, } } - alpm_sigresult_cleanup(&result); + alpm_siglist_cleanup(&siglist); return ret; } @@ -512,14 +513,14 @@ int _alpm_check_pgp_helper(alpm_handle_t *handle, const char *path, * @return a int value : 0 (valid), 1 (invalid), -1 (an error occurred) */ int SYMEXPORT alpm_pkg_check_pgp_signature(alpm_pkg_t *pkg, - alpm_sigresult_t *result) + alpm_siglist_t *siglist) { ASSERT(pkg != NULL, return -1); - ASSERT(result != NULL, RET_ERR(pkg->handle, ALPM_ERR_WRONG_ARGS, -1)); + ASSERT(siglist != NULL, RET_ERR(pkg->handle, ALPM_ERR_WRONG_ARGS, -1)); pkg->handle->pm_errno = 0; return _alpm_gpgme_checksig(pkg->handle, alpm_pkg_get_filename(pkg), - pkg->base64_sig, result); + pkg->base64_sig, siglist); } /** @@ -528,30 +529,29 @@ int SYMEXPORT alpm_pkg_check_pgp_signature(alpm_pkg_t *pkg, * @return a int value : 0 (valid), 1 (invalid), -1 (an error occurred) */ int SYMEXPORT alpm_db_check_pgp_signature(alpm_db_t *db, - alpm_sigresult_t *result) + alpm_siglist_t *siglist) { ASSERT(db != NULL, return -1); - ASSERT(result != NULL, RET_ERR(db->handle, ALPM_ERR_WRONG_ARGS, -1)); + ASSERT(siglist != NULL, RET_ERR(db->handle, ALPM_ERR_WRONG_ARGS, -1)); db->handle->pm_errno = 0; - return _alpm_gpgme_checksig(db->handle, _alpm_db_path(db), NULL, result); + return _alpm_gpgme_checksig(db->handle, _alpm_db_path(db), NULL, siglist); } -int SYMEXPORT alpm_sigresult_cleanup(alpm_sigresult_t *result) +int SYMEXPORT alpm_siglist_cleanup(alpm_siglist_t *siglist) { - ASSERT(result != NULL, return -1); - /* Because it is likely result is on the stack, uid and status may have bogus - * values in the struct. Only look at them if count is greater than 0. */ - if(result->count > 0) { - free(result->status); - if(result->uid) { - int i; - for(i = 0; i < result->count; i++) { - free(result->uid[i]); - } - free(result->uid); + ASSERT(siglist != NULL, return -1); + size_t num; + for(num = 0; num < siglist->count; num++) { + alpm_sigresult_t *result = siglist->results + num; + if(result->key.data) { + gpgme_key_unref(result->key.data); + } else { + free(result->key.fingerprint); } } + FREE(siglist->results); + siglist->count = 0; return 0; } diff --git a/lib/libalpm/signing.h b/lib/libalpm/signing.h index d07057a..6cb1656 100644 --- a/lib/libalpm/signing.h +++ b/lib/libalpm/signing.h @@ -23,7 +23,7 @@ char *_alpm_sigpath(alpm_handle_t *handle, const char *path); int _alpm_gpgme_checksig(alpm_handle_t *handle, const char *path, - const char *base64_sig, alpm_sigresult_t *result); + const char *base64_sig, alpm_siglist_t *result); int _alpm_check_pgp_helper(alpm_handle_t *handle, const char *path, const char *base64_sig, int optional, int marginal, int unknown, enum _alpm_errno_t invalid_err); diff --git a/src/pacman/package.c b/src/pacman/package.c index 7fe33a9..47b6623 100644 --- a/src/pacman/package.c +++ b/src/pacman/package.c @@ -149,17 +149,17 @@ void dump_pkg_full(alpm_pkg_t *pkg, enum pkg_from from, int extra) alpm_pkg_get_base64_sig(pkg) ? _("Yes") : _("None")); } if(from == PKG_FROM_FILE) { - alpm_sigresult_t result; - int err = alpm_pkg_check_pgp_signature(pkg, &result); + alpm_siglist_t siglist; + int err = alpm_pkg_check_pgp_signature(pkg, &siglist); if(err && alpm_errno(config->handle) == ALPM_ERR_SIG_MISSING) { string_display(_("Signatures :"), _("None")); } else if(err) { string_display(_("Signatures :"), alpm_strerror(alpm_errno(config->handle))); } else { - signature_display(_("Signatures :"), &result); + signature_display(_("Signatures :"), &siglist); } - alpm_sigresult_cleanup(&result); + alpm_siglist_cleanup(&siglist); } string_display(_("Description :"), alpm_pkg_get_desc(pkg)); diff --git a/src/pacman/util.c b/src/pacman/util.c index d2aa29b..585f402 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -664,7 +664,7 @@ void list_display_linebreak(const char *title, const alpm_list_t *list) } } -void signature_display(const char *title, alpm_sigresult_t *result) +void signature_display(const char *title, alpm_siglist_t *siglist) { int len = 0; @@ -672,13 +672,14 @@ void signature_display(const char *title, alpm_sigresult_t *result) len = string_length(title) + 1; printf("%s ", title); } - if(result->count == 0) { + if(siglist->count == 0) { printf(_("None")); } else { - int i; - for(i = 0; i < result->count; i++) { + size_t i; + for(i = 0; i < siglist->count; i++) { char sigline[PATH_MAX]; const char *status, *validity, *name; + alpm_sigresult_t *result = siglist->results + i; /* Don't re-indent the first result */ if(i != 0) { int j; @@ -686,7 +687,7 @@ void signature_display(const char *title, alpm_sigresult_t *result) printf(" "); } } - switch(result->status[i]) { + switch(result->status) { case ALPM_SIGSTATUS_VALID: status = _("Valid"); break; @@ -706,7 +707,7 @@ void signature_display(const char *title, alpm_sigresult_t *result) status = _("Signature error"); break; } - switch(result->validity[i]) { + switch(result->validity) { case ALPM_SIGVALIDITY_FULL: validity = _("full trust"); break; @@ -721,7 +722,7 @@ void signature_display(const char *title, alpm_sigresult_t *result) validity = _("unknown trust"); break; } - name = result->uid[i] ? result->uid[i] : _("{Key Unknown}"); + name = result->key.uid ? result->key.uid : result->key.fingerprint; snprintf(sigline, PATH_MAX, _("%s, %s from \"%s\""), status, validity, name); indentprint(sigline, len); diff --git a/src/pacman/util.h b/src/pacman/util.h index ee3dbd1..2411e65 100644 --- a/src/pacman/util.h +++ b/src/pacman/util.h @@ -57,7 +57,7 @@ double humanize_size(off_t bytes, const char target_unit, int long_labels, const int table_display(const char *title, const alpm_list_t *header, const alpm_list_t *rows); void list_display(const char *title, const alpm_list_t *list); void list_display_linebreak(const char *title, const alpm_list_t *list); -void signature_display(const char *title, alpm_sigresult_t *result); +void signature_display(const char *title, alpm_siglist_t *siglist); void display_targets(const alpm_list_t *pkgs, int install); int str_cmp(const void *s1, const void *s2); void display_new_optdepends(alpm_pkg_t *oldpkg, alpm_pkg_t *newpkg); -- 1.7.6
