This will prevent including in the delta the bits to update files that
are not world readable, so that we don't run into a permissions problem
when applying the deltas from a bare-user repository that has a bare
repository set as its parent.
This is the case for Endless when updating flatpak runtimes, as the
temporary directory created in ~/.local/share/flatpak/system-cache will
be of type bare-user with its parent set to /var/lib/flatpak which is a
bare repository in EOS, as it's shared with the one at /ostree/repo.
https://phabricator.endlessm.com/T14159Closes: #634
Approved by: cgwalters
I was having this thought today about making more of the OS readonly,
and ultimately if we got to the point where all ostree operations are
through the repo and sysroot dfds, we could have rpm-ostree be the
only process holding those fds open, and have a read-only bind mount
on top.
Anyways, we're not there, likely won't be soon, but this gets us
closer to being fully fd relative.
Closes: #628
Approved by: jlebon
This is a migration from the origin version. It's
nicer to have it in the remote, since that's what one
needs to change. Then tools don't need to mess with
the origin file.o
In fact in this scenario one can keep the "media source" like
`file:///install/repo` or whatever, since conceptually that's where it
came from. We're just providing a better error.
Closes: https://github.com/ostreedev/ostree/issues/626Closes: #627
Approved by: jlebon
These are out parameters, so add the (out) annotation and switch
(nullable) to (optional) since the latter is used for the purpose of
optional out parameters.
Closes: #629
Approved by: cgwalters
We were leaking in a few places that I noticed in an ASAN run. Also,
this was one of the last non-autoptr cleanup sections we have in
`out:` cleanup sections, making us a lot closer to a potential
full-tree rewrite to `return FALSE`.
Closes: #624
Approved by: jlebon
I installed `parallel` in my dev container, which got me
the sysroot locking tests, which caught this leak when
built with ASAN.
Closes: #623
Approved by: jlebon
The "remote cookies" code broke this. While I'm not sure anyone is
actually using ostree-without-http, it isn't too hard to keep the
build time conditional going. Further, this work is preparatory for
libcurl porting.
Closes: #621
Approved by: jlebon
Due to the way glib-mkenums runs the preprocessor itself, it
doesn't pick up the `AC_USE_SYSTEM_EXTENSIONS()` that we have in
`configure.ac`.
This blew up in an obscure way when I later wanted to `#include
"libglnx.h"` in one of the headers, since it needs the `basename()`
from `string.h` which is only available with `_GNU_SOURCE`.
Closes: #616
Approved by: jlebon
This is what we do for non-local (i.e. HTTP) pulls; we wnat to
correctly handle being interrupted during partial pulls.
Closes: https://github.com/ostreedev/ostree/issues/579Closes: #613
Approved by: jlebon
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
We should be religious about the "only set output variables on
success", otherwise it makes leaks more likely.
But the real leak was us simply not using autoptr in one place.
Closes: #598
Approved by: jlebon
And "move semantics" via `g_steal_pointer()`. Just a minor code
cleanup I noticed when I was hunting for a leak, which ended up being
elsewhere.
Closes: #598
Approved by: jlebon
glnx_make_lock_file requires that the dfd passed in survives the
lifetime of the lock. Since dfd_iter.fd gets cleaned up after the
function returns, this isn't the case. dfd_iter.fd should be equivalent
to tmpdir_dfd, since we iter on ".", and that survives past the
function, so just use that instead.
Closes: #591
Approved by: cgwalters
The fact that we weren't doing this is at best an oversight, and
for some deployment models a security vulnerability. Having both
`gpg-verify` and `gpg-verify-summary` shows that we were intending
them to be orthogonal/independent.
Lately I've been advocating moving towards pinned TLS instead of
gpg-signed summaries, and if we follow that path, performing GPG
verification of commit objects even if using deltas is more important,
as it provides an at-rest verifiable authenticity and integrity
mechanism.
Content providers which are signing their summary files and/or using
TLS (particularly pinned TLS) for transport should treat this as a
nice-to-have. However, for providers which are serving content over
plain HTTP and relying on GPG, this is a critical update.
Closes: https://github.com/ostreedev/ostree/issues/517Closes: #589
Approved by: jlebon
`-fsanitize=address` complained that the `refcount > 0` assertions
were reading without atomics. We can fix this by reworking them
to read the previous value.
Closes: #582
Approved by: jlebon
It turns out this is basically racy with the presence of other
threads. It was really cosmetic so let's stop doing it and make
`-fsanitize=thread` happy.
Closes: #582
Approved by: jlebon
This is actually fine in practice, but it triggers this
`-fsanitize=undefined` warning I saw in the test suite log:
```
src/libostree/ostree-repo-static-delta-compilation.c:160:10: runtime error: null pointer passed as argument 1, which is declared to never be null
```
Closes: #584
Approved by: jlebon
You'd expect
ostree commit --tree=ref=A --tree=ref=B
to produce a commit with the union of the trees given. Instead you'd get
a commit with the contents of just the latter commit. This was due to an
optimisation where we'd skip filling out the `files` and `subdirs`
members of the mtree, just filling in the metadata instead. This backfires
becuase this same code relies on checking the `files` and `subdirs` members
itself to work out whether the mtree is empty.
This commit removes the optimisation, fixing the bug. Maybe there's a way
to keep the optimisation and still fix the bug but it's not obvious to
me.
Closes: #581
Approved by: cgwalters
Conceptually we've been moving towards having our GPG verification
paths be per-remote. The code internally supports this, but we
didn't expose an API to use it conveniently.
This came up when trying to add a new `gpgkeypath` option, since
right now rpm-ostree manually finds keyrings for the remote, and
hence it wasn't looking at the keypath, and said "Unknown key"
in status.
Adding an API fixes this nicely.
Closes: #576
Approved by: giuseppe
For Project Atomic, we already have RPM signatures which use files in
`/etc/pki/rpm-gpg`. It's convenient to simply bind the OSTree remote
configuration to those file paths, rather than having duplicate key
data.
This does mean that we need to parse the files for verification, so we
end up importing them into the verifier's temporary keyring, which is
a bit ugly, but it's what other projects do.
Closes: https://github.com/ostreedev/ostree/issues/573Closes: #575
Approved by: giuseppe
When doing commit --tree=ref=XXX while at the same time applying some
form of modifier, ostree dies trying to read the xattrs using the
raw syscalls. We fix this by falling back to ostree_repo_file_get_xattrs()
in this case.
Also adds a testcase for this.
Closes: #577
Approved by: cgwalters
What in the code is called "scanning" is ensuring (potentially
recursively) have an object, and if not, fetching it. And then if
it's metadata, parsing it and finding new objects to fetch.
This logic has grown fairly complex. What I'm trying to fix
right now is that if we're doing a pull-local to a remote repository
via `sshfs` (FUSE) we still end up scanning, which is inefficient.
We can take advantage of the "commitpartial" logic here - if a commit
isn't partial, it's complete, hence we don't need to scan it.
At the same time, I'm changing the logic here to *always* do scans for
dirtree objects. This will fix cases where multiple commits share
dirtree objects. We have "commitpartial" metadata, but no such concept
of partial/complete for dirtrees.
But, we'll only ever scan dirtrees if we scan commits, which is
what the section above fixes.
Closes: https://github.com/ostreedev/ostree/issues/543Closes: #564
Approved by: alexlarsson
Some deployments may want to gate access to content based on things
like OAuth. In this model, the client system would normally compute a
token and pass it to the server via an API.
We could theoretically support this in the remote config too, but
that'd be a bit weird for OAuth as the information is dynamic.
Therefore this cleans up the code a little bit to more clearly handle
the case that the fetcher is initialized from both remote config
data plus pull options.
Closes: #574
Approved by: giuseppe
Otherwise it's possible for us to exhaust available file descriptors
or (on 32 bit) run up against mmap limits.
In the rollsum case, we didn't need to hold open the "from" object
at all. And in the bsdiff case, we weren't even looking at either of
the files until we started processing.
Also, while we have the patient open, switch to using O_TMPFILE
if available.
Closes: #567
Approved by: giuseppe
Private Cloudfront instances return 403 for objects which don't exist
rather then a 404.
Change the fetcher to assume 403 is ok for download that are "optional"
rather then erroring out at that step (e.g. trying to download a static
delta if the remote repo doesn't have those)
Closes: #531
Approved by: cgwalters
Optionally read cookie jars for a remote to be used when downloading
data. This can be used for private repositories which require specific
cookies to be present, e.g. repositories hosted on Amazon cloudfront
using signed cookies.
Closes: #531
Approved by: cgwalters
We should just download the commit objects directly, as it's
obviously a lot more efficient than deltas.
I had to generate a summary file in more places in the tests,
since once created, it needs to be updated.
Closes: https://github.com/ostreedev/ostree/issues/528Closes: #566
Approved by: jlebon
I was doing a chain of mirroring like A -> B -> C
And repo B had A as a remote. When I added B as
a remote to C, the summary file of B had a ref
upstream:foo/bar/baz, which caused all pulls from
B to C to fail, since the summary file is only
expected to have refs, not refspecs.
Closes: https://github.com/ostreedev/ostree/issues/561Closes: #565
Approved by: jlebon
Various bootloader add kernel commandline options dynamically, filter
these out when grabbing boot options from /proc/cmdline. Specifically
grub adds BOOT_IMAGE and systemd-boot adds initrd.
Closes: #560
Approved by: cgwalters
Found by valgrind memcheck. g_variant_new_from_bytes takes a ref to the
bytes, so we need to release the original ref.
Signed-off-by: Simon McVittie <smcv@debian.org>
Closes: #556
Approved by: cgwalters
ostree_repo_pull_with_options() and ostree_repo_remote_change() don't
sink floating GVariant arguments, and doing so now would be an
ABI change; so don't rely on them to do so.
Leak found with valgrind memcheck.
Signed-off-by: Simon McVittie <smcv@debian.org>
Closes: #556
Approved by: cgwalters
g_variant_get_strv is (transfer container): the caller is expected to
free the array, but not the individual strings.
Leak found with valgrind memcheck.
Signed-off-by: Simon McVittie <smcv@debian.org>
Closes: #556
Approved by: cgwalters
It appears the result of assign_bootserials() is never actually used,
but I haven't changed it to return void right now.
Leak found with valgrind memcheck.
Signed-off-by: Simon McVittie <smcv@debian.org>
Closes: #556
Approved by: cgwalters
The cleanup attribute doesn't clean up before calling a noreturn
function like exit(). Explicitly clean up the pointer variables
(but don't assume that a simple g_object_unref() would be OK either,
in case the behaviour of the cleanup attribute changes).
This isn't a real leak since we're about to exit anyway, but if we
don't fix it then valgrind memcheck will make the tests fail.
Signed-off-by: Simon McVittie <smcv@debian.org>
Closes: #559
Approved by: cgwalters
We are relying on the GPtrArray to free its contents, but we only
give it a free-function on one code path.
Found by valgrind memcheck.
Signed-off-by: Simon McVittie <smcv@debian.org>
Closes: #559
Approved by: cgwalters
If !existed, then we add arg to kargs->order, where it will be freed
by that array's free-function. However, if the kernel argument did
already exist, we have to either free arg ourselves (and make sure
the old key is what appears in the hash table), or do a linear search
on kargs->order to replace the old key with the new.
Leak found by valgrind memcheck.
Signed-off-by: Simon McVittie <smcv@debian.org>
Closes: #559
Approved by: cgwalters
Improve the heuristic to use only the part before the first '.' when
looking for a similar file in the current directory.
last versions of dracut generate reproducible initramfs files, but we
still fallback to the full file download if there is any minimal
change that causes a different checksum and file name.
This change extends that case to deal better with similar files that
have a different suffix.
This is the difference generating a static delta from
fedora-atomic/f24/x86_64/docker-host to fedora-atomic/f24/x86_64/testing/docker-host
before the patch:
fallback for 111ec866aa7ce3688407fa4a1ae7c9fca93dcee0b851fc9434c59ff947830cc7 (47.0 MB)
fallback for c6a898265de22b02c89ea2f35d132628d0ee1c0a058052ed14fee5799c17904c (47.0 MB)
fallback for fbce656249ece77260887ed873e445561b9d43bcb28a32e759c0b1bab89e7137 (6.6 MB)
fallback for cfdb51457e47e0a0fe0bac38991a21279d2646ff2f019630c7b52a0cd3451397 (6.6 MB)
part 0 n:1972 compressed:11239809 uncompressed:33747412
part 1 n:1079 compressed:9683681 uncompressed:55641397
part 2 n:1507 compressed:15050265 uncompressed:44448838
part 3 n:101 compressed:1865881 uncompressed:31896086
part 4 n:278 compressed:2452585 uncompressed:52811323
part 5 n:18 compressed:67621 uncompressed:100220
uncompressed=218645276 compressed=40359842 loose=545102
rollsum=49 objects, 2117254 bytes
bsdiff=4067 objects
after the patch:
part 0 n:843 compressed:19844109 uncompressed:95443178
part 1 n:1223 compressed:11188609 uncompressed:33330401
part 2 n:990 compressed:15762905 uncompressed:61214132
part 3 n:1441 compressed:20614573 uncompressed:31534195
part 4 n:163 compressed:2734997 uncompressed:51356423
part 5 n:285 compressed:2480813 uncompressed:52902904
part 6 n:14 compressed:59125 uncompressed:75341
uncompressed=325856574 compressed=72685131 loose=533283
rollsum=51 objects, 57235332 bytes
bsdiff=4073 objects
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Closes: #554
Approved by: cgwalters
If we have a partial commit it is not an error for a dirmeta to be
missing (in fact, that is likely), so instead of returning a not-found
error from ostree_repo_traverse_commit() we ignore the error and
continue.
In particular, this means we don't stop early at the first
missing dirmeta, which previously caused ostree_repo_prune() to
thing the dirmetas after that to be unreached and thus purged.
Also, we remove the special casing in ostree_repo_prune() to
not report errors for commitpartial, because these should not
be reported anymore.
This fixes https://github.com/ostreedev/ostree/issues/541Closes: #542
Approved by: cgwalters
If there is a transaction active, then we put writes to detached
metadata into the staging dir, and when reading it we look there
first. This allows transactions to be aborted half-way without
writing the detached metadata into the repository (possibly
overwriting any old metadata from there).
This fixes https://github.com/ostreedev/ostree/issues/526Closes: #539
Approved by: giuseppe
If the detached metadata is not in the repo, try in the parent
repo if that is set.
Without this a commit will not gpg validate in the child repo
Closes: #539
Approved by: giuseppe
Commit 1d4f1b8878 started using hardlinks
checkouts of symlinks. However, symlinks are not stored as symlink in the
repo for bare-user repos, so this breaks user-mode checkouts of such repos.
We fix this by checking for !is_symlink in the bare-user case.
This fixes:
https://github.com/ostreedev/ostree/issues/537Closes: #538
Approved by: giuseppe
I was noticing a recent performance issue with checkouts
which seemed to be mostly us going back to doing a `fsync()` on
directories.
Regardless, while looking at that, I saw we were spending time
creating new symlinks. Even though symlinks are small, it's still
better to hardlink them.
Going way back in time, the reason we weren't doing this is
because we were hitting `EMFILE` on ext4, but that was for
gnome-continuous which creates *many* build roots. Even
there though, they're just a cache, and we handle `EMFILE`.
For ostree-for-host-system, we don't expect to have many roots (just 3
at most transiently), so hardlinking symlinks does make sense.
Closes: #521
Approved by: jlebon
Just noticed this while inspecting the code. The deployments retrieved
by `_ostree_sysroot_list_deployment_dirs_for_os` will forcibly already
have a matching osname since it indirectly uses that same variable to
construct them. Having a check there makes it look like there may be
subtle corner cases, when there aren't.
Closes: #529
Approved by: cgwalters
If this is true, don't initiate, abort of commit a transaction, instead
it is assumed that the caller initiated the transaction, and that it
will eventually be commited.
This allows you to do multiple pulls or a combination of pulls and
commits in a single transaction.
Closes: #525
Approved by: cgwalters
This pulls in a new compilation flag for wrpseudo compatibility. Also
note we need to add some includes since glnx-libcontainer went away,
and with it some includes for `sys/mount.h` etc.
Closes: #522
Approved by: cgwalters
We were seeing some weird potential memory corruption in this code
when using it for `rpm-ostree-toolbox installer`, which is almost
certainly not its fault, but let's use it as an excuse to port
(mostly) to fd-relative and away from GFile.
Dropping the last GFile use here is a bit tricky as it does have a
nice high level wrapper around inotify.
Closes: #512
Approved by: jlebon
My previous change in https://github.com/ostreedev/ostree/pull/425
actually broke things so we basically used the repository defaults =(
This is a subtle mess since we're only trying to flip things off
for the *new* API.
Clean this up so that the "default repo inheritance" lives only in one
place - in the compat layer for the old checkout API. The new
checkout API defaults to off period, so the repository state is
irrelevant.
Closes: #520
Approved by: jlebon
/var/log is another one of those core directories that should be made
available and properly labeled during early boot before tmpfiles.d
starts up.
Related: https://bugzilla.redhat.com/show_bug.cgi?id=1265295Closes: #513
Approved by: cgwalters
While looking at a slow update issue (which I'm guessing is
unpredictable I/O latency in an OpenStack instance), I noticed
in one of the traces we were inside a fsync here.
Dropping the fsync here is just another of a long series of unwinding
them - we `syncfs()` the sysroot fd and `/boot` and we have a big
`sync()` anyways.
Closes: #508
Approved by: jlebon
More fsync pruning. Since we have a public API for writing the origin
file and it did a fsync before, let's preserve that. But when writing
deployments as part of a full transaction, we rely on the global
`syncfs()`, so add an internal function for origin file writing that
doesn't.
Closes: #509
Approved by: giuseppe
If ostree_sysroot_unload() was called explicitly, then sysroot_fd
would be closed again at finalization time, possibly closing a
random file descriptor belonging to some other part of the
application.
Closes: #507
Approved by: cgwalters
Dan Nicholson
Mario Sanchez Prada
Colin Walters
Alexander Larsson
Jasper St. Pierre
William Manley
Sjoerd Simons
Simon McVittie
Giuseppe Scrivano
Jonathan Lebon
Owen W. Taylor