deltas: Handle untrusted checksums faster and more robustly

When reworking the ostree core [to use O_TMPFILE](https://github.com/ostreedev/ostree/pull/369),
I hit an issue in the way the untrusted delta codepath ends up trying
to re-open the file to checksum it.  That's not possible with
`O_TMPFILE` since the fd (which we opened `O_WRONLY`) is the only
accessible reference to the content.

Fix this by changing the delta processing code to update a checksum as
we're doing writes, which is also faster, and ends up simplifying the
code as well.

What would be an even larger simplification here is if we e.g. used a
separate thread calling `write_object()` or something like that; the
main issue I see there is somehow bridging the fact that function
wants a `GInputStream*` but the delta code is generating stream of
writes.

Closes: #392
Approved by: jlebon
This commit is contained in:
Colin Walters 2016-07-11 09:29:18 -04:00 committed by Atomic Bot
parent fb0bf27d10
commit 6ffcb24d22
5 changed files with 126 additions and 195 deletions

View File

@ -63,6 +63,10 @@ G_BEGIN_DECLS
*/
#define _OSTREE_ZLIB_FILE_HEADER_GVARIANT_FORMAT G_VARIANT_TYPE ("(tuuuusa(ayay))")
GVariant *_ostree_file_header_new (GFileInfo *file_info,
GVariant *xattrs);
GVariant *_ostree_zlib_file_header_new (GFileInfo *file_info,
GVariant *xattrs);

View File

@ -203,9 +203,9 @@ ostree_validate_rev (const char *rev,
return ret;
}
static GVariant *
file_header_new (GFileInfo *file_info,
GVariant *xattrs)
GVariant *
_ostree_file_header_new (GFileInfo *file_info,
GVariant *xattrs)
{
guint32 uid;
guint32 gid;
@ -518,7 +518,7 @@ ostree_raw_file_to_content_stream (GInputStream *input,
g_autoptr(GVariant) file_header = NULL;
guint64 header_size;
file_header = file_header_new (file_info, xattrs);
file_header = _ostree_file_header_new (file_info, xattrs);
if (!header_and_input_to_stream (file_header,
input,
out_input,
@ -772,7 +772,7 @@ ostree_checksum_file_from_input (GFileInfo *file_info,
{
g_autoptr(GVariant) file_header = NULL;
file_header = file_header_new (file_info, xattrs);
file_header = _ostree_file_header_new (file_info, xattrs);
if (!write_file_header_update_checksum (NULL, file_header, checksum,
cancellable, error))

View File

@ -446,109 +446,14 @@ fallocate_stream (GFileDescriptorBased *stream,
}
gboolean
_ostree_repo_open_untrusted_content_bare (OstreeRepo *self,
const char *expected_checksum,
guint64 content_len,
OstreeRepoContentBareCommit *out_state,
GOutputStream **out_stream,
gboolean *out_have_object,
GCancellable *cancellable,
GError **error)
{
/* The trusted codepath is fine here */
return _ostree_repo_open_trusted_content_bare (self,
expected_checksum,
content_len,
out_state,
out_stream,
out_have_object,
cancellable,
error);
}
gboolean
_ostree_repo_commit_untrusted_content_bare (OstreeRepo *self,
const char *expected_checksum,
OstreeRepoContentBareCommit *state,
guint32 uid,
guint32 gid,
guint32 mode,
GVariant *xattrs,
GCancellable *cancellable,
GError **error)
{
gboolean ret = FALSE;
if (state->fd != -1)
{
GMappedFile *mapped;
g_autoptr(GBytes) bytes = NULL;
g_autoptr(GInputStream) raw_in = NULL;
g_autoptr(GInputStream) in = NULL;
g_autoptr(GFileInfo) file_info = NULL;
g_autofree guchar *actual_csum = NULL;
g_autofree char *actual_checksum = NULL;
int fd;
fd = openat (self->tmp_dir_fd, state->temp_filename, O_RDONLY);
if (fd == -1)
{
glnx_set_error_from_errno (error);
goto out;
}
mapped = g_mapped_file_new_from_fd (fd, FALSE, error);
close (fd);
if (mapped == NULL)
goto out;
bytes = g_mapped_file_get_bytes (mapped);
g_mapped_file_unref (mapped);
raw_in = g_memory_input_stream_new_from_bytes (bytes);
file_info = _ostree_header_gfile_info_new (mode, uid, gid);
if (!ostree_raw_file_to_content_stream (raw_in, file_info, xattrs, &in, NULL, cancellable, error))
goto out;
if (!ot_gio_checksum_stream (in, &actual_csum, cancellable, error))
goto out;
actual_checksum = ostree_checksum_from_bytes (actual_csum);
if (strcmp (actual_checksum, expected_checksum) != 0)
{
g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
"Corrupted object %s (actual checksum is %s)",
expected_checksum, actual_checksum);
goto out;
}
if (!commit_loose_object_trusted (self, expected_checksum, OSTREE_OBJECT_TYPE_FILE,
state->temp_filename,
FALSE, uid, gid, mode,
xattrs, state->fd,
cancellable, error))
goto out;
}
ret = TRUE;
out:
g_free (state->temp_filename);
return ret;
}
gboolean
_ostree_repo_open_trusted_content_bare (OstreeRepo *self,
const char *checksum,
guint64 content_len,
OstreeRepoContentBareCommit *out_state,
GOutputStream **out_stream,
gboolean *out_have_object,
GCancellable *cancellable,
GError **error)
_ostree_repo_open_content_bare (OstreeRepo *self,
const char *checksum,
guint64 content_len,
OstreeRepoContentBareCommit *out_state,
GOutputStream **out_stream,
gboolean *out_have_object,
GCancellable *cancellable,
GError **error)
{
gboolean ret = FALSE;
g_autofree char *temp_filename = NULL;

View File

@ -265,14 +265,14 @@ typedef struct {
} OstreeRepoContentBareCommit;
gboolean
_ostree_repo_open_trusted_content_bare (OstreeRepo *self,
const char *checksum,
guint64 content_len,
OstreeRepoContentBareCommit *out_state,
GOutputStream **out_stream,
gboolean *out_have_object,
GCancellable *cancellable,
GError **error);
_ostree_repo_open_content_bare (OstreeRepo *self,
const char *checksum,
guint64 content_len,
OstreeRepoContentBareCommit *out_state,
GOutputStream **out_stream,
gboolean *out_have_object,
GCancellable *cancellable,
GError **error);
gboolean
_ostree_repo_commit_trusted_content_bare (OstreeRepo *self,

View File

@ -60,6 +60,7 @@ typedef struct {
OstreeRepoContentBareCommit barecommitstate;
guint64 content_size;
GOutputStream *content_out;
GChecksum *content_checksum;
char checksum[OSTREE_SHA256_STRING_LEN+1];
char *read_source_object;
int read_source_fd;
@ -386,6 +387,29 @@ validate_ofs (StaticDeltaExecutionState *state,
return TRUE;
}
static gboolean
content_out_write (OstreeRepo *repo,
StaticDeltaExecutionState *state,
const guint8* buf,
gsize len,
GCancellable *cancellable,
GError **error)
{
gsize bytes_written;
if (state->content_checksum)
g_checksum_update (state->content_checksum, buf, len);
/* Ignore bytes_written since we discard partial content */
if (!g_output_stream_write_all (state->content_out,
buf, len,
&bytes_written,
cancellable, error))
return FALSE;
return TRUE;
}
static gboolean
do_content_open_generic (OstreeRepo *repo,
StaticDeltaExecutionState *state,
@ -451,7 +475,6 @@ dispatch_bspatch (OstreeRepo *repo,
g_autofree guchar *buf = NULL;
struct bspatch_stream stream;
struct bzpatch_opaque_s opaque;
gsize bytes_written;
if (!read_varuint64 (state, &offset, error))
goto out;
@ -484,14 +507,9 @@ dispatch_bspatch (OstreeRepo *repo,
&stream) < 0)
goto out;
if (!g_output_stream_write_all (state->content_out,
buf,
state->content_size,
&bytes_written,
cancellable, error))
if (!content_out_write (repo, state, buf, state->content_size,
cancellable, error))
goto out;
g_assert (bytes_written == state->content_size);
}
ret = TRUE;
@ -499,6 +517,35 @@ dispatch_bspatch (OstreeRepo *repo,
return ret;
}
/* When processing untrusted static deltas, we need to checksum the
* file content, which includes a header. Compare with what
* ostree_checksum_file_from_input() is doing too.
*/
static gboolean
handle_untrusted_content_checksum (OstreeRepo *repo,
StaticDeltaExecutionState *state,
GCancellable *cancellable,
GError **error)
{
g_autoptr(GVariant) header = NULL;
g_autoptr(GFileInfo) finfo = NULL;
gsize bytes_written;
if (state->trusted)
return TRUE;
finfo = _ostree_header_gfile_info_new (state->mode, state->uid, state->gid);
header = _ostree_file_header_new (finfo, state->xattrs);
state->content_checksum = g_checksum_new (G_CHECKSUM_SHA256);
if (!_ostree_write_variant_with_size (NULL, header, 0, &bytes_written, state->content_checksum,
cancellable, error))
return FALSE;
return TRUE;
}
static gboolean
dispatch_open_splice_and_close (OstreeRepo *repo,
StaticDeltaExecutionState *state,
@ -557,7 +604,6 @@ dispatch_open_splice_and_close (OstreeRepo *repo,
{
guint64 content_offset;
guint64 objlen;
gsize bytes_written;
g_autoptr(GInputStream) object_input = NULL;
g_autoptr(GInputStream) memin = NULL;
@ -582,34 +628,23 @@ dispatch_open_splice_and_close (OstreeRepo *repo,
(repo->mode == OSTREE_REPO_MODE_BARE ||
repo->mode == OSTREE_REPO_MODE_BARE_USER))
{
if (state->trusted)
{
if (!_ostree_repo_open_trusted_content_bare (repo, state->checksum,
state->content_size,
&state->barecommitstate,
&state->content_out,
&state->have_obj,
cancellable, error))
goto out;
}
else
{
if (!_ostree_repo_open_untrusted_content_bare (repo, state->checksum,
state->content_size,
&state->barecommitstate,
&state->content_out,
&state->have_obj,
cancellable, error))
goto out;
}
if (!_ostree_repo_open_content_bare (repo, state->checksum,
state->content_size,
&state->barecommitstate,
&state->content_out,
&state->have_obj,
cancellable, error))
goto out;
if (!state->have_obj)
{
if (!g_output_stream_write_all (state->content_out,
state->payload_data + content_offset,
state->content_size,
&bytes_written,
cancellable, error))
if (!handle_untrusted_content_checksum (repo, state, cancellable, error))
goto out;
if (!content_out_write (repo, state,
state->payload_data + content_offset,
state->content_size,
cancellable, error))
goto out;
}
}
@ -705,27 +740,17 @@ dispatch_open (OstreeRepo *repo,
ret = TRUE;
goto out;
}
if (!_ostree_repo_open_content_bare (repo, state->checksum,
state->content_size,
&state->barecommitstate,
&state->content_out,
&state->have_obj,
cancellable, error))
goto out;
if (state->trusted)
{
if (!_ostree_repo_open_trusted_content_bare (repo, state->checksum,
state->content_size,
&state->barecommitstate,
&state->content_out,
&state->have_obj,
cancellable, error))
goto out;
}
else
{
if (!_ostree_repo_open_untrusted_content_bare (repo, state->checksum,
state->content_size,
&state->barecommitstate,
&state->content_out,
&state->have_obj,
cancellable, error))
goto out;
}
if (!handle_untrusted_content_checksum (repo, state, cancellable, error))
goto out;
ret = TRUE;
out:
@ -743,7 +768,6 @@ dispatch_write (OstreeRepo *repo,
gboolean ret = FALSE;
guint64 content_size;
guint64 content_offset;
gsize bytes_written;
if (!read_varuint64 (state, &content_size, error))
goto out;
@ -785,11 +809,8 @@ dispatch_write (OstreeRepo *repo,
goto out;
}
if (!g_output_stream_write_all (state->content_out,
buf,
bytes_read,
&bytes_written,
cancellable, error))
if (!content_out_write (repo, state, (guint8*)buf, bytes_read,
cancellable, error))
goto out;
content_size -= bytes_read;
@ -800,11 +821,8 @@ dispatch_write (OstreeRepo *repo,
if (!validate_ofs (state, content_offset, content_size, error))
goto out;
if (!g_output_stream_write_all (state->content_out,
state->payload_data + content_offset,
content_size,
&bytes_written,
cancellable, error))
if (!content_out_write (repo, state, state->payload_data + content_offset, content_size,
cancellable, error))
goto out;
}
}
@ -898,22 +916,26 @@ dispatch_close (OstreeRepo *repo,
if (!g_output_stream_flush (state->content_out, cancellable, error))
goto out;
if (state->trusted)
if (state->content_checksum)
{
if (!_ostree_repo_commit_trusted_content_bare (repo, state->checksum, &state->barecommitstate,
state->uid, state->gid, state->mode,
state->xattrs,
cancellable, error))
goto out;
}
else
{
if (!_ostree_repo_commit_untrusted_content_bare (repo, state->checksum, &state->barecommitstate,
state->uid, state->gid, state->mode,
state->xattrs,
cancellable, error))
goto out;
const char *actual_checksum = g_checksum_get_string (state->content_checksum);
g_assert (!state->trusted);
if (strcmp (actual_checksum, state->checksum) != 0)
{
g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
"Corrupted object %s (actual checksum is %s)",
state->checksum, actual_checksum);
goto out;
}
}
if (!_ostree_repo_commit_trusted_content_bare (repo, state->checksum, &state->barecommitstate,
state->uid, state->gid, state->mode,
state->xattrs,
cancellable, error))
goto out;
}
if (!dispatch_unset_read_source (repo, state, cancellable, error))