diff --git a/src/libostree/ostree-repo-pull-private.h b/src/libostree/ostree-repo-pull-private.h index 86d1ffee..fd17baee 100644 --- a/src/libostree/ostree-repo-pull-private.h +++ b/src/libostree/ostree-repo-pull-private.h @@ -67,8 +67,6 @@ typedef struct { gboolean gpg_verify; gboolean gpg_verify_summary; - gboolean sign_verify; - gboolean sign_verify_summary; gboolean require_static_deltas; gboolean disable_static_deltas; gboolean has_tombstone_commits; @@ -124,7 +122,8 @@ typedef struct { gboolean is_commit_only; OstreeRepoImportFlags importflags; - GPtrArray *signapi_verifiers; + GPtrArray *signapi_commit_verifiers; + GPtrArray *signapi_summary_verifiers; GPtrArray *dirs; @@ -140,11 +139,12 @@ typedef struct { GSource *idle_src; } OtPullData; -GPtrArray * -_signapi_verifiers_for_remote (OstreeRepo *repo, - const char *remote_name, - GError **error); - +gboolean +_signapi_init_for_remote (OstreeRepo *repo, + const char *remote_name, + GPtrArray **out_commit_verifiers, + GPtrArray **out_summary_verifiers, + GError **error); gboolean _sign_verify_for_remote (GPtrArray *signers, GBytes *signed_data, diff --git a/src/libostree/ostree-repo-pull-verify.c b/src/libostree/ostree-repo-pull-verify.c index 36d877ac..ab680daf 100644 --- a/src/libostree/ostree-repo-pull-verify.c +++ b/src/libostree/ostree-repo-pull-verify.c @@ -71,6 +71,7 @@ static gboolean _signapi_load_public_keys (OstreeSign *sign, OstreeRepo *repo, const gchar *remote_name, + gboolean required, GError **error) { g_autofree gchar *pk_ascii = NULL; @@ -95,6 +96,8 @@ _signapi_load_public_keys (OstreeSign *sign, * and call it here for loading with method and file structure * specific for signature type. */ + if (required) + return glnx_throw (error, "No keys found for required signapi type %s", ostree_sign_get_name (sign)); return TRUE; } @@ -142,31 +145,120 @@ _signapi_load_public_keys (OstreeSign *sign, return TRUE; } -/* Create a new array of OstreeSign objects and load the public - * keys as described by the remote configuration. - */ -GPtrArray * -_signapi_verifiers_for_remote (OstreeRepo *repo, - const char *remote_name, - GError **error) +static gboolean +string_is_gkeyfile_truthy (const char *value, + gboolean *out_truth) { - g_autoptr(GPtrArray) signers = ostree_sign_get_all (); - g_assert_cmpuint (signers->len, >=, 1); - for (guint i = 0; i < signers->len; i++) + /* See https://gitlab.gnome.org/GNOME/glib/-/blob/20fb5bf868added5aec53c013ae85ec78ba2eedc/glib/gkeyfile.c#L4528 */ + if (g_str_equal (value, "true") || g_str_equal (value, "1")) { - OstreeSign *sign = signers->pdata[i]; - /* Try to load public key(s) according remote's configuration */ - if (!_signapi_load_public_keys (sign, repo, remote_name, error)) - return FALSE; + *out_truth = TRUE; + return TRUE; } - return g_steal_pointer (&signers); + else if (g_str_equal (value, "false") || g_str_equal (value, "0")) + { + *out_truth = FALSE; + return TRUE; + } + return FALSE; } -/* Iterate over the configured signers, and require the commit is signed +static gboolean +verifiers_from_config (OstreeRepo *repo, + const char *remote_name, + const char *key, + GPtrArray **out_verifiers, + GError **error) +{ + g_autoptr(GPtrArray) verifiers = NULL; + + g_autofree char *raw_value = NULL; + if (!ostree_repo_get_remote_option (repo, remote_name, + key, NULL, + &raw_value, error)) + return FALSE; + if (raw_value == NULL || g_str_equal (raw_value, "")) + { + *out_verifiers = NULL; + return TRUE; + } + gboolean sign_verify_bool = FALSE; + /* Is the value "truthy" according to GKeyFile's rules? If so, + * then we take this to be "accept signatures from any compiled + * type that happens to have keys configured". + */ + if (string_is_gkeyfile_truthy (raw_value, &sign_verify_bool)) + { + if (sign_verify_bool) + { + verifiers = ostree_sign_get_all (); + for (guint i = 0; i < verifiers->len; i++) + { + OstreeSign *sign = verifiers->pdata[i]; + /* Try to load public key(s) according remote's configuration; + * this one is optional. + */ + if (!_signapi_load_public_keys (sign, repo, remote_name, FALSE, error)) + return FALSE; + } + } + } + else + { + /* If the value isn't "truthy", then it must be an explicit list */ + g_auto(GStrv) sign_types = NULL; + if (!ostree_repo_get_remote_list_option (repo, remote_name, + key, &sign_types, + error)) + return FALSE; + verifiers = g_ptr_array_new_with_free_func ((GDestroyNotify) g_object_unref); + for (char **iter = sign_types; iter && *iter; iter++) + { + const char *sign_type = *iter; + OstreeSign *verifier = ostree_sign_get_by_name (sign_type, error); + if (!verifier) + return FALSE; + if (!_signapi_load_public_keys (verifier, repo, remote_name, TRUE, error)) + return FALSE; + g_ptr_array_add (verifiers, verifier); + } + g_assert_cmpuint (verifiers->len, >=, 1); + } + + *out_verifiers = g_steal_pointer (&verifiers); + return TRUE; +} + +/* Create a new array of OstreeSign objects and load the public + * keys as described by the remote configuration. If the + * remote does not have signing verification enabled, then + * the resulting verifier list will be NULL. + */ +gboolean +_signapi_init_for_remote (OstreeRepo *repo, + const char *remote_name, + GPtrArray **out_commit_verifiers, + GPtrArray **out_summary_verifiers, + GError **error) +{ + g_autoptr(GPtrArray) commit_verifiers = NULL; + g_autoptr(GPtrArray) summary_verifiers = NULL; + + if (!verifiers_from_config (repo, remote_name, "sign-verify", &commit_verifiers, error)) + return FALSE; + if (!verifiers_from_config (repo, remote_name, "sign-verify-summary", &summary_verifiers, error)) + return FALSE; + + ot_transfer_out_value (out_commit_verifiers, &commit_verifiers); + ot_transfer_out_value (out_summary_verifiers, &summary_verifiers); + return TRUE; +} + +/* Iterate over the configured verifiers, and require the commit is signed * by at least one. */ gboolean -_sign_verify_for_remote (GPtrArray *signers, +_sign_verify_for_remote (GPtrArray *verifiers, GBytes *signed_data, GVariant *metadata, GError **error) @@ -175,10 +267,10 @@ _sign_verify_for_remote (GPtrArray *signers, g_autoptr (GError) last_sig_error = NULL; gboolean found_sig = FALSE; - g_assert_cmpuint (signers->len, >=, 1); - for (guint i = 0; i < signers->len; i++) + g_assert_cmpuint (verifiers->len, >=, 1); + for (guint i = 0; i < verifiers->len; i++) { - OstreeSign *sign = signers->pdata[i]; + OstreeSign *sign = verifiers->pdata[i]; const gchar *signature_key = ostree_sign_metadata_key (sign); GVariantType *signature_format = (GVariantType *) ostree_sign_metadata_format (sign); g_autoptr (GVariant) signatures = @@ -257,7 +349,7 @@ _verify_unwritten_commit (OtPullData *pull_data, GError **error) { - if (pull_data->gpg_verify || pull_data->sign_verify) + if (pull_data->gpg_verify || pull_data->signapi_commit_verifiers) /* Shouldn't happen, but see comment in process_gpg_verify_result() */ if (g_hash_table_contains (pull_data->verified_commits, checksum)) return TRUE; @@ -284,13 +376,13 @@ _verify_unwritten_commit (OtPullData *pull_data, } #endif /* OSTREE_DISABLE_GPGME */ - if (pull_data->sign_verify) + if (pull_data->signapi_commit_verifiers) { /* Nothing to check if detached metadata is absent */ if (detached_metadata == NULL) return glnx_throw (error, "Can't verify commit without detached metadata"); - if (!_sign_verify_for_remote (pull_data->signapi_verifiers, signed_data, detached_metadata, error)) + if (!_sign_verify_for_remote (pull_data->signapi_commit_verifiers, signed_data, detached_metadata, error)) return glnx_prefix_error (error, "Can't verify commit"); /* Mark the commit as verified to avoid double verification diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 7116c3dc..5a57bfa6 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -1537,17 +1537,16 @@ scan_commit_object (OtPullData *pull_data, } #endif /* OSTREE_DISABLE_GPGME */ - if (pull_data->sign_verify && + if (pull_data->signapi_commit_verifiers && !g_hash_table_contains (pull_data->verified_commits, checksum)) { g_autoptr(GError) last_verification_error = NULL; gboolean found_any_signature = FALSE; gboolean found_valid_signature = FALSE; - g_assert (pull_data->signapi_verifiers); - for (guint i = 0; i < pull_data->signapi_verifiers->len; i++) + for (guint i = 0; i < pull_data->signapi_commit_verifiers->len; i++) { - OstreeSign *sign = pull_data->signapi_verifiers->pdata[i]; + OstreeSign *sign = pull_data->signapi_commit_verifiers->pdata[i]; found_any_signature = TRUE; @@ -3552,31 +3551,6 @@ ostree_repo_pull_with_options (OstreeRepo *self, if (!ostree_repo_remote_get_gpg_verify_summary (self, pull_data->remote_name, &pull_data->gpg_verify_summary, error)) goto out; - /* signapi differs from GPG in that it can only be explicitly *disabled* - * transiently during pulls, not enabled. - */ - if (disable_sign_verify) - { - pull_data->sign_verify = FALSE; - } - else - { - if (!ostree_repo_get_remote_boolean_option (self, pull_data->remote_name, - "sign-verify", FALSE, - &pull_data->sign_verify, error)) - goto out; - } - if (disable_sign_verify_summary) - { - pull_data->sign_verify_summary = FALSE; - } - else - { - if (!ostree_repo_get_remote_boolean_option (self, pull_data->remote_name, - "sign-verify-summary", FALSE, - &pull_data->sign_verify_summary, error)) - goto out; - } /* NOTE: If changing this, see the matching implementation in * ostree-sysroot-upgrader.c @@ -3595,13 +3569,13 @@ ostree_repo_pull_with_options (OstreeRepo *self, } } - if (pull_data->sign_verify || pull_data->sign_verify_summary) + if (pull_data->remote_name && !(disable_sign_verify && disable_sign_verify_summary)) { - g_assert (pull_data->remote_name != NULL); - pull_data->signapi_verifiers = _signapi_verifiers_for_remote (pull_data->repo, pull_data->remote_name, error); - if (!pull_data->signapi_verifiers) - goto out; - g_assert_cmpint (pull_data->signapi_verifiers->len, >=, 1); + if (!_signapi_init_for_remote (pull_data->repo, pull_data->remote_name, + &pull_data->signapi_commit_verifiers, + &pull_data->signapi_summary_verifiers, + error)) + return FALSE; } pull_data->phase = OSTREE_PULL_PHASE_FETCHING_REFS; @@ -3967,9 +3941,9 @@ ostree_repo_pull_with_options (OstreeRepo *self, } #endif /* OSTREE_DISABLE_GPGME */ - if (pull_data->sign_verify_summary) + if (pull_data->signapi_summary_verifiers) { - if (!bytes_sig && pull_data->sign_verify_summary) + if (!bytes_sig && pull_data->signapi_summary_verifiers) { g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, "Signatures verification enabled, but no summary.sig found (use sign-verify-summary=false in remote config to disable)"); @@ -3984,8 +3958,8 @@ ostree_repo_pull_with_options (OstreeRepo *self, bytes_sig, FALSE); - g_assert (pull_data->signapi_verifiers); - if (!_sign_verify_for_remote (pull_data->signapi_verifiers, bytes_summary, signatures, &temp_error)) + g_assert (pull_data->signapi_summary_verifiers); + if (!_sign_verify_for_remote (pull_data->signapi_summary_verifiers, bytes_summary, signatures, &temp_error)) { if (summary_from_cache) { @@ -4014,7 +3988,7 @@ ostree_repo_pull_with_options (OstreeRepo *self, cancellable, error)) goto out; - if (!_sign_verify_for_remote (pull_data->signapi_verifiers, bytes_summary, signatures, error)) + if (!_sign_verify_for_remote (pull_data->signapi_summary_verifiers, bytes_summary, signatures, error)) goto out; } else @@ -4536,7 +4510,7 @@ ostree_repo_pull_with_options (OstreeRepo *self, g_string_append_printf (msg, "\nsecurity: GPG: %s ", gpg_verify_state); const char *sign_verify_state; - sign_verify_state = (pull_data->sign_verify ? "commit" : "disabled"); + sign_verify_state = (pull_data->signapi_commit_verifiers ? "commit" : "disabled"); g_string_append_printf (msg, "\nsecurity: SIGN: %s ", sign_verify_state); OstreeFetcherURI *first_uri = pull_data->meta_mirrorlist->pdata[0]; @@ -4629,7 +4603,8 @@ ostree_repo_pull_with_options (OstreeRepo *self, g_free (pull_data->remote_refspec_name); g_free (pull_data->remote_name); g_free (pull_data->append_user_agent); - g_clear_pointer (&pull_data->signapi_verifiers, (GDestroyNotify) g_ptr_array_unref); + g_clear_pointer (&pull_data->signapi_commit_verifiers, (GDestroyNotify) g_ptr_array_unref); + g_clear_pointer (&pull_data->signapi_summary_verifiers, (GDestroyNotify) g_ptr_array_unref); g_clear_pointer (&pull_data->meta_mirrorlist, (GDestroyNotify) g_ptr_array_unref); g_clear_pointer (&pull_data->content_mirrorlist, (GDestroyNotify) g_ptr_array_unref); g_clear_pointer (&pull_data->summary_data, (GDestroyNotify) g_bytes_unref); @@ -6050,7 +6025,7 @@ ostree_repo_remote_fetch_summary_with_options (OstreeRepo *self, g_autoptr(GBytes) summary = NULL; g_autoptr(GBytes) signatures = NULL; gboolean gpg_verify_summary; - gboolean sign_verify_summary; + g_autoptr(GPtrArray) signapi_summary_verifiers = NULL; gboolean ret = FALSE; gboolean summary_is_from_cache; @@ -6107,11 +6082,12 @@ ostree_repo_remote_fetch_summary_with_options (OstreeRepo *self, } } - if (!ostree_repo_get_remote_boolean_option (self, name, "sign-verify-summary", - FALSE, &sign_verify_summary, error)) - goto out; + if (!_signapi_init_for_remote (self, name, NULL, + &signapi_summary_verifiers, + error)) + goto out; - if (sign_verify_summary) + if (signapi_summary_verifiers) { if (summary == NULL) { @@ -6134,10 +6110,8 @@ ostree_repo_remote_fetch_summary_with_options (OstreeRepo *self, sig_variant = g_variant_new_from_bytes (OSTREE_SUMMARY_SIG_GVARIANT_FORMAT, signatures, FALSE); - g_autoptr(GPtrArray) signapi_verifiers = _signapi_verifiers_for_remote (self, name, error); - if (!signapi_verifiers) - goto out; - if (!_sign_verify_for_remote (signapi_verifiers, summary, sig_variant, error)) + + if (!_sign_verify_for_remote (signapi_summary_verifiers, summary, sig_variant, error)) goto out; } } diff --git a/tests/test-signed-pull.sh b/tests/test-signed-pull.sh index b207eac2..fe78321a 100755 --- a/tests/test-signed-pull.sh +++ b/tests/test-signed-pull.sh @@ -23,7 +23,7 @@ set -euo pipefail . $(dirname $0)/libtest.sh -echo "1..16" +echo "1..20" # This is explicitly opt in for testing export OSTREE_DUMMY_SIGN_ENABLED=1 @@ -102,6 +102,31 @@ test_signed_pull "dummy" "" repo_init --sign-verify=dummy=inline:${DUMMYSIGN} test_signed_pull "dummy" "from remote opt" +# And now explicitly limit it to dummy +repo_init +${CMD_PREFIX} ostree --repo=repo config set 'remote "origin"'.sign-verify dummy +${CMD_PREFIX} ostree --repo=repo config set 'remote "origin"'.verification-dummy-key "${DUMMYSIGN}" +test_signed_pull "dummy" "explicit value" + +# dummy, but no key configured +repo_init +${CMD_PREFIX} ostree --repo=repo config set 'remote "origin"'.sign-verify dummy +if ${CMD_PREFIX} ostree --repo=repo pull origin main 2>err.txt; then + assert_not_reached "pull with nosuchsystem succeeded" +fi +assert_file_has_content err.txt 'No keys found for required signapi type dummy' +echo "ok explicit dummy but unconfigured" + +# Set it to an unknown explicit value +repo_init +${CMD_PREFIX} ostree --repo=repo config set 'remote "origin"'.sign-verify nosuchsystem; +${CMD_PREFIX} ostree --repo=repo config set 'remote "origin"'.verification-dummy-key "${DUMMYSIGN}" +if ${CMD_PREFIX} ostree --repo=repo pull origin main 2>err.txt; then + assert_not_reached "pull with nosuchsystem succeeded" +fi +assert_file_has_content err.txt 'Requested signature type is not implemented' +echo "ok pull failure for unknown system" + repo_init if ${CMD_PREFIX} ostree --repo=repo remote add other --sign-verify=trustme=inline:ok http://localhost 2>err.txt; then assert_not_reached "remote add with invalid keytype succeeded"