[pacman-dev] [PATCH 0/5] Manage signing keys using a WKD (FS#63171)

newer
[pacman-dev] [GIT] The official...

older
[pacman-dev] [GIT] The official...

Jonas Witschel

17 Jul 2019 17 Jul '19

8:06 p.m.

Hi, as explained in https://bugs.archlinux.org/task/63171, it would be nice if pacman could receive new PGP keys and refresh expired ones using a Web Key Directory (WKD) instead of using keyservers. This patch series adds the corresponding functionality to pacman and pacman-key. Since WKD is not enabled on archlinux.org yet, it might be a little early to merge this as I have only been able to test it locally on my machine. However since this is my first potential pacman contribution, I wanted to get it out early to gather some feedback on the code. Unfortunately GnuPG/GPGME handles WKD keys quite differently from keyserver keys, so you have to jump through some hoops to enable the usual cofirm-then-import workflow for keys. WKD support for pacman is complete, so it would be possible to remove the keyserver code and rely solely on WKD if the need should arise (though this would require changes to the Arch Linux packaging policy, see the bug report). pacman-key on the other hand is lacking a convenient way of refreshing all keys using WKD, currently every key needs to be refreshed manually by e-mail address using "pacman-key -r". This is because "gpg --refresh-keys" is hardwired to use keyservers. Hopefully the situation will change in future versions of GnuPG, see the corresponding commit message for reference, if not we need to work around this by listing every key in the keyring by email and doing a refresh using WKD. Cheers, Jonas Jonas Witschel (5): common: move rmrf to util-common signing: add ability to import keys using a WKD sync: lookup missing keys in the WKD using the packager email be_package: lookup missing keys in the WKD using the packager email pacman-key: receive keys from WKD with -r/--recv-keys lib/libalpm/alpm.h | 1 + lib/libalpm/be_package.c | 12 ++- lib/libalpm/signing.c | 175 ++++++++++++++++++++++++++++----------- lib/libalpm/signing.h | 2 +- lib/libalpm/sync.c | 9 +- lib/libalpm/util.c | 23 +++++ lib/libalpm/util.h | 1 + scripts/pacman-key.sh.in | 19 +++-- src/common/util-common.c | 42 ++++++++++ src/common/util-common.h | 2 + src/pacman/util.c | 40 --------- src/pacman/util.h | 1 - 12 files changed, 226 insertions(+), 101 deletions(-) -- 2.22.0

Show replies by date

Jonas Witschel

17 Jul 17 Jul

8:06 p.m.

New subject: [pacman-dev] [PATCH 1/5] common: move rmrf to util-common

We will need to remove temporary directories in libalpm, therefore move this helper function to the common utilities. Signed-off-by: Jonas Witschel --- src/common/util-common.c | 42 ++++++++++++++++++++++++++++++++++++++++ src/common/util-common.h | 2 ++ src/pacman/util.c | 40 -------------------------------------- src/pacman/util.h | 1 - 4 files changed, 44 insertions(+), 41 deletions(-) diff --git a/src/common/util-common.c b/src/common/util-common.c index 3aa0eac9..098ea890 100644 --- a/src/common/util-common.c +++ b/src/common/util-common.c @@ -21,6 +21,8 @@ #include #include #include +#include +#include #include "util-common.h" @@ -197,3 +199,43 @@ char *strndup(const char *s, size_t n) return (char *)memcpy(new, s, len); } #endif + +/* does the same thing as 'rm -rf' */ +int rmrf(const char *path) +{ + int errflag = 0; + struct dirent *dp; + DIR *dirp; + + if(!unlink(path)) { + return 0; + } else { + switch(errno) { + case ENOENT: + return 0; + case EPERM: + case EISDIR: + break; + default: + /* not a directory */ + return 1; + } + + dirp = opendir(path); + if(!dirp) { + return 1; + } + for(dp = readdir(dirp); dp != NULL; dp = readdir(dirp)) { + if(strcmp(dp->d_name, "..") != 0 && strcmp(dp->d_name, ".") != 0) { + char name[PATH_MAX]; + snprintf(name, PATH_MAX, "%s/%s", path, dp->d_name); + errflag += rmrf(name); + } + } + closedir(dirp); + if(rmdir(path)) { + errflag++; + } + return errflag; + } +} diff --git a/src/common/util-common.h b/src/common/util-common.h index 3434e1eb..6ab1150a 100644 --- a/src/common/util-common.h +++ b/src/common/util-common.h @@ -36,6 +36,8 @@ size_t strtrim(char *str); char *strndup(const char *s, size_t n); #endif +int rmrf(const char *path); + #define ARRAYSIZE(a) (sizeof (a) / sizeof (a[0])) #endif /* PM_UTIL_COMMON_H */ diff --git a/src/pacman/util.c b/src/pacman/util.c index 8f6290db..14fce9e6 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -247,46 +247,6 @@ unsigned short getcols(void) return c; } -/* does the same thing as 'rm -rf' */ -int rmrf(const char *path) -{ - int errflag = 0; - struct dirent *dp; - DIR *dirp; - - if(!unlink(path)) { - return 0; - } else { - switch(errno) { - case ENOENT: - return 0; - case EPERM: - case EISDIR: - break; - default: - /* not a directory */ - return 1; - } - - dirp = opendir(path); - if(!dirp) { - return 1; - } - for(dp = readdir(dirp); dp != NULL; dp = readdir(dirp)) { - if(strcmp(dp->d_name, "..") != 0 && strcmp(dp->d_name, ".") != 0) { - char name[PATH_MAX]; - snprintf(name, PATH_MAX, "%s/%s", path, dp->d_name); - errflag += rmrf(name); - } - } - closedir(dirp); - if(rmdir(path)) { - errflag++; - } - return errflag; - } -} - /* output a string, but wrap words properly with a specified indentation */ void indentprint(const char *str, unsigned short indent, unsigned short cols) diff --git a/src/pacman/util.h b/src/pacman/util.h index a1fbef46..1dd36882 100644 --- a/src/pacman/util.h +++ b/src/pacman/util.h @@ -53,7 +53,6 @@ int check_syncdbs(size_t need_repos, int check_valid); int sync_syncdbs(int level, alpm_list_t *syncs); unsigned short getcols(void); void columns_cache_reset(void); -int rmrf(const char *path); void indentprint(const char *str, unsigned short indent, unsigned short cols); char *strreplace(const char *str, const char *needle, const char *replace); void string_display(const char *title, const char *string, unsigned short cols); -- 2.22.0

Jonas Witschel

8:06 p.m.

New subject: [pacman-dev] [PATCH 2/5] signing: add ability to import keys using a WKD

Currently pacman relies on the SKS keyserver network to fetch unknown PGP keys. These keyservers are vulnerable to signature spamming attacks, potentionally making it impossible to import the required keys. An alternative to keyservers is a so-called Web Key Directory (WKD), a well-known, trusted location on a server from where the keys can be fetched. This commit adds the ability to retrieve keys from the WKD. Due to the mentioned vulnerabilities, the WKD is tried first, falling back to the keyservers only if no appropriate key is found there. In contrast to keyservers, keys on the WKD are not looked up using their fingerprint, but by email address. Since the email address of the signing key is usually not included in the signature, we will use the email packager email address. In contrast to the direct fingerprint lookup, this might not work or result in a different key, so we check the fingerprint of the received key and fall back to a keyserver if necessary. The extraction of the email address is not performed here, this will be included in the subsequent commits. GnuPG/GPGME handles keys from a WKD very differently than keys from a keyserver: gpgme_get_key directly imports the key into the keyring if it is available without giving the user the choice to confirm it first. On the other hand gpgme_op_import_keys does not import keys from a WKD because it is hard-coded to use keyservers (gpg --recv-keys). Therefore we set a temporary keyring for the initial lookup and use gpgme_get_key after the user has confirmed he wants to import the key. This makes it necessary to add a new member "location" to _alpm_pgpkey_t in order to distinguish the two methods to retrieve the key. Also see FS#63171. Signed-off-by: Jonas Witschel --- lib/libalpm/alpm.h | 1 + lib/libalpm/be_package.c | 2 +- lib/libalpm/signing.c | 175 ++++++++++++++++++++++++++++----------- lib/libalpm/signing.h | 2 +- lib/libalpm/sync.c | 2 +- 5 files changed, 130 insertions(+), 52 deletions(-) diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index ffb2ad96..e010676d 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -300,6 +300,7 @@ typedef struct _alpm_pgpkey_t { unsigned int length; unsigned int revoked; char pubkey_algo; + char location; } alpm_pgpkey_t; /** diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c index ac911bdb..fbb0d43e 100644 --- a/lib/libalpm/be_package.c +++ b/lib/libalpm/be_package.c @@ -755,7 +755,7 @@ int SYMEXPORT alpm_pkg_load(alpm_handle_t *handle, const char *filename, int ful 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) { + if(_alpm_key_import(handle, NULL, key) == -1) { fail = 1; } } diff --git a/lib/libalpm/signing.c b/lib/libalpm/signing.c index 92598b0e..4d246bf8 100644 --- a/lib/libalpm/signing.c +++ b/lib/libalpm/signing.c @@ -259,14 +259,13 @@ error: /** * Search for a GPG key in a remote location. - * This requires GPGME to call the gpg binary and have a keyserver previously - * defined in a gpg.conf configuration file. + * This requires GPGME to call the gpg binary. * @param handle the context handle * @param fpr the fingerprint key ID to look up * @param pgpkey storage location for the given key if found * @return 1 on success, 0 on key not found, -1 on error */ -static int key_search(alpm_handle_t *handle, const char *fpr, +static int key_search(alpm_handle_t *handle, const char *email, const char *fpr, alpm_pgpkey_t *pgpkey) { gpgme_error_t gpg_err; @@ -276,6 +275,9 @@ static int key_search(alpm_handle_t *handle, const char *fpr, int ret = -1; size_t fpr_len; char *full_fpr; + const char *tmpdir; + char *tmp_gnupgdir; + size_t tmp_gnupgdir_len; /* gpg2 goes full retard here. For key searches ONLY, we need to prefix the * key fingerprint with 0x, or the lookup will fail. */ @@ -287,36 +289,81 @@ static int key_search(alpm_handle_t *handle, const char *fpr, gpg_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; - gpg_err = gpgme_set_keylist_mode(ctx, mode); - CHECK_ERR(); - - _alpm_log(handle, ALPM_LOG_DEBUG, "looking up key %s remotely\n", fpr); - - gpg_err = gpgme_get_key(ctx, full_fpr, &key, 0); - if(gpg_err_code(gpg_err) == GPG_ERR_EOF) { - _alpm_log(handle, ALPM_LOG_DEBUG, "key lookup failed, unknown key\n"); - /* Try an alternate lookup using the 8 character fingerprint value, since - * busted-ass keyservers can't support lookups using subkeys with the full - * value as of now. This is why 2012 is not the year of PGP encryption. */ - if(fpr_len > 8) { - const char *short_fpr = memcpy(&full_fpr[fpr_len - 8], "0x", 2); - _alpm_log(handle, ALPM_LOG_DEBUG, - "looking up key %s remotely\n", short_fpr); - gpg_err = gpgme_get_key(ctx, short_fpr, &key, 0); - if(gpg_err_code(gpg_err) == GPG_ERR_EOF) { - _alpm_log(handle, ALPM_LOG_DEBUG, "key lookup failed, unknown key\n"); - ret = 0; - } + /* Using GPGME_KEYLIST_MODE_LOCATE to lookup a key using WKD directly + * imports it into the keyring, so we use a temporary GnuPG home directory + * to retrieve the key. */ + tmpdir = getenv("TMPDIR"); + if(!tmpdir) { + tmpdir = "/tmp"; + } + tmp_gnupgdir_len = strlen(tmpdir) + strlen("/alpm_gnupg_XXXXXX") + 1; + MALLOC(tmp_gnupgdir, tmp_gnupgdir_len, RET_ERR(handle, ALPM_ERR_MEMORY, -1)); + snprintf(tmp_gnupgdir, tmp_gnupgdir_len, "%s/alpm_gnupg_XXXXXX", tmpdir); + if(!mkdtemp(tmp_gnupgdir)) { + _alpm_log(handle, ALPM_LOG_DEBUG, "could not create temp directory\n"); + } else { + gpg_err = gpgme_ctx_set_engine_info(ctx, GPGME_PROTOCOL_OpenPGP, NULL, tmp_gnupgdir); + CHECK_ERR(); + + mode = gpgme_get_keylist_mode(ctx); + mode |= GPGME_KEYLIST_MODE_LOCATE; + gpg_err = gpgme_set_keylist_mode(ctx, mode); + CHECK_ERR(); + _alpm_log(handle, ALPM_LOG_DEBUG, "looking up key for %s using WKD\n", email); + gpg_err = gpgme_get_key(ctx, email, &key, 0); + + if((gpg_err_code(gpg_err) == GPG_ERR_NO_ERROR) && (strcmp(key->subkeys->fpr, fpr) == 0)) { + ret = 1; + pgpkey->location = 'w'; } else { + _alpm_log(handle, ALPM_LOG_DEBUG, "key lookup failed, unknown key\n"); ret = 0; } + rmrf(tmp_gnupgdir); + free(tmp_gnupgdir); } - CHECK_ERR(); + if(ret != 1) { + /* For keyserver lookup, we want to use the options specified in pacman's + * gpg.conf, so we destroy the context with the temporary home directory + * and recreate one with the default one set in init_gpgme. */ + gpgme_release(ctx); + memset(&ctx, 0, sizeof(ctx)); + gpg_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; + gpg_err = gpgme_set_keylist_mode(ctx, mode); + CHECK_ERR(); + + _alpm_log(handle, ALPM_LOG_DEBUG, "looking up key %s on keyserver\n", fpr); + + gpg_err = gpgme_get_key(ctx, full_fpr, &key, 0); + if(gpg_err_code(gpg_err) == GPG_ERR_EOF) { + _alpm_log(handle, ALPM_LOG_DEBUG, "key lookup failed, unknown key\n"); + /* Try an alternate lookup using the 8 character fingerprint value, since + * busted-ass keyservers can't support lookups using subkeys with the full + * value as of now. This is why 2012 is not the year of PGP encryption. */ + if(fpr_len > 8) { + const char *short_fpr = memcpy(&full_fpr[fpr_len - 8], "0x", 2); + _alpm_log(handle, ALPM_LOG_DEBUG, + "looking up key %s on keyserver\n", short_fpr); + gpg_err = gpgme_get_key(ctx, short_fpr, &key, 0); + if(gpg_err_code(gpg_err) == GPG_ERR_EOF) { + _alpm_log(handle, ALPM_LOG_DEBUG, "key lookup failed, unknown key\n"); + ret = 0; + } + } else { + ret = 0; + } + } + + CHECK_ERR(); + pgpkey->location = 'k'; + } /* should only get here if key actually exists */ pgpkey->data = key; @@ -327,7 +374,15 @@ static int key_search(alpm_handle_t *handle, const char *fpr, } pgpkey->uid = key->uids->uid; pgpkey->name = key->uids->name; - pgpkey->email = key->uids->email; + if(pgpkey->location == 'w') { + /* We have found the key in a WKD using the provided email address. To + * make sure we can import it into the keyring later, record the email + * address in the key. Otherwise the email address of a different, + * non-WKD-enabled user ID might be used, leading to an import failure. */ + pgpkey->email = strdup(email); + } else { + pgpkey->email = key->uids->email; + } pgpkey->created = key->subkeys->timestamp; pgpkey->expires = key->subkeys->expires; pgpkey->length = key->subkeys->length; @@ -396,6 +451,8 @@ static int key_import(alpm_handle_t *handle, alpm_pgpkey_t *key) gpgme_key_t keys[2]; gpgme_import_result_t result; int ret = -1; + gpgme_keylist_mode_t mode; + gpgme_key_t UNUSED gpgme_key; if(_alpm_access(handle, handle->gpgdir, "pubring.gpg", W_OK)) { /* no chance of import succeeding if pubring isn't writable */ @@ -409,20 +466,37 @@ static int key_import(alpm_handle_t *handle, alpm_pgpkey_t *key) _alpm_log(handle, ALPM_LOG_DEBUG, "importing key\n"); - keys[0] = key->data; - keys[1] = NULL; - gpg_err = gpgme_op_import_keys(ctx, keys); - CHECK_ERR(); - result = gpgme_op_import_result(ctx); - /* we know we tried to import exactly one key, so check for this */ - if(result->considered != 1 || !result->imports) { - _alpm_log(handle, ALPM_LOG_DEBUG, "could not import key, 0 results\n"); - ret = -1; - } else if(result->imports->result != GPG_ERR_NO_ERROR) { - _alpm_log(handle, ALPM_LOG_DEBUG, "gpg error: %s\n", gpgme_strerror(gpg_err)); - ret = -1; - } else { - ret = 0; + switch(key->location) { + case 'w': + mode = gpgme_get_keylist_mode(ctx); + mode |= GPGME_KEYLIST_MODE_LOCATE; + gpg_err = gpgme_set_keylist_mode(ctx, mode); + CHECK_ERR(); + gpg_err = gpgme_get_key(ctx, key->email, &gpgme_key, 0); + free(gpgme_key); + CHECK_ERR(); + ret = 0; + break; + case 'k': + keys[0] = key->data; + keys[1] = NULL; + gpg_err = gpgme_op_import_keys(ctx, keys); + CHECK_ERR(); + result = gpgme_op_import_result(ctx); + /* we know we tried to import exactly one key, so check for this */ + if(result->considered != 1 || !result->imports) { + _alpm_log(handle, ALPM_LOG_DEBUG, "could not import key, 0 results\n"); + ret = -1; + } else if(result->imports->result != GPG_ERR_NO_ERROR) { + _alpm_log(handle, ALPM_LOG_DEBUG, "gpg error: %s\n", gpgme_strerror(gpg_err)); + ret = -1; + } else { + ret = 0; + } + break; + default: + _alpm_log(handle, ALPM_LOG_DEBUG, "unknown key location '%c'\n", key->location); + ret = -1; } gpg_error: @@ -436,15 +510,15 @@ gpg_error: * @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 _alpm_key_import(alpm_handle_t *handle, const char *email, const char *fpr) { int ret = -1; alpm_pgpkey_t fetch_key; memset(&fetch_key, 0, sizeof(fetch_key)); - if(key_search(handle, fpr, &fetch_key) == 1) { + if(key_search(handle, email, fpr, &fetch_key) == 1) { _alpm_log(handle, ALPM_LOG_DEBUG, - "unknown key, found %s on keyserver\n", fetch_key.uid); + "unknown key, found %s remotely\n", fetch_key.uid); if(!_alpm_access(handle, handle->gpgdir, "pubring.gpg", W_OK)) { alpm_question_import_key_t question = { .type = ALPM_QUESTION_IMPORT_KEY, @@ -463,7 +537,7 @@ int _alpm_key_import(alpm_handle_t *handle, const char *fpr) } 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"), + _("key %s, \"%s\" found remotely, keyring is not writable\n"), fetch_key.fingerprint, fetch_key.uid); } } else { @@ -714,7 +788,8 @@ 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) +int _alpm_key_import(alpm_handle_t UNUSED *handle, const char UNUSED *email, + const char UNUSED *fpr) { return -1; } @@ -900,7 +975,9 @@ 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); - if(_alpm_key_import(handle, result->key.fingerprint) == 0) { + /* No information on the User ID is available, so we cannot search by + * email address. */ + if(_alpm_key_import(handle, NULL, result->key.fingerprint) == 0) { retry = 1; } diff --git a/lib/libalpm/signing.h b/lib/libalpm/signing.h index f8b84b94..d67709da 100644 --- a/lib/libalpm/signing.h +++ b/lib/libalpm/signing.h @@ -32,6 +32,6 @@ 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); +int _alpm_key_import(alpm_handle_t *handle, const char *email, const char *fpr); #endif /* ALPM_SIGNING_H */ diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index cbd072e6..efad77ba 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -927,7 +927,7 @@ static int check_keyring(alpm_handle_t *handle) alpm_list_t *k; for(k = errors; k; k = k->next) { char *key = k->data; - if(_alpm_key_import(handle, key) == -1) { + if(_alpm_key_import(handle, NULL, key) == -1) { fail = 1; } } -- 2.22.0

Jonas Witschel

11:50 p.m.

New subject: [pacman-dev] [PATCH 2/5] signing: add ability to import keys using a WKD

On 2019-07-17 22:06, Jonas Witschel wrote:

...

+ if((gpg_err_code(gpg_err) == GPG_ERR_NO_ERROR) && (strcmp(key->subkeys->fpr, fpr) == 0)) {

Sorry, I just realised this will never be true because "fpr" comes from "parse_subpacket" and is always the 8 byte key ID, while "key->subkeys->fpr" is the full 20 byte fingerprint. We need to compare with "fpr" with "key->subkeys->keyid" instead, see attached for the corrected patch that also adds some more debugging output in case of a key mismatch.

Jonas Witschel

11:50 p.m.

New subject: [pacman-dev] [PATCH v2] signing: add ability to import keys using a WKD

Currently pacman relies on the SKS keyserver network to fetch unknown PGP keys. These keyservers are vulnerable to signature spamming attacks, potentionally making it impossible to import the required keys. An alternative to keyservers is a so-called Web Key Directory (WKD), a well-known, trusted location on a server from where the keys can be fetched. This commit adds the ability to retrieve keys from the WKD. Due to the mentioned vulnerabilities, the WKD is tried first, falling back to the keyservers only if no appropriate key is found there. In contrast to keyservers, keys on the WKD are not looked up using their fingerprint, but by email address. Since the email address of the signing key is usually not included in the signature, we will use the email packager email address. In contrast to the direct fingerprint lookup, this might not work or result in a different key, so we check the fingerprint of the received key and fall back to a keyserver if necessary. The extraction of the email address is not performed here, this will be included in the subsequent commits. GnuPG/GPGME handles keys from a WKD very differently than keys from a keyserver: gpgme_get_key directly imports the key into the keyring if it is available without giving the user the choice to confirm it first. On the other hand gpgme_op_import_keys does not import keys from a WKD because it is hard-coded to use keyservers (gpg --recv-keys). Therefore we set a temporary keyring for the initial lookup and use gpgme_get_key after the user has confirmed he wants to import the key. This makes it necessary to add a new member "location" to _alpm_pgpkey_t in order to distinguish the two methods to retrieve the key. Also see FS#63171. Signed-off-by: Jonas Witschel --- lib/libalpm/alpm.h | 1 + lib/libalpm/be_package.c | 2 +- lib/libalpm/signing.c | 176 ++++++++++++++++++++++++++++----------- lib/libalpm/signing.h | 2 +- lib/libalpm/sync.c | 2 +- 5 files changed, 133 insertions(+), 50 deletions(-) diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index ffb2ad96..e010676d 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -300,6 +300,7 @@ typedef struct _alpm_pgpkey_t { unsigned int length; unsigned int revoked; char pubkey_algo; + char location; } alpm_pgpkey_t; /** diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c index ac911bdb..fbb0d43e 100644 --- a/lib/libalpm/be_package.c +++ b/lib/libalpm/be_package.c @@ -755,7 +755,7 @@ int SYMEXPORT alpm_pkg_load(alpm_handle_t *handle, const char *filename, int ful 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) { + if(_alpm_key_import(handle, NULL, key) == -1) { fail = 1; } } diff --git a/lib/libalpm/signing.c b/lib/libalpm/signing.c index 92598b0e..b38583ec 100644 --- a/lib/libalpm/signing.c +++ b/lib/libalpm/signing.c @@ -259,14 +259,13 @@ error: /** * Search for a GPG key in a remote location. - * This requires GPGME to call the gpg binary and have a keyserver previously - * defined in a gpg.conf configuration file. + * This requires GPGME to call the gpg binary. * @param handle the context handle * @param fpr the fingerprint key ID to look up * @param pgpkey storage location for the given key if found * @return 1 on success, 0 on key not found, -1 on error */ -static int key_search(alpm_handle_t *handle, const char *fpr, +static int key_search(alpm_handle_t *handle, const char *email, const char *fpr, alpm_pgpkey_t *pgpkey) { gpgme_error_t gpg_err; @@ -276,6 +275,9 @@ static int key_search(alpm_handle_t *handle, const char *fpr, int ret = -1; size_t fpr_len; char *full_fpr; + const char *tmpdir; + char *tmp_gnupgdir; + size_t tmp_gnupgdir_len; /* gpg2 goes full retard here. For key searches ONLY, we need to prefix the * key fingerprint with 0x, or the lookup will fail. */ @@ -287,36 +289,86 @@ static int key_search(alpm_handle_t *handle, const char *fpr, gpg_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; - gpg_err = gpgme_set_keylist_mode(ctx, mode); - CHECK_ERR(); - - _alpm_log(handle, ALPM_LOG_DEBUG, "looking up key %s remotely\n", fpr); - - gpg_err = gpgme_get_key(ctx, full_fpr, &key, 0); - if(gpg_err_code(gpg_err) == GPG_ERR_EOF) { - _alpm_log(handle, ALPM_LOG_DEBUG, "key lookup failed, unknown key\n"); - /* Try an alternate lookup using the 8 character fingerprint value, since - * busted-ass keyservers can't support lookups using subkeys with the full - * value as of now. This is why 2012 is not the year of PGP encryption. */ - if(fpr_len > 8) { - const char *short_fpr = memcpy(&full_fpr[fpr_len - 8], "0x", 2); - _alpm_log(handle, ALPM_LOG_DEBUG, - "looking up key %s remotely\n", short_fpr); - gpg_err = gpgme_get_key(ctx, short_fpr, &key, 0); - if(gpg_err_code(gpg_err) == GPG_ERR_EOF) { - _alpm_log(handle, ALPM_LOG_DEBUG, "key lookup failed, unknown key\n"); + /* Using GPGME_KEYLIST_MODE_LOCATE to lookup a key using WKD directly + * imports it into the keyring, so we use a temporary GnuPG home directory + * to retrieve the key. */ + tmpdir = getenv("TMPDIR"); + if(!tmpdir) { + tmpdir = "/tmp"; + } + tmp_gnupgdir_len = strlen(tmpdir) + strlen("/alpm_gnupg_XXXXXX") + 1; + MALLOC(tmp_gnupgdir, tmp_gnupgdir_len, RET_ERR(handle, ALPM_ERR_MEMORY, -1)); + snprintf(tmp_gnupgdir, tmp_gnupgdir_len, "%s/alpm_gnupg_XXXXXX", tmpdir); + if(!mkdtemp(tmp_gnupgdir)) { + _alpm_log(handle, ALPM_LOG_DEBUG, "could not create temp directory\n"); + } else { + gpg_err = gpgme_ctx_set_engine_info(ctx, GPGME_PROTOCOL_OpenPGP, NULL, tmp_gnupgdir); + CHECK_ERR(); + + mode = gpgme_get_keylist_mode(ctx); + mode |= GPGME_KEYLIST_MODE_LOCATE; + gpg_err = gpgme_set_keylist_mode(ctx, mode); + CHECK_ERR(); + _alpm_log(handle, ALPM_LOG_DEBUG, "looking up key for %s using WKD\n", email); + gpg_err = gpgme_get_key(ctx, email, &key, 0); + + if(gpg_err_code(gpg_err) == GPG_ERR_NO_ERROR) { + if((key->subkeys->keyid) && (strcmp(key->subkeys->keyid, fpr) != 0)) { + _alpm_log(handle, ALPM_LOG_DEBUG, "key ID mismatch, got %s\n", key->subkeys->keyid); ret = 0; + } else { + ret = 1; + pgpkey->location = 'w'; } } else { + _alpm_log(handle, ALPM_LOG_DEBUG, "key lookup failed, unknown key\n"); ret = 0; } + rmrf(tmp_gnupgdir); + free(tmp_gnupgdir); } - CHECK_ERR(); + if(ret != 1) { + /* For keyserver lookup, we want to use the options specified in pacman's + * gpg.conf, so we destroy the context with the temporary home directory + * and recreate one with the default one set in init_gpgme. */ + gpgme_release(ctx); + memset(&ctx, 0, sizeof(ctx)); + gpg_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; + gpg_err = gpgme_set_keylist_mode(ctx, mode); + CHECK_ERR(); + + _alpm_log(handle, ALPM_LOG_DEBUG, "looking up key %s on keyserver\n", fpr); + + gpg_err = gpgme_get_key(ctx, full_fpr, &key, 0); + if(gpg_err_code(gpg_err) == GPG_ERR_EOF) { + _alpm_log(handle, ALPM_LOG_DEBUG, "key lookup failed, unknown key\n"); + /* Try an alternate lookup using the 8 character fingerprint value, since + * busted-ass keyservers can't support lookups using subkeys with the full + * value as of now. This is why 2012 is not the year of PGP encryption. */ + if(fpr_len > 8) { + const char *short_fpr = memcpy(&full_fpr[fpr_len - 8], "0x", 2); + _alpm_log(handle, ALPM_LOG_DEBUG, + "looking up key %s on keyserver\n", short_fpr); + gpg_err = gpgme_get_key(ctx, short_fpr, &key, 0); + if(gpg_err_code(gpg_err) == GPG_ERR_EOF) { + _alpm_log(handle, ALPM_LOG_DEBUG, "key lookup failed, unknown key\n"); + ret = 0; + } + } else { + ret = 0; + } + } + + CHECK_ERR(); + pgpkey->location = 'k'; + } /* should only get here if key actually exists */ pgpkey->data = key; @@ -327,7 +379,15 @@ static int key_search(alpm_handle_t *handle, const char *fpr, } pgpkey->uid = key->uids->uid; pgpkey->name = key->uids->name; - pgpkey->email = key->uids->email; + if(pgpkey->location == 'w') { + /* We have found the key in a WKD using the provided email address. To + * make sure we can import it into the keyring later, record the email + * address in the key. Otherwise the email address of a different, + * non-WKD-enabled user ID might be used, leading to an import failure. */ + pgpkey->email = strdup(email); + } else { + pgpkey->email = key->uids->email; + } pgpkey->created = key->subkeys->timestamp; pgpkey->expires = key->subkeys->expires; pgpkey->length = key->subkeys->length; @@ -396,6 +456,8 @@ static int key_import(alpm_handle_t *handle, alpm_pgpkey_t *key) gpgme_key_t keys[2]; gpgme_import_result_t result; int ret = -1; + gpgme_keylist_mode_t mode; + gpgme_key_t UNUSED gpgme_key; if(_alpm_access(handle, handle->gpgdir, "pubring.gpg", W_OK)) { /* no chance of import succeeding if pubring isn't writable */ @@ -409,20 +471,37 @@ static int key_import(alpm_handle_t *handle, alpm_pgpkey_t *key) _alpm_log(handle, ALPM_LOG_DEBUG, "importing key\n"); - keys[0] = key->data; - keys[1] = NULL; - gpg_err = gpgme_op_import_keys(ctx, keys); - CHECK_ERR(); - result = gpgme_op_import_result(ctx); - /* we know we tried to import exactly one key, so check for this */ - if(result->considered != 1 || !result->imports) { - _alpm_log(handle, ALPM_LOG_DEBUG, "could not import key, 0 results\n"); - ret = -1; - } else if(result->imports->result != GPG_ERR_NO_ERROR) { - _alpm_log(handle, ALPM_LOG_DEBUG, "gpg error: %s\n", gpgme_strerror(gpg_err)); - ret = -1; - } else { - ret = 0; + switch(key->location) { + case 'w': + mode = gpgme_get_keylist_mode(ctx); + mode |= GPGME_KEYLIST_MODE_LOCATE; + gpg_err = gpgme_set_keylist_mode(ctx, mode); + CHECK_ERR(); + gpg_err = gpgme_get_key(ctx, key->email, &gpgme_key, 0); + free(gpgme_key); + CHECK_ERR(); + ret = 0; + break; + case 'k': + keys[0] = key->data; + keys[1] = NULL; + gpg_err = gpgme_op_import_keys(ctx, keys); + CHECK_ERR(); + result = gpgme_op_import_result(ctx); + /* we know we tried to import exactly one key, so check for this */ + if(result->considered != 1 || !result->imports) { + _alpm_log(handle, ALPM_LOG_DEBUG, "could not import key, 0 results\n"); + ret = -1; + } else if(result->imports->result != GPG_ERR_NO_ERROR) { + _alpm_log(handle, ALPM_LOG_DEBUG, "gpg error: %s\n", gpgme_strerror(gpg_err)); + ret = -1; + } else { + ret = 0; + } + break; + default: + _alpm_log(handle, ALPM_LOG_DEBUG, "unknown key location '%c'\n", key->location); + ret = -1; } gpg_error: @@ -436,15 +515,15 @@ gpg_error: * @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 _alpm_key_import(alpm_handle_t *handle, const char *email, const char *fpr) { int ret = -1; alpm_pgpkey_t fetch_key; memset(&fetch_key, 0, sizeof(fetch_key)); - if(key_search(handle, fpr, &fetch_key) == 1) { + if(key_search(handle, email, fpr, &fetch_key) == 1) { _alpm_log(handle, ALPM_LOG_DEBUG, - "unknown key, found %s on keyserver\n", fetch_key.uid); + "unknown key, found %s remotely\n", fetch_key.uid); if(!_alpm_access(handle, handle->gpgdir, "pubring.gpg", W_OK)) { alpm_question_import_key_t question = { .type = ALPM_QUESTION_IMPORT_KEY, @@ -463,7 +542,7 @@ int _alpm_key_import(alpm_handle_t *handle, const char *fpr) } 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"), + _("key %s, \"%s\" found remotely, keyring is not writable\n"), fetch_key.fingerprint, fetch_key.uid); } } else { @@ -714,7 +793,8 @@ 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) +int _alpm_key_import(alpm_handle_t UNUSED *handle, const char UNUSED *email, + const char UNUSED *fpr) { return -1; } @@ -900,7 +980,9 @@ 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); - if(_alpm_key_import(handle, result->key.fingerprint) == 0) { + /* No information on the User ID is available, so we cannot search by + * email address. */ + if(_alpm_key_import(handle, NULL, result->key.fingerprint) == 0) { retry = 1; } diff --git a/lib/libalpm/signing.h b/lib/libalpm/signing.h index f8b84b94..d67709da 100644 --- a/lib/libalpm/signing.h +++ b/lib/libalpm/signing.h @@ -32,6 +32,6 @@ 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); +int _alpm_key_import(alpm_handle_t *handle, const char *email, const char *fpr); #endif /* ALPM_SIGNING_H */ diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index cbd072e6..efad77ba 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -927,7 +927,7 @@ static int check_keyring(alpm_handle_t *handle) alpm_list_t *k; for(k = errors; k; k = k->next) { char *key = k->data; - if(_alpm_key_import(handle, key) == -1) { + if(_alpm_key_import(handle, NULL, key) == -1) { fail = 1; } } -- 2.22.0

Jonas Witschel

5 Aug 5 Aug

10:37 a.m.

New subject: [pacman-dev] [PATCH 2/5 v3] signing: add ability to import keys using a WKD

Currently pacman relies on the SKS keyserver network to fetch unknown PGP keys. These keyservers are vulnerable to signature spamming attacks, potentionally making it impossible to import the required keys. An alternative to keyservers is a so-called Web Key Directory (WKD), a well-known, trusted location on a server from where the keys can be fetched. This commit adds the ability to retrieve keys from the WKD. Due to the mentioned vulnerabilities, the WKD is tried first, falling back to the keyservers only if no appropriate key is found there. In contrast to keyservers, keys on the WKD are not looked up using their fingerprint, but by email address. Since the email address of the signing key is usually not included in the signature, we will use the email packager email address. In contrast to the direct fingerprint lookup, this might not work or result in a different key, so we check the fingerprint of the received key and fall back to a keyserver if necessary. The extraction of the email address is not performed here, this will be included in the subsequent commits. GnuPG/GPGME handles keys from a WKD very differently than keys from a keyserver: gpgme_get_key directly imports the key into the keyring if it is available without giving the user the choice to confirm it first. On the other hand gpgme_op_import_keys does not import keys from a WKD because it is hard-coded to use keyservers (gpg --recv-keys). Therefore we set a temporary keyring for the initial lookup and use gpgme_get_key after the user has confirmed he wants to import the key. This makes it necessary to add a new member "location" to _alpm_pgpkey_t in order to distinguish the two methods to retrieve the key. Also see FS#63171. Signed-off-by: Jonas Witschel --- lib/libalpm/alpm.h | 1 + lib/libalpm/be_package.c | 2 +- lib/libalpm/signing.c | 184 +++++++++++++++++++++++++++++---------- lib/libalpm/signing.h | 2 +- lib/libalpm/sync.c | 2 +- 5 files changed, 140 insertions(+), 51 deletions(-) diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index ffb2ad96..e010676d 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -300,6 +300,7 @@ typedef struct _alpm_pgpkey_t { unsigned int length; unsigned int revoked; char pubkey_algo; + char location; } alpm_pgpkey_t; /** diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c index ac911bdb..fbb0d43e 100644 --- a/lib/libalpm/be_package.c +++ b/lib/libalpm/be_package.c @@ -755,7 +755,7 @@ int SYMEXPORT alpm_pkg_load(alpm_handle_t *handle, const char *filename, int ful 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) { + if(_alpm_key_import(handle, NULL, key) == -1) { fail = 1; } } diff --git a/lib/libalpm/signing.c b/lib/libalpm/signing.c index 92598b0e..9dcd7cb8 100644 --- a/lib/libalpm/signing.c +++ b/lib/libalpm/signing.c @@ -259,14 +259,13 @@ error: /** * Search for a GPG key in a remote location. - * This requires GPGME to call the gpg binary and have a keyserver previously - * defined in a gpg.conf configuration file. + * This requires GPGME to call the gpg binary. * @param handle the context handle * @param fpr the fingerprint key ID to look up * @param pgpkey storage location for the given key if found * @return 1 on success, 0 on key not found, -1 on error */ -static int key_search(alpm_handle_t *handle, const char *fpr, +static int key_search(alpm_handle_t *handle, const char *email, const char *fpr, alpm_pgpkey_t *pgpkey) { gpgme_error_t gpg_err; @@ -276,6 +275,9 @@ static int key_search(alpm_handle_t *handle, const char *fpr, int ret = -1; size_t fpr_len; char *full_fpr; + const char *tmpdir; + char *tmp_gnupgdir; + size_t tmp_gnupgdir_len; /* gpg2 goes full retard here. For key searches ONLY, we need to prefix the * key fingerprint with 0x, or the lookup will fail. */ @@ -287,36 +289,92 @@ static int key_search(alpm_handle_t *handle, const char *fpr, gpg_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; - gpg_err = gpgme_set_keylist_mode(ctx, mode); - CHECK_ERR(); - - _alpm_log(handle, ALPM_LOG_DEBUG, "looking up key %s remotely\n", fpr); - - gpg_err = gpgme_get_key(ctx, full_fpr, &key, 0); - if(gpg_err_code(gpg_err) == GPG_ERR_EOF) { - _alpm_log(handle, ALPM_LOG_DEBUG, "key lookup failed, unknown key\n"); - /* Try an alternate lookup using the 8 character fingerprint value, since - * busted-ass keyservers can't support lookups using subkeys with the full - * value as of now. This is why 2012 is not the year of PGP encryption. */ - if(fpr_len > 8) { - const char *short_fpr = memcpy(&full_fpr[fpr_len - 8], "0x", 2); - _alpm_log(handle, ALPM_LOG_DEBUG, - "looking up key %s remotely\n", short_fpr); - gpg_err = gpgme_get_key(ctx, short_fpr, &key, 0); - if(gpg_err_code(gpg_err) == GPG_ERR_EOF) { - _alpm_log(handle, ALPM_LOG_DEBUG, "key lookup failed, unknown key\n"); - ret = 0; + /* Using GPGME_KEYLIST_MODE_LOCATE to lookup a key using WKD directly + * imports it into the keyring, so we use a temporary GnuPG home directory + * to retrieve the key. */ + tmpdir = getenv("TMPDIR"); + if(!tmpdir) { + tmpdir = "/tmp"; + } + tmp_gnupgdir_len = strlen(tmpdir) + strlen("/alpm_gnupg_XXXXXX") + 1; + MALLOC(tmp_gnupgdir, tmp_gnupgdir_len, RET_ERR(handle, ALPM_ERR_MEMORY, -1)); + snprintf(tmp_gnupgdir, tmp_gnupgdir_len, "%s/alpm_gnupg_XXXXXX", tmpdir); + if(!mkdtemp(tmp_gnupgdir)) { + _alpm_log(handle, ALPM_LOG_DEBUG, "could not create temp directory\n"); + } else { + gpg_err = gpgme_ctx_set_engine_info(ctx, GPGME_PROTOCOL_OpenPGP, NULL, tmp_gnupgdir); + CHECK_ERR(); + + mode = gpgme_get_keylist_mode(ctx); + mode |= GPGME_KEYLIST_MODE_LOCATE; + gpg_err = gpgme_set_keylist_mode(ctx, mode); + CHECK_ERR(); + _alpm_log(handle, ALPM_LOG_DEBUG, "looking up key for %s using WKD\n", email); + gpg_err = gpgme_get_key(ctx, email, &key, 0); + + if(gpg_err_code(gpg_err) == GPG_ERR_NO_ERROR) { + gpgme_subkey_t subkey = key->subkeys; + ret = 0; + while(subkey) { + if(strcmp(subkey->keyid, fpr) == 0) { + ret = 1; + pgpkey->location = 'w'; + break; + } + subkey = subkey->next; + } + if(ret != 1) { + _alpm_log(handle, ALPM_LOG_DEBUG, "key ID mismatch on WKD\n"); } } else { + _alpm_log(handle, ALPM_LOG_DEBUG, "key lookup failed, unknown key\n"); ret = 0; } + rmrf(tmp_gnupgdir); + free(tmp_gnupgdir); } - CHECK_ERR(); + if(ret != 1) { + /* For keyserver lookup, we want to use the options specified in pacman's + * gpg.conf, so we destroy the context with the temporary home directory + * and recreate one with the default one set in init_gpgme. */ + gpgme_release(ctx); + memset(&ctx, 0, sizeof(ctx)); + gpg_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; + gpg_err = gpgme_set_keylist_mode(ctx, mode); + CHECK_ERR(); + + _alpm_log(handle, ALPM_LOG_DEBUG, "looking up key %s on keyserver\n", fpr); + + gpg_err = gpgme_get_key(ctx, full_fpr, &key, 0); + if(gpg_err_code(gpg_err) == GPG_ERR_EOF) { + _alpm_log(handle, ALPM_LOG_DEBUG, "key lookup failed, unknown key\n"); + /* Try an alternate lookup using the 8 character fingerprint value, since + * busted-ass keyservers can't support lookups using subkeys with the full + * value as of now. This is why 2012 is not the year of PGP encryption. */ + if(fpr_len > 8) { + const char *short_fpr = memcpy(&full_fpr[fpr_len - 8], "0x", 2); + _alpm_log(handle, ALPM_LOG_DEBUG, + "looking up key %s on keyserver\n", short_fpr); + gpg_err = gpgme_get_key(ctx, short_fpr, &key, 0); + if(gpg_err_code(gpg_err) == GPG_ERR_EOF) { + _alpm_log(handle, ALPM_LOG_DEBUG, "key lookup failed, unknown key\n"); + ret = 0; + } + } else { + ret = 0; + } + } + + CHECK_ERR(); + pgpkey->location = 'k'; + } /* should only get here if key actually exists */ pgpkey->data = key; @@ -327,7 +385,15 @@ static int key_search(alpm_handle_t *handle, const char *fpr, } pgpkey->uid = key->uids->uid; pgpkey->name = key->uids->name; - pgpkey->email = key->uids->email; + if(pgpkey->location == 'w') { + /* We have found the key in a WKD using the provided email address. To + * make sure we can import it into the keyring later, record the email + * address in the key. Otherwise the email address of a different, + * non-WKD-enabled user ID might be used, leading to an import failure. */ + pgpkey->email = strdup(email); + } else { + pgpkey->email = key->uids->email; + } pgpkey->created = key->subkeys->timestamp; pgpkey->expires = key->subkeys->expires; pgpkey->length = key->subkeys->length; @@ -396,6 +462,8 @@ static int key_import(alpm_handle_t *handle, alpm_pgpkey_t *key) gpgme_key_t keys[2]; gpgme_import_result_t result; int ret = -1; + gpgme_keylist_mode_t mode; + gpgme_key_t UNUSED gpgme_key; if(_alpm_access(handle, handle->gpgdir, "pubring.gpg", W_OK)) { /* no chance of import succeeding if pubring isn't writable */ @@ -409,20 +477,37 @@ static int key_import(alpm_handle_t *handle, alpm_pgpkey_t *key) _alpm_log(handle, ALPM_LOG_DEBUG, "importing key\n"); - keys[0] = key->data; - keys[1] = NULL; - gpg_err = gpgme_op_import_keys(ctx, keys); - CHECK_ERR(); - result = gpgme_op_import_result(ctx); - /* we know we tried to import exactly one key, so check for this */ - if(result->considered != 1 || !result->imports) { - _alpm_log(handle, ALPM_LOG_DEBUG, "could not import key, 0 results\n"); - ret = -1; - } else if(result->imports->result != GPG_ERR_NO_ERROR) { - _alpm_log(handle, ALPM_LOG_DEBUG, "gpg error: %s\n", gpgme_strerror(gpg_err)); - ret = -1; - } else { - ret = 0; + switch(key->location) { + case 'w': + mode = gpgme_get_keylist_mode(ctx); + mode |= GPGME_KEYLIST_MODE_LOCATE; + gpg_err = gpgme_set_keylist_mode(ctx, mode); + CHECK_ERR(); + gpg_err = gpgme_get_key(ctx, key->email, &gpgme_key, 0); + free(gpgme_key); + CHECK_ERR(); + ret = 0; + break; + case 'k': + keys[0] = key->data; + keys[1] = NULL; + gpg_err = gpgme_op_import_keys(ctx, keys); + CHECK_ERR(); + result = gpgme_op_import_result(ctx); + /* we know we tried to import exactly one key, so check for this */ + if(result->considered != 1 || !result->imports) { + _alpm_log(handle, ALPM_LOG_DEBUG, "could not import key, 0 results\n"); + ret = -1; + } else if(result->imports->result != GPG_ERR_NO_ERROR) { + _alpm_log(handle, ALPM_LOG_DEBUG, "gpg error: %s\n", gpgme_strerror(gpg_err)); + ret = -1; + } else { + ret = 0; + } + break; + default: + _alpm_log(handle, ALPM_LOG_DEBUG, "unknown key location '%c'\n", key->location); + ret = -1; } gpg_error: @@ -436,15 +521,15 @@ gpg_error: * @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 _alpm_key_import(alpm_handle_t *handle, const char *email, const char *fpr) { int ret = -1; alpm_pgpkey_t fetch_key; memset(&fetch_key, 0, sizeof(fetch_key)); - if(key_search(handle, fpr, &fetch_key) == 1) { + if(key_search(handle, email, fpr, &fetch_key) == 1) { _alpm_log(handle, ALPM_LOG_DEBUG, - "unknown key, found %s on keyserver\n", fetch_key.uid); + "unknown key, found %s remotely\n", fetch_key.uid); if(!_alpm_access(handle, handle->gpgdir, "pubring.gpg", W_OK)) { alpm_question_import_key_t question = { .type = ALPM_QUESTION_IMPORT_KEY, @@ -463,7 +548,7 @@ int _alpm_key_import(alpm_handle_t *handle, const char *fpr) } 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"), + _("key %s, \"%s\" found remotely, keyring is not writable\n"), fetch_key.fingerprint, fetch_key.uid); } } else { @@ -714,7 +799,8 @@ 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) +int _alpm_key_import(alpm_handle_t UNUSED *handle, const char UNUSED *email, + const char UNUSED *fpr) { return -1; } @@ -900,7 +986,9 @@ 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); - if(_alpm_key_import(handle, result->key.fingerprint) == 0) { + /* No information on the User ID is available, so we cannot search by + * email address. */ + if(_alpm_key_import(handle, NULL, result->key.fingerprint) == 0) { retry = 1; } diff --git a/lib/libalpm/signing.h b/lib/libalpm/signing.h index f8b84b94..d67709da 100644 --- a/lib/libalpm/signing.h +++ b/lib/libalpm/signing.h @@ -32,6 +32,6 @@ 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); +int _alpm_key_import(alpm_handle_t *handle, const char *email, const char *fpr); #endif /* ALPM_SIGNING_H */ diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index cbd072e6..efad77ba 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -927,7 +927,7 @@ static int check_keyring(alpm_handle_t *handle) alpm_list_t *k; for(k = errors; k; k = k->next) { char *key = k->data; - if(_alpm_key_import(handle, key) == -1) { + if(_alpm_key_import(handle, NULL, key) == -1) { fail = 1; } } -- 2.22.0

Jonas Witschel

17 Jul 17 Jul

8:06 p.m.

New subject: [pacman-dev] [PATCH 3/5] sync: lookup missing keys in the WKD using the packager email

We assume that the packager is of the form "Example Name " and that the key used to sign the package can be resolved using WKD with this address. This means that the package signing key should have one user ID with the given email address, which does not need to be a valid address, but needs to be published in the WKD. Signed-off-by: Jonas Witschel --- lib/libalpm/sync.c | 9 +++++++-- lib/libalpm/util.c | 23 +++++++++++++++++++++++ lib/libalpm/util.h | 1 + 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index efad77ba..02acdf6d 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -872,6 +872,7 @@ static int check_keyring(alpm_handle_t *handle) size_t current = 0, numtargs; alpm_list_t *i, *errors = NULL; alpm_event_t event; + char *email; event.type = ALPM_EVENT_KEYRING_START; EVENT(handle, &event); @@ -905,6 +906,8 @@ static int check_keyring(alpm_handle_t *handle) char *key = k->data; if(!alpm_list_find_str(errors, key) && _alpm_key_in_keychain(handle, key) == 0) { + _alpm_email_from_uid(pkg->packager, &email); + errors = alpm_list_add(errors, email); errors = alpm_list_add(errors, strdup(key)); } } @@ -926,8 +929,10 @@ static int check_keyring(alpm_handle_t *handle) int fail = 0; alpm_list_t *k; for(k = errors; k; k = k->next) { - char *key = k->data; - if(_alpm_key_import(handle, NULL, key) == -1) { + email = k->data; + k = k->next; + char *fpr = k->data; + if(_alpm_key_import(handle, email, fpr) == -1) { fail = 1; } } diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c index d33eef2a..2089f84d 100644 --- a/lib/libalpm/util.c +++ b/lib/libalpm/util.c @@ -1491,3 +1491,26 @@ void _alpm_alloc_fail(size_t size) { fprintf(stderr, "alloc failure: could not allocate %zu bytes\n", size); } + +/** Extract the email address from a User ID + * @param uid User ID to parse in the form "Example Name " + * @param email to hold email address + * @return 0 on success, -1 on error + */ +int _alpm_email_from_uid(const char *uid, char **email) +{ + char *start, *end; + + start = strrchr(uid, '<'); + if(start) { + end = strrchr(start, '>'); + } + + if(start && end) { + STRNDUP(*email, start+1, end-start-1, return -1); + return 0; + } else { + email = NULL; + return -1; + } +} diff --git a/lib/libalpm/util.h b/lib/libalpm/util.h index 9a3942f1..1190f10f 100644 --- a/lib/libalpm/util.h +++ b/lib/libalpm/util.h @@ -147,6 +147,7 @@ int _alpm_fnmatch_patterns(alpm_list_t *patterns, const char *string); int _alpm_fnmatch(const void *pattern, const void *string); void *_alpm_realloc(void **data, size_t *current, const size_t required); void *_alpm_greedy_grow(void **data, size_t *current, const size_t required); +int _alpm_email_from_uid(const char *uid, char **email); #ifndef HAVE_STRSEP char *strsep(char **, const char *); -- 2.22.0

Allan McRae

5 Aug 5 Aug

11:14 a.m.

New subject: [pacman-dev] [PATCH 3/5] sync: lookup missing keys in the WKD using the packager email

On 18/7/19 6:06 am, Jonas Witschel wrote:

...

We assume that the packager is of the form "Example Name " and that the key used to sign the package can be resolved using WKD with this address. This means that the package signing key should have one user ID with the given email address, which does not need to be a valid address, but needs to be published in the WKD.

Signed-off-by: Jonas Witschel --- lib/libalpm/sync.c | 9 +++++++-- lib/libalpm/util.c | 23 +++++++++++++++++++++++ lib/libalpm/util.h | 1 + 3 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index efad77ba..02acdf6d 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -872,6 +872,7 @@ static int check_keyring(alpm_handle_t *handle) size_t current = 0, numtargs; alpm_list_t *i, *errors = NULL; alpm_event_t event; + char *email;

event.type = ALPM_EVENT_KEYRING_START; EVENT(handle, &event); @@ -905,6 +906,8 @@ static int check_keyring(alpm_handle_t *handle) char *key = k->data; if(!alpm_list_find_str(errors, key) && _alpm_key_in_keychain(handle, key) == 0) { + _alpm_email_from_uid(pkg->packager, &email); + errors = alpm_list_add(errors, email); errors = alpm_list_add(errors, strdup(key));

I don't like this. Storing two strings as adjacent items in the list. I'd prefer a small two item struct. Any other opinions on this? <snip>

...

} diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c index d33eef2a..2089f84d 100644 --- a/lib/libalpm/util.c +++ b/lib/libalpm/util.c @@ -1491,3 +1491,26 @@ void _alpm_alloc_fail(size_t size) { fprintf(stderr, "alloc failure: could not allocate %zu bytes\n", size); } + +/** Extract the email address from a User ID + * @param uid User ID to parse in the form "Example Name " + * @param email to hold email address + * @return 0 on success, -1 on error + */ +int _alpm_email_from_uid(const char *uid, char **email) +{ + char *start, *end; + + start = strrchr(uid, '<');

This makes a strong assumption that "<" is not used within an email address. The use of that character is technically valid, provided it is quoted. I am happy with that assumption, but we need to add a check in libmakpkeg to reject emails containing it. In fact, our PACKAGER variable has no enforced format at all...

...

+ if(start) { + end = strrchr(start, '>'); + } + + if(start && end) { + STRNDUP(*email, start+1, end-start-1, return -1); + return 0; + } else { + email = NULL; + return -1; + } +} diff --git a/lib/libalpm/util.h b/lib/libalpm/util.h index 9a3942f1..1190f10f 100644 --- a/lib/libalpm/util.h +++ b/lib/libalpm/util.h @@ -147,6 +147,7 @@ int _alpm_fnmatch_patterns(alpm_list_t *patterns, const char *string); int _alpm_fnmatch(const void *pattern, const void *string); void *_alpm_realloc(void **data, size_t *current, const size_t required); void *_alpm_greedy_grow(void **data, size_t *current, const size_t required); +int _alpm_email_from_uid(const char *uid, char **email);

Rename to: _alpm_email_from_packager()

...

#ifndef HAVE_STRSEP char *strsep(char **, const char *); -- 2.22.0 .

Jonas Witschel

3:32 p.m.

New subject: [pacman-dev] [PATCH 3/5 v2] sync: lookup missing keys in the WKD using the packager email

We assume that the packager is of the form "Example Name " and that the key used to sign the package can be resolved using WKD with this address. This means that the package signing key should have one user ID with the given email address, which does not need to be a valid address, but needs to be published in the WKD. Signed-off-by: Jonas Witschel --- lib/libalpm/sync.c | 20 +++++++++++++++++--- lib/libalpm/util.c | 23 +++++++++++++++++++++++ lib/libalpm/util.h | 1 + 3 files changed, 41 insertions(+), 3 deletions(-) diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index efad77ba..6e8e23f1 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -47,6 +47,11 @@ #include "diskspace.h" #include "signing.h" +struct keyinfo_t { + char* email; + char* keyid; +}; + /** Check for new version of pkg in sync repos * (only the first occurrence is considered in sync) */ @@ -872,6 +877,7 @@ static int check_keyring(alpm_handle_t *handle) size_t current = 0, numtargs; alpm_list_t *i, *errors = NULL; alpm_event_t event; + struct keyinfo_t *keyinfo; event.type = ALPM_EVENT_KEYRING_START; EVENT(handle, &event); @@ -905,7 +911,13 @@ static int check_keyring(alpm_handle_t *handle) char *key = k->data; if(!alpm_list_find_str(errors, key) && _alpm_key_in_keychain(handle, key) == 0) { - errors = alpm_list_add(errors, strdup(key)); + keyinfo = malloc(sizeof(struct keyinfo_t)); + if(!keyinfo) { + break; + } + _alpm_email_from_packager(pkg->packager, &keyinfo->email); + keyinfo->keyid = strdup(key); + errors = alpm_list_add(errors, keyinfo); } } FREELIST(keys); @@ -926,10 +938,12 @@ static int check_keyring(alpm_handle_t *handle) int fail = 0; alpm_list_t *k; for(k = errors; k; k = k->next) { - char *key = k->data; - if(_alpm_key_import(handle, NULL, key) == -1) { + keyinfo = k->data; + if(_alpm_key_import(handle, keyinfo->email, keyinfo->keyid) == -1) { fail = 1; } + free(keyinfo->email); + free(keyinfo->keyid); } event.type = ALPM_EVENT_KEY_DOWNLOAD_DONE; EVENT(handle, &event); diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c index d33eef2a..5e2e9770 100644 --- a/lib/libalpm/util.c +++ b/lib/libalpm/util.c @@ -1491,3 +1491,26 @@ void _alpm_alloc_fail(size_t size) { fprintf(stderr, "alloc failure: could not allocate %zu bytes\n", size); } + +/** Extract the email address from the packager field + * @param packager the packager to parse in the form "Example Name " + * @param email to hold email address + * @return 0 on success, -1 on error + */ +int _alpm_email_from_packager(const char *packager, char **email) +{ + char *start, *end; + + start = strrchr(packager, '<'); + if(start) { + end = strrchr(start, '>'); + } + + if(start && end) { + STRNDUP(*email, start+1, end-start-1, return -1); + return 0; + } else { + email = NULL; + return -1; + } +} diff --git a/lib/libalpm/util.h b/lib/libalpm/util.h index 9a3942f1..f1ff97be 100644 --- a/lib/libalpm/util.h +++ b/lib/libalpm/util.h @@ -147,6 +147,7 @@ int _alpm_fnmatch_patterns(alpm_list_t *patterns, const char *string); int _alpm_fnmatch(const void *pattern, const void *string); void *_alpm_realloc(void **data, size_t *current, const size_t required); void *_alpm_greedy_grow(void **data, size_t *current, const size_t required); +int _alpm_email_from_packager(const char *packager, char **email); #ifndef HAVE_STRSEP char *strsep(char **, const char *); -- 2.22.0

Jonas Witschel

3:32 p.m.

New subject: [pacman-dev] [PATCH 4/5 v2] be_package: lookup missing keys in the WKD using the packager email

In contrast to the preceeding commit we do not have a database with the required packager information to work with, so we need to extract the package temporarily to obtain this information. Signed-off-by: Jonas Witschel --- lib/libalpm/be_package.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c index fbb0d43e..ee148831 100644 --- a/lib/libalpm/be_package.c +++ b/lib/libalpm/be_package.c @@ -731,6 +731,8 @@ int SYMEXPORT alpm_pkg_load(alpm_handle_t *handle, const char *filename, int ful { int validation = 0; char *sigpath; + alpm_pkg_t *pkg_temp; + char *email; CHECK_HANDLE(handle, return -1); ASSERT(pkg != NULL, RET_ERR(handle, ALPM_ERR_WRONG_ARGS, -1)); @@ -755,9 +757,17 @@ int SYMEXPORT alpm_pkg_load(alpm_handle_t *handle, const char *filename, int ful for(k = keys; k; k = k->next) { char *key = k->data; if(_alpm_key_in_keychain(handle, key) == 0) { - if(_alpm_key_import(handle, NULL, key) == -1) { + pkg_temp = _alpm_pkg_load_internal(handle, filename, full); + if(pkg_temp) { + _alpm_email_from_packager(pkg_temp->packager, &email); + _alpm_pkg_free(pkg_temp); + } else { + email = NULL; + } + if(_alpm_key_import(handle, email, key) == -1) { fail = 1; } + free(email); } } FREELIST(keys); -- 2.22.0

Jonas Witschel

3:32 p.m.

New subject: [pacman-dev] [PATCH 5/5 v2] pacman-key: receive keys from WKD with -r/--recv-keys

If an email address is specified, we use --locate-key to look up the key using WKD and keyserver as a fallback. If the key is specified as a key ID, this doesn't work, so we use the normal keyserver-based --recv-keys. Note that --refresh-keys still uses the keyservers exclusively for refreshing, though the situation might potentially be improved in a new version of GnuPG: https://lists.gnupg.org/pipermail/gnupg-users/2019-July/062169.html Signed-off-by: Jonas Witschel --- scripts/pacman-key.sh.in | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in index b05754e5..a4bdbaa9 100644 --- a/scripts/pacman-key.sh.in +++ b/scripts/pacman-key.sh.in @@ -455,22 +455,29 @@ lsign_keys() { } receive_keys() { - local name id keyids + local name id keyids emails # if the key is not a hex ID, do a lookup for name; do if [[ $name = ?(0x)+([0-9a-fA-F]) ]]; then keyids+=("$name") - else - if id=$(key_lookup_from_name "$name"); then - keyids+=("$id") - fi + elif [[ $name = *@*.* ]]; then + emails+=("$name") + elif id=$(key_lookup_from_name "$name"); then + keyids+=("$id") fi done - (( ${#keyids[*]} > 0 )) || exit 1 + (( ${#keyids[*]}+${#emails[*]} > 0 )) || exit 1 + + if (( ${#emails[*]} > 0 )) && \ + ! "${GPG_PACMAN[@]}" --auto-key-locate nodefault,clear,wkd,keyserver \ + --locate-key "${emails[@]}" ; then + error "$(gettext "Remote key not fetched correctly from WKD or keyserver.")" + exit 1 + fi - if ! "${GPG_PACMAN[@]}" --recv-keys "${keyids[@]}" ; then + if (( ${#keyids[*]} > 0 )) && ! "${GPG_PACMAN[@]}" --recv-keys "${keyids[@]}" ; then error "$(gettext "Remote key not fetched correctly from keyserver.")" exit 1 fi -- 2.22.0

Allan McRae

7 Oct 7 Oct

8:38 a.m.

New subject: [pacman-dev] [PATCH 5/5 v2] pacman-key: receive keys from WKD with -r/--recv-keys

On 6/8/19 1:32 am, Jonas Witschel wrote:

...

If an email address is specified, we use --locate-key to look up the key using WKD and keyserver as a fallback. If the key is specified as a key ID, this doesn't work, so we use the normal keyserver-based --recv-keys.

Note that --refresh-keys still uses the keyservers exclusively for refreshing, though the situation might potentially be improved in a new version of GnuPG: https://lists.gnupg.org/pipermail/gnupg-users/2019-July/062169.html

Signed-off-by: Jonas Witschel ---

Some fairly minor changes below.

...

scripts/pacman-key.sh.in | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in index b05754e5..a4bdbaa9 100644 --- a/scripts/pacman-key.sh.in +++ b/scripts/pacman-key.sh.in @@ -455,22 +455,29 @@ lsign_keys() { }

receive_keys() { - local name id keyids + local name id keyids emails

# if the key is not a hex ID, do a lookup for name; do if [[ $name = ?(0x)+([0-9a-fA-F]) ]]; then keyids+=("$name") - else - if id=$(key_lookup_from_name "$name"); then - keyids+=("$id") - fi + elif [[ $name = *@*.* ]]; then + emails+=("$name") + elif id=$(key_lookup_from_name "$name"); then + keyids+=("$id") fi done

- (( ${#keyids[*]} > 0 )) || exit 1 + (( ${#keyids[*]}+${#emails[*]} > 0 )) || exit 1 + + if (( ${#emails[*]} > 0 )) && \ + ! "${GPG_PACMAN[@]}" --auto-key-locate nodefault,clear,wkd,keyserver \

...

From the man page:

clear Clear all defined mechanisms. This is useful to override mechanisms given in a config file. Note that a nodefault in mechanisms will also be cleared unless it is given af‐ ter the clear. so clear,nodefault,wkd,keyserver ?

...

+ --locate-key "${emails[@]}" ; then + error "$(gettext "Remote key not fetched correctly from WKD or keyserver.")" + exit 1

Instead of exiting here, catch the failure (ret=1), both here and...

...

+ fi

- if ! "${GPG_PACMAN[@]}" --recv-keys "${keyids[@]}" ; then + if (( ${#keyids[*]} > 0 )) && ! "${GPG_PACMAN[@]}" --recv-keys "${keyids[@]}" ; then error "$(gettext "Remote key not fetched correctly from keyserver.")" exit 1

here...

...

fi

and exit here if there was a failure.

...

-- 2.22.0 .

Jonas Witschel

10:56 a.m.

New subject: [pacman-dev] [PATCH v3] pacman-key: receive keys from WKD with -r/--recv-keys

If an email address is specified, we use --locate-key to look up the key using WKD and keyserver as a fallback. If the key is specified as a key ID, this doesn't work, so we use the normal keyserver-based --recv-keys. Note that --refresh-keys still uses the keyservers exclusively for refreshing, though the situation might potentially be improved in a new version of GnuPG: https://lists.gnupg.org/pipermail/gnupg-users/2019-July/062169.html Signed-off-by: Jonas Witschel --- Apply suggested changes: - Use "clear,nodefault" instead of "nodefault,clear" (verified with GnuPG 2.2.17 that the latter indeed doesn't clear the default "local") - Attempt lookups by key ID instead of bailing out early if lookup by email address failed scripts/pacman-key.sh.in | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in index 117acc40..8c8ffc3f 100644 --- a/scripts/pacman-key.sh.in +++ b/scripts/pacman-key.sh.in @@ -461,25 +461,34 @@ lsign_keys() { } receive_keys() { - local name id keyids + local ret=0 name id keyids emails # if the key is not a hex ID, do a lookup for name; do if [[ $name = ?(0x)+([0-9a-fA-F]) ]]; then keyids+=("$name") - else - if id=$(key_lookup_from_name "$name"); then - keyids+=("$id") - fi + elif [[ $name = *@*.* ]]; then + emails+=("$name") + elif id=$(key_lookup_from_name "$name"); then + keyids+=("$id") fi done - (( ${#keyids[*]} > 0 )) || exit 1 + (( ${#keyids[*]}+${#emails[*]} > 0 )) || exit 1 + + if (( ${#emails[*]} > 0 )) && \ + ! "${GPG_PACMAN[@]}" --auto-key-locate clear,nodefault,wkd,keyserver \ + --locate-key "${emails[@]}" ; then + error "$(gettext "Remote key not fetched correctly from WKD or keyserver.")" + ret=1 + fi - if ! "${GPG_PACMAN[@]}" --recv-keys "${keyids[@]}" ; then + if (( ${#keyids[*]} > 0 )) && ! "${GPG_PACMAN[@]}" --recv-keys "${keyids[@]}" ; then error "$(gettext "Remote key not fetched correctly from keyserver.")" - exit 1 + ret=1 fi + + exit $ret } refresh_keys() { -- 2.23.0

Allan McRae

11:40 a.m.

New subject: [pacman-dev] [PATCH v3] pacman-key: receive keys from WKD with -r/--recv-keys

On 7/10/19 8:56 pm, Jonas Witschel wrote:

...

If an email address is specified, we use --locate-key to look up the key using WKD and keyserver as a fallback. If the key is specified as a key ID, this doesn't work, so we use the normal keyserver-based --recv-keys.

Note that --refresh-keys still uses the keyservers exclusively for refreshing, though the situation might potentially be improved in a new version of GnuPG: https://lists.gnupg.org/pipermail/gnupg-users/2019-July/062169.html

Signed-off-by: Jonas Witschel ---

Apply suggested changes: - Use "clear,nodefault" instead of "nodefault,clear" (verified with GnuPG 2.2.17 that the latter indeed doesn't clear the default "local") - Attempt lookups by key ID instead of bailing out early if lookup by email address failed

Great - thanks!

...

scripts/pacman-key.sh.in | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in index 117acc40..8c8ffc3f 100644 --- a/scripts/pacman-key.sh.in +++ b/scripts/pacman-key.sh.in @@ -461,25 +461,34 @@ lsign_keys() { }

receive_keys() { - local name id keyids + local ret=0 name id keyids emails

# if the key is not a hex ID, do a lookup for name; do if [[ $name = ?(0x)+([0-9a-fA-F]) ]]; then keyids+=("$name") - else - if id=$(key_lookup_from_name "$name"); then - keyids+=("$id") - fi + elif [[ $name = *@*.* ]]; then + emails+=("$name") + elif id=$(key_lookup_from_name "$name"); then + keyids+=("$id") fi done

- (( ${#keyids[*]} > 0 )) || exit 1 + (( ${#keyids[*]}+${#emails[*]} > 0 )) || exit 1 + + if (( ${#emails[*]} > 0 )) && \ + ! "${GPG_PACMAN[@]}" --auto-key-locate clear,nodefault,wkd,keyserver \ + --locate-key "${emails[@]}" ; then + error "$(gettext "Remote key not fetched correctly from WKD or keyserver.")" + ret=1 + fi

- if ! "${GPG_PACMAN[@]}" --recv-keys "${keyids[@]}" ; then + if (( ${#keyids[*]} > 0 )) && ! "${GPG_PACMAN[@]}" --recv-keys "${keyids[@]}" ; then error "$(gettext "Remote key not fetched correctly from keyserver.")" - exit 1 + ret=1 fi + + exit $ret }

refresh_keys() {

Jonas Witschel

5 Aug 5 Aug

3:36 p.m.

New subject: [pacman-dev] [PATCH] libmakepkg: check if PACKAGER has the expected format for WKD lookup

pacman should be able to extract an email address from PACKAGER for WKD lookup, so issue a warning if it is not of the form "Example Name ". Neither the name nor the email address must contain additional angle brackets. Signed-off-by: Jonas Witschel --- scripts/libmakepkg/lint_config/variable.sh.in | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/scripts/libmakepkg/lint_config/variable.sh.in b/scripts/libmakepkg/lint_config/variable.sh.in index 55ed6d6d..fe1e6d63 100644 --- a/scripts/libmakepkg/lint_config/variable.sh.in +++ b/scripts/libmakepkg/lint_config/variable.sh.in @@ -60,5 +60,10 @@ lint_config_variables() { fi done + # pacman should be able to extract an email address from PACKAGER for WKD key lookup + if ! [[ $PACKAGER =~ ^[^\<\>]*\ \<[^\<\>]*\>$ ]]; then + warning "$(gettext "PACKAGER should have the format 'Example Name '")" + fi + return $ret } -- 2.22.0

Eli Schwartz

3:59 p.m.

New subject: [pacman-dev] [PATCH] libmakepkg: check if PACKAGER has the expected format for WKD lookup

On 8/5/19 11:36 AM, Jonas Witschel wrote:

...

pacman should be able to extract an email address from PACKAGER for WKD lookup, so issue a warning if it is not of the form "Example Name ". Neither the name nor the email address must contain additional angle brackets.

Signed-off-by: Jonas Witschel --- scripts/libmakepkg/lint_config/variable.sh.in | 5 +++++ 1 file changed, 5 insertions(+)

diff --git a/scripts/libmakepkg/lint_config/variable.sh.in b/scripts/libmakepkg/lint_config/variable.sh.in index 55ed6d6d..fe1e6d63 100644 --- a/scripts/libmakepkg/lint_config/variable.sh.in +++ b/scripts/libmakepkg/lint_config/variable.sh.in @@ -60,5 +60,10 @@ lint_config_variables() { fi done

+ # pacman should be able to extract an email address from PACKAGER for WKD key lookup + if ! [[ $PACKAGER =~ ^[^\<\>]*\ \<[^\<\>]*\>$ ]]; then

Too much escaping is making me seasick. local match='^[^<>]* <[^<>]*>$' if ! [[ $PACKAGER =~ $match ]]; then Since you're making the portion before the optional (* instead of +) this would require someone without a Name component to have a leading space. I recommend using this instead: match='^([^<>]+ )?<[^<>]*>$'

...

+ warning "$(gettext "PACKAGER should have the format 'Example Name '")" + fi + return $ret } -- 2.22.0

-- Eli Schwartz Bug Wrangler and Trusted User

Jonas Witschel

4:32 p.m.

New subject: [pacman-dev] [PATCH v2] libmakepkg: check if PACKAGER has the expected format for WKD lookup

pacman should be able to extract an email address from PACKAGER for WKD lookup, so issue a warning if it is not of the form "Example Name ". Neither the name nor the email address must contain additional angle brackets. Signed-off-by: Jonas Witschel --- v2: implement Eli's suggestion to improve regex and avoid escaping scripts/libmakepkg/lint_config/variable.sh.in | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/scripts/libmakepkg/lint_config/variable.sh.in b/scripts/libmakepkg/lint_config/variable.sh.in index 55ed6d6d..b0b731dd 100644 --- a/scripts/libmakepkg/lint_config/variable.sh.in +++ b/scripts/libmakepkg/lint_config/variable.sh.in @@ -60,5 +60,11 @@ lint_config_variables() { fi done + # pacman should be able to extract an email address from PACKAGER for WKD key lookup + local match='^([^<>]+ )?<[^<>]*>$' + if ! [[ $PACKAGER =~ $match ]]; then + warning "$(gettext "PACKAGER should have the format 'Example Name '")" + fi + return $ret } -- 2.22.0

Jonas Witschel

3:37 p.m.

New subject: [pacman-dev] [PATCH 3/5] sync: lookup missing keys in the WKD using the packager email

Hi, thank you for the review! On 2019-08-05 13:14, Allan McRae wrote:

...

...
+ errors = alpm_list_add(errors, email); errors = alpm_list_add(errors, strdup(key));

I don't like this. Storing two strings as adjacent items in the list.

I'd prefer a small two item struct.

Any other opinions on this?

Done, it now uses a struct with two members "email" and "keyid". A bit more work because we need to free the strings manually now, but I agree it is much cleaner.

...

...
+/** Extract the email address from a User ID + * @param uid User ID to parse in the form "Example Name " [...] + start = strrchr(uid, '<');

This makes a strong assumption that "<" is not used within an email address. The use of that character is technically valid, provided it is quoted.

I am happy with that assumption, but we need to add a check in libmakpkeg to reject emails containing it.

In fact, our PACKAGER variable has no enforced format at all...

I sent a separate patch for libmakepkg to issue a warning if PACKAGER doesn't have the expected format. I opted for a warning instead of a hard error because I don't know what other distributions using pacman do - for Arch the "Example Name " is used consistently by all packagers (except for Xyne, who doesn't use an email address at all).

...

...
+int _alpm_email_from_uid(const char *uid, char **email);

Rename to:

_alpm_email_from_packager()

Done, this also affected "[PATCH 4/5] be_package: lookup missing keys in the WKD using the packager email" because the function is used there as well. I also published the updated patch series as the "wkd-v2" branch of https://gitlab.com/diabonas/pacman Kind regards, Jonas

Eli Schwartz

4:07 p.m.

New subject: [pacman-dev] [PATCH 3/5] sync: lookup missing keys in the WKD using the packager email

On 8/5/19 11:37 AM, Jonas Witschel wrote:

...

Hi,

thank you for the review!

New suggestion: You can do inline comments for a patch using git send-email --annotate, and insert descriptions like: v2: changed foo to bar After the "---" which ends the commit message and before the "diff --git". There are already some comments there, e.g. scripts/libmakepkg/lint_config/variable.sh.in | 5 +++++ 1 file changed, 5 insertions(+) Common practice in many circles is to add commentary or changelogs on the patch, in between the "---" and this summary ... Describing what changed between v1 and v2 of a patch, in the patch email, makes it easier to follow the evolution of a patch, vs. consulting a followup email and trying to map each reply to the affected changeset. Could help. :) -- Eli Schwartz Bug Wrangler and Trusted User

Jonas Witschel

4:35 p.m.

New subject: [pacman-dev] [PATCH 3/5] sync: lookup missing keys in the WKD using the packager email

On 2019-08-05 18:07, Eli Schwartz wrote:

...

New suggestion:

You can do inline comments for a patch using git send-email --annotate, and insert descriptions like:

v2: changed foo to bar

After the "---" which ends the commit message and before the "diff --git".

Thanks, that is useful indeed! I did this for v2 of the libmakepkg patch, thank you for your suggestions there as well! Regards, Jonas

Jonas Witschel

17 Jul 17 Jul

8:06 p.m.

New subject: [pacman-dev] [PATCH 4/5] be_package: lookup missing keys in the WKD using the packager email

In contrast to the preceeding commit we do not have a database with the required packager information to work with, so we need to extract the package temporarily to obtain this information. Signed-off-by: Jonas Witschel --- lib/libalpm/be_package.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c index fbb0d43e..28f04371 100644 --- a/lib/libalpm/be_package.c +++ b/lib/libalpm/be_package.c @@ -731,6 +731,8 @@ int SYMEXPORT alpm_pkg_load(alpm_handle_t *handle, const char *filename, int ful { int validation = 0; char *sigpath; + alpm_pkg_t *pkg_temp; + char *email; CHECK_HANDLE(handle, return -1); ASSERT(pkg != NULL, RET_ERR(handle, ALPM_ERR_WRONG_ARGS, -1)); @@ -755,9 +757,17 @@ int SYMEXPORT alpm_pkg_load(alpm_handle_t *handle, const char *filename, int ful for(k = keys; k; k = k->next) { char *key = k->data; if(_alpm_key_in_keychain(handle, key) == 0) { - if(_alpm_key_import(handle, NULL, key) == -1) { + pkg_temp = _alpm_pkg_load_internal(handle, filename, full); + if(pkg_temp) { + _alpm_email_from_uid(pkg_temp->packager, &email); + _alpm_pkg_free(pkg_temp); + } else { + email = NULL; + } + if(_alpm_key_import(handle, email, key) == -1) { fail = 1; } + free(email); } } FREELIST(keys); -- 2.22.0

Jonas Witschel

8:06 p.m.

New subject: [pacman-dev] [PATCH 5/5] pacman-key: receive keys from WKD with -r/--recv-keys

If an email address is specified, we use --locate-key to look up the key using WKD and keyserver as a fallback. If the key is specified as a key ID, this doesn't work, so we use the normal keyserver-based --recv-keys. Note that --refresh-keys still uses the keyservers exclusively for refreshing, though the situation might potentially be improved in a new version of GnuPG: https://lists.gnupg.org/pipermail/gnupg-users/2019-July/062169.html Signed-off-by: Jonas Witschel --- scripts/pacman-key.sh.in | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in index b05754e5..cd214a2e 100644 --- a/scripts/pacman-key.sh.in +++ b/scripts/pacman-key.sh.in @@ -455,25 +455,30 @@ lsign_keys() { } receive_keys() { - local name id keyids + local name id keyids emails # if the key is not a hex ID, do a lookup for name; do if [[ $name = ?(0x)+([0-9a-fA-F]) ]]; then keyids+=("$name") - else - if id=$(key_lookup_from_name "$name"); then - keyids+=("$id") - fi + elif [[ $name = *@*.* ]]; then + emails+=("$name") + elif id=$(key_lookup_from_name "$name"); then + keyids+=("$id") fi done - (( ${#keyids[*]} > 0 )) || exit 1 + (( ${#keyids[*]}+${#emails[*]} > 0 )) || exit 1 - if ! "${GPG_PACMAN[@]}" --recv-keys "${keyids[@]}" ; then + if (( ${#keyids[*]} > 0 )) && ! "${GPG_PACMAN[@]}" --recv-keys "${keyids[@]}" ; then error "$(gettext "Remote key not fetched correctly from keyserver.")" exit 1 fi + + if (( ${#emails[*]} > 0 )) && ! "${GPG_PACMAN[@]}" --auto-key-locate nodefault,clear,wkd,keyserver --locate-key "${emails[@]}" ; then + error "$(gettext "Remote key not fetched correctly from WKD or keyserver.")" + exit 1 + fi } refresh_keys() { -- 2.22.0

Allan McRae

5 Aug 5 Aug

11:31 a.m.

New subject: [pacman-dev] [PATCH 5/5] pacman-key: receive keys from WKD with -r/--recv-keys

On 18/7/19 6:06 am, Jonas Witschel wrote:

...

If an email address is specified, we use --locate-key to look up the key using WKD and keyserver as a fallback. If the key is specified as a key ID, this doesn't work, so we use the normal keyserver-based --recv-keys.

Note that --refresh-keys still uses the keyservers exclusively for refreshing, though the situation might potentially be improved in a new version of GnuPG: https://lists.gnupg.org/pipermail/gnupg-users/2019-July/062169.html

Signed-off-by: Jonas Witschel --- scripts/pacman-key.sh.in | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in index b05754e5..cd214a2e 100644 --- a/scripts/pacman-key.sh.in +++ b/scripts/pacman-key.sh.in @@ -455,25 +455,30 @@ lsign_keys() { }

receive_keys() { - local name id keyids + local name id keyids emails

# if the key is not a hex ID, do a lookup for name; do if [[ $name = ?(0x)+([0-9a-fA-F]) ]]; then keyids+=("$name") - else - if id=$(key_lookup_from_name "$name"); then - keyids+=("$id") - fi + elif [[ $name = *@*.* ]]; then + emails+=("$name") + elif id=$(key_lookup_from_name "$name"); then + keyids+=("$id") fi done

- (( ${#keyids[*]} > 0 )) || exit 1 + (( ${#keyids[*]}+${#emails[*]} > 0 )) || exit 1

- if ! "${GPG_PACMAN[@]}" --recv-keys "${keyids[@]}" ; then + if (( ${#keyids[*]} > 0 )) && ! "${GPG_PACMAN[@]}" --recv-keys "${keyids[@]}" ; then error "$(gettext "Remote key not fetched correctly from keyserver.")" exit 1 fi + + if (( ${#emails[*]} > 0 )) && ! "${GPG_PACMAN[@]}" --auto-key-locate nodefault,clear,wkd,keyserver --locate-key "${emails[@]}" ; then

Super long line. Please split across two or more lines. I would also like to see this block placed above the --recv-keys one.

...

+ error "$(gettext "Remote key not fetched correctly from WKD or keyserver.")" + exit 1 + fi }

refresh_keys() { -- 2.22.0 .

Jonas Witschel

3:37 p.m.

New subject: [pacman-dev] [PATCH 5/5] pacman-key: receive keys from WKD with -r/--recv-keys

On 2019-08-05 13:31, Allan McRae wrote:

...

...
+ if (( ${#emails[*]} > 0 )) && ! "${GPG_PACMAN[@]}" --auto-key-locate nodefault,clear,wkd,keyserver --locate-key "${emails[@]}" ; then

Super long line. Please split across two or more lines.

I would also like to see this block placed above the --recv-keys one.

I split up the line and moved the block. Cheers, Jonas

Jonas Witschel

10:42 a.m.

Hi, good news everyone, thanks to Florian Pritz we now have a working WKD [1] I had to modify the fingerprint check again [2] because the key used for signing might be a subkey, so we need to loop trough all subkeys and check if one of these key IDs matches the one used for signing the package. The latest patch series now works with the keys published in the Arch Linux WKD. I tested this by creating a new keyring containing only the master signing keys, then installing a random package from each of the developers whose key is ready for WKD support (the ones with three "Yes" in the table in [3]). This worked successfully, so from my side, this code is now ready for merging. I welcome any feedback on the code and any testing whether everything works as expected. For convenience I cloned the pacman repository on Gitlab and provide the patch series in the "wkd" branch there [4]. A possible test setup looks like this: # Build pacman with WKD patches added git clone --branch wkd https://gitlab.com/diabonas/pacman.git mkdir pacman/build && cd pacman/build meson .. && ninja # Prepare keyring (similar to what pacman-key --populate archlinux would # do, but only import the master signing keys) fakeroot pacman-key --init --gpgdir keyring fakeroot pacman-key --gpgdir keyring --recv-keys \ $(cut -d':' -f1 /usr/share/pacman/keyrings/archlinux-trusted) fakeroot pacman-key --gpgdir keyring --lsign \ $(cut -d':' -f1 /usr/share/pacman/keyrings/archlinux-trusted) gpg --homedir keyring --import-ownertrust \ /usr/share/pacman/keyrings/archlinux-trusted mkdir -p root/var/lib/pacman # Install a package, key will be looked up in the WKD # Output should be: # debug: looking up key for arojas@archlinux.org using WKD # debug: unknown key, found Antonio Rojas remotely fakeroot ./pacman --root root --gpgdir keyring --debug -Syu libaio Other interesting test cases are: - Use a package by a packager that doesn't use an Arch Linux UID (first column in [3] is No), e.g. "linux-headers": the WKD key lookup should fail with "key lookup failed, unknown key", but the key should be found and imported from a keyserver. - Disable keyserver access by adding the line "keyserver broken.invalid" to "keyring/gpg.conf". Now importing the key for "libaio" from the WKD should still work, while installing "linux-headers" fails with 'error: key "A5E9288C4FA415FA" could not be looked up remotely'. (Don't forget to delete the keys from the pacman keyring using "gpg --homedir keyring --delete-keys 9D74DF6F91B7BDABD5815CA84AC5588F941C2A25 8218F88849AAC522E94CF470A5E9288C4FA415FA" first if you've already imported them.) - Test whether installing a file directly using fakeroot ./pacman --root root --gpgdir keyring --debug -U \ https://mex.mirror.pkgbuild.com/core/os/x86_64/libaio-0.3.112-1-x86_64.pkg.t... instead of using the database also retrieves the key from the WKD. Cheers, Jonas [1] https://bugs.archlinux.org/task/63171#comment180697 [2] https://lists.archlinux.org/pipermail/pacman-dev/2019-August/023518.html [3] https://wiki.archlinux.org/index.php/User:Diabonas/WKD_support_by_developer_... [4] https://gitlab.com/diabonas/pacman

Jonas Witschel

2 Oct 2 Oct

2:40 p.m.

New subject: [pacman-dev] [PATCH v3 0/3] Manage signing keys using a WKD (FS#63171)

Based on the feedback on #archlinux-pacman, I have reworked the WKD patches: we now ask the user whether they want to import a missing PGP key before doing any remote lookup, which eliminates the need for a second temporary keyring. Without a remote lookup, we only know the ID of the package signing key, so we display the packager in addition to the key ID for user convenience. This patch series entirely replaces all previously sent patches regarding WKD support. - PATCH v3 1/3 restructures the user confirmation in the described way. It incorporates the previous patches 3/5 and 4/5 because to have a standalone patch, we need to retrieve the user ID to display a user-friendly confirmation message. Other than that, it's mostly moving existing code around to fit the new workflow. - PATCH v3 2/3 is a simplified version of the previous patch 2/5, since doing the confirmation first allows us to drop the temporary keyring. Note that in contrast to the previous approach, we don't check any more whether the key retrieved from the WKD has the correct key ID, it is now the responsibility of the WKD maintainer to ensure this. The reason for this change is that at the time we are able to check the key ID, we have already imported the key anyway. - PATCH v3 3/3 is unchanged from "[PATCH v2] libmakepkg: check if PACKAGER has the expected format for WKD lookup", included simply for the convenience of having a complete patch series. Jonas Witschel (3): signing: move key import confirmation before key_search signing: add ability to import keys using a WKD libmakepkg: check if PACKAGER has the expected format for WKD lookup lib/libalpm/be_package.c | 12 +- lib/libalpm/signing.c | 120 ++++++++++++++---- lib/libalpm/signing.h | 2 +- lib/libalpm/sync.c | 22 +++- scripts/libmakepkg/lint_config/variable.sh.in | 6 + src/pacman/callback.c | 13 +- 6 files changed, 136 insertions(+), 39 deletions(-) -- 2.23.0

Jonas Witschel

2:40 p.m.

New subject: [pacman-dev] [PATCH v3 1/3] signing: move key import confirmation before key_search

Ask the user whether they want to import a missing key before even doing a search on the keyserver. This will be useful for getting Web Key Directory support in place: for a WKD, looking up and importing a key are a single action, so the current key_search -> QUESTION -> key_import workflow does not apply. Since only the ID of the package signing key is available before key_search, we display the packager variable in addition to the key ID for user convenience. Signed-off-by: Jonas Witschel --- lib/libalpm/be_package.c | 12 +++++++++- lib/libalpm/signing.c | 50 ++++++++++++++++++++++------------------ lib/libalpm/signing.h | 2 +- lib/libalpm/sync.c | 22 +++++++++++++++--- src/pacman/callback.c | 13 ++--------- 5 files changed, 60 insertions(+), 39 deletions(-) diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c index ac911bdb..6c93d12b 100644 --- a/lib/libalpm/be_package.c +++ b/lib/libalpm/be_package.c @@ -731,6 +731,8 @@ int SYMEXPORT alpm_pkg_load(alpm_handle_t *handle, const char *filename, int ful { int validation = 0; char *sigpath; + alpm_pkg_t *pkg_temp; + char *packager; CHECK_HANDLE(handle, return -1); ASSERT(pkg != NULL, RET_ERR(handle, ALPM_ERR_WRONG_ARGS, -1)); @@ -755,9 +757,17 @@ int SYMEXPORT alpm_pkg_load(alpm_handle_t *handle, const char *filename, int ful 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) { + pkg_temp = _alpm_pkg_load_internal(handle, filename, full); + if(pkg_temp) { + packager = pkg_temp->packager; + _alpm_pkg_free(pkg_temp); + } else { + packager = NULL; + } + if(_alpm_key_import(handle, packager, key) == -1) { fail = 1; } + free(packager); } } FREELIST(keys); diff --git a/lib/libalpm/signing.c b/lib/libalpm/signing.c index 92598b0e..c1ae5354 100644 --- a/lib/libalpm/signing.c +++ b/lib/libalpm/signing.c @@ -433,42 +433,45 @@ gpg_error: /** * Import a key defined by a fingerprint into the local keyring. * @param handle the context handle + * @param uid a user ID of the key to import * @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 _alpm_key_import(alpm_handle_t *handle, const char *uid, const char *fpr) { int ret = -1; alpm_pgpkey_t fetch_key; + + if(_alpm_access(handle, handle->gpgdir, "pubring.gpg", W_OK)) { + /* no chance of import succeeding if pubring isn't writable */ + _alpm_log(handle, ALPM_LOG_ERROR, _("keyring is not writable\n")); + return -1; + } + memset(&fetch_key, 0, sizeof(fetch_key)); + STRDUP(fetch_key.uid, uid, return -1); + STRDUP(fetch_key.fingerprint, fpr, return -1); - 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)) { - alpm_question_import_key_t question = { + alpm_question_import_key_t question = { .type = ALPM_QUESTION_IMPORT_KEY, .import = 0, .key = &fetch_key }; - QUESTION(handle, &question); - if(question.import) { - 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); - } + QUESTION(handle, &question); + if(question.import) { + if(key_search(handle, fpr, &fetch_key) == 1) { + _alpm_log(handle, ALPM_LOG_DEBUG, + _("key \"%s\" on keyserver\n"), fetch_key.uid); + 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); + _alpm_log(handle, ALPM_LOG_ERROR, + _("key \"%s\" could not be looked up remotely\n"), fpr); } - } else { - _alpm_log(handle, ALPM_LOG_ERROR, - _("key \"%s\" could not be looked up remotely\n"), fpr); } gpgme_key_unref(fetch_key.data); @@ -714,7 +717,8 @@ 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) +int _alpm_key_import(alpm_handle_t UNUSED *handle, const char UNUSED *uid, + const char UNUSED *fpr) { return -1; } @@ -900,7 +904,7 @@ 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); - if(_alpm_key_import(handle, result->key.fingerprint) == 0) { + if(_alpm_key_import(handle, result->key.uid, result->key.fingerprint) == 0) { retry = 1; } diff --git a/lib/libalpm/signing.h b/lib/libalpm/signing.h index f8b84b94..36b64384 100644 --- a/lib/libalpm/signing.h +++ b/lib/libalpm/signing.h @@ -32,6 +32,6 @@ 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); +int _alpm_key_import(alpm_handle_t *handle, const char *uid, const char *fpr); #endif /* ALPM_SIGNING_H */ diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index cbd072e6..dd695877 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -47,6 +47,12 @@ #include "diskspace.h" #include "signing.h" +struct keyinfo_t { + char* uid; + char* keyid; +}; + + /** Check for new version of pkg in sync repos * (only the first occurrence is considered in sync) */ @@ -872,6 +878,7 @@ static int check_keyring(alpm_handle_t *handle) size_t current = 0, numtargs; alpm_list_t *i, *errors = NULL; alpm_event_t event; + struct keyinfo_t *keyinfo; event.type = ALPM_EVENT_KEYRING_START; EVENT(handle, &event); @@ -905,7 +912,13 @@ static int check_keyring(alpm_handle_t *handle) char *key = k->data; if(!alpm_list_find_str(errors, key) && _alpm_key_in_keychain(handle, key) == 0) { - errors = alpm_list_add(errors, strdup(key)); + keyinfo = malloc(sizeof(struct keyinfo_t)); + if(!keyinfo) { + break; + } + keyinfo->uid = strdup(pkg->packager); + keyinfo->keyid = strdup(key); + errors = alpm_list_add(errors, keyinfo); } } FREELIST(keys); @@ -926,10 +939,13 @@ static int check_keyring(alpm_handle_t *handle) 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) { + keyinfo = k->data; + if(_alpm_key_import(handle, keyinfo->uid, keyinfo->keyid) == -1) { fail = 1; } + free(keyinfo->uid); + free(keyinfo->keyid); + } event.type = ALPM_EVENT_KEY_DOWNLOAD_DONE; EVENT(handle, &event); diff --git a/src/pacman/callback.c b/src/pacman/callback.c index fc4ce875..25528100 100644 --- a/src/pacman/callback.c +++ b/src/pacman/callback.c @@ -456,17 +456,8 @@ void cb_question(alpm_question_t *question) case ALPM_QUESTION_IMPORT_KEY: { alpm_question_import_key_t *q = &question->import_key; - char created[12]; - time_t time = (time_t)q->key->created; - strftime(created, 12, "%Y-%m-%d", localtime(&time)); - - if(q->key->revoked) { - q->import = yesno(_("Import PGP key %u%c/%s, \"%s\", created: %s (revoked)?"), - q->key->length, q->key->pubkey_algo, q->key->fingerprint, q->key->uid, created); - } else { - q->import = yesno(_("Import PGP key %u%c/%s, \"%s\", created: %s?"), - q->key->length, q->key->pubkey_algo, q->key->fingerprint, q->key->uid, created); - } + q->import = yesno(_("Import PGP key %s, \"%s\"?"), + q->key->fingerprint, q->key->uid); } break; } -- 2.23.0

Jonas Witschel

2:40 p.m.

New subject: [pacman-dev] [PATCH v3 2/3] signing: add ability to import keys using a WKD

Allan McRae

7 Oct 7 Oct

1:21 a.m.

New subject: [pacman-dev] [PATCH v3 2/3] signing: add ability to import keys using a WKD

On 3/10/19 12:40 am, Jonas Witschel wrote:

...

Currently pacman relies on the SKS keyserver network to fetch unknown PGP keys. These keyservers are vulnerable to signature spamming attacks, potentionally making it impossible to import the required keys. An alternative to keyservers is a so-called Web Key Directory (WKD), a well-known, trusted location on a server from where the keys can be fetched.

This commit adds the ability to retrieve keys from a WKD. Due to the mentioned vulnerabilities, the WKD is tried first, falling back to the keyservers only if no appropriate key is found there.

In contrast to keyservers, keys in a WKD are not looked up using their fingerprint, but by email address. Since the email address of the signing key is usually not included in the signature, we will use the packager email address to perform the lookup.

Also see FS#63171.

Signed-off-by: Jonas Witschel --- lib/libalpm/signing.c | 96 +++++++++++++++++++++++++++++++++++++------ 1 file changed, 83 insertions(+), 13 deletions(-)

Thanks - looks good apart from one change I made:

...

+ /* Try to import the key from a WKD first */ + email_from_uid(uid, &email); + ret = key_import_wkd(handle, email);

CC libalpm_la-signing.lo signing.c: In function ‘_alpm_key_import’: signing.c:285:12: error: ‘email’ may be used uninitialized in this function [-Werror=maybe-uninitialized] 285 | gpg_err = gpgme_get_key(ctx, email, &key, 0); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ signing.c:506:8: note: ‘email’ was declared here 506 | char *email; | ^~~~~ cc1: all warnings being treated as errors if(email_from_uid(uid, &email) == 0) { ret = key_import_wkd(handle, email); }

Jonas Witschel

2 Oct 2 Oct

2:40 p.m.

New subject: [pacman-dev] [PATCH v3 3/3] libmakepkg: check if PACKAGER has the expected format for WKD lookup

pacman should be able to extract an email address from PACKAGER for WKD lookup, so issue a warning if it is not of the form "Example Name ". Neither the name nor the email address must contain additional angle brackets. Signed-off-by: Jonas Witschel --- scripts/libmakepkg/lint_config/variable.sh.in | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/scripts/libmakepkg/lint_config/variable.sh.in b/scripts/libmakepkg/lint_config/variable.sh.in index 55ed6d6d..b0b731dd 100644 --- a/scripts/libmakepkg/lint_config/variable.sh.in +++ b/scripts/libmakepkg/lint_config/variable.sh.in @@ -60,5 +60,11 @@ lint_config_variables() { fi done + # pacman should be able to extract an email address from PACKAGER for WKD key lookup + local match='^([^<>]+ )?<[^<>]*>$' + if ! [[ $PACKAGER =~ $match ]]; then + warning "$(gettext "PACKAGER should have the format 'Example Name '")" + fi + return $ret } -- 2.23.0

Allan McRae

4 Oct 4 Oct

1:53 a.m.

New subject: [pacman-dev] [PATCH v3 3/3] libmakepkg: check if PACKAGER has the expected format for WKD lookup

On 3/10/19 12:40 am, Jonas Witschel wrote:

...

pacman should be able to extract an email address from PACKAGER for WKD lookup, so issue a warning if it is not of the form "Example Name ". Neither the name nor the email address must contain additional angle brackets.

Thanks. I have applied and will send through changes to makepkg.conf.5 that mention this restriction. Allan

1662

Age (days ago)

1744

Last active (days ago)

List overview

Download

30 comments

4 participants

participants (4)

Allan McRae
Eli Schwartz
Jonas Witschel
Jonas Witschel