lib: Always checksum content in deltas

This is a follow up to conversation on list - in practice, if we're
backing away from summary signing, then it makes sense to remove the
special casing for checksums in deltas around summary signatures.

This is also related to the recent change to enable GPG checking for
commits in deltas - now we have a more coherent story between the
previous pull path and deltas.

I didn't do any performance checking, and while it's slightly annoying
that we're now doing sha256 on the delta content twice (once for the
part and once per object)...sha256 is pretty fast, I think most users
are I/O bound anyways, and it'd drop even farther if we started using
openssl.

Closes: #612
Approved by: jlebon
This commit is contained in:
Colin Walters 2016-12-05 17:22:46 -05:00 committed by Atomic Bot
parent 66da1199f0
commit 4c8fc92aa0
4 changed files with 27 additions and 67 deletions

View File

@ -1010,8 +1010,6 @@ static_deltapart_fetch_on_complete (GObject *object,
_ostree_static_delta_part_execute_async (pull_data->repo, _ostree_static_delta_part_execute_async (pull_data->repo,
fetch_data->objects, fetch_data->objects,
part, part,
/* Trust checksums if summary was gpg signed */
pull_data->gpg_verify_summary && pull_data->summary_data_sig,
pull_data->cancellable, pull_data->cancellable,
on_static_delta_written, on_static_delta_written,
fetch_data); fetch_data);
@ -1661,7 +1659,6 @@ process_one_static_delta (OtPullData *pull_data,
g_autoptr(GBytes) inline_part_bytes = NULL; g_autoptr(GBytes) inline_part_bytes = NULL;
guint64 size, usize; guint64 size, usize;
guint32 version; guint32 version;
const gboolean trusted = pull_data->gpg_verify_summary && pull_data->summary_data_sig;
header = g_variant_get_child_value (headers, i); header = g_variant_get_child_value (headers, i);
g_variant_get (header, "(u@aytt@ay)", &version, &csum_v, &size, &usize, &objects); g_variant_get (header, "(u@aytt@ay)", &version, &csum_v, &size, &usize, &objects);
@ -1731,7 +1728,6 @@ process_one_static_delta (OtPullData *pull_data,
_ostree_static_delta_part_execute_async (pull_data->repo, _ostree_static_delta_part_execute_async (pull_data->repo,
fetch_data->objects, fetch_data->objects,
inline_delta_part, inline_delta_part,
trusted,
pull_data->cancellable, pull_data->cancellable,
on_static_delta_written, on_static_delta_written,
fetch_data); fetch_data);

View File

@ -435,8 +435,7 @@ ostree_repo_static_delta_execute_offline (OstreeRepo *self,
} }
if (!_ostree_static_delta_part_execute (self, objects, part, skip_validation, if (!_ostree_static_delta_part_execute (self, objects, part, skip_validation,
FALSE, NULL, NULL, cancellable, error))
cancellable, error))
{ {
g_prefix_error (error, "Executing delta part %i: ", i); g_prefix_error (error, "Executing delta part %i: ", i);
goto out; goto out;
@ -653,7 +652,7 @@ show_one_part (OstreeRepo *self,
(guint64)g_variant_n_children (ops)); (guint64)g_variant_n_children (ops));
if (!_ostree_static_delta_part_execute (self, objects, if (!_ostree_static_delta_part_execute (self, objects,
part, TRUE, TRUE, part, TRUE,
&stats, cancellable, error)) &stats, cancellable, error))
goto out; goto out;

View File

@ -141,7 +141,6 @@ typedef struct {
gboolean _ostree_static_delta_part_execute (OstreeRepo *repo, gboolean _ostree_static_delta_part_execute (OstreeRepo *repo,
GVariant *header, GVariant *header,
GVariant *part_payload, GVariant *part_payload,
gboolean trusted,
gboolean stats_only, gboolean stats_only,
OstreeDeltaExecuteStats *stats, OstreeDeltaExecuteStats *stats,
GCancellable *cancellable, GCancellable *cancellable,
@ -150,7 +149,6 @@ gboolean _ostree_static_delta_part_execute (OstreeRepo *repo,
void _ostree_static_delta_part_execute_async (OstreeRepo *repo, void _ostree_static_delta_part_execute_async (OstreeRepo *repo,
GVariant *header, GVariant *header,
GVariant *part_payload, GVariant *part_payload,
gboolean trusted,
GCancellable *cancellable, GCancellable *cancellable,
GAsyncReadyCallback callback, GAsyncReadyCallback callback,
gpointer user_data); gpointer user_data);

View File

@ -39,7 +39,6 @@
G_STATIC_ASSERT (sizeof (guint) >= sizeof (guint32)); G_STATIC_ASSERT (sizeof (guint) >= sizeof (guint32));
typedef struct { typedef struct {
gboolean trusted;
gboolean stats_only; gboolean stats_only;
OstreeRepo *repo; OstreeRepo *repo;
guint checksum_index; guint checksum_index;
@ -180,7 +179,6 @@ gboolean
_ostree_static_delta_part_execute (OstreeRepo *repo, _ostree_static_delta_part_execute (OstreeRepo *repo,
GVariant *objects, GVariant *objects,
GVariant *part, GVariant *part,
gboolean trusted,
gboolean stats_only, gboolean stats_only,
OstreeDeltaExecuteStats *stats, OstreeDeltaExecuteStats *stats,
GCancellable *cancellable, GCancellable *cancellable,
@ -200,7 +198,6 @@ _ostree_static_delta_part_execute (OstreeRepo *repo,
state->repo = repo; state->repo = repo;
state->async_error = error; state->async_error = error;
state->trusted = trusted;
state->stats_only = stats_only; state->stats_only = stats_only;
if (!_ostree_static_delta_parse_checksum_array (objects, if (!_ostree_static_delta_parse_checksum_array (objects,
@ -296,7 +293,6 @@ typedef struct {
GVariant *part; GVariant *part;
GCancellable *cancellable; GCancellable *cancellable;
GSimpleAsyncResult *result; GSimpleAsyncResult *result;
gboolean trusted;
} StaticDeltaPartExecuteAsyncData; } StaticDeltaPartExecuteAsyncData;
static void static void
@ -323,7 +319,6 @@ static_delta_part_execute_thread (GSimpleAsyncResult *res,
if (!_ostree_static_delta_part_execute (data->repo, if (!_ostree_static_delta_part_execute (data->repo,
data->header, data->header,
data->part, data->part,
data->trusted,
FALSE, NULL, FALSE, NULL,
cancellable, &error)) cancellable, &error))
g_simple_async_result_take_error (res, error); g_simple_async_result_take_error (res, error);
@ -333,7 +328,6 @@ void
_ostree_static_delta_part_execute_async (OstreeRepo *repo, _ostree_static_delta_part_execute_async (OstreeRepo *repo,
GVariant *header, GVariant *header,
GVariant *part, GVariant *part,
gboolean trusted,
GCancellable *cancellable, GCancellable *cancellable,
GAsyncReadyCallback callback, GAsyncReadyCallback callback,
gpointer user_data) gpointer user_data)
@ -344,7 +338,6 @@ _ostree_static_delta_part_execute_async (OstreeRepo *repo,
asyncdata->repo = g_object_ref (repo); asyncdata->repo = g_object_ref (repo);
asyncdata->header = g_variant_ref (header); asyncdata->header = g_variant_ref (header);
asyncdata->part = g_variant_ref (part); asyncdata->part = g_variant_ref (part);
asyncdata->trusted = trusted;
asyncdata->cancellable = cancellable ? g_object_ref (cancellable) : NULL; asyncdata->cancellable = cancellable ? g_object_ref (cancellable) : NULL;
asyncdata->result = g_simple_async_result_new ((GObject*) repo, asyncdata->result = g_simple_async_result_new ((GObject*) repo,
@ -517,9 +510,9 @@ dispatch_bspatch (OstreeRepo *repo,
return ret; return ret;
} }
/* When processing untrusted static deltas, we need to checksum the /* Before, we had a distinction between "trusted" and "untrusted" deltas
* file content, which includes a header. Compare with what * which we've decided wasn't a good idea. Now, we always checksum the content.
* ostree_checksum_file_from_input() is doing too. * Compare with what ostree_checksum_file_from_input() is doing too.
*/ */
static gboolean static gboolean
handle_untrusted_content_checksum (OstreeRepo *repo, handle_untrusted_content_checksum (OstreeRepo *repo,
@ -531,9 +524,6 @@ handle_untrusted_content_checksum (OstreeRepo *repo,
g_autoptr(GFileInfo) finfo = NULL; g_autoptr(GFileInfo) finfo = NULL;
gsize bytes_written; gsize bytes_written;
if (state->trusted)
return TRUE;
finfo = _ostree_header_gfile_info_new (state->mode, state->uid, state->gid); finfo = _ostree_header_gfile_info_new (state->mode, state->uid, state->gid);
header = _ostree_file_header_new (finfo, state->xattrs); header = _ostree_file_header_new (finfo, state->xattrs);
@ -579,26 +569,16 @@ dispatch_open_splice_and_close (OstreeRepo *repo,
metadata = g_variant_new_from_data (ostree_metadata_variant_type (state->output_objtype), metadata = g_variant_new_from_data (ostree_metadata_variant_type (state->output_objtype),
state->payload_data + offset, length, TRUE, NULL, NULL); state->payload_data + offset, length, TRUE, NULL, NULL);
if (state->trusted) {
{ g_autofree guchar *actual_csum = NULL;
if (!ostree_repo_write_metadata_trusted (state->repo, state->output_objtype,
state->checksum,
metadata,
cancellable,
error))
goto out;
}
else
{
g_autofree guchar *actual_csum = NULL;
if (!ostree_repo_write_metadata (state->repo, state->output_objtype, if (!ostree_repo_write_metadata (state->repo, state->output_objtype,
state->checksum, state->checksum,
metadata, &actual_csum, metadata, &actual_csum,
cancellable, cancellable,
error)) error))
goto out; goto out;
} }
} }
else else
{ {
@ -673,28 +653,17 @@ dispatch_open_splice_and_close (OstreeRepo *repo,
cancellable, error)) cancellable, error))
goto out; goto out;
if (state->trusted) {
{ g_autofree guchar *actual_csum = NULL;
if (!ostree_repo_write_content_trusted (state->repo, if (!ostree_repo_write_content (state->repo,
state->checksum, state->checksum,
object_input, object_input,
objlen, objlen,
cancellable, &actual_csum,
error)) cancellable,
goto out; error))
} goto out;
else }
{
g_autofree guchar *actual_csum = NULL;
if (!ostree_repo_write_content (state->repo,
state->checksum,
object_input,
objlen,
&actual_csum,
cancellable,
error))
goto out;
}
} }
} }
@ -920,8 +889,6 @@ dispatch_close (OstreeRepo *repo,
{ {
const char *actual_checksum = g_checksum_get_string (state->content_checksum); const char *actual_checksum = g_checksum_get_string (state->content_checksum);
g_assert (!state->trusted);
if (strcmp (actual_checksum, state->checksum) != 0) if (strcmp (actual_checksum, state->checksum) != 0)
{ {
g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,