pull: Update key loading function to match error style

This code wasn't written with idiomatic GError usage; it's not standard
to construct an error up front and continually append to its
message.  The exit from a function is usually `return TRUE`,
with error conditions before that.

Updating it to match style reveals what I think is a bug;
we were silently ignoring failure to parse key files.
This commit is contained in:
Colin Walters 2020-04-05 18:22:49 +00:00
parent a16fe86b36
commit 47539874b8
2 changed files with 8 additions and 16 deletions

View File

@ -1488,14 +1488,10 @@ _load_public_keys (OstreeSign *sign,
const gchar *remote_name, const gchar *remote_name,
GError **error) GError **error)
{ {
g_autofree gchar *pk_ascii = NULL; g_autofree gchar *pk_ascii = NULL;
g_autofree gchar *pk_file = NULL; g_autofree gchar *pk_file = NULL;
gboolean loaded_from_file = TRUE; gboolean loaded_from_file = TRUE;
gboolean loaded_inlined = TRUE; gboolean loaded_inlined = TRUE;
g_autoptr (GError) verification_error = NULL;
glnx_throw (&verification_error, "no public keys loaded");
ostree_repo_get_remote_option (repo, ostree_repo_get_remote_option (repo,
remote_name, remote_name,
@ -1536,10 +1532,8 @@ _load_public_keys (OstreeSign *sign,
loaded_from_file = TRUE; loaded_from_file = TRUE;
else else
{ {
g_debug("Unable to load public keys for '%s' from file '%s': %s", return glnx_throw (error, "Failed loading '%s' keys from '%s",
ostree_sign_get_name(sign), pk_file, local_error->message); ostree_sign_get_name (sign), pk_file);
/* Save error message for better reason detection later if needed */
glnx_prefix_error (&verification_error, "%s", local_error->message);
} }
} }
@ -1556,19 +1550,16 @@ _load_public_keys (OstreeSign *sign,
if (!loaded_inlined) if (!loaded_inlined)
{ {
g_debug("Unable to load public key '%s' for '%s': %s", return glnx_throw (error, "Failed loading '%s' keys from inline `verification-key`",
pk_ascii, ostree_sign_get_name (sign), local_error->message); ostree_sign_get_name (sign));
/* Save error message for better reason detection later if needed */
glnx_prefix_error (&verification_error, "%s", local_error->message);
} }
} }
/* Return true if able to load from any source */ /* Return true if able to load from any source */
if (loaded_from_file || loaded_inlined) if (!(loaded_from_file || loaded_inlined))
return TRUE; return glnx_throw (error, "No keys found");
return glnx_throw (error, "%s", verification_error->message); return TRUE;
} }
static gboolean static gboolean

View File

@ -93,6 +93,7 @@ echo "ok pull failure with incorrect keys file option"
# Test with correct dummy key # Test with correct dummy key
${CMD_PREFIX} ostree --repo=repo config set 'remote "origin"'.verification-key "${DUMMYSIGN}" ${CMD_PREFIX} ostree --repo=repo config set 'remote "origin"'.verification-key "${DUMMYSIGN}"
${CMD_PREFIX} ostree --repo=repo config unset 'remote "origin"'.verification-file
test_signed_pull "dummy" "" test_signed_pull "dummy" ""
if ! has_libsodium; then if ! has_libsodium; then