From 8ef18fd850d53fa01e7a3d8fe47fdd069b276b85 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 21 Dec 2017 22:31:18 +0100 Subject: [PATCH 01/30] ci: Make rust build nonblocking for now Will debug at some point but for now let's unblock other things. ``` /usr/bin/ld: /var/tmp/checkout/target/release/libbupsplit_rs.a(bupsplit_rs-db7d02fa07221ce3.bupsplit_rs0.rust-cgu.o): undefined reference to symbol 'dladdr@@GLIBC_2.2.5' ``` Closes: #1387 Approved by: jlebon --- .papr.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.papr.yml b/.papr.yml index 2016c61b..a9ee15e2 100644 --- a/.papr.yml +++ b/.papr.yml @@ -53,6 +53,7 @@ tests: context: f26-rust inherit: true +required: false container: image: registry.fedoraproject.org/fedora:26 env: @@ -67,6 +68,7 @@ tests: context: f26-gnutls inherit: true +required: true container: image: registry.fedoraproject.org/fedora:26 env: From 117d5c9f771e919dc33bdc8a616867212713f0bd Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 20 Dec 2017 10:15:10 +0100 Subject: [PATCH 02/30] build-sys: Post-release version bump Closes: #1387 Approved by: jlebon --- configure.ac | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index 478d7349..697007d5 100644 --- a/configure.ac +++ b/configure.ac @@ -4,10 +4,10 @@ dnl update libostree-released.sym from libostree-devel.sym, and update the check dnl in test-symbols.sh, and also set is_release_build=yes below. Then make dnl another post-release commit to bump the version, and set is_release_build=no. m4_define([year_version], [2017]) -m4_define([release_version], [15]) +m4_define([release_version], [16]) m4_define([package_version], [year_version.release_version]) AC_INIT([libostree], [package_version], [walters@verbum.org]) -is_release_build=yes +is_release_build=no AC_CONFIG_HEADER([config.h]) AC_CONFIG_MACRO_DIR([buildutil]) AC_CONFIG_AUX_DIR([build-aux]) From cac42bb6f576696033e70a2b1cb2c947b95e2306 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Thu, 21 Dec 2017 18:01:44 +0000 Subject: [PATCH 03/30] build: Fix typo in -Wparentheses warning GCC supports -Wparentheses, not -Wparenthesis. https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wno-parentheses Signed-off-by: Philip Withnall Closes: #1388 Approved by: jlebon --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 697007d5..fa436ea6 100644 --- a/configure.ac +++ b/configure.ac @@ -43,7 +43,7 @@ CC_CHECK_FLAGS_APPEND([WARN_CFLAGS], [CFLAGS], [\ -Werror=switch \ -Werror=overflow \ -Werror=int-conversion \ - -Werror=parenthesis \ + -Werror=parentheses \ -Werror=undef \ -Werror=incompatible-pointer-types \ -Werror=misleading-indentation \ From 8d3d14503b3105f93db635022e881e02688f2db4 Mon Sep 17 00:00:00 2001 From: Marcus Folkesson Date: Thu, 21 Dec 2017 10:25:45 +0100 Subject: [PATCH 04/30] lib/pull: allways include ostree-repo-pull-private.h Allways include ostree-repo-pull-private.h to get rid of the following build error when HAVE_LIBCURL_OR_LIBSOUP is not defined: src/libostree/ostree-repo-pull.c:1493:1: error: no previous prototype for '_ostree_repo_verify_bindings' [-Werror=missing-prototypes] Signed-off-by: Marcus Folkesson Closes: #1389 Approved by: cgwalters --- src/libostree/ostree-repo-pull.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 2e6e308c..b2a00ea2 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -27,12 +27,12 @@ #include "libglnx.h" #include "ostree.h" #include "otutil.h" +#include "ostree-repo-pull-private.h" #ifdef HAVE_LIBCURL_OR_LIBSOUP #include "ostree-core-private.h" #include "ostree-repo-private.h" -#include "ostree-repo-pull-private.h" #include "ostree-repo-static-delta-private.h" #include "ostree-metalink.h" #include "ostree-fetcher-util.h" From f63e62fbd272edbdd9695a10d841fc6b5eb69df8 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Sun, 10 Dec 2017 19:39:38 +0000 Subject: [PATCH 05/30] tests: Don't assume uid == primary gid Nothing guarantees that each user has a group containing only themselves. Even if they do, nothing guarantees that its group ID equals the user ID, particularly if another user earlier in the same range was created without a corresponding group or vice versa. Signed-off-by: Simon McVittie Closes: #1390 Approved by: cgwalters --- tests/test-basic-user.sh | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/tests/test-basic-user.sh b/tests/test-basic-user.sh index 7f970b5c..6987e4ad 100755 --- a/tests/test-basic-user.sh +++ b/tests/test-basic-user.sh @@ -104,18 +104,20 @@ cd ${test_tmpdir} rm -rf test2-checkout $OSTREE checkout -H -U test2 test2-checkout # With --link-checkout-speedup, specifying --owner-uid should "win" by default. -myid=$(id -u) -newid=$((${myid} + 1)) -$OSTREE commit ${COMMIT_ARGS} --owner-uid ${newid} --owner-gid ${newid} \ +myuid=$(id -u) +mygid=$(id -g) +newuid=$((${myuid} + 1)) +newgid=$((${mygid} + 1)) +$OSTREE commit ${COMMIT_ARGS} --owner-uid ${newuid} --owner-gid ${newgid} \ --link-checkout-speedup -b test2-linkcheckout-test --tree=dir=test2-checkout $OSTREE ls test2-linkcheckout-test /baz/cow > ls.txt -assert_file_has_content ls.txt "^-006.. ${newid} ${newid} .*/baz/cow" +assert_file_has_content ls.txt "^-006.. ${newuid} ${newgid} .*/baz/cow" # But --devino-canonical should override that -$OSTREE commit ${COMMIT_ARGS} --owner-uid ${newid} --owner-gid ${newid} \ +$OSTREE commit ${COMMIT_ARGS} --owner-uid ${newuid} --owner-gid ${newgid} \ -I -b test2-devino-test --tree=dir=test2-checkout $OSTREE ls test2-devino-test /baz/cow > ls.txt -assert_file_has_content ls.txt "^-006.. ${myid} ${myid} .*/baz/cow" +assert_file_has_content ls.txt "^-006.. ${myuid} ${mygid} .*/baz/cow" $OSTREE refs --delete test2-{linkcheckout,devino}-test echo "ok commit with -I" From 1f832597fc83fda6cb8daf48c4495a9e1590774c Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 2 Jan 2018 09:54:52 -0500 Subject: [PATCH 06/30] build-sys: Link with -ldl for rust build I didn't dive into this too much, it looks like something in rust changed that broke our build. Probably libstd gained a dependency on `-ldl` or so, and that's handled by cargo? Anyways linking against it isn't going to hurt. Closes: #1391 Approved by: smcv --- Makefile-libostree.am | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Makefile-libostree.am b/Makefile-libostree.am index 0a4de6de..19f8184e 100644 --- a/Makefile-libostree.am +++ b/Makefile-libostree.am @@ -202,6 +202,10 @@ libostree_1_la_CFLAGS = $(AM_CFLAGS) -I$(srcdir)/bsdiff -I$(srcdir)/libglnx -I$( libostree_1_la_LDFLAGS = -version-number 1:0:0 -Bsymbolic-functions $(addprefix $(wl_versionscript_arg),$(symbol_files)) libostree_1_la_LIBADD = libotutil.la libglnx.la libbsdiff.la libostree-kernel-args.la $(OT_INTERNAL_GIO_UNIX_LIBS) $(OT_INTERNAL_GPGME_LIBS) \ $(OT_DEP_LZMA_LIBS) $(OT_DEP_ZLIB_LIBS) $(OT_DEP_CRYPTO_LIBS) +# Some change between rust-1.21.0-1.fc27 and rust-1.22.1-1.fc27.x86_64 +if ENABLE_RUST +libostree_1_la_LIBADD += -ldl +endif libostree_1_la_LIBADD += $(bupsplitpath) EXTRA_libostree_1_la_DEPENDENCIES = $(symbol_files) From 95f9b392a44974a91bd3bf61e33779cdedda29e7 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 2 Jan 2018 10:00:17 -0500 Subject: [PATCH 07/30] Revert "ci: Make rust build nonblocking for now" This reverts commit 8ef18fd850d53fa01e7a3d8fe47fdd069b276b85. Closes: #1391 Approved by: smcv --- .papr.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.papr.yml b/.papr.yml index a9ee15e2..2016c61b 100644 --- a/.papr.yml +++ b/.papr.yml @@ -53,7 +53,6 @@ tests: context: f26-rust inherit: true -required: false container: image: registry.fedoraproject.org/fedora:26 env: @@ -68,7 +67,6 @@ tests: context: f26-gnutls inherit: true -required: true container: image: registry.fedoraproject.org/fedora:26 env: From 994cd66744e559c92644f36028c6c262605ad75a Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Wed, 3 Jan 2018 08:23:10 +0000 Subject: [PATCH 08/30] tests: Assert that byte-order is swapped on LE but not BE CPUs Closes: #1392 Signed-off-by: Simon McVittie Closes: #1393 Approved by: cgwalters --- Makefile-tests.am | 8 +++++++- tests/basic-test.sh | 14 +++++++++++++- tests/get-byte-order.c | 12 ++++++++++++ 3 files changed, 32 insertions(+), 2 deletions(-) create mode 100644 tests/get-byte-order.c diff --git a/Makefile-tests.am b/Makefile-tests.am index 2b335556..25a8d08b 100644 --- a/Makefile-tests.am +++ b/Makefile-tests.am @@ -130,7 +130,13 @@ experimental_test_scripts = \ tests/test-summary-collections.sh \ tests/test-pull-collections.sh \ $(NULL) -test_extra_programs = $(NULL) +test_extra_programs = \ + tests/get-byte-order \ + $(NULL) + +tests_get_byte_order_SOURCES = tests/get-byte-order.c +tests_get_byte_order_CFLAGS = $(AM_CFLAGS) $(GLIB_CFLAGS) +tests_get_byte_order_LDADD = $(GLIB_LIBS) tests_repo_finder_mount_SOURCES = tests/repo-finder-mount.c tests_repo_finder_mount_CFLAGS = $(common_tests_cflags) diff --git a/tests/basic-test.sh b/tests/basic-test.sh index 87cb9fa2..c4eb9cad 100644 --- a/tests/basic-test.sh +++ b/tests/basic-test.sh @@ -759,8 +759,20 @@ $OSTREE show --print-metadata-key=FOO test2 > test2-meta assert_file_has_content test2-meta "BAR" $OSTREE show --print-metadata-key=KITTENS test2 > test2-meta assert_file_has_content test2-meta "CUTE" + $OSTREE show --print-metadata-key=SOMENUM test2 > test2-meta -assert_file_has_content test2-meta "uint64 3026418949592973312" +case "$("${test_builddir}/get-byte-order")" in + (4321) + assert_file_has_content test2-meta "uint64 42" + ;; + (1234) + assert_file_has_content test2-meta "uint64 3026418949592973312" + ;; + (*) + fatal "neither little-endian nor big-endian?" + ;; +esac + $OSTREE show -B --print-metadata-key=SOMENUM test2 > test2-meta assert_file_has_content test2-meta "uint64 42" $OSTREE show --print-detached-metadata-key=SIGNATURE test2 > test2-meta diff --git a/tests/get-byte-order.c b/tests/get-byte-order.c new file mode 100644 index 00000000..7e7ba31b --- /dev/null +++ b/tests/get-byte-order.c @@ -0,0 +1,12 @@ +/* Helper for OSTree tests: return host byte order */ + +#include "config.h" + +#include + +int +main (void) +{ + g_print ("%d\n", G_BYTE_ORDER); + return 0; +} From 46a841a062936f35cefaae5a4dbbb989859fc471 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 14 Dec 2017 11:05:00 -0500 Subject: [PATCH 09/30] rofiles: Add --copyup option MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sadly https://sourceware.org/bugzilla/show_bug.cgi?id=22089 is I think going to actually force us to cave here. Even if we got the glibc patch in today, we need to support the RHEL glibc. See also discussion about fish as part of the general Fedora tracker. This is basically needed to unblock rpm-ostree unified core 🌐: https://github.com/projectatomic/rpm-ostree/issues/729 Closes: https://github.com/ostreedev/ostree/issues/1377 Closes: #1382 Approved by: jlebon --- src/rofiles-fuse/Makefile-inc.am | 6 +- src/rofiles-fuse/main.c | 143 +++++++++++++++++++++++-------- tests/test-rofiles-fuse.sh | 22 ++++- 3 files changed, 133 insertions(+), 38 deletions(-) diff --git a/src/rofiles-fuse/Makefile-inc.am b/src/rofiles-fuse/Makefile-inc.am index 5510a2bd..6754aa70 100644 --- a/src/rofiles-fuse/Makefile-inc.am +++ b/src/rofiles-fuse/Makefile-inc.am @@ -19,5 +19,7 @@ bin_PROGRAMS += rofiles-fuse rofiles_fuse_SOURCES = src/rofiles-fuse/main.c -rofiles_fuse_CFLAGS = $(AM_CFLAGS) -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 $(BUILDOPT_FUSE_CFLAGS) $(OT_INTERNAL_GIO_UNIX_CFLAGS) -I$(srcdir)/libglnx $(NULL) -rofiles_fuse_LDADD = libglnx.la $(BUILDOPT_FUSE_LIBS) $(OT_INTERNAL_GIO_UNIX_LIBS) +rofiles_fuse_CFLAGS = $(AM_CFLAGS) -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 $(BUILDOPT_FUSE_CFLAGS) \ + $(OT_INTERNAL_GIO_UNIX_CFLAGS) -I $(srcdir)/src/libostree -I $(builddir)/src/libostree \ + -I$(srcdir)/libglnx +rofiles_fuse_LDADD = libglnx.la $(BUILDOPT_FUSE_LIBS) $(OT_INTERNAL_GIO_UNIX_LIBS) libostree-1.la diff --git a/src/rofiles-fuse/main.c b/src/rofiles-fuse/main.c index 9e04274b..2e1c1346 100644 --- a/src/rofiles-fuse/main.c +++ b/src/rofiles-fuse/main.c @@ -35,9 +35,14 @@ #include #include "libglnx.h" +#include "ostree.h" // Global to store our read-write path static int basefd = -1; +/* Whether or not to automatically "copyup" (in overlayfs terms). + * What we're really doing is breaking hardlinks. + */ +static gboolean opt_copyup; static inline const char * ENSURE_RELPATH (const char *path) @@ -200,52 +205,97 @@ callback_link (const char *from, const char *to) /* Check whether @stbuf refers to a hardlinked regfile or symlink, and if so * return -EROFS. Otherwise return 0. */ -static int -can_write_stbuf (struct stat *stbuf) +static gboolean +can_write_stbuf (const struct stat *stbuf) { /* If it's not a regular file or symlink, ostree won't hardlink it, so allow * writes - it might be a FIFO or device that somehow * ended up underneath our mount. */ if (!(S_ISREG (stbuf->st_mode) || S_ISLNK (stbuf->st_mode))) - return 0; + return TRUE; /* If the object isn't hardlinked, it's OK to write */ if (stbuf->st_nlink <= 1) - return 0; + return TRUE; /* Otherwise, it's a hardlinked file or symlink; it must be * immutable. */ - return -EROFS; + return FALSE; } -/* Check whether @path refers to a hardlinked regfile or symlink, and if so - * return -EROFS. Otherwise return 0. - */ static int -can_write (const char *path) +gioerror_to_errno (GIOErrorEnum e) { - struct stat stbuf; - if (fstatat (basefd, path, &stbuf, AT_SYMLINK_NOFOLLOW) == -1) + /* It's obviously crappy to have to do this but + * we also don't want to try to have "raw errno" versions + * of everything down in ostree_break_hardlink() so... + * let's just reverse map a few ones I think are going to be common. + */ + switch (e) { - if (errno == ENOENT) - return 0; - else - return -errno; + case G_IO_ERROR_NOT_FOUND: + return ENOENT; + case G_IO_ERROR_IS_DIRECTORY: + return EISDIR; + case G_IO_ERROR_PERMISSION_DENIED: + return EPERM; + case G_IO_ERROR_NO_SPACE: + return ENOSPC; + default: + return EIO; } - return can_write_stbuf (&stbuf); } -#define VERIFY_WRITE(path) do { \ - int r = can_write (path); \ - if (r != 0) \ - return r; \ +static int +verify_write_or_copyup (const char *path, const struct stat *stbuf) +{ + struct stat stbuf_local; + + /* If a stbuf wasn't provided, gather it now */ + if (!stbuf) + { + if (fstatat (basefd, path, &stbuf_local, AT_SYMLINK_NOFOLLOW) == -1) + { + if (errno == ENOENT) + return 0; + else + return -errno; + } + stbuf = &stbuf_local; + } + + /* Verify writability, if that fails, perform copy-up if enabled */ + if (!can_write_stbuf (stbuf)) + { + if (opt_copyup) + { + g_autoptr(GError) tmp_error = NULL; + if (!ostree_break_hardlink (basefd, path, FALSE, NULL, &tmp_error)) + return -gioerror_to_errno ((GIOErrorEnum)tmp_error->code); + } + else + return -EROFS; + } + + return 0; +} + +/* Given a path (which is absolute), convert it + * to a relative path (even for the caller) and + * perform either write verification or copy-up. + */ +#define PATH_WRITE_ENTRYPOINT(path) do { \ + path = ENSURE_RELPATH (path); \ + int r = verify_write_or_copyup (path, NULL); \ + if (r != 0) \ + return r; \ } while (0) static int callback_chmod (const char *path, mode_t mode) { - path = ENSURE_RELPATH (path); - VERIFY_WRITE(path); + PATH_WRITE_ENTRYPOINT (path); + /* Note we can't use AT_SYMLINK_NOFOLLOW yet; * https://marc.info/?l=linux-kernel&m=148830147803162&w=2 * https://marc.info/?l=linux-fsdevel&m=149193779929561&w=2 @@ -258,8 +308,8 @@ callback_chmod (const char *path, mode_t mode) static int callback_chown (const char *path, uid_t uid, gid_t gid) { - path = ENSURE_RELPATH (path); - VERIFY_WRITE(path); + PATH_WRITE_ENTRYPOINT (path); + if (fchownat (basefd, path, uid, gid, AT_SYMLINK_NOFOLLOW) != 0) return -errno; return 0; @@ -268,12 +318,9 @@ callback_chown (const char *path, uid_t uid, gid_t gid) static int callback_truncate (const char *path, off_t size) { - glnx_autofd int fd = -1; + PATH_WRITE_ENTRYPOINT (path); - path = ENSURE_RELPATH (path); - VERIFY_WRITE(path); - - fd = openat (basefd, path, O_NOFOLLOW|O_WRONLY); + glnx_autofd int fd = openat (basefd, path, O_NOFOLLOW|O_WRONLY); if (fd == -1) return -errno; @@ -286,6 +333,9 @@ callback_truncate (const char *path, off_t size) static int callback_utimens (const char *path, const struct timespec tv[2]) { + /* This one isn't write-verified, we support changing times + * even for hardlinked files. + */ path = ENSURE_RELPATH (path); if (utimensat (basefd, path, tv, AT_SYMLINK_NOFOLLOW) == -1) @@ -324,20 +374,38 @@ do_open (const char *path, mode_t mode, struct fuse_file_info *finfo) return -errno; } - int r = can_write_stbuf (&stbuf); + int r = verify_write_or_copyup (path, &stbuf); if (r != 0) { (void) close (fd); return r; } - /* Handle O_TRUNC here only after verifying hardlink state */ - if (finfo->flags & O_TRUNC) + /* In the copyup case, we need to re-open */ + if (opt_copyup) { - if (ftruncate (fd, 0) == -1) + (void) close (fd); + /* Note that unlike the initial open, we will pass through + * O_TRUNC. More ideally in this copyup case we'd avoid copying + * the whole file in the first place, but eh. It's not like we're + * high performance anyways. + */ + fd = openat (basefd, path, finfo->flags & ~(O_EXCL|O_CREAT), mode); + if (fd == -1) + return -errno; + } + else + { + /* In the non-copyup case we handle O_TRUNC here, after we've verified + * the hardlink state above with verify_write_or_copyup(). + */ + if (finfo->flags & O_TRUNC) { - (void) close (fd); - return -errno; + if (ftruncate (fd, 0) == -1) + { + (void) close (fd); + return -errno; + } } } } @@ -521,6 +589,7 @@ struct fuse_operations callback_oper = { enum { KEY_HELP, KEY_VERSION, + KEY_COPYUP, }; static void @@ -565,6 +634,9 @@ rofs_parse_opt (void *data, const char *arg, int key, case KEY_HELP: usage (outargs->argv[0]); exit (EXIT_SUCCESS); + case KEY_COPYUP: + opt_copyup = TRUE; + return 0; default: fprintf (stderr, "see `%s -h' for usage\n", outargs->argv[0]); exit (EXIT_FAILURE); @@ -577,6 +649,7 @@ static struct fuse_opt rofs_opts[] = { FUSE_OPT_KEY ("--help", KEY_HELP), FUSE_OPT_KEY ("-V", KEY_VERSION), FUSE_OPT_KEY ("--version", KEY_VERSION), + FUSE_OPT_KEY ("--copyup", KEY_COPYUP), FUSE_OPT_END }; diff --git a/tests/test-rofiles-fuse.sh b/tests/test-rofiles-fuse.sh index 6222929e..805a7188 100755 --- a/tests/test-rofiles-fuse.sh +++ b/tests/test-rofiles-fuse.sh @@ -26,7 +26,7 @@ skip_without_user_xattrs setup_test_repository "bare" -echo "1..8" +echo "1..11" cd ${test_tmpdir} mkdir mnt @@ -117,3 +117,23 @@ echo "ok checkout copy fallback" # check that O_RDONLY|O_CREAT is handled correctly; used by flock(1) at least flock mnt/nonexistent-file echo "ok create file in ro mode" +echo "ok flock" + +# And now with --copyup enabled + +fusermount -u ${test_tmpdir}/mnt +assert_not_has_file mnt/firstfile +rofiles-fuse --copyup checkout-test2 mnt +assert_file_has_content mnt/firstfile first +echo "ok copyup mount" + +firstfile_orig_inode=$(stat -c %i checkout-test2/firstfile) +for path in firstfile{,-link}; do + echo truncating > mnt/${path} + assert_file_has_content mnt/${path} truncating + assert_not_file_has_content mnt/${path} first +done +firstfile_new_inode=$(stat -c %i checkout-test2/firstfile) +assert_not_streq "${firstfile_orig_inode}" "${firstfile_new_inode}" + +echo "ok copyup" From 3b9304b5d76968c92378b708a76239fa5f08dcf9 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 5 Jan 2018 16:02:58 -0500 Subject: [PATCH 10/30] rofiles: Fix --copyup when creating a new file This tripped up the `docbook-dtds` `%post` in my experiments with doing rpm-ostree for buildroots. I cloned and built [xfstests](https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git) but haven't yet investigated actually running it. In the meantime let's do the obvious fix here; we need to distinguish between "copyup enabled" and "actually did a copyup" in the open path at least, since if we didn't do a copyup we don't need to re-open. Closes: #1396 Approved by: jlebon --- src/rofiles-fuse/main.c | 15 ++++++++--- tests/test-rofiles-fuse.sh | 51 ++++++++++++++++++++++++++++++++------ 2 files changed, 54 insertions(+), 12 deletions(-) diff --git a/src/rofiles-fuse/main.c b/src/rofiles-fuse/main.c index 2e1c1346..77c2f3d7 100644 --- a/src/rofiles-fuse/main.c +++ b/src/rofiles-fuse/main.c @@ -247,10 +247,14 @@ gioerror_to_errno (GIOErrorEnum e) } static int -verify_write_or_copyup (const char *path, const struct stat *stbuf) +verify_write_or_copyup (const char *path, const struct stat *stbuf, + gboolean *out_did_copyup) { struct stat stbuf_local; + if (out_did_copyup) + *out_did_copyup = FALSE; + /* If a stbuf wasn't provided, gather it now */ if (!stbuf) { @@ -272,6 +276,8 @@ verify_write_or_copyup (const char *path, const struct stat *stbuf) g_autoptr(GError) tmp_error = NULL; if (!ostree_break_hardlink (basefd, path, FALSE, NULL, &tmp_error)) return -gioerror_to_errno ((GIOErrorEnum)tmp_error->code); + if (out_did_copyup) + *out_did_copyup = TRUE; } else return -EROFS; @@ -286,7 +292,7 @@ verify_write_or_copyup (const char *path, const struct stat *stbuf) */ #define PATH_WRITE_ENTRYPOINT(path) do { \ path = ENSURE_RELPATH (path); \ - int r = verify_write_or_copyup (path, NULL); \ + int r = verify_write_or_copyup (path, NULL, NULL); \ if (r != 0) \ return r; \ } while (0) @@ -374,7 +380,8 @@ do_open (const char *path, mode_t mode, struct fuse_file_info *finfo) return -errno; } - int r = verify_write_or_copyup (path, &stbuf); + gboolean did_copyup; + int r = verify_write_or_copyup (path, &stbuf, &did_copyup); if (r != 0) { (void) close (fd); @@ -382,7 +389,7 @@ do_open (const char *path, mode_t mode, struct fuse_file_info *finfo) } /* In the copyup case, we need to re-open */ - if (opt_copyup) + if (did_copyup) { (void) close (fd); /* Note that unlike the initial open, we will pass through diff --git a/tests/test-rofiles-fuse.sh b/tests/test-rofiles-fuse.sh index 805a7188..1c91a5cd 100755 --- a/tests/test-rofiles-fuse.sh +++ b/tests/test-rofiles-fuse.sh @@ -121,19 +121,54 @@ echo "ok flock" # And now with --copyup enabled -fusermount -u ${test_tmpdir}/mnt -assert_not_has_file mnt/firstfile -rofiles-fuse --copyup checkout-test2 mnt +copyup_reset() { + cd ${test_tmpdir} + fusermount -u mnt + rm checkout-test2 -rf + $OSTREE checkout -H test2 checkout-test2 + rofiles-fuse --copyup checkout-test2 mnt +} + +assert_test_file() { + t=$1 + f=$2 + if ! test ${t} "${f}"; then + ls -al "${f}" + fatal "Failed test ${t} ${f}" + fi +} + +copyup_reset assert_file_has_content mnt/firstfile first echo "ok copyup mount" +# Test O_TRUNC directly firstfile_orig_inode=$(stat -c %i checkout-test2/firstfile) -for path in firstfile{,-link}; do - echo truncating > mnt/${path} - assert_file_has_content mnt/${path} truncating - assert_not_file_has_content mnt/${path} first -done +echo -n truncating > mnt/firstfile +assert_streq "$(cat mnt/firstfile)" truncating firstfile_new_inode=$(stat -c %i checkout-test2/firstfile) assert_not_streq "${firstfile_orig_inode}" "${firstfile_new_inode}" +assert_test_file -f checkout-test2/firstfile + +copyup_reset +firstfile_link_orig_inode=$(stat -c %i checkout-test2/firstfile-link) +firstfile_orig_inode=$(stat -c %i checkout-test2/firstfile) +# Now write via the symlink +echo -n truncating > mnt/firstfile-link +assert_streq "$(cat mnt/firstfile)" truncating +firstfile_new_inode=$(stat -c %i checkout-test2/firstfile) +firstfile_link_new_inode=$(stat -c %i checkout-test2/firstfile-link) +assert_not_streq "${firstfile_orig_inode}" "${firstfile_new_inode}" +assert_streq "${firstfile_link_orig_inode}" "${firstfile_link_new_inode}" +assert_test_file -f checkout-test2/firstfile +# Verify we didn't replace the link with a regfile somehow +assert_test_file -L checkout-test2/firstfile-link + +# These both end up creating new files; in the sed case we'll then do a rename() +copyup_reset +echo "hello new file" > mnt/a-new-non-copyup-file +assert_file_has_content_literal mnt/a-new-non-copyup-file "hello new file" +sed -i -e s,first,second, mnt/firstfile +assert_file_has_content_literal mnt/firstfile "second" echo "ok copyup" From c8d9da8d96d06b60e4760094695b125121f95868 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 8 Jan 2018 09:28:47 -0500 Subject: [PATCH 11/30] bin: Fix cookie builtin build with curl but no soup Prep for supporting `--with-curl --without-soup`. Closes: #1397 Approved by: cgwalters --- src/ostree/ot-builtin-remote.c | 2 +- src/ostree/ot-remote-builtins.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ostree/ot-builtin-remote.c b/src/ostree/ot-builtin-remote.c index dfb07d03..b89e8ba3 100644 --- a/src/ostree/ot-builtin-remote.c +++ b/src/ostree/ot-builtin-remote.c @@ -41,7 +41,7 @@ static OstreeCommand remote_subcommands[] = { { "gpg-import", OSTREE_BUILTIN_FLAG_NONE, ot_remote_builtin_gpg_import, "Import GPG keys" }, -#ifdef HAVE_LIBSOUP +#ifdef HAVE_LIBCURL_OR_LIBSOUP { "add-cookie", OSTREE_BUILTIN_FLAG_NONE, ot_remote_builtin_add_cookie, "Add a cookie to remote" }, diff --git a/src/ostree/ot-remote-builtins.h b/src/ostree/ot-remote-builtins.h index ce788524..60f4165e 100644 --- a/src/ostree/ot-remote-builtins.h +++ b/src/ostree/ot-remote-builtins.h @@ -31,7 +31,7 @@ BUILTINPROTO(add); BUILTINPROTO(delete); BUILTINPROTO(gpg_import); BUILTINPROTO(list); -#ifdef HAVE_LIBSOUP +#ifdef HAVE_LIBCURL_OR_LIBSOUP BUILTINPROTO(add_cookie); BUILTINPROTO(list_cookies); BUILTINPROTO(delete_cookie); From 353fb175c628e0d1d767a6ae323a495afdc03fb2 Mon Sep 17 00:00:00 2001 From: Anton Gerasimov Date: Mon, 8 Jan 2018 14:53:46 +0100 Subject: [PATCH 12/30] build-sys: Allow building with curl, but without libsoup Some people (particularly embedded) may find it simpler to drop libsoup from the build dependency side, but still use libcurl. Note though this currently neuters almost all of the tests. Signed-off-by: Anton Gerasimov Closes: #1397 Approved by: cgwalters --- Makefile-tests.am | 8 ++++++++ configure.ac | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/Makefile-tests.am b/Makefile-tests.am index 25a8d08b..9198eb31 100644 --- a/Makefile-tests.am +++ b/Makefile-tests.am @@ -381,6 +381,14 @@ dist_test_scripts += $(_installed_or_uninstalled_test_scripts) test_programs += $(_installed_or_uninstalled_test_programs) endif +if !USE_LIBSOUP +no-soup-for-you-warning: + @echo "WARNING: $(PACKAGE) was built without libsoup, which is currently" 1>&2 + @echo "WARNING: required for many unit tests." 1>&2 + sleep 10 +check: no-soup-for-you-warning +endif + # Unfortunately the glib test data APIs don't actually handle # non-recursive Automake, so we change our code to canonically look # for tests/ which is just a symlink when installed. diff --git a/configure.ac b/configure.ac index fa436ea6..e26c903c 100644 --- a/configure.ac +++ b/configure.ac @@ -197,7 +197,7 @@ AM_COND_IF(BUILDOPT_TRIVIAL_HTTPD, ) AS_IF([test x$with_curl = xyes && test x$with_soup = xno], [ - AC_MSG_ERROR([Curl enabled, but libsoup is not; libsoup is needed for tests]) + AC_MSG_WARN([Curl enabled, but libsoup is not; libsoup is needed for tests (make check, etc.)]) ]) AM_CONDITIONAL(USE_CURL_OR_SOUP, test x$with_curl != xno || test x$with_soup != xno) AS_IF([test x$with_curl != xno || test x$with_soup != xno], From 9fe6ddbaef9cd34a711c0322197b39e87571aa78 Mon Sep 17 00:00:00 2001 From: Will Thompson Date: Tue, 9 Jan 2018 11:51:04 +0000 Subject: [PATCH 13/30] ostree-grub-generator: fix typo in comment Closes: #1398 Approved by: jlebon --- src/boot/grub2/ostree-grub-generator | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/boot/grub2/ostree-grub-generator b/src/boot/grub2/ostree-grub-generator index 82e66bd7..ad8c2b83 100644 --- a/src/boot/grub2/ostree-grub-generator +++ b/src/boot/grub2/ostree-grub-generator @@ -1,6 +1,6 @@ #!/bin/sh -# To use a custrom script for generating grub.cfg, set the --with-grub2-mkconfig=no +# To use a custom script for generating grub.cfg, set the --with-grub2-mkconfig=no # configure switch when configuring and building OSTree. # # This script is called by ostree/src/libostree/ostree-bootloader-grub2.c whenever From 2c2e6799bed208306dba600c8a2de813660ffd93 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 9 Jan 2018 10:22:50 -0500 Subject: [PATCH 14/30] grub2: Exit gracefully if there's no system ostree repository Apparently there testing systems that literally install *all* packages. Having `ostree-grub2` currently causes grub2 to fail on a non-ostree managed system. Let's just gracefully exit if there's no system repository. https://bugzilla.redhat.com/show_bug.cgi?id=1532668 Closes: #1399 Approved by: jlebon --- src/boot/grub2/grub2-15_ostree | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/boot/grub2/grub2-15_ostree b/src/boot/grub2/grub2-15_ostree index dfff6e89..0b9bf930 100644 --- a/src/boot/grub2/grub2-15_ostree +++ b/src/boot/grub2/grub2-15_ostree @@ -17,10 +17,14 @@ # Free Software Foundation, Inc., 59 Temple Place, Suite 330, # Boston, MA 02111-1307, USA. -# Gracefully exit if ostree is not installed +# Gracefully exit if ostree is not installed, or there's +# no system repository initialized. if ! which ostree >/dev/null 2>/dev/null; then exit 0 fi +if ! test -d /ostree/repo; then + exit 0 +fi # Make sure we're in the right environment if ! test -n "${GRUB_DEVICE}"; then From 95e574d09b36846e1611b5e89e1caf068b2468c3 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Tue, 9 Jan 2018 20:00:24 +0000 Subject: [PATCH 15/30] bin/commit: move parent checking code higher up No functional change. Prep for the next commit. Closes: #1402 Approved by: cgwalters --- src/ostree/ot-builtin-commit.c | 64 +++++++++++++++++----------------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/src/ostree/ot-builtin-commit.c b/src/ostree/ot-builtin-commit.c index 9c05428e..842d07d1 100644 --- a/src/ostree/ot-builtin-commit.c +++ b/src/ostree/ot-builtin-commit.c @@ -466,6 +466,38 @@ ostree_builtin_commit (int argc, char **argv, OstreeCommandInvocation *invocatio goto out; } + if (!(opt_branch || opt_orphan)) + { + g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED, + "A branch must be specified with --branch, or use --orphan"); + goto out; + } + + if (opt_parent) + { + if (g_str_equal (opt_parent, "none")) + parent = NULL; + else + { + if (!ostree_validate_checksum_string (opt_parent, error)) + goto out; + parent = g_strdup (opt_parent); + } + } + else if (!opt_orphan) + { + if (!ostree_repo_resolve_rev (repo, opt_branch, TRUE, &parent, error)) + { + if (g_error_matches (*error, G_IO_ERROR, G_IO_ERROR_IS_DIRECTORY)) + { + /* A folder exists with the specified ref name, + * which is handled by _ostree_repo_write_ref */ + g_clear_error (error); + } + else goto out; + } + } + if (opt_metadata_strings || opt_metadata_variants) { g_autoptr(GVariantBuilder) builder = @@ -493,13 +525,6 @@ ostree_builtin_commit (int argc, char **argv, OstreeCommandInvocation *invocatio detached_metadata = g_variant_ref_sink (g_variant_builder_end (builder)); } - if (!(opt_branch || opt_orphan)) - { - g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED, - "A branch must be specified with --branch, or use --orphan"); - goto out; - } - if (opt_no_xattrs) flags |= OSTREE_REPO_COMMIT_MODIFIER_FLAGS_SKIP_XATTRS; if (opt_consume) @@ -543,31 +568,6 @@ ostree_builtin_commit (int argc, char **argv, OstreeCommandInvocation *invocatio } } - if (opt_parent) - { - if (g_str_equal (opt_parent, "none")) - parent = NULL; - else - { - if (!ostree_validate_checksum_string (opt_parent, error)) - goto out; - parent = g_strdup (opt_parent); - } - } - else if (!opt_orphan) - { - if (!ostree_repo_resolve_rev (repo, opt_branch, TRUE, &parent, error)) - { - if (g_error_matches (*error, G_IO_ERROR, G_IO_ERROR_IS_DIRECTORY)) - { - /* A folder exists with the specified ref name, - * which is handled by _ostree_repo_write_ref */ - g_clear_error (error); - } - else goto out; - } - } - if (opt_editor) { if (!commit_editor (repo, opt_branch, &opt_subject, &commit_body, cancellable, error)) From 939791b4fa528bd69d25ecb9354b0a3013c45be8 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Tue, 9 Jan 2018 20:29:22 +0000 Subject: [PATCH 16/30] bin/commit: add --keep-metadata option Clients of libostree such as rpm-ostree make extensive use of the `ostree commit -b foo --tree=ref=foo` pattern in their tests, e.g. to simulate an update. What I'm trying to solve here is that it's often the case that we want to keep metadata from the previous commit without having to be too verbose (i.e. reading from the parent, then passing it as an argument). The new `--keep-metadata` switch makes this really easy. I intend to use this in the rpm-ostree testsuite to make sure we always carry over the `source-title` metadata as well as during set up for tests that require `rpmostree.rpmdb.pkglist` metadata. I initially implemented this in a small wrapper script that uses the API directly, though we make use of so many other `ostree commit` functions that it'd require re-implementing a lot of it. Closes: #1402 Approved by: cgwalters --- bash/ostree | 1 + src/ostree/ot-builtin-commit.c | 37 +++++++++++++++++++++++++++++++++- tests/basic-test.sh | 14 ++++++++++++- 3 files changed, 50 insertions(+), 2 deletions(-) diff --git a/bash/ostree b/bash/ostree index 0ba135e7..a6eb56de 100644 --- a/bash/ostree +++ b/bash/ostree @@ -783,6 +783,7 @@ _ostree_commit() { local options_with_args=" --add-detached-metadata-string --add-metadata-string + --keep-metadata --bind-ref --body -m --body-file -F diff --git a/src/ostree/ot-builtin-commit.c b/src/ostree/ot-builtin-commit.c index 842d07d1..5e5cddd1 100644 --- a/src/ostree/ot-builtin-commit.c +++ b/src/ostree/ot-builtin-commit.c @@ -45,6 +45,7 @@ static char *opt_skiplist_file; static char **opt_metadata_strings; static char **opt_metadata_variants; static char **opt_detached_metadata_strings; +static char **opt_metadata_keep; static gboolean opt_link_checkout_speedup; static gboolean opt_skip_if_unchanged; static gboolean opt_tar_autocreate_parents; @@ -96,6 +97,7 @@ static GOptionEntry options[] = { { "tree", 0, 0, G_OPTION_ARG_STRING_ARRAY, &opt_trees, "Overlay the given argument as a tree", "dir=PATH or tar=TARFILE or ref=COMMIT" }, { "add-metadata-string", 0, 0, G_OPTION_ARG_STRING_ARRAY, &opt_metadata_strings, "Add a key/value pair to metadata", "KEY=VALUE" }, { "add-metadata", 0, 0, G_OPTION_ARG_STRING_ARRAY, &opt_metadata_variants, "Add a key/value pair to metadata, where the KEY is a string, an VALUE is g_variant_parse() formatted", "KEY=VALUE" }, + { "keep-metadata", 0, 0, G_OPTION_ARG_STRING_ARRAY, &opt_metadata_keep, "Keep metadata KEY and its associated VALUE from parent", "KEY" }, { "add-detached-metadata-string", 0, 0, G_OPTION_ARG_STRING_ARRAY, &opt_detached_metadata_strings, "Add a key/value pair to detached metadata", "KEY=VALUE" }, { "owner-uid", 0, 0, G_OPTION_ARG_INT, &opt_owner_uid, "Set file ownership user id", "UID" }, { "owner-gid", 0, 0, G_OPTION_ARG_INT, &opt_owner_gid, "Set file ownership group id", "GID" }, @@ -498,7 +500,15 @@ ostree_builtin_commit (int argc, char **argv, OstreeCommandInvocation *invocatio } } - if (opt_metadata_strings || opt_metadata_variants) + if (!parent && opt_metadata_keep) + { + g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED, + "Either --branch or --parent must be specified when using " + "--keep-metadata"); + goto out; + } + + if (opt_metadata_strings || opt_metadata_variants || opt_metadata_keep) { g_autoptr(GVariantBuilder) builder = g_variant_builder_new (G_VARIANT_TYPE ("a{sv}")); @@ -511,6 +521,31 @@ ostree_builtin_commit (int argc, char **argv, OstreeCommandInvocation *invocatio !parse_keyvalue_strings (builder, opt_metadata_variants, TRUE, error)) goto out; + if (opt_metadata_keep) + { + g_assert (parent); + + g_autoptr(GVariant) parent_commit = NULL; + if (!ostree_repo_load_commit (repo, parent, &parent_commit, NULL, error)) + goto out; + + g_auto(GVariantDict) dict; + g_variant_dict_init (&dict, g_variant_get_child_value (parent_commit, 0)); + for (char **keyp = opt_metadata_keep; keyp && *keyp; keyp++) + { + const char *key = *keyp; + g_autoptr(GVariant) val = g_variant_dict_lookup_value (&dict, key, NULL); + if (!val) + { + g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, + "Missing metadata key '%s' from commit '%s'", key, parent); + goto out; + } + + g_variant_builder_add (builder, "{sv}", key, val); + } + } + metadata = g_variant_ref_sink (g_variant_builder_end (builder)); } diff --git a/tests/basic-test.sh b/tests/basic-test.sh index c4eb9cad..9c8771e5 100644 --- a/tests/basic-test.sh +++ b/tests/basic-test.sh @@ -19,7 +19,7 @@ set -euo pipefail -echo "1..$((78 + ${extra_basic_tests:-0}))" +echo "1..$((79 + ${extra_basic_tests:-0}))" CHECKOUT_U_ARG="" CHECKOUT_H_ARGS="-H" @@ -779,6 +779,18 @@ $OSTREE show --print-detached-metadata-key=SIGNATURE test2 > test2-meta assert_file_has_content test2-meta "HANCOCK" echo "ok metadata commit with strings" +$OSTREE commit ${COMMIT_ARGS} -b test2 --tree=ref=test2 \ + --add-detached-metadata-string=SIGNATURE=HANCOCK \ + --keep-metadata=KITTENS --keep-metadata=SOMENUM +if $OSTREE show --print-metadata-key=FOO test2; then + assert_not_reached "FOO was kept without explicit --keep-metadata?" +fi +$OSTREE show --print-metadata-key=KITTENS test2 > test2-meta +assert_file_has_content test2-meta "CUTE" +$OSTREE show -B --print-metadata-key=SOMENUM test2 > test2-meta +assert_file_has_content test2-meta "uint64 42" +echo "ok keep metadata from parent" + cd ${test_tmpdir} $OSTREE show --print-metadata-key=ostree.ref-binding test2 > test2-ref-binding assert_file_has_content test2-ref-binding 'test2' From 94bbbdf3ca37507197b8db62271b69b9fd1374e4 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Tue, 9 Jan 2018 21:08:09 +0000 Subject: [PATCH 17/30] bash/ostree: add missing --add-metadata option Closes: #1402 Approved by: cgwalters --- bash/ostree | 1 + 1 file changed, 1 insertion(+) diff --git a/bash/ostree b/bash/ostree index a6eb56de..fe8e3d2c 100644 --- a/bash/ostree +++ b/bash/ostree @@ -783,6 +783,7 @@ _ostree_commit() { local options_with_args=" --add-detached-metadata-string --add-metadata-string + --add-metadata --keep-metadata --bind-ref --body -m From 62cb078973f2dacd0975a7d67da907d267dbd8bd Mon Sep 17 00:00:00 2001 From: Gatis Paeglis Date: Fri, 12 Aug 2016 08:50:29 +0200 Subject: [PATCH 18/30] ostree-prepare-root: enabler for simpler kernel arg With the current approach, when ostree-prepare-root is used on the kernel command line as init=, it always assumes that the next value in the argument list is a path to the sysroot. The code for falling back to a default path (if none is provided), would only work if init= is the last arg in the argument list. We can not rely on that and have to explicitly provide the path to the sysroot. Which defeats the purpose of a default path selection code. To keep command line neater assume that sysroot is on / when using ostree-prepare-root as init. This probably is what most people want anyways. Also _ostree_kernel_args* API assumes that args are space separated list. Which is problematic for: "init=${ostree}/usr/lib/ostree/ostree-prepare-root /" as it gets split in two. Closes: #1401 Approved by: cgwalters --- src/switchroot/ostree-prepare-root.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/switchroot/ostree-prepare-root.c b/src/switchroot/ostree-prepare-root.c index 9b8c3381..43e15fcc 100644 --- a/src/switchroot/ostree-prepare-root.c +++ b/src/switchroot/ostree-prepare-root.c @@ -102,10 +102,16 @@ main(int argc, char *argv[]) struct stat stbuf; int we_mounted_proc = 0; - if (argc < 2) - root_arg = "/"; + if (getpid() == 1) + { + root_arg = "/"; + } else - root_arg = argv[1]; + { + if (argc < 2) + err (EXIT_FAILURE, "usage: ostree-prepare-root SYSROOT"); + root_arg = argv[1]; + } if (stat ("/proc/cmdline", &stbuf) < 0) { From 652d9dd98a610ba6ebda3e3e6c5202f53d3dc439 Mon Sep 17 00:00:00 2001 From: Gatis Paeglis Date: Fri, 12 Aug 2016 11:51:04 +0200 Subject: [PATCH 19/30] deploy: add --karg-none argument If the current deployment has "rootwait root=/dev/sda2", but the new deployment does not need "rootwait" anymore, there is no way to clear this arg at the moment (as opposed to "karg=root=", which overrides any earlier argument with the same name). With "--karg-none" users can now clear all the previous args and set new "root=": ostree admin deploy --karg-none --karg=root=LABEL=rootfs Closes: #1401 Approved by: cgwalters --- src/ostree/ot-admin-builtin-deploy.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/ostree/ot-admin-builtin-deploy.c b/src/ostree/ot-admin-builtin-deploy.c index 83550331..8da9dbc3 100644 --- a/src/ostree/ot-admin-builtin-deploy.c +++ b/src/ostree/ot-admin-builtin-deploy.c @@ -40,6 +40,7 @@ static char **opt_kernel_argv_append; static gboolean opt_kernel_proc_cmdline; static char *opt_osname; static char *opt_origin_path; +static gboolean opt_kernel_arg_none; /* ATTENTION: * Please remember to update the bash-completion script (bash/ostree) and @@ -56,6 +57,7 @@ static GOptionEntry options[] = { { "karg-proc-cmdline", 0, 0, G_OPTION_ARG_NONE, &opt_kernel_proc_cmdline, "Import current /proc/cmdline", NULL }, { "karg", 0, 0, G_OPTION_ARG_STRING_ARRAY, &opt_kernel_argv, "Set kernel argument, like root=/dev/sda1; this overrides any earlier argument with the same name", "NAME=VALUE" }, { "karg-append", 0, 0, G_OPTION_ARG_STRING_ARRAY, &opt_kernel_argv_append, "Append kernel argument; useful with e.g. console= that can be used multiple times", "NAME=VALUE" }, + { "karg-none", 0, 0, G_OPTION_ARG_NONE, &opt_kernel_arg_none, "Do not import kernel arguments", NULL }, { NULL } }; @@ -79,6 +81,12 @@ ot_admin_builtin_deploy (int argc, char **argv, OstreeCommandInvocation *invocat return FALSE; } + if (opt_kernel_proc_cmdline && opt_kernel_arg_none) + { + ot_util_usage_error (context, "Can't specify both --karg-proc-cmdline and --karg-none", error); + return FALSE; + } + const char *refspec = argv[1]; OstreeRepo *repo = ostree_sysroot_repo (sysroot); @@ -130,7 +138,7 @@ ot_admin_builtin_deploy (int argc, char **argv, OstreeCommandInvocation *invocat if (!_ostree_kernel_args_append_proc_cmdline (kargs, cancellable, error)) return FALSE; } - else if (merge_deployment) + else if (merge_deployment && !opt_kernel_arg_none) { OstreeBootconfigParser *bootconfig = ostree_deployment_get_bootconfig (merge_deployment); g_auto(GStrv) previous_args = g_strsplit (ostree_bootconfig_parser_get (bootconfig, "options"), " ", -1); From 4233b1db199d5207138b9f2946b35d28557cc7a5 Mon Sep 17 00:00:00 2001 From: Gatis Paeglis Date: Wed, 24 Aug 2016 13:26:47 +0200 Subject: [PATCH 20/30] Support for booting without initramfs Previously when initramfs-* was not found in a deployment's boot directory, it was assumed that rootfs is prepared for ostree booting by a kernel patch. With this patch, the behaviour changes to be - if initramfs-* is not found, assume that system is using a static ostree-prepare-root as init process. Booting without initramfs is a common use case on embedded systems. This approach is also more convenient, than having to patch the kernel. Closes: #1401 Approved by: cgwalters --- Makefile-tests.am | 1 + src/boot/grub2/ostree-grub-generator | 8 +++-- src/libostree/ostree-sysroot-deploy.c | 18 +++++++--- tests/test-no-initramfs.sh | 52 +++++++++++++++++++++++++++ 4 files changed, 71 insertions(+), 8 deletions(-) create mode 100755 tests/test-no-initramfs.sh diff --git a/Makefile-tests.am b/Makefile-tests.am index 9198eb31..350209de 100644 --- a/Makefile-tests.am +++ b/Makefile-tests.am @@ -116,6 +116,7 @@ _installed_or_uninstalled_test_scripts = \ tests/test-pull-mirrorlist.sh \ tests/test-summary-update.sh \ tests/test-summary-view.sh \ + tests/test-no-initramfs.sh \ $(NULL) experimental_test_scripts = \ diff --git a/src/boot/grub2/ostree-grub-generator b/src/boot/grub2/ostree-grub-generator index ad8c2b83..3985008f 100644 --- a/src/boot/grub2/ostree-grub-generator +++ b/src/boot/grub2/ostree-grub-generator @@ -28,7 +28,7 @@ entries_path=$(dirname $new_grub2_cfg)/entries read_config() { - config_file=${entries_path}/${1} + config_file=${1} title="" initrd="" options="" @@ -67,11 +67,13 @@ populate_menu() else boot_prefix="${OSTREE_BOOT_PARTITION}" fi - for config in $(ls ${entries_path}); do + for config in $(ls $entries_path/*.conf); do read_config ${config} menu="${menu}menuentry '${title}' {\n" menu="${menu}\t linux ${boot_prefix}${linux} ${options}\n" - menu="${menu}\t initrd ${boot_prefix}${initrd}\n" + if [ -n "${initrd}" ] ; then + menu="${menu}\t initrd ${boot_prefix}${initrd}\n" + fi menu="${menu}}\n\n" done # The printf command seems to be more reliable across shells for special character (\n, \t) evaluation diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index 29c90ea7..5dc5bde0 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -1700,21 +1700,29 @@ install_deployment_kernel (OstreeSysroot *sysroot, g_autofree char * boot_relpath = g_strconcat ("/", bootcsumdir, "/", kernel_layout->kernel_namever, NULL); ostree_bootconfig_parser_set (bootconfig, "linux", boot_relpath); + val = ostree_bootconfig_parser_get (bootconfig, "options"); + g_autoptr(OstreeKernelArgs) kargs = _ostree_kernel_args_from_string (val); + if (kernel_layout->initramfs_namever) { g_autofree char * boot_relpath = g_strconcat ("/", bootcsumdir, "/", kernel_layout->initramfs_namever, NULL); ostree_bootconfig_parser_set (bootconfig, "initrd", boot_relpath); } - - val = ostree_bootconfig_parser_get (bootconfig, "options"); + else + { + g_autofree char *prepare_root_arg = NULL; + prepare_root_arg = g_strdup_printf ("init=/ostree/boot.%d/%s/%s/%d/usr/lib/ostree/ostree-prepare-root", + new_bootversion, osname, bootcsum, + ostree_deployment_get_bootserial (deployment)); + _ostree_kernel_args_replace_take (kargs, g_steal_pointer (&prepare_root_arg)); + } /* Note this is parsed in ostree-impl-system-generator.c */ g_autofree char *ostree_kernel_arg = g_strdup_printf ("ostree=/ostree/boot.%d/%s/%s/%d", new_bootversion, osname, bootcsum, ostree_deployment_get_bootserial (deployment)); - g_autoptr(OstreeKernelArgs) kargs = _ostree_kernel_args_from_string (val); - _ostree_kernel_args_replace_take (kargs, ostree_kernel_arg); - ostree_kernel_arg = NULL; + _ostree_kernel_args_replace_take (kargs, g_steal_pointer (&ostree_kernel_arg)); + g_autofree char *options_key = _ostree_kernel_args_to_string (kargs); ostree_bootconfig_parser_set (bootconfig, "options", options_key); diff --git a/tests/test-no-initramfs.sh b/tests/test-no-initramfs.sh new file mode 100755 index 00000000..dcd21ee1 --- /dev/null +++ b/tests/test-no-initramfs.sh @@ -0,0 +1,52 @@ +#!/bin/bash + +. $(dirname $0)/libtest.sh + +echo "1..3" + +setup_os_repository "archive-z2" "uboot" + +cd ${test_tmpdir} + +${CMD_PREFIX} ostree --repo=sysroot/ostree/repo remote add --set=gpg-verify=false testos $(cat httpd-address)/ostree/testos-repo +${CMD_PREFIX} ostree --repo=sysroot/ostree/repo pull testos testos/buildmaster/x86_64-runtime +${CMD_PREFIX} ostree admin deploy --karg=root=LABEL=rootfs --os=testos testos:testos/buildmaster/x86_64-runtime + +assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'root=LABEL=rootfs' +assert_not_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'init=' + +echo "ok deployment with initramfs" + +cd ${test_tmpdir}/osdata/boot +rm -f initramfs* vmlinuz* +echo "the kernel only" > vmlinuz-3.6.0 +bootcsum=$(cat vmlinuz-3.6.0 | sha256sum | cut -f 1 -d ' ') +mv vmlinuz-3.6.0 vmlinuz-3.6.0-${bootcsum} +cd - +${CMD_PREFIX} ostree --repo=${test_tmpdir}/testos-repo commit --tree=dir=osdata/ -b testos/buildmaster/x86_64-runtime +${CMD_PREFIX} ostree pull testos:testos/buildmaster/x86_64-runtime +${CMD_PREFIX} ostree admin deploy --os=testos --karg=root=/dev/sda2 --karg=rootwait testos:testos/buildmaster/x86_64-runtime +assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'rootwait' +assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'init=' +assert_not_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'initrd' + +echo "ok switching to bootdir with no initramfs" + +cd ${test_tmpdir}/osdata/boot +rm -f initramfs* vmlinuz* +echo "the kernel" > vmlinuz-3.6.0 +echo "initramfs to assist the kernel" > initramfs-3.6.0 +bootcsum=$(cat vmlinuz-3.6.0 initramfs-3.6.0 | sha256sum | cut -f 1 -d ' ') +mv vmlinuz-3.6.0 vmlinuz-3.6.0-${bootcsum} +mv initramfs-3.6.0 initramfs-3.6.0-${bootcsum} +cd - +${CMD_PREFIX} ostree --repo=${test_tmpdir}/testos-repo commit --tree=dir=osdata/ -b testos/buildmaster/x86_64-runtime +${CMD_PREFIX} ostree pull testos:testos/buildmaster/x86_64-runtime +${CMD_PREFIX} ostree admin deploy --os=testos --karg-none --karg=root=LABEL=rootfs testos:testos/buildmaster/x86_64-runtime +assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'initrd' +assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'root=LABEL=rootfs' +assert_not_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'rootwait' +assert_not_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'init=' + +echo "ok switching from no initramfs to initramfs enabled sysroot" + From 3724692d9e5a982ab74a7993d8c4527870b8073d Mon Sep 17 00:00:00 2001 From: Gatis Paeglis Date: Wed, 24 Aug 2016 14:02:18 +0200 Subject: [PATCH 21/30] ostree-grub-generator: update outdated comment Closes: #1401 Approved by: cgwalters --- src/boot/grub2/ostree-grub-generator | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/boot/grub2/ostree-grub-generator b/src/boot/grub2/ostree-grub-generator index 3985008f..97ef4d69 100644 --- a/src/boot/grub2/ostree-grub-generator +++ b/src/boot/grub2/ostree-grub-generator @@ -1,7 +1,6 @@ #!/bin/sh -# To use a custom script for generating grub.cfg, set the --with-grub2-mkconfig=no -# configure switch when configuring and building OSTree. +# The builtin grub.cfg generator. # # This script is called by ostree/src/libostree/ostree-bootloader-grub2.c whenever # boot loader configuration file needs to be updated. It can be used as a template From 3318db548eb204c3d0561a59e56c3a0fc5666276 Mon Sep 17 00:00:00 2001 From: William Manley Date: Tue, 7 Mar 2017 12:57:26 +0000 Subject: [PATCH 22/30] Tests: test-no-initramfs: Test both legacy and new kernel locations Closes: #1401 Approved by: cgwalters --- tests/test-no-initramfs.sh | 86 +++++++++++++++++++++++++------------- 1 file changed, 56 insertions(+), 30 deletions(-) diff --git a/tests/test-no-initramfs.sh b/tests/test-no-initramfs.sh index dcd21ee1..7c23a3e7 100755 --- a/tests/test-no-initramfs.sh +++ b/tests/test-no-initramfs.sh @@ -2,7 +2,7 @@ . $(dirname $0)/libtest.sh -echo "1..3" +echo "1..7" setup_os_repository "archive-z2" "uboot" @@ -17,36 +17,62 @@ assert_not_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'in echo "ok deployment with initramfs" -cd ${test_tmpdir}/osdata/boot -rm -f initramfs* vmlinuz* -echo "the kernel only" > vmlinuz-3.6.0 -bootcsum=$(cat vmlinuz-3.6.0 | sha256sum | cut -f 1 -d ' ') -mv vmlinuz-3.6.0 vmlinuz-3.6.0-${bootcsum} -cd - -${CMD_PREFIX} ostree --repo=${test_tmpdir}/testos-repo commit --tree=dir=osdata/ -b testos/buildmaster/x86_64-runtime -${CMD_PREFIX} ostree pull testos:testos/buildmaster/x86_64-runtime -${CMD_PREFIX} ostree admin deploy --os=testos --karg=root=/dev/sda2 --karg=rootwait testos:testos/buildmaster/x86_64-runtime -assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'rootwait' -assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'init=' -assert_not_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'initrd' +pull_test_tree() { + kernel_contents=$1 + initramfs_contents=$2 -echo "ok switching to bootdir with no initramfs" + printf "TEST SETUP:\n kernel: %s\n initramfs: %s\n layout: %s\n" \ + "$kernel_contents" "$initramfs_contents" "$layout" -cd ${test_tmpdir}/osdata/boot -rm -f initramfs* vmlinuz* -echo "the kernel" > vmlinuz-3.6.0 -echo "initramfs to assist the kernel" > initramfs-3.6.0 -bootcsum=$(cat vmlinuz-3.6.0 initramfs-3.6.0 | sha256sum | cut -f 1 -d ' ') -mv vmlinuz-3.6.0 vmlinuz-3.6.0-${bootcsum} -mv initramfs-3.6.0 initramfs-3.6.0-${bootcsum} -cd - -${CMD_PREFIX} ostree --repo=${test_tmpdir}/testos-repo commit --tree=dir=osdata/ -b testos/buildmaster/x86_64-runtime -${CMD_PREFIX} ostree pull testos:testos/buildmaster/x86_64-runtime -${CMD_PREFIX} ostree admin deploy --os=testos --karg-none --karg=root=LABEL=rootfs testos:testos/buildmaster/x86_64-runtime -assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'initrd' -assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'root=LABEL=rootfs' -assert_not_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'rootwait' -assert_not_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'init=' + rm -rf ${test_tmpdir}/osdata/usr/lib/modules/3.6.0/{initramfs.img,vmlinuz} \ + ${test_tmpdir}/osdata/usr/lib/ostree-boot \ + ${test_tmpdir}/osdata/boot + if [ "$layout" = "/usr/lib/modules" ]; then + # Fedora compatible layout + cd ${test_tmpdir}/osdata/usr/lib/modules/3.6.0 + echo -n "$kernel_contents" > vmlinuz + [ -n "$initramfs_contents" ] && echo -n "$initramfs_contents" > initramfs.img + elif [ "$layout" = "/usr/lib/ostree-boot" ] || [ "$layout" = "/boot" ]; then + # "Legacy" layout + mkdir -p "${test_tmpdir}/osdata/$layout" + cd "${test_tmpdir}/osdata/$layout" + bootcsum=$(echo -n "$kernel_contents$initramfs_contents" \ + | sha256sum | cut -f 1 -d ' ') + echo -n "$kernel_contents" > vmlinuz-${bootcsum} + [ -n "$initramfs_contents" ] && echo -n "$initramfs_contents" > initramfs-${bootcsum} + else + exit 1 + fi + cd - + ${CMD_PREFIX} ostree --repo=${test_tmpdir}/testos-repo commit --tree=dir=osdata/ -b testos/buildmaster/x86_64-runtime + ${CMD_PREFIX} ostree pull testos:testos/buildmaster/x86_64-runtime +} -echo "ok switching from no initramfs to initramfs enabled sysroot" +get_key_from_bootloader_conf() { + conffile=$1 + key=$2 + assert_file_has_content "$conffile" "^$key" + awk "/^$key/ { print \$2 }" "$conffile" +} + +for layout in /usr/lib/modules /usr/lib/ostree-boot /boot; +do + pull_test_tree "the kernel only" + ${CMD_PREFIX} ostree admin deploy --os=testos --karg=root=/dev/sda2 --karg=rootwait testos:testos/buildmaster/x86_64-runtime + assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'rootwait' + assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'init=' + assert_not_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'initrd' + + echo "ok switching to bootdir with no initramfs layout=$layout" + + pull_test_tree "the kernel" "initramfs to assist the kernel" + ${CMD_PREFIX} ostree admin deploy --os=testos --karg-none --karg=root=LABEL=rootfs testos:testos/buildmaster/x86_64-runtime + assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'initrd' + assert_file_has_content sysroot/boot/$(get_key_from_bootloader_conf sysroot/boot/loader/entries/ostree-testos-0.conf "initrd") "initramfs to assist the kernel" + assert_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'root=LABEL=rootfs' + assert_not_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'rootwait' + assert_not_file_has_content sysroot/boot/loader/entries/ostree-testos-0.conf 'init=' + + echo "ok switching from no initramfs to initramfs enabled sysroot layout=$layout" +done From 2c932d9721fbce43a49d453a759d584ac4265444 Mon Sep 17 00:00:00 2001 From: Matthew Leeds Date: Tue, 9 Jan 2018 23:36:40 -0800 Subject: [PATCH 23/30] find-remotes: Add --finders option It can be helpful to be able to choose which OstreeRepoFinder instances to use when using the find-remotes command. For example, if the tests need to run in an environment that can't have an Avahi daemon, this allows you to disable the Avahi (LAN) finder. This commit adds the --finders option for this purpose. Closes: #1407 Approved by: cgwalters --- src/ostree/ot-builtin-find-remotes.c | 106 ++++++++++++++++++++++++++- 1 file changed, 103 insertions(+), 3 deletions(-) diff --git a/src/ostree/ot-builtin-find-remotes.c b/src/ostree/ot-builtin-find-remotes.c index b15bce8d..b99f6e6a 100644 --- a/src/ostree/ot-builtin-find-remotes.c +++ b/src/ostree/ot-builtin-find-remotes.c @@ -30,12 +30,14 @@ #include "ostree-remote-private.h" static gchar *opt_cache_dir = NULL; +static gchar *opt_finders = NULL; static gboolean opt_disable_fsync = FALSE; static gboolean opt_pull = FALSE; static GOptionEntry options[] = { { "cache-dir", 0, 0, G_OPTION_ARG_FILENAME, &opt_cache_dir, "Use custom cache dir", NULL }, + { "finders", 0, 0, G_OPTION_ARG_STRING, &opt_finders, "Use the specified comma separated list of finders (e.g. config,lan,mount)", "FINDERS" }, { "disable-fsync", 0, 0, G_OPTION_ARG_NONE, &opt_disable_fsync, "Do not invoke fsync()", NULL }, { "pull", 0, 0, G_OPTION_ARG_NONE, &opt_pull, "Pull the updates after finding them", NULL }, { NULL } @@ -116,6 +118,52 @@ collection_ref_free0 (OstreeCollectionRef *ref) ostree_collection_ref_free (ref); } +static gboolean +validate_finders_list (char **finders, + GOptionContext *context, + GError **error) +{ + typedef struct { + gchar *finder_name; + gboolean already_used; + } Finder; + Finder valid_finders[] = { + {.finder_name = "config", .already_used = FALSE}, + {.finder_name = "lan", .already_used = FALSE}, + {.finder_name = "mount", .already_used = FALSE} + }; + + if (finders == NULL || *finders == NULL) + { + ot_util_usage_error (context, "List of finders in --finders option must not be empty", error); + return FALSE; + } + + for (char **iter = finders; iter && *iter; iter++) + { + gboolean is_valid_finder = FALSE; + for (int i = 0; i < 3; i++) + { + if (valid_finders[i].already_used == TRUE) + continue; + if (g_strcmp0 (*iter, valid_finders[i].finder_name) == 0) + { + is_valid_finder = TRUE; + valid_finders[i].already_used = TRUE; + } + } + if (!is_valid_finder) + { + g_autofree gchar *error_msg = NULL; + error_msg = g_strdup_printf ("Unknown or duplicate finder type given in --finders option: ā€˜%s’", *iter); + ot_util_usage_error (context, error_msg, error); + return FALSE; + } + } + + return TRUE; +} + /* TODO: Add a man page. */ gboolean ostree_builtin_find_remotes (int argc, @@ -127,12 +175,17 @@ ostree_builtin_find_remotes (int argc, g_autoptr(GOptionContext) context = NULL; g_autoptr(OstreeRepo) repo = NULL; g_autoptr(GPtrArray) refs = NULL; /* (element-type OstreeCollectionRef) */ + g_autoptr(GPtrArray) finders = NULL; /* (element-type OstreeRepoFinder) */ + g_autoptr(OstreeRepoFinder) finder_config = NULL; + g_autoptr(OstreeRepoFinder) finder_mount = NULL; + g_autoptr(OstreeRepoFinder) finder_avahi = NULL; g_autoptr(OstreeAsyncProgress) progress = NULL; gsize i; g_autoptr(GAsyncResult) find_result = NULL, pull_result = NULL; g_auto(OstreeRepoFinderResultv) results = NULL; g_auto(GLnxConsoleRef) console = { 0, }; g_autoptr(GHashTable) refs_found = NULL; /* set (element-type OstreeCollectionRef) */ + g_auto(GStrv) finders_strings = NULL; context = g_option_context_new ("COLLECTION-ID REF [COLLECTION-ID REF...]"); @@ -176,18 +229,65 @@ ostree_builtin_find_remotes (int argc, g_ptr_array_add (refs, NULL); + /* Build the array of OstreeRepoFinder instances */ + if (opt_finders != NULL) + { + g_auto(GStrv) finders_strings = NULL; + + finders_strings = g_strsplit (opt_finders, ",", 0); + if (!validate_finders_list (finders_strings, context, error)) + return FALSE; + + finders = g_ptr_array_new (); + for (char **iter =finders_strings; iter && *iter; iter++) + { + if (g_strcmp0 (*iter, "config") == 0) + { + finder_config = OSTREE_REPO_FINDER (ostree_repo_finder_config_new ()); + g_ptr_array_add (finders, finder_config); + } + else if (g_strcmp0 (*iter, "mount") == 0) + { + finder_mount = OSTREE_REPO_FINDER (ostree_repo_finder_mount_new (NULL)); + g_ptr_array_add (finders, finder_mount); + } + else if (g_strcmp0 (*iter, "lan") == 0) + { +#ifdef HAVE_AVAHI + GMainContext *main_context = g_main_context_get_thread_default (); + g_autoptr(GError) local_error = NULL; + + finder_avahi = OSTREE_REPO_FINDER (ostree_repo_finder_avahi_new (main_context)); + ostree_repo_finder_avahi_start (OSTREE_REPO_FINDER_AVAHI (finder_avahi), &local_error); + + if (local_error != NULL) + { + g_warning ("Avahi finder failed; removing it: %s", local_error->message); + g_clear_object (&finder_avahi); + } + else + g_ptr_array_add (finders, finder_avahi); +#else + ot_util_usage_error (context, "LAN repo finder requested but ostree was compiled without Avahi support", error); + return FALSE; +#endif /* HAVE_AVAHI */ + } + else + g_assert_not_reached (); + } + g_ptr_array_add (finders, NULL); + } + /* Run the operation. */ glnx_console_lock (&console); if (console.is_tty) progress = ostree_async_progress_new_and_connect (ostree_repo_pull_default_console_progress_changed, &console); - /* FIXME: Eventually some command line options for customising the finders - * list would be good. */ ostree_repo_find_remotes_async (repo, (const OstreeCollectionRef * const *) refs->pdata, NULL /* no options */, - NULL /* default finders */, + finders != NULL ? (OstreeRepoFinder **) finders->pdata : NULL, progress, cancellable, get_result_cb, &find_result); From 0be95ded9914eaa903fe9e5d38cf5c97b4567ac4 Mon Sep 17 00:00:00 2001 From: Matthew Leeds Date: Tue, 9 Jan 2018 23:42:49 -0800 Subject: [PATCH 24/30] tests: Use --finders option for find-remotes All the current uses of the find-remotes command in the tests use it to find configured remotes or mounted (USB) remotes, so using --finders=config and --finders=mount in the tests respectively shouldn't affect the correctness of the tests. It does however allow the tests to be run in an environment that doesn't have an Avahi daemon. Closes: #1407 Approved by: cgwalters --- tests/test-find-remotes.sh | 28 ++++++++++----------- tests/test-repo-finder-mount-integration.sh | 4 +-- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/tests/test-find-remotes.sh b/tests/test-find-remotes.sh index 0bbe54f0..bc3888f4 100755 --- a/tests/test-find-remotes.sh +++ b/tests/test-find-remotes.sh @@ -86,7 +86,7 @@ assert_file_has_content local-mirror/refs/mirrors/org.example.OsCollection/os/am for repo in local local-mirror; do # Try finding an update for an existing branch. - ${CMD_PREFIX} ostree --repo=$repo find-remotes org.example.AppsCollection app1 > find + ${CMD_PREFIX} ostree --repo=$repo find-remotes --finders=config org.example.AppsCollection app1 > find assert_file_has_content find "^Result [0-9]\+: file://$(pwd)/apps-collection$" assert_file_has_content find "^ - Keyring: apps-remote.trustedkeys.gpg$" assert_file_has_content find "^ - (org.example.AppsCollection, app1) = $(cat app1-checksum)$" @@ -94,7 +94,7 @@ for repo in local local-mirror; do assert_not_file_has_content find "^No results.$" # Find several updates for several existing branches. - ${CMD_PREFIX} ostree --repo=$repo find-remotes org.example.AppsCollection app1 org.example.OsCollection os/amd64/master > find + ${CMD_PREFIX} ostree --repo=$repo find-remotes --finders=config org.example.AppsCollection app1 org.example.OsCollection os/amd64/master > find assert_file_has_content find "^Result [0-9]\+: file://$(pwd)/apps-collection$" assert_file_has_content find "^ - Keyring: apps-remote.trustedkeys.gpg$" assert_file_has_content find "^ - (org.example.AppsCollection, app1) = $(cat app1-checksum)$" @@ -105,7 +105,7 @@ for repo in local local-mirror; do assert_not_file_has_content find "^No results.$" # Find some updates and a new branch. - ${CMD_PREFIX} ostree --repo=$repo find-remotes org.example.AppsCollection app1 org.example.AppsCollection app2 org.example.OsCollection os/amd64/master > find + ${CMD_PREFIX} ostree --repo=$repo find-remotes --finders=config org.example.AppsCollection app1 org.example.AppsCollection app2 org.example.OsCollection os/amd64/master > find assert_file_has_content find "^Result [0-9]\+: file://$(pwd)/apps-collection$" assert_file_has_content find "^ - Keyring: apps-remote.trustedkeys.gpg$" assert_file_has_content find "^ - (org.example.AppsCollection, app1) = $(cat app1-checksum)$" @@ -117,7 +117,7 @@ for repo in local local-mirror; do assert_not_file_has_content find "^No results.$" # Find an update and a non-existent branch. - ${CMD_PREFIX} ostree --repo=$repo find-remotes org.example.AppsCollection app1 org.example.AppsCollection not-an-app > find + ${CMD_PREFIX} ostree --repo=$repo find-remotes --finders=config org.example.AppsCollection app1 org.example.AppsCollection not-an-app > find assert_file_has_content find "^Result [0-9]\+: file://$(pwd)/apps-collection$" assert_file_has_content find "^ - Keyring: apps-remote.trustedkeys.gpg$" assert_file_has_content find "^ - (org.example.AppsCollection, not-an-app) = (not found)$" @@ -128,20 +128,20 @@ for repo in local local-mirror; do assert_not_file_has_content find "^No results.$" # Do all the above, but pull this time. - ${CMD_PREFIX} ostree --repo=$repo find-remotes --pull org.example.AppsCollection app1 > pull || true + ${CMD_PREFIX} ostree --repo=$repo find-remotes --finders=config --pull org.example.AppsCollection app1 > pull || true assert_file_has_content pull "^1/1 refs were found.$" assert_file_has_content pull "^Pulled 1/1 refs successfully.$" assert_not_file_has_content pull "Failed to pull some refs from the remotes" assert_ref $repo app1 $(cat app1-checksum) - ${CMD_PREFIX} ostree --repo=$repo find-remotes --pull org.example.AppsCollection app1 org.example.OsCollection os/amd64/master > pull + ${CMD_PREFIX} ostree --repo=$repo find-remotes --finders=config --pull org.example.AppsCollection app1 org.example.OsCollection os/amd64/master > pull assert_file_has_content pull "^2/2 refs were found.$" assert_file_has_content pull "^Pulled 2/2 refs successfully.$" assert_not_file_has_content pull "Failed to pull some refs from the remotes" assert_ref $repo app1 $(cat app1-checksum) assert_ref $repo os/amd64/master $(cat os-checksum) - ${CMD_PREFIX} ostree --repo=$repo find-remotes --pull org.example.AppsCollection app1 org.example.AppsCollection app2 org.example.OsCollection os/amd64/master > pull + ${CMD_PREFIX} ostree --repo=$repo find-remotes --finders=config --pull org.example.AppsCollection app1 org.example.AppsCollection app2 org.example.OsCollection os/amd64/master > pull assert_file_has_content pull "^3/3 refs were found.$" assert_file_has_content pull "^Pulled 3/3 refs successfully.$" assert_not_file_has_content pull "Failed to pull some refs from the remotes" @@ -149,7 +149,7 @@ for repo in local local-mirror; do assert_ref $repo app2 $(cat app2-checksum) assert_ref $repo os/amd64/master $(cat os-checksum) - ${CMD_PREFIX} ostree --repo=$repo find-remotes --pull org.example.AppsCollection app1 org.example.AppsCollection not-an-app > pull + ${CMD_PREFIX} ostree --repo=$repo find-remotes --finders=config --pull org.example.AppsCollection app1 org.example.AppsCollection not-an-app > pull assert_file_has_content pull "^1/2 refs were found.$" assert_not_file_has_content pull "Failed to pull some refs from the remotes" assert_ref $repo app1 $(cat app1-checksum) @@ -164,7 +164,7 @@ ${CMD_PREFIX} ostree --repo=os-collection summary --update --gpg-homedir=${TEST_ for repo in local-mirror; do # Try finding an update for that branch. - ${CMD_PREFIX} ostree --repo=$repo find-remotes org.example.OsCollection os/amd64/master > find + ${CMD_PREFIX} ostree --repo=$repo find-remotes --finders=config org.example.OsCollection os/amd64/master > find assert_file_has_content find "^Result [0-9]\+: file://$(pwd)/os-collection$" assert_file_has_content find "^ - Keyring: os-remote.trustedkeys.gpg$" assert_file_has_content find "^ - (org.example.OsCollection, os/amd64/master) = $(cat os-checksum-2)$" @@ -172,7 +172,7 @@ for repo in local-mirror; do assert_not_file_has_content find "^No results.$" # Pull it. - ${CMD_PREFIX} ostree --repo=$repo find-remotes --pull org.example.OsCollection os/amd64/master > pull || true + ${CMD_PREFIX} ostree --repo=$repo find-remotes --finders=config --pull org.example.OsCollection os/amd64/master > pull || true assert_file_has_content pull "^1/1 refs were found.$" assert_file_has_content pull "^Pulled 1/1 refs successfully.$" assert_not_file_has_content pull "Failed to pull some refs from the remotes" @@ -191,7 +191,7 @@ ${CMD_PREFIX} ostree --repo=local remote add os-remote-local-mirror file://$(pwd for repo in local; do # Try finding an update for that branch. - ${CMD_PREFIX} ostree --repo=$repo find-remotes org.example.OsCollection os/amd64/master > find + ${CMD_PREFIX} ostree --repo=$repo find-remotes --finders=config org.example.OsCollection os/amd64/master > find assert_file_has_content find "^Result [0-9]\+: file://$(pwd)/os-collection$" assert_file_has_content find "^ - Keyring: os-remote.trustedkeys.gpg$" assert_file_has_content find "^ - (org.example.OsCollection, os/amd64/master) = $(cat os-checksum-2)$" @@ -202,7 +202,7 @@ for repo in local; do assert_not_file_has_content find "^No results.$" # Pull it. - ${CMD_PREFIX} ostree --repo=$repo find-remotes --pull org.example.OsCollection os/amd64/master > pull || true + ${CMD_PREFIX} ostree --repo=$repo find-remotes --finders=config --pull org.example.OsCollection os/amd64/master > pull || true assert_file_has_content pull "^1/1 refs were found.$" assert_file_has_content pull "^Pulled 1/1 refs successfully.$" assert_not_file_has_content pull "Failed to pull some refs from the remotes" @@ -218,7 +218,7 @@ ${CMD_PREFIX} ostree --repo=os-collection summary --update --gpg-homedir=${TEST_ for repo in local; do # Try finding an update for that branch. - ${CMD_PREFIX} ostree --repo=$repo find-remotes org.example.OsCollection os/amd64/master > find + ${CMD_PREFIX} ostree --repo=$repo find-remotes --finders=config org.example.OsCollection os/amd64/master > find assert_file_has_content find "^Result [0-9]\+: file://$(pwd)/os-collection$" assert_file_has_content find "^ - Keyring: os-remote.trustedkeys.gpg$" assert_file_has_content find "^ - (org.example.OsCollection, os/amd64/master) = $(cat os-checksum-3)$" @@ -226,7 +226,7 @@ for repo in local; do assert_not_file_has_content find "^No results.$" # Pull it. - ${CMD_PREFIX} ostree --repo=$repo find-remotes --pull org.example.OsCollection os/amd64/master > pull || true + ${CMD_PREFIX} ostree --repo=$repo find-remotes --finders=config --pull org.example.OsCollection os/amd64/master > pull || true assert_file_has_content pull "^1/1 refs were found.$" assert_file_has_content pull "^Pulled 1/1 refs successfully.$" assert_not_file_has_content pull "Failed to pull some refs from the remotes" diff --git a/tests/test-repo-finder-mount-integration.sh b/tests/test-repo-finder-mount-integration.sh index 9ce34cb2..33959af2 100755 --- a/tests/test-repo-finder-mount-integration.sh +++ b/tests/test-repo-finder-mount-integration.sh @@ -109,12 +109,12 @@ for fs_type in ext4 vfat; do ostree_repo_init peer-repo_$fs_type ${CMD_PREFIX} ostree --repo=peer-repo_$fs_type remote add remote1 file://just-here-for-the-keyring --collection-id org.example.Collection1 --gpg-import="${test_tmpdir}/gpghome/key1.asc" - ${CMD_PREFIX} ostree --repo=peer-repo_$fs_type find-remotes org.example.Collection1 test-1 > find-results + ${CMD_PREFIX} ostree --repo=peer-repo_$fs_type find-remotes --finders=mount org.example.Collection1 test-1 > find-results assert_not_file_has_content find-results "^No results.$" assert_file_has_content find-results "^Result 0: file://${usb_mount}" assert_file_has_content find-results "(org.example.Collection1, test-1) = $(cat ref1-checksum)$" - ${CMD_PREFIX} ostree --repo=peer-repo_$fs_type find-remotes --pull org.example.Collection1 test-1 > pull-results + ${CMD_PREFIX} ostree --repo=peer-repo_$fs_type find-remotes --finders=mount --pull org.example.Collection1 test-1 > pull-results assert_file_has_content pull-results "^Pulled 1/1 refs successfully.$" ${CMD_PREFIX} ostree --repo=peer-repo_$fs_type refs --collections > refs From fdf7e2c560719d7e18f31c90b04a24349c6cc0e2 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 10 Jan 2018 16:02:59 -0500 Subject: [PATCH 25/30] lib/fetcher: Add version to USER_AGENT string This came up in allowing Fedora infrastructure to work around a libcurl bug with HTTP2: https://pagure.io/atomic-wg/issue/405 Closes: https://github.com/ostreedev/ostree/issues/1405 Closes: #1406 Approved by: jlebon --- src/libostree/ostree-fetcher-curl.c | 2 +- src/libostree/ostree-fetcher-soup.c | 2 +- src/libostree/ostree-fetcher-util.h | 6 ++++++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/libostree/ostree-fetcher-curl.c b/src/libostree/ostree-fetcher-curl.c index 272d609a..2d01257d 100644 --- a/src/libostree/ostree-fetcher-curl.c +++ b/src/libostree/ostree-fetcher-curl.c @@ -714,7 +714,7 @@ initiate_next_curl_request (FetcherRequest *req, curl_easy_setopt (req->easy, CURLOPT_URL, uri); } - curl_easy_setopt (req->easy, CURLOPT_USERAGENT, "ostree "); + curl_easy_setopt (req->easy, CURLOPT_USERAGENT, OSTREE_FETCHER_USERAGENT_STRING); if (self->extra_headers) curl_easy_setopt (req->easy, CURLOPT_HTTPHEADER, self->extra_headers); diff --git a/src/libostree/ostree-fetcher-soup.c b/src/libostree/ostree-fetcher-soup.c index e023d34e..cab8dff6 100644 --- a/src/libostree/ostree-fetcher-soup.c +++ b/src/libostree/ostree-fetcher-soup.c @@ -479,7 +479,7 @@ ostree_fetcher_session_thread (gpointer data) g_main_context_push_thread_default (mainctx); /* We retain ownership of the SoupSession reference. */ - closure->session = soup_session_async_new_with_options (SOUP_SESSION_USER_AGENT, "ostree ", + closure->session = soup_session_async_new_with_options (SOUP_SESSION_USER_AGENT, OSTREE_FETCHER_USERAGENT_STRING, SOUP_SESSION_SSL_USE_SYSTEM_CA_FILE, TRUE, SOUP_SESSION_USE_THREAD_CONTEXT, TRUE, SOUP_SESSION_ADD_FEATURE_BY_TYPE, SOUP_TYPE_REQUESTER, diff --git a/src/libostree/ostree-fetcher-util.h b/src/libostree/ostree-fetcher-util.h index 29293816..48528e0f 100644 --- a/src/libostree/ostree-fetcher-util.h +++ b/src/libostree/ostree-fetcher-util.h @@ -25,6 +25,12 @@ G_BEGIN_DECLS +/* We used to only send "ostree/" but now include the version + * https://github.com/ostreedev/ostree/issues/1405 + * This came up in allowing Fedora infrastructure to work around a libcurl bug with HTTP2. + */ +#define OSTREE_FETCHER_USERAGENT_STRING (PACKAGE_NAME "/" PACKAGE_VERSION) + static inline gboolean _ostree_fetcher_tmpf_from_flags (OstreeFetcherRequestFlags flags, int dfd, From 854a823e05d6fe8b610c02c2a71eaeb2bf1e98a6 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Thu, 11 Jan 2018 20:54:26 +0000 Subject: [PATCH 26/30] tests/libtest-core: support multiple literal checks `grep` supports checking multiple fixed strings separated by newlines, but it's mostly just easier to pass them as separate arguments, so let's support that. This is now at parity with the similar `assert_file_has_content`. Closes: #1409 Approved by: cgwalters --- tests/libtest-core.sh | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/libtest-core.sh b/tests/libtest-core.sh index 2144e1ac..7223f4d2 100644 --- a/tests/libtest-core.sh +++ b/tests/libtest-core.sh @@ -112,9 +112,12 @@ assert_file_has_content () { } assert_file_has_content_literal () { - if ! grep -q -F -e "$2" "$1"; then - _fatal_print_file "$1" "File '$1' doesn't match fixed string list '$2'" - fi + fpath=$1; shift + for s in "$@"; do + if ! grep -q -F -e "$s" "$fpath"; then + _fatal_print_file "$fpath" "File '$fpath' doesn't match fixed string list '$s'" + fi + done } assert_file_has_mode () { From 2b78df25f469f01f96616ac3bcb5bc17bd68ab2e Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 12 Jan 2018 09:01:52 -0500 Subject: [PATCH 27/30] tests: Add a test case for path traversal in a dirtree I was reading about a recent security issue with both EMC and VMWare: https://arstechnica.com/information-technology/2018/01/emc-vmware-security-bugs-throw-gasoline-on-cloud-security-fire/ It's a classic path traversal problem, and that made me think more about our handling of this in libostree. Fortunately of course, not being new to this rodeo, long ago I *did* consider path traversal. Inside the pull code, we call `ot_util_filename_validate()`. Also, `fsck` does this too. I have further followups here, but let's add some test cases for this. I crafted a repository with a `../` in a dirtree object by patching libostree to inject it, and that's included as a tarball. This patch covers the two cases where we do already have checks; pulling via HTTP, and in `fsck`. Closes: #1412 Approved by: jlebon --- Makefile-tests.am | 1 + cfg.mk | 2 +- tests/ostree-path-traverse.tar.gz | Bin 0 -> 1265 bytes tests/pull-test.sh | 17 ++++++++++++++++- tests/test-corruption.sh | 12 +++++++++++- 5 files changed, 29 insertions(+), 3 deletions(-) create mode 100644 tests/ostree-path-traverse.tar.gz diff --git a/Makefile-tests.am b/Makefile-tests.am index 350209de..284dc76f 100644 --- a/Makefile-tests.am +++ b/Makefile-tests.am @@ -178,6 +178,7 @@ dist_installed_test_data = tests/archive-test.sh \ tests/pre-endian-deltas-repo-little.tar.xz \ tests/fah-deltadata-old.tar.xz \ tests/fah-deltadata-new.tar.xz \ + tests/ostree-path-traverse.tar.gz \ tests/libtest-core.sh \ $(NULL) diff --git a/cfg.mk b/cfg.mk index 0eb05b89..5947a141 100644 --- a/cfg.mk +++ b/cfg.mk @@ -39,4 +39,4 @@ sc_glnx_no_fd_close: show-vc-list-except: @$(VC_LIST_EXCEPT) -VC_LIST_ALWAYS_EXCLUDE_REGEX = ^ABOUT-NLS|cfg.mk|maint.mk|*.gpg|*.sig|.xz$$ +VC_LIST_ALWAYS_EXCLUDE_REGEX = ^ABOUT-NLS|cfg.mk|maint.mk|*.gpg|*.sig|.xz|.gz$$ diff --git a/tests/ostree-path-traverse.tar.gz b/tests/ostree-path-traverse.tar.gz new file mode 100644 index 0000000000000000000000000000000000000000..9dec3b73c76a149d34db24015d4da5918dd96558 GIT binary patch literal 1265 zcmV1MQqoXk0}c$F~*ylYoDEXwg#0L5OLyoB2QQ9a^Ilu^PN6 z6e>k$=FLmCcK3z%HX#Hr9t!m)coE5=XuXJ7w6vg}Jc*voMW`SeFSY>@JXAs7+axw= zvU$7N-FZp)C5_nH^0aD9l^d~|wQ*v{O1%Z8NXfGjOODo; zYm_i0gi9e2CnbcbkhC>GlTbB>q?QmxzJ?cS39Z)9_aj{FsNpnsYGfc`GX(SORS!0G(I;{SvY9Q1cVTl#0UYnA1OO&XoGk(d8tgg5F> z2+8z^{J;MopCiNwHx^r?1@U#d`1(eL*po(~&?Az0+$Y>>FASLRJtYFAeXBxi6h^Q$ zu(!#H|5`PUqqxfzU%w6TWl(iGcjy39a>y^f_x9y+{Fe;Qt6b z|8qmW{*3j+|F^mSx2iuUkpDZOGFBNIpNQ&{6V>_~eQIN!|8Hu2SD9`1Z@3o_#?AbH zqEG$Vz?%MX6m6T<&=jHzhvpfDz&@}&-3<=2p-SW(>qI&?iCqMorEyVu` z@P97Y9sZAO_J0`sp9|WG|G@e*P+T&?6+;qfj>#h&nxqRz#vv~a2^P}HBHS)sf zi{+y)EtXG}D?jab+drU>)!ua-Ty;r z0sapF`b>WOt3v#rfb&020RIR6FE{>u#e;NB#;W;bPAOt7#ZoIyHKvx@P@|Ce0{JFj zo-ZucDzw%zqk=$l@PE4)7y9SV2%9vM}8sCf8qR(6LyFHqnrI70srTMw&K6LBSIAu)oRsy@yojhaG`&$|0A3H z9~1EW*EwfS-(33i{j1IXmoI;I;>2H%jJ&?^{oD1!SC)VI?#8t|cZc@x zD|#3}2egj=nR+^JPscV%Y*1dE49YwDL71lh>*V|wA@KVj00000000000000000000 b000000000000000unqqLd};Rh0C)fZpA?^( literal 0 HcmV?d00001 diff --git a/tests/pull-test.sh b/tests/pull-test.sh index e6317fbf..463b41ef 100644 --- a/tests/pull-test.sh +++ b/tests/pull-test.sh @@ -52,7 +52,7 @@ function verify_initial_contents() { assert_file_has_content baz/cow '^moo$' } -echo "1..33" +echo "1..34" # Try both syntaxes repo_init --no-gpg-verify @@ -217,6 +217,21 @@ else echo "ok corruption (skipped)" fi + +cd ${test_tmpdir}/ostree-srv +tar xf ${test_srcdir}/ostree-path-traverse.tar.gz +cd ${test_tmpdir} +rm corruptrepo -rf +ostree_repo_init corruptrepo --mode=archive +${CMD_PREFIX} ostree --repo=corruptrepo remote add --set=gpg-verify=false pathtraverse $(cat httpd-address)/ostree/ostree-path-traverse/repo +if ${CMD_PREFIX} ostree --repo=corruptrepo pull pathtraverse pathtraverse-test 2>err.txt; then + fatal "Pulled a repo with path traversal in dirtree" +fi +assert_file_has_content_literal err.txt 'Invalid / in filename ../afile' +rm corruptrepo -rf +echo "ok path traversal checked on pull" + + cd ${test_tmpdir} rm mirrorrepo/refs/remotes/* -rf ${CMD_PREFIX} ostree --repo=mirrorrepo prune --refs-only diff --git a/tests/test-corruption.sh b/tests/test-corruption.sh index cb5e9c09..626929e7 100755 --- a/tests/test-corruption.sh +++ b/tests/test-corruption.sh @@ -19,7 +19,7 @@ set -euo pipefail -echo "1..4" +echo "1..5" . $(dirname $0)/libtest.sh @@ -72,3 +72,13 @@ fi assert_file_has_content_literal err.txt "Loading commit for ref test2: No such metadata object" echo "ok missing commit" + +cd ${test_tmpdir} +tar xf ${test_srcdir}/ostree-path-traverse.tar.gz +if ${CMD_PREFIX} ostree --repo=ostree-path-traverse/repo fsck -q 2>err.txt; then + fatal "fsck unexpectedly succeeded" +fi +assert_file_has_content_literal err.txt '.dirtree: Invalid / in filename ../afile' + +echo "ok path traverse" + From f3ae36ff4360c58158963ca2c20862ae94ac0775 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 12 Jan 2018 10:40:36 -0500 Subject: [PATCH 28/30] lib/checkout: Validate pathnames during checkout While we do protect against path traversal during pull, let's also validate during checkout; it's a cheap operation and provides good last-mile protection. Closes: #1412 Approved by: jlebon --- src/libostree/ostree-repo-checkout.c | 13 +++++++++++++ tests/test-corruption.sh | 11 ++++++++--- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/src/libostree/ostree-repo-checkout.c b/src/libostree/ostree-repo-checkout.c index 962b503d..6334ae50 100644 --- a/src/libostree/ostree-repo-checkout.c +++ b/src/libostree/ostree-repo-checkout.c @@ -535,6 +535,10 @@ checkout_one_file_at (OstreeRepo *repo, GCancellable *cancellable, GError **error) { + /* Validate this up front to prevent path traversal attacks */ + if (!ot_util_filename_validate (destination_name, error)) + return FALSE; + gboolean need_copy = TRUE; gboolean is_bare_user_symlink = FALSE; char loose_path_buf[_OSTREE_LOOSE_PATH_MAX]; @@ -897,6 +901,15 @@ checkout_tree_at_recurse (OstreeRepo *self, while (g_variant_iter_loop (&viter, "(&s@ay@ay)", &dname, &subdirtree_csum_v, &subdirmeta_csum_v)) { + /* Validate this up front to prevent path traversal attacks. Note that + * we don't validate at the top of this function like we do for + * checkout_one_file_at() becuase I believe in some cases this function + * can be called *initially* with user-specified paths for the root + * directory. + */ + if (!ot_util_filename_validate (dname, error)) + return FALSE; + const size_t origlen = selabel_path_buf ? selabel_path_buf->len : 0; if (selabel_path_buf) { diff --git a/tests/test-corruption.sh b/tests/test-corruption.sh index 626929e7..74ce8e83 100755 --- a/tests/test-corruption.sh +++ b/tests/test-corruption.sh @@ -19,7 +19,7 @@ set -euo pipefail -echo "1..5" +echo "1..6" . $(dirname $0)/libtest.sh @@ -79,6 +79,11 @@ if ${CMD_PREFIX} ostree --repo=ostree-path-traverse/repo fsck -q 2>err.txt; then fatal "fsck unexpectedly succeeded" fi assert_file_has_content_literal err.txt '.dirtree: Invalid / in filename ../afile' +echo "ok path traverse fsck" -echo "ok path traverse" - +cd ${test_tmpdir} +if ${CMD_PREFIX} ostree --repo=ostree-path-traverse/repo checkout pathtraverse-test pathtraverse-test 2>err.txt; then + fatal "checkout with path traversal unexpectedly succeeded" +fi +assert_file_has_content_literal err.txt 'Invalid / in filename ../afile' +echo "ok path traverse checkout" From 8e6e64a5adb69a2cb0e84b035a5bc56009735bc7 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 12 Jan 2018 09:15:21 -0500 Subject: [PATCH 29/30] lib: Validate metadata structure more consistently during pull Previously we were doing e.g. `ot_util_filename_validate()` specifically inline in dirtree objects, but only *after* writing them into the staging directory (by default). In (non-default) cases such as not using a transaction, such an object could be written directly into the repo. A notable gap here is that `pull-local --untrusted` was *not* doing this verification, just checksums. We harden that (and also the static delta writing path, really *everything* that calls `ostree_repo_write_metadata()` to also do "structure" validation which includes path traversal checks. Basically, let's try hard to avoid having badly structured objects even in the repo. One thing that sucks in this patch is that we need to allocate a "bounce buffer" for metadata in the static delta path, because GVariant imposes alignment requirements, which I screwed up and didn't fulfill when designing deltas. It actually didn't matter before because we weren't parsing them, but now we are. In theory we could check alignment but ...eh, not worth it, at least not until we change the delta compiler to emit aligned metadata which actually may be quite tricky. (Big picture I doubt this really matters much right now but I'm not going to pull out a profiler yet for this) The pull test was extended to check we didn't even write a dirtree with path traversal into the staging directory. There's a bit of code motion in extracting `_ostree_validate_structureof_metadata()` from `fsck_metadata_object()`. Then `_ostree_verify_metadata_object()` builds on that to do checksum verification too. Closes: #1412 Approved by: jlebon --- src/libostree/ostree-core-private.h | 11 +++ src/libostree/ostree-core.c | 68 +++++++++++++++++++ src/libostree/ostree-repo-commit.c | 8 +++ src/libostree/ostree-repo-pull.c | 41 ++++++----- .../ostree-repo-static-delta-processing.c | 9 ++- src/libostree/ostree-repo.c | 35 +--------- tests/pull-test.sh | 5 +- tests/test-pull-untrusted.sh | 15 +++- 8 files changed, 138 insertions(+), 54 deletions(-) diff --git a/src/libostree/ostree-core-private.h b/src/libostree/ostree-core-private.h index 08809e4a..162677c3 100644 --- a/src/libostree/ostree-core-private.h +++ b/src/libostree/ostree-core-private.h @@ -164,6 +164,17 @@ _ostree_loose_path (char *buf, OstreeObjectType objtype, OstreeRepoMode repo_mode); +gboolean _ostree_validate_structureof_metadata (OstreeObjectType objtype, + GVariant *commit, + GError **error); + +gboolean +_ostree_verify_metadata_object (OstreeObjectType objtype, + const char *expected_checksum, + GVariant *metadata, + GError **error); + + #define _OSTREE_METADATA_GPGSIGS_NAME "ostree.gpgsigs" #define _OSTREE_METADATA_GPGSIGS_TYPE G_VARIANT_TYPE ("aay") diff --git a/src/libostree/ostree-core.c b/src/libostree/ostree-core.c index 679c9529..2256a2c0 100644 --- a/src/libostree/ostree-core.c +++ b/src/libostree/ostree-core.c @@ -2071,6 +2071,74 @@ validate_variant (GVariant *variant, return TRUE; } +/* TODO: make this public later; just wraps the previously public + * commit/dirtree/dirmeta verifiers. + */ +gboolean +_ostree_validate_structureof_metadata (OstreeObjectType objtype, + GVariant *metadata, + GError **error) +{ + g_assert (OSTREE_OBJECT_TYPE_IS_META (objtype)); + + switch (objtype) + { + case OSTREE_OBJECT_TYPE_COMMIT: + if (!ostree_validate_structureof_commit (metadata, error)) + return FALSE; + break; + case OSTREE_OBJECT_TYPE_DIR_TREE: + if (!ostree_validate_structureof_dirtree (metadata, error)) + return FALSE; + break; + case OSTREE_OBJECT_TYPE_DIR_META: + if (!ostree_validate_structureof_dirmeta (metadata, error)) + return FALSE; + break; + case OSTREE_OBJECT_TYPE_TOMBSTONE_COMMIT: + case OSTREE_OBJECT_TYPE_COMMIT_META: + /* TODO */ + break; + case OSTREE_OBJECT_TYPE_FILE: + g_assert_not_reached (); + break; + } + + return TRUE; +} + +/* Used by fsck as well as pull. Verify the checksum of a metadata object + * and its "structure" or the additional schema we impose on GVariants such + * as ensuring the "ay" checksum entries are of length 32. Another important + * one is checking for path traversal in dirtree objects. + */ +gboolean +_ostree_verify_metadata_object (OstreeObjectType objtype, + const char *expected_checksum, + GVariant *metadata, + GError **error) +{ + g_assert (expected_checksum); + + g_auto(OtChecksum) hasher = { 0, }; + ot_checksum_init (&hasher); + ot_checksum_update (&hasher, g_variant_get_data (metadata), g_variant_get_size (metadata)); + + char actual_checksum[OSTREE_SHA256_STRING_LEN+1]; + ot_checksum_get_hexdigest (&hasher, actual_checksum, sizeof (actual_checksum)); + if (!_ostree_compare_object_checksum (objtype, expected_checksum, actual_checksum, error)) + return FALSE; + + /* Add the checksum + objtype prefix here */ + { const char *error_prefix = glnx_strjoina (expected_checksum, ".", ostree_object_type_to_string (objtype)); + GLNX_AUTO_PREFIX_ERROR(error_prefix, error); + if (!_ostree_validate_structureof_metadata (objtype, metadata, error)) + return FALSE; + } + + return TRUE; +} + /** * ostree_validate_structureof_commit: * @commit: A commit object, %OSTREE_OBJECT_TYPE_COMMIT diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index 4392f700..44434c66 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -2027,6 +2027,13 @@ ostree_repo_write_metadata (OstreeRepo *self, if (!metadata_size_valid (objtype, g_variant_get_size (normalized), error)) return FALSE; + /* For untrusted objects, verify their structure here */ + if (expected_checksum) + { + if (!_ostree_validate_structureof_metadata (objtype, object, error)) + return FALSE; + } + g_autoptr(GBytes) vdata = g_variant_get_data_as_bytes (normalized); if (!write_metadata_object (self, objtype, expected_checksum, vdata, out_csum, cancellable, error)) @@ -4101,6 +4108,7 @@ _ostree_repo_import_object (OstreeRepo *self, &variant, error)) return FALSE; + /* Note this one also now verifies structure in the !trusted case */ g_autofree guchar *real_csum = NULL; if (!ostree_repo_write_metadata (self, objtype, checksum, variant, diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index b2a00ea2..ab59c33a 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -736,6 +736,12 @@ scan_dirtree_object (OtPullData *pull_data, g_variant_get_child (files_variant, i, "(&s@ay)", &filename, &csum); + /* Note this is now obsoleted by the _ostree_validate_structureof_metadata() + * but I'm keeping this since: + * 1) It's cheap + * 2) We want to continue to do validation for objects written to disk + * before libostree's validation was strengthened. + */ if (!ot_util_filename_validate (filename, error)) return FALSE; @@ -810,6 +816,7 @@ scan_dirtree_object (OtPullData *pull_data, g_variant_get_child (dirs_variant, i, "(&s@ay@ay)", &dirname, &tree_csum, &meta_csum); + /* See comment above for files */ if (!ot_util_filename_validate (dirname, error)) return FALSE; @@ -1222,24 +1229,20 @@ meta_fetch_on_complete (GObject *object, FALSE, &metadata, error)) goto out; - /* For commit objects, compute the hash and check the GPG signature before - * writing to the repo, and also write the .commitpartial to say that - * we're still processing this commit. + /* Compute checksum and verify structure now. Note this is a recent change + * (Jan 2018) - we used to verify the checksum only when writing down + * below. But we want to do "structure" verification early on as well + * before the object is written even to the staging directory. + */ + if (!_ostree_verify_metadata_object (objtype, checksum, metadata, error)) + goto out; + + /* For commit objects, check the GPG signature before writing to the repo, + * and also write the .commitpartial to say that we're still processing + * this commit. */ if (objtype == OSTREE_OBJECT_TYPE_COMMIT) { - /* Verify checksum */ - OtChecksum hasher = { 0, }; - ot_checksum_init (&hasher); - { g_autoptr(GBytes) bytes = g_variant_get_data_as_bytes (metadata); - ot_checksum_update_bytes (&hasher, bytes); - } - char hexdigest[OSTREE_SHA256_STRING_LEN+1]; - ot_checksum_get_hexdigest (&hasher, hexdigest, sizeof (hexdigest)); - - if (!_ostree_compare_object_checksum (objtype, checksum, hexdigest, error)) - goto out; - /* Do GPG verification. `detached_data` may be NULL if no detached * metadata was found during pull; that's handled by * gpg_verify_unwritten_commit(). If we ever change the pull code to @@ -1256,7 +1259,13 @@ meta_fetch_on_complete (GObject *object, goto out; } - ostree_repo_write_metadata_async (pull_data->repo, objtype, checksum, metadata, + /* Note that we now (Jan 2018) pass NULL for checksum, which means "don't + * verify checksum", since we just did it above. Related to this...now + * that we're doing all the verification here, one thing we could do later + * just `glnx_link_tmpfile_at()` into the repository, like the content + * fetch path does for trusted commits. + */ + ostree_repo_write_metadata_async (pull_data->repo, objtype, NULL, metadata, pull_data->cancellable, on_metadata_written, fetch_data); pull_data->n_outstanding_metadata_write_requests++; diff --git a/src/libostree/ostree-repo-static-delta-processing.c b/src/libostree/ostree-repo-static-delta-processing.c index 3b9fd49f..bb6238e4 100644 --- a/src/libostree/ostree-repo-static-delta-processing.c +++ b/src/libostree/ostree-repo-static-delta-processing.c @@ -497,8 +497,13 @@ dispatch_open_splice_and_close (OstreeRepo *repo, goto out; } - metadata = g_variant_new_from_data (ostree_metadata_variant_type (state->output_objtype), - state->payload_data + offset, length, TRUE, NULL, NULL); + /* Unfortunately we need a copy because GVariant wants pointer-alignment + * and we didn't guarantee that in static deltas. We can do so in the + * future. + */ + g_autoptr(GBytes) metadata_copy = g_bytes_new (state->payload_data + offset, length); + metadata = g_variant_new_from_bytes (ostree_metadata_variant_type (state->output_objtype), + metadata_copy, FALSE); { g_autofree guchar *actual_csum = NULL; diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index ec509e9d..858a5555 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -3941,6 +3941,7 @@ ostree_repo_delete_object (OstreeRepo *self, return TRUE; } +/* Thin wrapper for _ostree_verify_metadata_object() */ static gboolean fsck_metadata_object (OstreeRepo *self, OstreeObjectType objtype, @@ -3956,39 +3957,7 @@ fsck_metadata_object (OstreeRepo *self, cancellable, error)) return FALSE; - g_auto(OtChecksum) hasher = { 0, }; - ot_checksum_init (&hasher); - ot_checksum_update (&hasher, g_variant_get_data (metadata), g_variant_get_size (metadata)); - - char actual_checksum[OSTREE_SHA256_STRING_LEN+1]; - ot_checksum_get_hexdigest (&hasher, actual_checksum, sizeof (actual_checksum)); - if (!_ostree_compare_object_checksum (objtype, sha256, actual_checksum, error)) - return FALSE; - - switch (objtype) - { - case OSTREE_OBJECT_TYPE_COMMIT: - if (!ostree_validate_structureof_commit (metadata, error)) - return FALSE; - break; - case OSTREE_OBJECT_TYPE_DIR_TREE: - if (!ostree_validate_structureof_dirtree (metadata, error)) - return FALSE; - break; - case OSTREE_OBJECT_TYPE_DIR_META: - if (!ostree_validate_structureof_dirmeta (metadata, error)) - return FALSE; - break; - case OSTREE_OBJECT_TYPE_TOMBSTONE_COMMIT: - case OSTREE_OBJECT_TYPE_COMMIT_META: - /* TODO */ - break; - case OSTREE_OBJECT_TYPE_FILE: - g_assert_not_reached (); - break; - } - - return TRUE; + return _ostree_verify_metadata_object (objtype, sha256, metadata, error); } static gboolean diff --git a/tests/pull-test.sh b/tests/pull-test.sh index 463b41ef..8018e91a 100644 --- a/tests/pull-test.sh +++ b/tests/pull-test.sh @@ -227,7 +227,10 @@ ${CMD_PREFIX} ostree --repo=corruptrepo remote add --set=gpg-verify=false pathtr if ${CMD_PREFIX} ostree --repo=corruptrepo pull pathtraverse pathtraverse-test 2>err.txt; then fatal "Pulled a repo with path traversal in dirtree" fi -assert_file_has_content_literal err.txt 'Invalid / in filename ../afile' +assert_file_has_content_literal err.txt 'ae9a5d2701a02740aa2ee317ba53b13e3efb0f29609cd4896e1bafeee4caddb5.dirtree: Invalid / in filename ../afile' +# And verify we didn't write the object into the staging directory even +find corruptrepo/tmp -name '9a5d2701a02740aa2ee317ba53b13e3efb0f29609cd4896e1bafeee4caddb5.dirtree' >find.txt +assert_not_file_has_content find.txt '9a5d2701a02740aa2ee317ba53b13e3efb0f29609cd4896e1bafeee4caddb5' rm corruptrepo -rf echo "ok path traversal checked on pull" diff --git a/tests/test-pull-untrusted.sh b/tests/test-pull-untrusted.sh index 247a34f9..5e35c1c3 100755 --- a/tests/test-pull-untrusted.sh +++ b/tests/test-pull-untrusted.sh @@ -1,6 +1,7 @@ #!/bin/bash # # Copyright (C) 2014 Alexander Larsson +# Copyright (C) 2018 Red Hat, Inc. # # This library is free software; you can redistribute it and/or # modify it under the terms of the GNU Lesser General Public @@ -22,7 +23,7 @@ set -euo pipefail . $(dirname $0)/libtest.sh -echo '1..3' +echo '1..4' setup_test_repository "bare" @@ -60,10 +61,20 @@ else fi rm -rf repo2 -mkdir repo2 ostree_repo_init repo2 --mode="bare" if ${CMD_PREFIX} ostree --repo=repo2 pull-local --untrusted repo; then assert_not_reached "corrupted untrusted pull unexpectedly failed!" else echo "ok untrusted pull with corruption failed" fi + + +cd ${test_tmpdir} +tar xf ${test_srcdir}/ostree-path-traverse.tar.gz +rm -rf repo2 +ostree_repo_init repo2 --mode=archive +if ${CMD_PREFIX} ostree --repo=repo2 pull-local --untrusted ostree-path-traverse/repo pathtraverse-test 2>err.txt; then + fatal "pull-local unexpectedly succeeded" +fi +assert_file_has_content_literal err.txt 'Invalid / in filename ../afile' +echo "ok untrusted pull-local path traversal" From d3fa95023e1d944f5923064d5b19cc6ee62cf80a Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 12 Jan 2018 15:27:44 -0500 Subject: [PATCH 30/30] Release 2018.1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In particular I'd like to get the `--copyup` changes out for an rpm-ostree release that will use them. But there are other good changes here, and let's keep up a regular release train šŸš„ in general. Closes: #1413 Approved by: jlebon --- configure.ac | 6 +++--- src/libostree/libostree-devel.sym | 4 ++-- src/libostree/libostree-released.sym | 3 +++ tests/test-symbols.sh | 2 +- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/configure.ac b/configure.ac index e26c903c..a4e1e74f 100644 --- a/configure.ac +++ b/configure.ac @@ -3,11 +3,11 @@ dnl If doing a final release, remember to follow the instructions to dnl update libostree-released.sym from libostree-devel.sym, and update the checksum dnl in test-symbols.sh, and also set is_release_build=yes below. Then make dnl another post-release commit to bump the version, and set is_release_build=no. -m4_define([year_version], [2017]) -m4_define([release_version], [16]) +m4_define([year_version], [2018]) +m4_define([release_version], [1]) m4_define([package_version], [year_version.release_version]) AC_INIT([libostree], [package_version], [walters@verbum.org]) -is_release_build=no +is_release_build=yes AC_CONFIG_HEADER([config.h]) AC_CONFIG_MACRO_DIR([buildutil]) AC_CONFIG_AUX_DIR([build-aux]) diff --git a/src/libostree/libostree-devel.sym b/src/libostree/libostree-devel.sym index 5ab4b99c..cdd6b7ca 100644 --- a/src/libostree/libostree-devel.sym +++ b/src/libostree/libostree-devel.sym @@ -18,8 +18,8 @@ ***/ /* Add new symbols here. Release commits should copy this section into -released.sym. */ -LIBOSTREE_2017.16 { -} LIBOSTREE_2017.15; +LIBOSTREE_2018.2 { +} LIBOSTREE_2018.1; /* Stub section for the stable release *after* this development one; don't * edit this other than to update the last number. This is just a copy/paste diff --git a/src/libostree/libostree-released.sym b/src/libostree/libostree-released.sym index b7d57853..51cd31a1 100644 --- a/src/libostree/libostree-released.sym +++ b/src/libostree/libostree-released.sym @@ -451,6 +451,9 @@ LIBOSTREE_2017.15 { ostree_break_hardlink; } LIBOSTREE_2017.14; +LIBOSTREE_2018.1 { +} LIBOSTREE_2017.15; + /* NOTE: Only add more content here in release commits! See the * comments at the top of this file. */ diff --git a/tests/test-symbols.sh b/tests/test-symbols.sh index 51137d6c..02facd2c 100755 --- a/tests/test-symbols.sh +++ b/tests/test-symbols.sh @@ -52,7 +52,7 @@ echo 'ok documented symbols' # ONLY update this checksum in release commits! cat > released-sha256.txt <