From 7b64e29d53caa3443427d3cd964c7af795cb7bb5 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 13 Feb 2017 17:09:20 -0500 Subject: [PATCH 1/3] unpacker: Check for filter errors while committing tmpfiles too This masked an issue in a change I was working on in the filter. --- src/libpriv/rpmostree-unpacker.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/libpriv/rpmostree-unpacker.c b/src/libpriv/rpmostree-unpacker.c index 98586d934f..225f5b2553 100644 --- a/src/libpriv/rpmostree-unpacker.c +++ b/src/libpriv/rpmostree-unpacker.c @@ -721,6 +721,13 @@ import_rpm_to_repo (RpmOstreeUnpacker *self, if (!ostree_repo_write_dfd_to_mtree (repo, tmpdir_dfd, ".", mtree, modifier, cancellable, error)) goto out; + + /* check if any of the cbs set an error */ + if (cb_error != NULL) + { + *error = cb_error; + goto out; + } } if (!ostree_repo_write_mtree (repo, mtree, &root, cancellable, error)) From ea51dd0922b55c581fbdf62e0b52dc7e781b5b93 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 13 Feb 2017 11:33:50 -0500 Subject: [PATCH 2/3] importer: Error importing RPMs which install to /opt (outside of /usr) See https://github.com/projectatomic/rpm-ostree/issues/233 - for RPMs which place files in e.g. `/opt`, we have different behavior in the treecompose case (silently drop it) versus package layering (does the wrong thing). Since the unpacker right now is only used in the layering case, this just ensures we'll get a consistent error there. --- Makefile-tests.am | 1 + src/libpriv/rpmostree-unpacker.c | 11 +++++++++++ tests/common/compose/yum/test-opt.spec | 22 ++++++++++++++++++++++ tests/vmcheck/test-layering-basic.sh | 8 ++++++++ 4 files changed, 42 insertions(+) create mode 100644 tests/common/compose/yum/test-opt.spec diff --git a/Makefile-tests.am b/Makefile-tests.am index 0ca30f74cf..85c0fb4c6d 100644 --- a/Makefile-tests.am +++ b/Makefile-tests.am @@ -31,6 +31,7 @@ testpackages = \ tests/common/compose/yum/repo/packages/x86_64/scriptpkg1-1.0-1.x86_64.rpm \ tests/common/compose/yum/repo/packages/x86_64/nonrootcap-1.0-1.x86_64.rpm \ tests/common/compose/yum/repo/packages/x86_64/test-post-rofiles-violation-1.0-1.x86_64.rpm \ + tests/common/compose/yum/repo/packages/x86_64/test-opt-1.0-1.x86_64.rpm \ $(NULL) # Create a rule for each testpkg with their respective spec file as dep. diff --git a/src/libpriv/rpmostree-unpacker.c b/src/libpriv/rpmostree-unpacker.c index 225f5b2553..6b0abad9f8 100644 --- a/src/libpriv/rpmostree-unpacker.c +++ b/src/libpriv/rpmostree-unpacker.c @@ -609,6 +609,17 @@ compose_filter_cb (OstreeRepo *repo, { g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, "RPM had unexpected non-root owned path \"%s\", marked as %u:%u)", path, uid, gid); + return OSTREE_REPO_COMMIT_FILTER_SKIP; + } + /* And ensure the RPM installs into supported paths */ + else if (!(g_str_has_prefix (path, "/usr/") || g_str_has_prefix (path, "/bin/") || + g_str_has_prefix (path, "/sbin/") || g_str_has_prefix (path, "/lib/") || + g_str_has_prefix (path, "/lib64/"))) + { + g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, + "Unsupported path: %s; See %s", + path, "https://github.com/projectatomic/rpm-ostree/issues/233"); + return OSTREE_REPO_COMMIT_FILTER_SKIP; } } diff --git a/tests/common/compose/yum/test-opt.spec b/tests/common/compose/yum/test-opt.spec new file mode 100644 index 0000000000..274c6a961d --- /dev/null +++ b/tests/common/compose/yum/test-opt.spec @@ -0,0 +1,22 @@ +Summary: Test package which installs in /opt +Name: test-opt +Version: 1.0 +Release: 1 +License: GPLv2+ +Group: Development/Tools +URL: http://example.com +BuildArch: x86_64 + +%description +%{summary} + +%prep + +%build + +%install +mkdir -p %{buildroot}/opt/app/bin +touch %{buildroot}/opt/app/bin/foo + +%files +/opt/app diff --git a/tests/vmcheck/test-layering-basic.sh b/tests/vmcheck/test-layering-basic.sh index 8e12858d1f..79e9562a0f 100755 --- a/tests/vmcheck/test-layering-basic.sh +++ b/tests/vmcheck/test-layering-basic.sh @@ -43,6 +43,14 @@ if vm_cmd "runuser -u bin rpm-ostree pkg-add foo-1.0"; then assert_not_reached "Was able to install a package as non-root!" fi +# Be sure an unprivileged user exists +if vm_rpmostree install test-opt-1.0 2>err.txt; then + assert_not_reached "Was able to install a package in /opt" +fi +assert_file_has_content err.txt "See https://github.com/projectatomic/rpm-ostree/issues/233" + +echo "ok failed to install in opt" + vm_rpmostree pkg-add foo-1.0 vm_cmd ostree --repo=/sysroot/ostree/repo/extensions/rpmostree/pkgcache refs |grep /foo/> refs.txt pkgref=$(head -1 refs.txt) From 5e7cc94a8704175c1bcc7d353de0e04c3d503558 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 13 Feb 2017 17:23:32 -0500 Subject: [PATCH 3/3] fixup! importer: Error importing RPMs which install to /opt (outside of /usr) --- src/libpriv/rpmostree-unpacker.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/src/libpriv/rpmostree-unpacker.c b/src/libpriv/rpmostree-unpacker.c index 6b0abad9f8..3ed75c68f2 100644 --- a/src/libpriv/rpmostree-unpacker.c +++ b/src/libpriv/rpmostree-unpacker.c @@ -575,6 +575,23 @@ append_tmpfiles_d (RpmOstreeUnpacker *self, } } +/* When we do a unified core, we'll likely need to add /boot to pick up + * kernels here at least. This is intended short term to address + * https://github.com/projectatomic/rpm-ostree/issues/233 + */ +static gboolean +path_is_ostree_compliant (const char *path) +{ + g_assert (*path == '/'); + path++; + return (*path == '\0' || + g_str_equal (path, "usr") || g_str_has_prefix (path, "usr/") || + g_str_equal (path, "bin") || g_str_has_prefix (path, "bin/") || + g_str_equal (path, "sbin") || g_str_has_prefix (path, "sbin/") || + g_str_equal (path, "lib") || g_str_has_prefix (path, "lib/") || + g_str_equal (path, "lib64") || g_str_has_prefix (path, "lib64/")); +} + static OstreeRepoCommitFilterResult compose_filter_cb (OstreeRepo *repo, const char *path, @@ -612,9 +629,7 @@ compose_filter_cb (OstreeRepo *repo, return OSTREE_REPO_COMMIT_FILTER_SKIP; } /* And ensure the RPM installs into supported paths */ - else if (!(g_str_has_prefix (path, "/usr/") || g_str_has_prefix (path, "/bin/") || - g_str_has_prefix (path, "/sbin/") || g_str_has_prefix (path, "/lib/") || - g_str_has_prefix (path, "/lib64/"))) + else if (!path_is_ostree_compliant (path)) { g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, "Unsupported path: %s; See %s",