From b2ded5a4da58940159f5d337e9e29c5de5cc0f7a Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 2 Feb 2017 10:00:06 -0500 Subject: [PATCH 1/2] repo: Enable GPG by default This increases compatibility with dnf/yum. It was very surprising to me that we didn't do this. Most repos do seem to use `gpgcheck=1`, but I'm fairly certain I've seen repo configs in the wild that actually depend on it, though after looking at a few (ceph, postgres), I can't find one yet. Still, I think it's worth making this change. I suspect very few people will edit `/etc/yum/yum.conf` to globally disable GPG checking, versus disabling it per repository. --- libdnf/dnf-repo.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/libdnf/dnf-repo.c b/libdnf/dnf-repo.c index 08e0d8b..017c82d 100644 --- a/libdnf/libdnf/dnf-repo.c +++ b/libdnf/libdnf/dnf-repo.c @@ -858,7 +858,16 @@ dnf_repo_set_keyfile_data(DnfRepo *repo, GError **error) /* gpgkey is optional for gpgcheck=1, but required for repo_gpgcheck=1 */ g_free(priv->gpgkey); priv->gpgkey = g_key_file_get_string(priv->keyfile, priv->id, "gpgkey", NULL); - priv->gpgcheck_pkgs = dnf_repo_get_boolean(priv->keyfile, priv->id, "gpgcheck", NULL); + /* Currently, we don't have a global configuration file. The way this worked in yum + * is that the yum package enabled gpgcheck=1 by default in /etc/yum.conf. Basically, + * I don't think many people changed that. It's just saner to disable it in the individual + * repo files as required. To claim compatibility with yum repository files, I think + * we need to basically hard code the yum.conf defaults here. + */ + if (!g_key_file_has_key (priv->keyfile, priv->id, "gpgcheck", NULL)) + priv->gpgcheck_pkgs = TRUE; + else + priv->gpgcheck_pkgs = dnf_repo_get_boolean(priv->keyfile, priv->id, "gpgcheck", NULL); priv->gpgcheck_md = dnf_repo_get_boolean(priv->keyfile, priv->id, "repo_gpgcheck", NULL); if (priv->gpgcheck_md && priv->gpgkey == NULL) { g_set_error_literal(error, -- 2.9.3 From 0d68ebac7d39a9278ebb5259c4912efa55c080da Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 31 Jan 2017 17:14:37 -0500 Subject: [PATCH 2/2] transaction: Export parts of the GPG functionality rpm-ostree doesn't use `dnf_transaction_commit()`, because it uses libostree to store the RPMs, and bubblewrap to run `%post`s. However, we do need the GPG functionality. --- libdnf/libdnf/dnf-transaction.c | 193 ++++++++++++++++++++++++++++------------------- libdnf/libdnf/dnf-transaction.h | 11 +++ 2 files changed, 127 insertions(+), 77 deletions(-) diff --git a/libdnf/libdnf/dnf-transaction.c b/libdnf/libdnf/dnf-transaction.c index 62fcc51..dd73245 100644 --- a/libdnf/libdnf/dnf-transaction.c +++ b/libdnf/libdnf/dnf-transaction.c @@ -328,17 +328,88 @@ dnf_transaction_ensure_repo_list(DnfTransaction *transaction, return TRUE; } +gboolean +dnf_transaction_gpgcheck_package (DnfTransaction *transaction, + DnfPackage *pkg, + GError **error) +{ + DnfTransactionPrivate *priv = GET_PRIVATE(transaction); + GError *error_local = NULL; + DnfRepo *repo; + const gchar *fn; + + /* ensure the filename is set */ + if (!dnf_transaction_ensure_repo(transaction, pkg, error)) { + g_prefix_error(error, "Failed to check untrusted: "); + return FALSE; + } + + /* find the location of the local file */ + fn = dnf_package_get_filename(pkg); + if (fn == NULL) { + g_set_error(error, + DNF_ERROR, + DNF_ERROR_FILE_NOT_FOUND, + "Downloaded file for %s not found", + dnf_package_get_name(pkg)); + return FALSE; + } + + /* check file */ + if (!dnf_keyring_check_untrusted_file(priv->keyring, fn, &error_local)) { + + /* probably an i/o error */ + if (!g_error_matches(error_local, + DNF_ERROR, + DNF_ERROR_GPG_SIGNATURE_INVALID)) { + g_propagate_error(error, error_local); + return FALSE; + } + + /* if the repo is signed this is ALWAYS an error */ + repo = dnf_package_get_repo(pkg); + if (repo != NULL && dnf_repo_get_gpgcheck(repo)) { + g_set_error(error, + DNF_ERROR, + DNF_ERROR_FILE_INVALID, + "package %s cannot be verified " + "and repo %s is GPG enabled: %s", + dnf_package_get_nevra(pkg), + dnf_repo_get_id(repo), + error_local->message); + g_error_free(error_local); + return FALSE; + } + + /* we can only install signed packages in this mode */ + if ((priv->flags & DNF_TRANSACTION_FLAG_ONLY_TRUSTED) > 0) { + g_propagate_error(error, error_local); + return FALSE; + } + + /* we can install unsigned packages */ + g_warning("ignoring as allow-untrusted: %s", + error_local->message); + g_clear_error(&error_local); + } + + return TRUE; +} + /** * dnf_transaction_check_untrusted: + * @transaction: Transaction + * @goal: Target goal + * @error: Error + * + * Verify GPG signatures for all pending packages to be changed as part + * of @goal. */ -static gboolean +gboolean dnf_transaction_check_untrusted(DnfTransaction *transaction, HyGoal goal, GError **error) { - DnfTransactionPrivate *priv = GET_PRIVATE(transaction); - DnfPackage *pkg; - const gchar *fn; guint i; g_autoptr(GPtrArray) install = NULL; @@ -354,64 +425,10 @@ dnf_transaction_check_untrusted(DnfTransaction *transaction, /* find any packages in untrusted repos */ for (i = 0; i < install->len; i++) { - GError *error_local = NULL; - DnfRepo *repo; - pkg = g_ptr_array_index(install, i); + DnfPackage *pkg = g_ptr_array_index(install, i); - /* ensure the filename is set */ - if (!dnf_transaction_ensure_repo(transaction, pkg, error)) { - g_prefix_error(error, "Failed to check untrusted: "); + if (!dnf_transaction_gpgcheck_package (transaction, pkg, error)) return FALSE; - } - - /* find the location of the local file */ - fn = dnf_package_get_filename(pkg); - if (fn == NULL) { - g_set_error(error, - DNF_ERROR, - DNF_ERROR_FILE_NOT_FOUND, - "Downloaded file for %s not found", - dnf_package_get_name(pkg)); - return FALSE; - } - - /* check file */ - if (!dnf_keyring_check_untrusted_file(priv->keyring, fn, &error_local)) { - - /* probably an i/o error */ - if (!g_error_matches(error_local, - DNF_ERROR, - DNF_ERROR_GPG_SIGNATURE_INVALID)) { - g_propagate_error(error, error_local); - return FALSE; - } - - /* if the repo is signed this is ALWAYS an error */ - repo = dnf_package_get_repo(pkg); - if (repo != NULL && dnf_repo_get_gpgcheck(repo)) { - g_set_error(error, - DNF_ERROR, - DNF_ERROR_FILE_INVALID, - "package %s cannot be verified " - "and repo %s is GPG enabled: %s", - dnf_package_get_nevra(pkg), - dnf_repo_get_id(repo), - error_local->message); - g_error_free(error_local); - return FALSE; - } - - /* we can only install signed packages in this mode */ - if ((priv->flags & DNF_TRANSACTION_FLAG_ONLY_TRUSTED) > 0) { - g_propagate_error(error, error_local); - return FALSE; - } - - /* we can install unsigned packages */ - g_debug("ignoring as allow-untrusted: %s", - error_local->message); - g_clear_error(&error_local); - } } return TRUE; } @@ -1167,6 +1184,44 @@ dnf_transaction_reset(DnfTransaction *transaction) } /** + * dnf_transaction_import_keys: + * @transaction: a #DnfTransaction instance. + * @error: A #GError or %NULL + * + * Imports all keys from /etc/pki/rpm-gpg as well as any + * downloaded per-repo keys. Note this is called automatically + * by dnf_transaction_commit(). + **/ +gboolean +dnf_transaction_import_keys(DnfTransaction *transaction, + GError **error) +{ + guint i; + + DnfTransactionPrivate *priv = GET_PRIVATE(transaction); + /* import all system wide GPG keys */ + if (!dnf_keyring_add_public_keys(priv->keyring, error)) + return FALSE; + + /* import downloaded repo GPG keys */ + for (i = 0; i < priv->repos->len; i++) { + DnfRepo *repo = g_ptr_array_index(priv->repos, i); + const gchar *pubkey; + + /* does this file actually exist */ + pubkey = dnf_repo_get_public_key(repo); + if (g_file_test(pubkey, G_FILE_TEST_EXISTS)) { + /* import */ + if (!dnf_keyring_add_public_key(priv->keyring, pubkey, error)) + return FALSE; + } + } + + return TRUE; +} + + +/** * dnf_transaction_commit: * @transaction: a #DnfTransaction instance. * @goal: A #HyGoal @@ -1237,26 +1292,10 @@ dnf_transaction_commit(DnfTransaction *transaction, if (!ret) goto out; - /* import all system wide GPG keys */ - ret = dnf_keyring_add_public_keys(priv->keyring, error); + ret = dnf_transaction_import_keys(transaction, error); if (!ret) goto out; - /* import downloaded repo GPG keys */ - for (i = 0; i < priv->repos->len; i++) { - DnfRepo *repo = g_ptr_array_index(priv->repos, i); - const gchar *pubkey; - - /* does this file actually exist */ - pubkey = dnf_repo_get_public_key(repo); - if (g_file_test(pubkey, G_FILE_TEST_EXISTS)) { - /* import */ - ret = dnf_keyring_add_public_key(priv->keyring, pubkey, error); - if (!ret) - goto out; - } - } - /* find any packages without valid GPG signatures */ ret = dnf_transaction_check_untrusted(transaction, goal, error); if (!ret) diff --git a/libdnf/libdnf/dnf-transaction.h b/libdnf/libdnf/dnf-transaction.h index 6131374..84d1365 100644 --- a/libdnf/libdnf/dnf-transaction.h +++ b/libdnf/libdnf/dnf-transaction.h @@ -95,6 +95,17 @@ gboolean dnf_transaction_depsolve (DnfTransaction *transac gboolean dnf_transaction_download (DnfTransaction *transaction, DnfState *state, GError **error); + +gboolean dnf_transaction_import_keys (DnfTransaction *transaction, + GError **error); + +gboolean dnf_transaction_gpgcheck_package (DnfTransaction *transaction, + DnfPackage *pkg, + GError **error); +gboolean dnf_transaction_check_untrusted (DnfTransaction *transaction, + HyGoal goal, + GError **error); + gboolean dnf_transaction_commit (DnfTransaction *transaction, HyGoal goal, DnfState *state, -- 2.9.3