Add more error prefixing when parsing commit objects

I've got more debug information in the error case that motivated
bae4347abe
"pull: Add error prefixing for corrupt checksums"
where the sole error is

`error: Invalid checksum of length 0 expected 32`

This must be coming from the pull code in the case where we've
already fetched the commit object.

- Add some error prefixing here in the core commit validation code
- Ensure that we do the validation immediately after loading, including
  of the parent commit reference where I think this error must be coming
  from
- Then the pull code can just safely call `ostree_commit_get_parent`
  which already does the hex conversion etc.
This commit is contained in:
Colin Walters 2023-06-15 14:26:12 -04:00
parent ff805f1c4f
commit c71fc3d994
2 changed files with 17 additions and 23 deletions

View File

@ -2152,18 +2152,18 @@ ostree_validate_structureof_commit (GVariant *commit, GError **error)
if (n_elts > 0)
{
if (!ostree_validate_structureof_csum_v (parent_csum_v, error))
return FALSE;
return glnx_prefix_error (error, "Invalid commit parent");
}
g_autoptr (GVariant) content_csum_v = NULL;
g_variant_get_child (commit, 6, "@ay", &content_csum_v);
if (!ostree_validate_structureof_csum_v (content_csum_v, error))
return FALSE;
return glnx_prefix_error (error, "Invalid commit tree content checksum");
g_autoptr (GVariant) metadata_csum_v = NULL;
g_variant_get_child (commit, 7, "@ay", &metadata_csum_v);
if (!ostree_validate_structureof_csum_v (metadata_csum_v, error))
return FALSE;
return glnx_prefix_error (error, "Invalid commit tree metadata checksum");
return TRUE;
}

View File

@ -1538,6 +1538,10 @@ scan_commit_object (OtPullData *pull_data, const char *checksum, guint recursion
if (!ostree_repo_load_commit (pull_data->repo, checksum, &commit, &commitstate, error))
return FALSE;
/* Do this early because if it's corrupt, something else is going wrong */
if (!ostree_validate_structureof_commit (commit, error))
return glnx_prefix_error (error, "Validating commit %s", checksum);
if (!pull_data->disable_verify_bindings)
{
/* If ref is non-NULL then the commit we fetched was requested through
@ -1590,27 +1594,17 @@ scan_commit_object (OtPullData *pull_data, const char *checksum, guint recursion
/* If we found a legacy transaction flag, assume all commits are partial */
gboolean is_partial = commitstate_is_partial (pull_data, commitstate);
/* PARSE OSTREE_SERIALIZED_COMMIT_VARIANT */
g_autoptr (GVariant) parent_csum = NULL;
const guchar *parent_csum_bytes = NULL;
g_variant_get_child (commit, 1, "@ay", &parent_csum);
if (g_variant_n_children (parent_csum) > 0)
if (pull_data->maxdepth == -1 || depth > 0)
{
parent_csum_bytes = ostree_checksum_bytes_peek_validate (parent_csum, error);
if (parent_csum_bytes == NULL)
return FALSE;
}
if (parent_csum_bytes != NULL && (pull_data->maxdepth == -1 || depth > 0))
{
char parent_checksum[OSTREE_SHA256_STRING_LEN + 1];
ostree_checksum_inplace_from_bytes (parent_csum_bytes, parent_checksum);
int parent_depth = (depth > 0) ? depth - 1 : -1;
g_hash_table_insert (pull_data->commit_to_depth, g_strdup (parent_checksum),
GINT_TO_POINTER (parent_depth));
queue_scan_one_metadata_object_c (pull_data, parent_csum_bytes, OSTREE_OBJECT_TYPE_COMMIT,
NULL, recursion_depth + 1, NULL);
g_autofree char *parent_checksum = ostree_commit_get_parent (commit);
if (parent_checksum)
{
int parent_depth = (depth > 0) ? depth - 1 : -1;
g_hash_table_insert (pull_data->commit_to_depth, g_strdup (parent_checksum),
GINT_TO_POINTER (parent_depth));
queue_scan_one_metadata_object (pull_data, parent_checksum, OSTREE_OBJECT_TYPE_COMMIT,
NULL, recursion_depth + 1, NULL);
}
}
/* We only recurse to looking whether we need dirtree/dirmeta