Skip to content

Commit

Permalink
Merge pull request #2105 from cgwalters/pull-signapi-explicit
Browse files Browse the repository at this point in the history
pull: Add support for sign-verify=<list>
  • Loading branch information
openshift-merge-robot authored May 24, 2020
2 parents 8801e38 + 5cb9d0d commit 8e02597
Show file tree
Hide file tree
Showing 4 changed files with 173 additions and 82 deletions.
16 changes: 8 additions & 8 deletions src/libostree/ostree-repo-pull-private.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand All @@ -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,
Expand Down
136 changes: 114 additions & 22 deletions src/libostree/ostree-repo-pull-verify.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
}

Expand Down Expand Up @@ -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)
{
/* See https://gitlab.gnome.org/GNOME/glib/-/blob/20fb5bf868added5aec53c013ae85ec78ba2eedc/glib/gkeyfile.c#L4528 */
if (g_str_equal (value, "true") || g_str_equal (value, "1"))
{
*out_truth = TRUE;
return TRUE;
}
else if (g_str_equal (value, "false") || g_str_equal (value, "0"))
{
*out_truth = FALSE;
return TRUE;
}
return FALSE;
}

static gboolean
verifiers_from_config (OstreeRepo *repo,
const char *remote_name,
const char *key,
GPtrArray **out_verifiers,
GError **error)
{
g_autoptr(GPtrArray) signers = ostree_sign_get_all ();
g_assert_cmpuint (signers->len, >=, 1);
for (guint i = 0; i < signers->len; i++)
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))
{
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))
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);
}
return g_steal_pointer (&signers);

*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 signers, and require the commit is signed
/* 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)
Expand All @@ -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 =
Expand Down Expand Up @@ -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;
Expand All @@ -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
Expand Down
76 changes: 25 additions & 51 deletions src/libostree/ostree-repo-pull.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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
Expand All @@ -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;
Expand Down Expand Up @@ -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)");
Expand All @@ -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)
{
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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];
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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)
{
Expand All @@ -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;
}
}
Expand Down
Loading

0 comments on commit 8e02597

Please sign in to comment.