pull: Do GPG verify commit objects when using deltas
The fact that we weren't doing this is at best an oversight, and for some deployment models a security vulnerability. Having both `gpg-verify` and `gpg-verify-summary` shows that we were intending them to be orthogonal/independent. Lately I've been advocating moving towards pinned TLS instead of gpg-signed summaries, and if we follow that path, performing GPG verification of commit objects even if using deltas is more important, as it provides an at-rest verifiable authenticity and integrity mechanism. Content providers which are signing their summary files and/or using TLS (particularly pinned TLS) for transport should treat this as a nice-to-have. However, for providers which are serving content over plain HTTP and relying on GPG, this is a critical update. Closes: https://github.com/ostreedev/ostree/issues/517 Closes: #589 Approved by: jlebon
This commit is contained in:
Colin Walters
Atomic Bot
parent
cb57338a12
commit
41ef2aeb38
|
|
@ -1027,6 +1027,57 @@ static_deltapart_fetch_on_complete (GObject *object,
|
|||
fetch_static_delta_data_free (fetch_data);
|
||||
}
|
||||
|
||||
static gboolean
|
||||
process_verify_result (OtPullData *pull_data,
|
||||
const char *checksum,
|
||||
OstreeGpgVerifyResult *result,
|
||||
GError **error)
|
||||
{
|
||||
if (result == NULL)
|
||||
return FALSE;
|
||||
|
||||
/* Allow callers to output the results immediately. */
|
||||
g_signal_emit_by_name (pull_data->repo,
|
||||
"gpg-verify-result",
|
||||
checksum, result);
|
||||
|
||||
return ostree_gpg_verify_result_require_valid_signature (result, error);
|
||||
}
|
||||
|
||||
static gboolean
|
||||
gpg_verify_unwritten_commit (OtPullData *pull_data,
|
||||
const char *checksum,
|
||||
GVariant *commit,
|
||||
GVariant *detached_metadata,
|
||||
GCancellable *cancellable,
|
||||
GError **error)
|
||||
{
|
||||
if (pull_data->gpg_verify)
|
||||
{
|
||||
glnx_unref_object OstreeGpgVerifyResult *result = NULL;
|
||||
g_autoptr(GBytes) signed_data = g_variant_get_data_as_bytes (commit);
|
||||
|
||||
if (!detached_metadata)
|
||||
{
|
||||
g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED,
|
||||
"No detached metadata found for GPG verification");
|
||||
return FALSE;
|
||||
}
|
||||
|
||||
result = _ostree_repo_gpg_verify_with_metadata (pull_data->repo,
|
||||
signed_data,
|
||||
detached_metadata,
|
||||
pull_data->remote_name,
|
||||
NULL, NULL,
|
||||
cancellable,
|
||||
error);
|
||||
if (!process_verify_result (pull_data, checksum, result, error))
|
||||
return FALSE;
|
||||
}
|
||||
|
||||
return TRUE;
|
||||
}
|
||||
|
||||
static gboolean
|
||||
scan_commit_object (OtPullData *pull_data,
|
||||
const char *checksum,
|
||||
|
|
@ -1075,16 +1126,7 @@ scan_commit_object (OtPullData *pull_data,
|
|||
pull_data->remote_name,
|
||||
cancellable,
|
||||
error);
|
||||
|
||||
if (result == NULL)
|
||||
goto out;
|
||||
|
||||
/* Allow callers to output the results immediately. */
|
||||
g_signal_emit_by_name (pull_data->repo,
|
||||
"gpg-verify-result",
|
||||
checksum, result);
|
||||
|
||||
if (!ostree_gpg_verify_result_require_valid_signature (result, error))
|
||||
if (!process_verify_result (pull_data, checksum, result, error))
|
||||
goto out;
|
||||
}
|
||||
|
||||
|
|
@ -1563,7 +1605,6 @@ process_one_static_delta (OtPullData *pull_data,
|
|||
{
|
||||
g_autoptr(GVariant) to_csum_v = NULL;
|
||||
g_autofree char *to_checksum = NULL;
|
||||
g_autoptr(GVariant) to_commit = NULL;
|
||||
gboolean have_to_commit;
|
||||
|
||||
to_csum_v = g_variant_get_child_value (delta_superblock, 3);
|
||||
|
|
@ -1578,10 +1619,16 @@ process_one_static_delta (OtPullData *pull_data,
|
|||
if (!have_to_commit)
|
||||
{
|
||||
FetchObjectData *fetch_data;
|
||||
g_autoptr(GVariant) to_commit = g_variant_get_child_value (delta_superblock, 4);
|
||||
g_autofree char *detached_path = _ostree_get_relative_static_delta_path (from_revision, to_revision, "commitmeta");
|
||||
g_autoptr(GVariant) detached_data = NULL;
|
||||
|
||||
detached_data = g_variant_lookup_value (metadata, detached_path, G_VARIANT_TYPE("a{sv}"));
|
||||
|
||||
if (!gpg_verify_unwritten_commit (pull_data, to_revision, to_commit, detached_data,
|
||||
cancellable, error))
|
||||
goto out;
|
||||
|
||||
if (detached_data && !ostree_repo_write_commit_detached_metadata (pull_data->repo,
|
||||
to_revision,
|
||||
detached_data,
|
||||
|
|
@ -1595,8 +1642,6 @@ process_one_static_delta (OtPullData *pull_data,
|
|||
fetch_data->is_detached_meta = FALSE;
|
||||
fetch_data->object_is_stored = FALSE;
|
||||
|
||||
to_commit = g_variant_get_child_value (delta_superblock, 4);
|
||||
|
||||
ostree_repo_write_metadata_async (pull_data->repo, OSTREE_OBJECT_TYPE_COMMIT, to_checksum,
|
||||
to_commit,
|
||||
pull_data->cancellable,
|
||||
|
|
|
|||
|
|
@ -26,7 +26,7 @@ unset OSTREE_GPG_HOME
|
|||
|
||||
setup_fake_remote_repo1 "archive-z2"
|
||||
|
||||
echo "1..2"
|
||||
echo "1..4"
|
||||
|
||||
cd ${test_tmpdir}
|
||||
mkdir repo
|
||||
|
|
@ -171,4 +171,39 @@ fi
|
|||
assert_file_has_content err.txt "GPG signatures found, but none are in trusted keyring"
|
||||
|
||||
echo "ok"
|
||||
|
||||
# Test deltas with signed commits; this test is a bit
|
||||
# weird here but this file has separate per-remote keys.
|
||||
cd ${test_tmpdir}
|
||||
rm repo/refs/remotes/* -rf
|
||||
${OSTREE} prune --refs-only
|
||||
echo $(date) > workdir/testfile-for-deltas-1
|
||||
# Sign with keyid 1 for first commit
|
||||
${CMD_PREFIX} ostree --repo=${test_tmpdir}/ostree-srv/gnomerepo commit -b main --gpg-sign ${TEST_GPG_KEYID_1} --gpg-homedir ${test_tmpdir}/gpghome workdir
|
||||
prevrev=$(${CMD_PREFIX} ostree --repo=${test_tmpdir}/ostree-srv/gnomerepo rev-parse main)
|
||||
# Pull the previous revision
|
||||
${OSTREE} pull R1:main
|
||||
assert_streq $(${OSTREE} rev-parse R1:main) ${prevrev}
|
||||
# Sign with keyid 2, but use remote r1
|
||||
echo $(date) > workdir/testfile-for-deltas-2
|
||||
${CMD_PREFIX} ostree --repo=${test_tmpdir}/ostree-srv/gnomerepo commit -b main --gpg-sign ${TEST_GPG_KEYID_2} --gpg-homedir ${test_tmpdir}/gpghome workdir
|
||||
${CMD_PREFIX} ostree --repo=${test_tmpdir}/ostree-srv/gnomerepo static-delta generate main
|
||||
# Summary is signed with key1
|
||||
${CMD_PREFIX} ostree --repo=${test_tmpdir}/ostree-srv/gnomerepo summary -u --gpg-sign ${TEST_GPG_KEYID_1} --gpg-homedir ${test_tmpdir}/gpghome
|
||||
newrev=$(${CMD_PREFIX} ostree --repo=${test_tmpdir}/ostree-srv/gnomerepo rev-parse main)
|
||||
if ${OSTREE} pull --require-static-deltas R1:main 2>err.txt; then
|
||||
assert_not_reached "Unexpectedly succeeded at pulling commit signed with untrusted key"
|
||||
fi
|
||||
assert_file_has_content err.txt "GPG signatures found, but none are in trusted keyring"
|
||||
|
||||
echo "ok gpg untrusted signed commit for delta upgrades"
|
||||
|
||||
${CMD_PREFIX} ostree --repo=${test_tmpdir}/ostree-srv/gnomerepo reset main{,^}
|
||||
${CMD_PREFIX} ostree --repo=${test_tmpdir}/ostree-srv/gnomerepo commit -b main --gpg-sign ${TEST_GPG_KEYID_1} --gpg-homedir ${test_tmpdir}/gpghome workdir
|
||||
${CMD_PREFIX} ostree --repo=${test_tmpdir}/ostree-srv/gnomerepo static-delta generate main
|
||||
${CMD_PREFIX} ostree --repo=${test_tmpdir}/ostree-srv/gnomerepo summary -u --gpg-sign ${TEST_GPG_KEYID_1} --gpg-homedir ${test_tmpdir}/gpghome
|
||||
${OSTREE} pull --require-static-deltas R1:main
|
||||
|
||||
echo "ok gpg trusted signed commit for delta upgrades"
|
||||
|
||||
libtest_cleanup_gpg
|
||||
|
|
|
|||
Loading…
Reference in New Issue