[pacman-dev] [PATCH v2] Fallback to detached signatures during keyring check
Andrew Gregory
andrew.gregory.8 at gmail.com
Sun May 31 04:31:18 UTC 2020
On 05/30/20 at 07:51pm, Anatol Pomozov wrote:
> Pacman has a 'key in keyring' verification step that makes sure the signatures
> have a valid keyid. Currently pacman parses embedded package signatures only.
>
> Add a fallback to detached signatures. If embedded signature is missing then it
> tries to read corresponding *.sig file and get keyid from there.
>
> Verification:
> debug: found cached pkg: /var/cache/pacman/pkg/glib-networking-2.64.3-1-x86_64.pkg.tar.zst
> debug: found detached signature /var/cache/pacman/pkg/glib-networking-2.64.3-1-x86_64.pkg.tar.zst.sig with size 310
> debug: found signature key: A5E9288C4FA415FA
> debug: looking up key A5E9288C4FA415FA locally
> debug: key lookup success, key exists
>
> Signed-off-by: Anatol Pomozov <anatol.pomozov at gmail.com>
> ---
> lib/libalpm/alpm.h | 10 ++++++++++
> lib/libalpm/package.c | 31 +++++++++++++++++++++++++++++++
> lib/libalpm/sync.c | 17 ++++++++---------
> lib/libalpm/util.c | 35 +++++++++++++++++++++++++++++++++++
> lib/libalpm/util.h | 1 +
> 5 files changed, 85 insertions(+), 9 deletions(-)
>
> diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h
> index c6a13273..9c5fff73 100644
> --- a/lib/libalpm/alpm.h
> +++ b/lib/libalpm/alpm.h
> @@ -1403,6 +1403,16 @@ alpm_db_t *alpm_pkg_get_db(alpm_pkg_t *pkg);
> */
> const char *alpm_pkg_get_base64_sig(alpm_pkg_t *pkg);
>
> +/** Extracts package signature either from embedded package signature
> + * or if it is absent then reads data from detached signature file.
> + * @param pkg a pointer to package
> + * @param sig output parameter for signature data. Callee function allocates
> + * buffer needed for the signature data. Caller is responsible for
> + * freeing this buffer.
> + * @return size of a signature or negative number if error.
> + */
> +int alpm_pkg_get_sig(alpm_pkg_t *pkg, unsigned char **sig);
> +
> /** Returns the method used to validate a package during install.
> * @param pkg a pointer to package
> * @return an enum member giving the validation method
> diff --git a/lib/libalpm/package.c b/lib/libalpm/package.c
> index 5c5fa073..e0e4d987 100644
> --- a/lib/libalpm/package.c
> +++ b/lib/libalpm/package.c
> @@ -268,6 +268,37 @@ const char SYMEXPORT *alpm_pkg_get_base64_sig(alpm_pkg_t *pkg)
> return pkg->base64_sig;
> }
>
> +int SYMEXPORT alpm_pkg_get_sig(alpm_pkg_t *pkg, unsigned char **sig)
This API is weird, why are you returning an int when neither of the
successful values are int and can potentially overflow an int? You're
returning the length of an allocated string, size_t is the appropriate
type.
> +{
> + ASSERT(pkg != NULL, return -1);
> + pkg->handle->pm_errno = ALPM_ERR_OK;
I don't like clearing pm_errno without a good reason. It generally
doesn't serve any purpose and can get us into trouble if a function
gets called during cleanup somewhere.
> + if(pkg->base64_sig) {
> + size_t sig_len;
> + int ret = alpm_decode_signature(pkg->base64_sig, sig, &sig_len);
> + return ret == 0 ? (int)sig_len : -1;
> + } else {
> + char *pkgpath, *sigpath;
> + int len;
> +
> + pkgpath = _alpm_filecache_find(pkg->handle, pkg->filename);
> + if(!pkgpath) {
> + RET_ERR(pkg->handle, ALPM_ERR_PKG_NOT_FOUND, -1);
> + }
> + sigpath = _alpm_sigpath(pkg->handle, pkgpath);
> + if(!sigpath || _alpm_access(pkg->handle, NULL, sigpath, R_OK)) {
> + FREE(pkgpath);
> + FREE(sigpath);
> + RET_ERR(pkg->handle, ALPM_ERR_SIG_MISSING, -1);
> + }
> + len = _alpm_read_file(sigpath, sig);
You need to check for and handle failure.
> + _alpm_log(pkg->handle, ALPM_LOG_DEBUG, "found detached signature %s with size %d\n", sigpath, len);
> + FREE(pkgpath);
> + FREE(sigpath);
> + return len;
> + }
> +}
> +
> const char SYMEXPORT *alpm_pkg_get_arch(alpm_pkg_t *pkg)
> {
> ASSERT(pkg != NULL, return NULL);
> diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c
> index 8c01ad95..0ab0fe26 100644
> --- a/lib/libalpm/sync.c
> +++ b/lib/libalpm/sync.c
> @@ -880,18 +880,17 @@ static int check_keyring(alpm_handle_t *handle)
> }
>
> level = alpm_db_get_siglevel(alpm_pkg_get_db(pkg));
> - if((level & ALPM_SIG_PACKAGE) && pkg->base64_sig) {
> - unsigned char *decoded_sigdata = NULL;
> - size_t data_len;
> - int decode_ret = alpm_decode_signature(pkg->base64_sig,
> - &decoded_sigdata, &data_len);
> - if(decode_ret == 0) {
> + if((level & ALPM_SIG_PACKAGE)) {
> + unsigned char *signature = NULL;
> + int sig_len = alpm_pkg_get_sig(pkg, &signature);
> + if(sig_len > 0) {
> alpm_list_t *keys = NULL;
> - if(alpm_extract_keyid(handle, pkg->name, decoded_sigdata,
> - data_len, &keys) == 0) {
> + if(alpm_extract_keyid(handle, pkg->name, signature,
> + sig_len, &keys) == 0) {
> alpm_list_t *k;
> for(k = keys; k; k = k->next) {
> char *key = k->data;
> + _alpm_log(handle, ALPM_LOG_DEBUG, "found signature key: %s\n", key);
> if(!alpm_list_find(errors, key, key_cmp) &&
> _alpm_key_in_keychain(handle, key) == 0) {
> keyinfo = malloc(sizeof(struct keyinfo_t));
> @@ -905,8 +904,8 @@ static int check_keyring(alpm_handle_t *handle)
> }
> FREELIST(keys);
> }
> - free(decoded_sigdata);
> }
> + free(signature);
> }
> }
>
> diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c
> index 76728eb4..3d57817b 100644
> --- a/lib/libalpm/util.c
> +++ b/lib/libalpm/util.c
> @@ -1489,3 +1489,38 @@ void _alpm_alloc_fail(size_t size)
> {
> fprintf(stderr, "alloc failure: could not allocate %zu bytes\n", size);
> }
> +
> +/** This functions reads file content.
> + *
> + * Memory buffer is allocated by the callee function. It is responsibility
> + * of the caller to free the buffer
> + *
> + * @param filepath filepath to read
> + * @param data pointer to output buffer
> + * @return size of the data read or negative number in case of error
> + */
> +int _alpm_read_file(const char *filepath, unsigned char **data)
Same as above.
> +{
> + struct stat st;
> + FILE *fp;
> +
> + if((fp = fopen(filepath, "rb")) == NULL) {
> + return -1;
> + }
> +
> + if(fstat(fileno(fp), &st) != 0) {
> + fclose(fp);
> + return -1;
> + }
> +
> + MALLOC(*data, st.st_size, fclose(fp); return -1);
> +
> + if(fread(*data, st.st_size, 1, fp) != 1) {
> + FREE(*data);
> + fclose(fp);
> + return -1;
> + }
> +
> + fclose(fp);
> + return st.st_size;
> +}
> diff --git a/lib/libalpm/util.h b/lib/libalpm/util.h
> index 4fc6e718..50a94489 100644
> --- a/lib/libalpm/util.h
> +++ b/lib/libalpm/util.h
> @@ -155,6 +155,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_read_file(const char *filepath, unsigned char **data);
>
> #ifndef HAVE_STRSEP
> char *strsep(char **, const char *);
> --
> 2.26.2
More information about the pacman-dev
mailing list