[pacman-dev] [PATCH 0/8] [RFC] Signature checking overhaul
The main change here is that pacman will now check the needed keys are in the keyring before doing package validation. Example output: :: Retrieving packages ... systemd-sysvcompat-... 5.4 KiB 671K/s 00:00 [######################] 100% (1/1) checking keys in keyring [######################] 100% :: Import PGP key 2048R/F56C0C53, "Dave Reisner <d@falconindy.com>", created: 2011-06-26? [Y/n] (1/1) checking package integrity [######################] 100% (1/1) loading package files [######################] 100% This removed the repeat validation after key downloading and made the following much easier to implement: 1) packages with bad signatures get the "pkg is corrupt, delete?" type message 2) pacman -U now downloads a signature if needed. These patches need a very good review... Allan McRae (8): Make key_in_keychain available in library Move key importing into separate function Add function to extract key id from signatures Make decode_signature available to the library Check keys are in keyring before package validation Remove retry path from signature validation Prompt to delete packages with signature fails Import key if needed when installing package from file lib/libalpm/alpm.h | 9 ++- lib/libalpm/be_package.c | 40 ++++++++++ lib/libalpm/signing.c | 197 ++++++++++++++++++++++++++++++++++++++--------- lib/libalpm/signing.h | 7 ++ lib/libalpm/sync.c | 87 ++++++++++++++++++--- src/pacman/callback.c | 9 +++ 6 files changed, 300 insertions(+), 49 deletions(-) -- 1.8.0
In preparation for checking key presence and downloading needed keys before conflict checking. Signed-off-by: Allan McRae <allan@archlinux.org> --- lib/libalpm/signing.c | 7 ++++--- lib/libalpm/signing.h | 2 ++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/libalpm/signing.c b/lib/libalpm/signing.c index 6eb2da5..09463ff 100644 --- a/lib/libalpm/signing.c +++ b/lib/libalpm/signing.c @@ -185,7 +185,7 @@ error: * @param fpr the fingerprint key ID to look up * @return 1 if key is known, 0 if key is unknown, -1 on error */ -static int key_in_keychain(alpm_handle_t *handle, const char *fpr) +int _alpm_key_in_keychain(alpm_handle_t *handle, const char *fpr) { gpgme_error_t err; gpgme_ctx_t ctx; @@ -631,10 +631,11 @@ error: } #else /* HAVE_LIBGPGME */ -static int key_in_keychain(alpm_handle_t UNUSED *handle, const char UNUSED *fpr) +int _alpm_key_in_keychain(alpm_handle_t UNUSED *handle, const char UNUSED *fpr) { return -1; } + int _alpm_gpgme_checksig(alpm_handle_t UNUSED *handle, const char UNUSED *path, const char UNUSED *base64_sig, alpm_siglist_t UNUSED *siglist) { @@ -810,7 +811,7 @@ int _alpm_process_siglist(alpm_handle_t *handle, const char *identifier, case ALPM_SIGSTATUS_KEY_UNKNOWN: /* ensure this key is still actually unknown; we may have imported it * on an earlier call to this function. */ - if(key_in_keychain(handle, result->key.fingerprint) == 1) { + if(_alpm_key_in_keychain(handle, result->key.fingerprint) == 1) { break; } _alpm_log(handle, ALPM_LOG_ERROR, diff --git a/lib/libalpm/signing.h b/lib/libalpm/signing.h index 19ffef2..cc3e979 100644 --- a/lib/libalpm/signing.h +++ b/lib/libalpm/signing.h @@ -31,6 +31,8 @@ int _alpm_check_pgp_helper(alpm_handle_t *handle, const char *path, int _alpm_process_siglist(alpm_handle_t *handle, const char *identifier, alpm_siglist_t *siglist, int optional, int marginal, int unknown); +int _alpm_key_in_keychain(alpm_handle_t *handle, const char *fpr); + #endif /* _ALPM_SIGNING_H */ /* vim: set ts=2 sw=2 noet: */ -- 1.8.0
This will be useful for checking the availablity of all keys before perfoming validation in sync operations and for downloading a needed key in upgrade operations. Signed-off-by: Allan McRae <allan@archlinux.org> --- lib/libalpm/signing.c | 81 +++++++++++++++++++++++++++++++-------------------- lib/libalpm/signing.h | 1 + 2 files changed, 50 insertions(+), 32 deletions(-) diff --git a/lib/libalpm/signing.c b/lib/libalpm/signing.c index 09463ff..9d56aba 100644 --- a/lib/libalpm/signing.c +++ b/lib/libalpm/signing.c @@ -372,6 +372,46 @@ error: } /** + * Import a key defined by a fingerprint into the local keyring. + * @param handle the context handle + * @param fpr the fingerprint key ID to import + * @return 0 on success, -1 on error + */ +int _alpm_key_import(alpm_handle_t *handle, const char *fpr) { + int answer = 0, ret = -1; + alpm_pgpkey_t fetch_key; + memset(&fetch_key, 0, sizeof(fetch_key)); + + if(key_search(handle, fpr, &fetch_key) == 1) { + _alpm_log(handle, ALPM_LOG_DEBUG, + "unknown key, found %s on keyserver\n", fetch_key.uid); + if(!_alpm_access(handle, handle->gpgdir, "pubring.gpg", W_OK)) { + QUESTION(handle, ALPM_QUESTION_IMPORT_KEY, + &fetch_key, NULL, NULL, &answer); + if(answer) { + if(key_import(handle, &fetch_key) == 0) { + ret = 0; + } else { + _alpm_log(handle, ALPM_LOG_ERROR, + _("key \"%s\" could not be imported\n"), fetch_key.uid); + } + } + } else { + /* keyring directory was not writable, so we don't even try */ + _alpm_log(handle, ALPM_LOG_WARNING, + _("key %s, \"%s\" found on keyserver, keyring is not writable\n"), + fetch_key.fingerprint, fetch_key.uid); + } + } else { + _alpm_log(handle, ALPM_LOG_ERROR, + _("key \"%s\" could not be looked up remotely\n"), fpr); + } + gpgme_key_unref(fetch_key.data); + + return ret; +} + +/** * Decode a loaded signature in base64 form. * @param base64_data the signature to attempt to decode * @param data the decoded data; must be freed by the caller @@ -636,6 +676,11 @@ int _alpm_key_in_keychain(alpm_handle_t UNUSED *handle, const char UNUSED *fpr) return -1; } +int _alpm_key_import(alpm_handle_t UNUSED *handle, const char UNUSED *fpr) +{ + return -1; +} + int _alpm_gpgme_checksig(alpm_handle_t UNUSED *handle, const char UNUSED *path, const char UNUSED *base64_sig, alpm_siglist_t UNUSED *siglist) { @@ -816,39 +861,11 @@ int _alpm_process_siglist(alpm_handle_t *handle, const char *identifier, } _alpm_log(handle, ALPM_LOG_ERROR, _("%s: key \"%s\" is unknown\n"), identifier, name); -#ifdef HAVE_LIBGPGME - { - int answer = 0; - alpm_pgpkey_t fetch_key; - memset(&fetch_key, 0, sizeof(fetch_key)); - - if(key_search(handle, result->key.fingerprint, &fetch_key) == 1) { - _alpm_log(handle, ALPM_LOG_DEBUG, - "unknown key, found %s on keyserver\n", fetch_key.uid); - if(!_alpm_access(handle, handle->gpgdir, "pubring.gpg", W_OK)) { - QUESTION(handle, ALPM_QUESTION_IMPORT_KEY, - &fetch_key, NULL, NULL, &answer); - if(answer) { - if(key_import(handle, &fetch_key) == 0) { - retry = 1; - } else { - _alpm_log(handle, ALPM_LOG_ERROR, - _("key \"%s\" could not be imported\n"), fetch_key.uid); - } - } - } else { - /* keyring directory was not writable, so we don't even try */ - _alpm_log(handle, ALPM_LOG_WARNING, - _("key %s, \"%s\" found on keyserver, keyring is not writable\n"), - fetch_key.fingerprint, fetch_key.uid); - } - } else { - _alpm_log(handle, ALPM_LOG_ERROR, - _("key \"%s\" could not be looked up remotely\n"), name); - } - gpgme_key_unref(fetch_key.data); + + if(_alpm_key_import(handle, result->key.fingerprint) == 0) { + retry = 1; } -#endif + break; case ALPM_SIGSTATUS_KEY_DISABLED: _alpm_log(handle, ALPM_LOG_ERROR, diff --git a/lib/libalpm/signing.h b/lib/libalpm/signing.h index cc3e979..a78e4b7 100644 --- a/lib/libalpm/signing.h +++ b/lib/libalpm/signing.h @@ -32,6 +32,7 @@ int _alpm_process_siglist(alpm_handle_t *handle, const char *identifier, alpm_siglist_t *siglist, int optional, int marginal, int unknown); int _alpm_key_in_keychain(alpm_handle_t *handle, const char *fpr); +int _alpm_key_import(alpm_handle_t *handle, const char *fpr); #endif /* _ALPM_SIGNING_H */ -- 1.8.0
This does not support all possibilities of RFC4880, but it does cover every key currently used in Arch Linux. Signed-off-by: Allan McRae <allan@archlinux.org> --- lib/libalpm/signing.c | 100 ++++++++++++++++++++++++++++++++++++++++++++++++++ lib/libalpm/signing.h | 2 + 2 files changed, 102 insertions(+) diff --git a/lib/libalpm/signing.c b/lib/libalpm/signing.c index 9d56aba..6b13522 100644 --- a/lib/libalpm/signing.c +++ b/lib/libalpm/signing.c @@ -948,4 +948,104 @@ int SYMEXPORT alpm_siglist_cleanup(alpm_siglist_t *siglist) return 0; } +/** + * Extract the Issuer Key ID from a signature + * @param sig PGP signature + * @param len length of signature + * @param keys a pointer to storage for key IDs + * @return 0 on success, -1 on error + */ +int _alpm_extract_keyid(const unsigned char *sig, size_t len, alpm_list_t **keys) +{ + size_t pos, spos, blen, hlen, ulen, slen; + pos = 0; + + while(pos < len) { + if(!(sig[pos] & 0x80)) { + return -1; + } + + if(sig[pos] & 0x40) { + // "new" packet format is not supported + return -1; + } + + if(((sig[pos] & 0x3f) >> 2) != 2) { + // signature is not a "Signature Packet" + return -1; + } + + switch (sig[pos] & 0x03) { + case 0: + blen = sig[pos + 1]; + pos = pos + 2; + break; + + case 1: + blen = (sig[pos + 1] << 8) | sig[pos + 2]; + pos = pos + 3; + break; + + case 2: + blen = (sig[pos + 1] << 24) | (sig[pos + 2] << 16) | (sig[pos + 3] << 8) | sig[pos + 4]; + pos = pos + 5; + break; + + case 3: + // partial body length not supported + return -1; + } + + if(sig[pos] != 4) { + // only support version 4 signature packet format + return -1; + } + + if(sig[pos + 1] != 0x00) { + // not a signature of a binary document + return -1; + } + + pos = pos + 4; + + hlen = (sig[pos] << 8) | sig[pos + 1]; + pos = pos + hlen + 2; + + ulen = (sig[pos] << 8) | sig[pos + 1]; + pos = pos + 2; + + spos = pos; + + while(spos < pos + ulen) { + if(sig[spos] < 192) { + slen = sig[spos]; + spos = spos + 1; + } else if(sig[spos] < 255) { + slen = (sig[spos] << 8) | sig[spos + 1]; + spos = spos + 2; + } else { + slen = (sig[spos + 1] << 24) | (sig[spos + 2] << 16) | (sig[spos + 3] << 8) | sig[spos + 4]; + spos = spos + 5; + } + + if(sig[spos] == 16) { + // issuer key ID + char key[16]; + size_t i; + for (i = 0; i < 8; i++) { + sprintf(&key[i * 2], "%02hhX", sig[spos + i + 1]); + } + *keys = alpm_list_add(*keys, strdup(key)); + break; + } + + spos = spos + slen; + } + + pos = pos + (blen - hlen - 8); + } + + return 0; +} + /* vim: set ts=2 sw=2 noet: */ diff --git a/lib/libalpm/signing.h b/lib/libalpm/signing.h index a78e4b7..6537ee3 100644 --- a/lib/libalpm/signing.h +++ b/lib/libalpm/signing.h @@ -34,6 +34,8 @@ int _alpm_process_siglist(alpm_handle_t *handle, const char *identifier, int _alpm_key_in_keychain(alpm_handle_t *handle, const char *fpr); int _alpm_key_import(alpm_handle_t *handle, const char *fpr); +int _alpm_extract_keyid(const unsigned char *sig, size_t len, alpm_list_t **keys); + #endif /* _ALPM_SIGNING_H */ /* vim: set ts=2 sw=2 noet: */ -- 1.8.0
On Sat, Nov 03, 2012 at 01:28:17AM +1000, Allan McRae wrote:
This does not support all possibilities of RFC4880, but it does cover every key currently used in Arch Linux.
Hmmm, this function doesn't seem to distinguish between "not a key" and "not a supported key". Can we return more useful error codes, like -ENOSYS for unsupported, -EINVAL for not-a-signature packet...
Signed-off-by: Allan McRae <allan@archlinux.org> --- lib/libalpm/signing.c | 100 ++++++++++++++++++++++++++++++++++++++++++++++++++ lib/libalpm/signing.h | 2 + 2 files changed, 102 insertions(+)
diff --git a/lib/libalpm/signing.c b/lib/libalpm/signing.c index 9d56aba..6b13522 100644 --- a/lib/libalpm/signing.c +++ b/lib/libalpm/signing.c @@ -948,4 +948,104 @@ int SYMEXPORT alpm_siglist_cleanup(alpm_siglist_t *siglist) return 0; }
+/** + * Extract the Issuer Key ID from a signature + * @param sig PGP signature + * @param len length of signature + * @param keys a pointer to storage for key IDs + * @return 0 on success, -1 on error + */ +int _alpm_extract_keyid(const unsigned char *sig, size_t len, alpm_list_t **keys) +{ + size_t pos, spos, blen, hlen, ulen, slen; + pos = 0; + + while(pos < len) { + if(!(sig[pos] & 0x80)) { + return -1; + } + + if(sig[pos] & 0x40) { + // "new" packet format is not supported + return -1; + } + + if(((sig[pos] & 0x3f) >> 2) != 2) { + // signature is not a "Signature Packet" + return -1; + } + + switch (sig[pos] & 0x03) {
style nit: switch(foo)
+ case 0: + blen = sig[pos + 1]; + pos = pos + 2; + break; + + case 1: + blen = (sig[pos + 1] << 8) | sig[pos + 2]; + pos = pos + 3; + break; + + case 2: + blen = (sig[pos + 1] << 24) | (sig[pos + 2] << 16) | (sig[pos + 3] << 8) | sig[pos + 4]; + pos = pos + 5; + break; + + case 3: + // partial body length not supported + return -1; + } + + if(sig[pos] != 4) { + // only support version 4 signature packet format + return -1; + } + + if(sig[pos + 1] != 0x00) { + // not a signature of a binary document + return -1; + } + + pos = pos + 4; + + hlen = (sig[pos] << 8) | sig[pos + 1]; + pos = pos + hlen + 2; + + ulen = (sig[pos] << 8) | sig[pos + 1]; + pos = pos + 2; + + spos = pos; + + while(spos < pos + ulen) { + if(sig[spos] < 192) { + slen = sig[spos]; + spos = spos + 1; + } else if(sig[spos] < 255) { + slen = (sig[spos] << 8) | sig[spos + 1]; + spos = spos + 2; + } else { + slen = (sig[spos + 1] << 24) | (sig[spos + 2] << 16) | (sig[spos + 3] << 8) | sig[spos + 4]; + spos = spos + 5; + } + + if(sig[spos] == 16) { + // issuer key ID + char key[16]; + size_t i; + for (i = 0; i < 8; i++) { + sprintf(&key[i * 2], "%02hhX", sig[spos + i + 1]); + } + *keys = alpm_list_add(*keys, strdup(key));
Would be nice to be robust against strdup() memory allocation failures here.
+ break; + } + + spos = spos + slen; + } + + pos = pos + (blen - hlen - 8); + } + + return 0; +} + /* vim: set ts=2 sw=2 noet: */ diff --git a/lib/libalpm/signing.h b/lib/libalpm/signing.h index a78e4b7..6537ee3 100644 --- a/lib/libalpm/signing.h +++ b/lib/libalpm/signing.h @@ -34,6 +34,8 @@ int _alpm_process_siglist(alpm_handle_t *handle, const char *identifier, int _alpm_key_in_keychain(alpm_handle_t *handle, const char *fpr); int _alpm_key_import(alpm_handle_t *handle, const char *fpr);
+int _alpm_extract_keyid(const unsigned char *sig, size_t len, alpm_list_t **keys); + #endif /* _ALPM_SIGNING_H */
/* vim: set ts=2 sw=2 noet: */ -- 1.8.0
On 03/11/12 01:40, Dave Reisner wrote:
On Sat, Nov 03, 2012 at 01:28:17AM +1000, Allan McRae wrote:
This does not support all possibilities of RFC4880, but it does cover every key currently used in Arch Linux.
Hmmm, this function doesn't seem to distinguish between "not a key" and "not a supported key". Can we return more useful error codes, like -ENOSYS for unsupported, -EINVAL for not-a-signature packet...
Instead of doing this I have added ALPM_LOG_ERROR messages that output why the key has failed. I think this is a more clear way to allow a user to report and issue allowing us to follow up any failure in the future. Allan
On Fri, Nov 2, 2012 at 10:28 AM, Allan McRae <allan@archlinux.org> wrote:
This does not support all possibilities of RFC4880, but it does cover every key currently used in Arch Linux.
Signed-off-by: Allan McRae <allan@archlinux.org> --- lib/libalpm/signing.c | 100 ++++++++++++++++++++++++++++++++++++++++++++++++++ lib/libalpm/signing.h | 2 + 2 files changed, 102 insertions(+)
diff --git a/lib/libalpm/signing.c b/lib/libalpm/signing.c index 9d56aba..6b13522 100644 --- a/lib/libalpm/signing.c +++ b/lib/libalpm/signing.c @@ -948,4 +948,104 @@ int SYMEXPORT alpm_siglist_cleanup(alpm_siglist_t *siglist) return 0; }
+/** + * Extract the Issuer Key ID from a signature + * @param sig PGP signature + * @param len length of signature + * @param keys a pointer to storage for key IDs + * @return 0 on success, -1 on error + */ +int _alpm_extract_keyid(const unsigned char *sig, size_t len, alpm_list_t **keys) +{ + size_t pos, spos, blen, hlen, ulen, slen; + pos = 0; + + while(pos < len) { + if(!(sig[pos] & 0x80)) { + return -1; + } + + if(sig[pos] & 0x40) { + // "new" packet format is not supported Respect the preferred coding style for comments, please. :)
-Dan
Signed-off-by: Allan McRae <allan@archlinux.org> --- lib/libalpm/signing.c | 4 ++-- lib/libalpm/signing.h | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/libalpm/signing.c b/lib/libalpm/signing.c index 6b13522..703e3ea 100644 --- a/lib/libalpm/signing.c +++ b/lib/libalpm/signing.c @@ -418,7 +418,7 @@ int _alpm_key_import(alpm_handle_t *handle, const char *fpr) { * @param data_len the length of the returned data * @return 0 on success, -1 on failure to properly decode */ -static int decode_signature(const char *base64_data, +int _alpm_decode_signature(const char *base64_data, unsigned char **data, size_t *data_len) { size_t len = strlen(base64_data); unsigned char *usline = (unsigned char *)base64_data; @@ -517,7 +517,7 @@ int _alpm_gpgme_checksig(alpm_handle_t *handle, const char *path, if(base64_sig) { /* memory-based, we loaded it from a sync DB */ size_t data_len; - int decode_ret = decode_signature(base64_sig, + int decode_ret = _alpm_decode_signature(base64_sig, &decoded_sigdata, &data_len); if(decode_ret) { handle->pm_errno = ALPM_ERR_SIG_INVALID; diff --git a/lib/libalpm/signing.h b/lib/libalpm/signing.h index 6537ee3..d4d7c78 100644 --- a/lib/libalpm/signing.h +++ b/lib/libalpm/signing.h @@ -34,6 +34,8 @@ int _alpm_process_siglist(alpm_handle_t *handle, const char *identifier, int _alpm_key_in_keychain(alpm_handle_t *handle, const char *fpr); int _alpm_key_import(alpm_handle_t *handle, const char *fpr); +int _alpm_decode_signature(const char *base64_data, + unsigned char **data, size_t *data_len); int _alpm_extract_keyid(const unsigned char *sig, size_t len, alpm_list_t **keys); #endif /* _ALPM_SIGNING_H */ -- 1.8.0
Keys used to create signatures are checked for presence in the keyring before package validation is performed. Signed-off-by: Allan McRae <allan@archlinux.org> --- lib/libalpm/alpm.h | 9 +++++-- lib/libalpm/sync.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++ src/pacman/callback.c | 9 +++++++ 3 files changed, 88 insertions(+), 2 deletions(-) diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index 1d6a8c6..8f3cfb0 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -347,7 +347,11 @@ typedef enum _alpm_event_t { /** Disk space usage will be computed for a package */ ALPM_EVENT_DISKSPACE_START, /** Disk space usage was computed for a package */ - ALPM_EVENT_DISKSPACE_DONE + ALPM_EVENT_DISKSPACE_DONE, + /** Checking keys used to create signatures are in keyring. */ + ALPM_EVENT_KEYRING_START, + /** Keyring checking is finished. */ + ALPM_EVENT_KEYRING_DONE } alpm_event_t; /** Event callback */ @@ -381,7 +385,8 @@ typedef enum _alpm_progress_t { ALPM_PROGRESS_CONFLICTS_START, ALPM_PROGRESS_DISKSPACE_START, ALPM_PROGRESS_INTEGRITY_START, - ALPM_PROGRESS_LOAD_START + ALPM_PROGRESS_LOAD_START, + ALPM_PROGRESS_KEYRING_START } alpm_progress_t; /** Progress callback */ diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index ca6b507..dcfefd1 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -969,6 +969,71 @@ finish: return errors; } +static int check_keyring(alpm_handle_t *handle) +{ + size_t current = 0, numtargs; + alpm_list_t *i, *errors = NULL; + + EVENT(handle, ALPM_EVENT_KEYRING_START, NULL, NULL); + + numtargs = alpm_list_count(handle->trans->add); + + for(i = handle->trans->add; i; i = i->next, current++) { + alpm_pkg_t *pkg = i->data; + alpm_siglevel_t level; + + int percent = (current * 100) / numtargs; + PROGRESS(handle, ALPM_PROGRESS_KEYRING_START, "", percent, + numtargs, current); + + if(pkg->origin == ALPM_PKG_FROM_FILE) { + continue; /* pkg_load() has been already called, this package is valid */ + } + + level = alpm_db_get_siglevel(alpm_pkg_get_db(pkg)); + if((level & ALPM_SIG_PACKAGE) && pkg->base64_sig) { + unsigned char *decoded_sigdata = NULL; + size_t data_len; + int decode_ret = _alpm_decode_signature(pkg->base64_sig, + &decoded_sigdata, &data_len); + if(decode_ret == 0) { + alpm_list_t *keys = NULL; + if(_alpm_extract_keyid(decoded_sigdata, data_len, &keys) == 0) { + alpm_list_t *k; + for(k = keys; k; k = k->next) { + char *key = k->data; + if(_alpm_key_in_keychain(handle, key) == 0) { + errors = alpm_list_add(errors, strdup(key)); + } + } + FREELIST(keys); + } + } + } + } + + PROGRESS(handle, ALPM_PROGRESS_KEYRING_START, "", 100, + numtargs, current); + EVENT(handle, ALPM_EVENT_KEYRING_DONE, NULL, NULL); + + if(errors) { + int fail = 0; + alpm_list_t *k; + for(k = errors; k; k = k->next) { + char *key = k->data; + if(_alpm_key_import(handle, key) == -1) { + fail = 1; + } + } + if(fail) { + _alpm_log(handle, ALPM_LOG_ERROR, _("required key missing from keyring\n")); + return -1; + } + } + + return 0; +} + static int check_validity(alpm_handle_t *handle, size_t total, size_t total_bytes) { @@ -1134,6 +1199,13 @@ int _alpm_sync_commit(alpm_handle_t *handle, alpm_list_t **data) return -1; } +#if HAVE_LIBGPGME + /* make sure all required signatures are in keyring */ + if(check_keyring(handle)) { + return -1; + } +#endif + /* get the total size of all packages so we can adjust the progress bar more * realistically if there are small and huge packages involved */ for(i = trans->add; i; i = i->next) { diff --git a/src/pacman/callback.c b/src/pacman/callback.c index 01c6b61..4e2f153 100644 --- a/src/pacman/callback.c +++ b/src/pacman/callback.c @@ -208,6 +208,11 @@ void cb_event(alpm_event_t event, void *data1, void *data2) printf(_("checking package integrity...\n")); } break; + case ALPM_EVENT_KEYRING_START: + if(config->noprogressbar) { + printf(_("checking keyring...\n")); + } + break; case ALPM_EVENT_LOAD_START: if(config->noprogressbar) { printf(_("loading package files...\n")); @@ -245,6 +250,7 @@ void cb_event(alpm_event_t event, void *data1, void *data2) case ALPM_EVENT_RESOLVEDEPS_DONE: case ALPM_EVENT_INTERCONFLICTS_DONE: case ALPM_EVENT_INTEGRITY_DONE: + case ALPM_EVENT_KEYRING_DONE: case ALPM_EVENT_LOAD_DONE: case ALPM_EVENT_DELTA_INTEGRITY_DONE: case ALPM_EVENT_DELTA_PATCHES_DONE: @@ -432,6 +438,9 @@ void cb_progress(alpm_progress_t event, const char *pkgname, int percent, case ALPM_PROGRESS_INTEGRITY_START: opr = _("checking package integrity"); break; + case ALPM_PROGRESS_KEYRING_START: + opr = _("checking keys in keyring"); + break; case ALPM_PROGRESS_LOAD_START: opr = _("loading package files"); break; -- 1.8.0
Now that the keyring is checked for all needed keys before the validation, we can not reach a point of a missing key when doing validity checks for sync operations. Signed-off-by: Allan McRae <allan@archlinux.org> --- lib/libalpm/sync.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index dcfefd1..031034d 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -1084,15 +1084,13 @@ static int check_validity(alpm_handle_t *handle, EVENT(handle, ALPM_EVENT_INTEGRITY_DONE, NULL, NULL); if(errors) { - 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, + _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); } @@ -1103,14 +1101,10 @@ static int check_validity(alpm_handle_t *handle, } alpm_list_free(errors); - if(tryagain == 0) { - if(!handle->pm_errno) { - RET_ERR(handle, ALPM_ERR_PKG_INVALID, -1); - } - return -1; + if(!handle->pm_errno) { + RET_ERR(handle, ALPM_ERR_PKG_INVALID, -1); } - /* we were told at least once we can try again */ - return 1; + return -1; } return 0; -- 1.8.0
Offer to remove the bad package when a signature fails to validate as is done for checksum failures. Signed-off-by: Allan McRae <allan@archlinux.org> --- lib/libalpm/sync.c | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index 031034d..2546579 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -1091,6 +1091,7 @@ static int check_validity(alpm_handle_t *handle, v->level & ALPM_SIG_PACKAGE_OPTIONAL, v->level & ALPM_SIG_PACKAGE_MARGINAL_OK, v->level & ALPM_SIG_PACKAGE_UNKNOWN_OK); + prompt_to_delete(handle, v->path, v->error); } else if(v->error == ALPM_ERR_PKG_INVALID_CHECKSUM) { prompt_to_delete(handle, v->path, v->error); } -- 1.8.0
On Sat, Nov 03, 2012 at 01:28:21AM +1000, Allan McRae wrote:
Offer to remove the bad package when a signature fails to validate as is done for checksum failures.
Signed-off-by: Allan McRae <allan@archlinux.org> ---
FS#28014 This is totally anti-climactic.
lib/libalpm/sync.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index 031034d..2546579 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -1091,6 +1091,7 @@ static int check_validity(alpm_handle_t *handle, v->level & ALPM_SIG_PACKAGE_OPTIONAL, v->level & ALPM_SIG_PACKAGE_MARGINAL_OK, v->level & ALPM_SIG_PACKAGE_UNKNOWN_OK); + prompt_to_delete(handle, v->path, v->error); } else if(v->error == ALPM_ERR_PKG_INVALID_CHECKSUM) { prompt_to_delete(handle, v->path, v->error); } -- 1.8.0
When installing a package with "pacman -U" that has a detached signature, check if the needed key is in the keyring and download if necessary. Signed-off-by: Allan McRae <allan@archlinux.org> --- lib/libalpm/be_package.c | 40 ++++++++++++++++++++++++++++++++++++++++ lib/libalpm/signing.c | 5 +++++ 2 files changed, 45 insertions(+) diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c index 8c5b2d1..710a063 100644 --- a/lib/libalpm/be_package.c +++ b/lib/libalpm/be_package.c @@ -519,6 +519,46 @@ 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)); + char *sigpath = _alpm_sigpath(handle, filename); + if(sigpath && !_alpm_access(handle, NULL, sigpath, R_OK)) { + if(level & ALPM_SIG_PACKAGE) { + struct stat info; + alpm_list_t *keys = NULL; + int fail = 0; + size_t len; + unsigned char *sig; + FILE *fp; + + stat(sigpath, &info); + len = info.st_size; + sig = malloc(len); + fp = fopen(sigpath, "rb"); + fread(sig, len, 1, fp); + fclose(fp); + + if(_alpm_extract_keyid(sig, len, &keys) == 0) { + alpm_list_t *k; + for(k = keys; k; k = k->next) { + char *key = k->data; + if(_alpm_key_in_keychain(handle, key) == 0) { + if(_alpm_key_import(handle, key) == -1) { + fail = 1; + } + } + } + FREELIST(keys); + } + + free(sig); + + if(fail) { + _alpm_log(handle, ALPM_LOG_ERROR, _("required key missing from keyring\n")); + return -1; + } + } + } + free(sigpath); + if(_alpm_pkg_validate_internal(handle, filename, NULL, level, NULL, &validation) == -1) { /* pm_errno is set by pkg_validate */ diff --git a/lib/libalpm/signing.c b/lib/libalpm/signing.c index 703e3ea..4f03853 100644 --- a/lib/libalpm/signing.c +++ b/lib/libalpm/signing.c @@ -192,6 +192,11 @@ int _alpm_key_in_keychain(alpm_handle_t *handle, const char *fpr) gpgme_key_t key; int ret = -1; + if(init_gpgme(handle)) { + /* pm_errno was set in gpgme_init() */ + goto error; + } + memset(&ctx, 0, sizeof(ctx)); err = gpgme_new(&ctx); CHECK_ERR(); -- 1.8.0
On Sat, Nov 03, 2012 at 01:28:22AM +1000, Allan McRae wrote:
When installing a package with "pacman -U" that has a detached signature, check if the needed key is in the keyring and download if necessary.
Signed-off-by: Allan McRae <allan@archlinux.org> ---
What ever happened to our musings about adding another SigLevel-ish option for handling this sort of thing? Are we satisfied with just letting the global value dictate this behavior?
lib/libalpm/be_package.c | 40 ++++++++++++++++++++++++++++++++++++++++ lib/libalpm/signing.c | 5 +++++ 2 files changed, 45 insertions(+)
diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c index 8c5b2d1..710a063 100644 --- a/lib/libalpm/be_package.c +++ b/lib/libalpm/be_package.c @@ -519,6 +519,46 @@ 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));
+ char *sigpath = _alpm_sigpath(handle, filename); + if(sigpath && !_alpm_access(handle, NULL, sigpath, R_OK)) { + if(level & ALPM_SIG_PACKAGE) { + struct stat info;
nitpick: we use 'st' in a lot of places to denote the name of a stat struct. I don't feel incredibly strongly about this, but consistency is nice.
+ alpm_list_t *keys = NULL; + int fail = 0; + size_t len;
If you're going to use this as a quick reference to a stat field, mark it const (or just get rid of it and reference info.st_size).
+ unsigned char *sig; + FILE *fp; + + stat(sigpath, &info);
Check for stat failure?
+ len = info.st_size; + sig = malloc(len);
Check for null return?
+ fp = fopen(sigpath, "rb");
Check for null fp?
+ fread(sig, len, 1, fp); + fclose(fp); + + if(_alpm_extract_keyid(sig, len, &keys) == 0) { + alpm_list_t *k; + for(k = keys; k; k = k->next) { + char *key = k->data; + if(_alpm_key_in_keychain(handle, key) == 0) { + if(_alpm_key_import(handle, key) == -1) { + fail = 1; + } + } + } + FREELIST(keys); + } + + free(sig); + + if(fail) { + _alpm_log(handle, ALPM_LOG_ERROR, _("required key missing from keyring\n")); + return -1; + } + } + } + free(sigpath); + if(_alpm_pkg_validate_internal(handle, filename, NULL, level, NULL, &validation) == -1) { /* pm_errno is set by pkg_validate */ diff --git a/lib/libalpm/signing.c b/lib/libalpm/signing.c index 703e3ea..4f03853 100644 --- a/lib/libalpm/signing.c +++ b/lib/libalpm/signing.c @@ -192,6 +192,11 @@ int _alpm_key_in_keychain(alpm_handle_t *handle, const char *fpr) gpgme_key_t key; int ret = -1;
+ if(init_gpgme(handle)) { + /* pm_errno was set in gpgme_init() */ + goto error; + } + memset(&ctx, 0, sizeof(ctx)); err = gpgme_new(&ctx); CHECK_ERR(); -- 1.8.0
On 03/11/12 01:48, Dave Reisner wrote:
On Sat, Nov 03, 2012 at 01:28:22AM +1000, Allan McRae wrote:
When installing a package with "pacman -U" that has a detached signature, check if the needed key is in the keyring and download if necessary.
Signed-off-by: Allan McRae <allan@archlinux.org> ---
What ever happened to our musings about adding another SigLevel-ish option for handling this sort of thing? Are we satisfied with just letting the global value dictate this behavior?
For reference: https://mailman.archlinux.org/pipermail/pacman-dev/2012-February/015155.html
participants (3)
-
Allan McRae
-
Dan McGee
-
Dave Reisner