Skip to content

Commit

Permalink
Add support for rpm-ostree deploy --ex-cliwrap=true
Browse files Browse the repository at this point in the history
This is a better alternative to coreos/fedora-coreos-config#830

Basically rather than trying to send this out to all FCOS users,
it's much saner to allow people to opt-in to it locally.

If we'd finished coreos#2326
then this would be something as trivial as:
```
$ echo 'cliwrap: true' > /etc/rpm-ostree.d/cliwrap.yaml
$ rpm-ostree rebuild
```

Unfortunately that's not the world we live in, so a whole lot of
layers here need crossing to just propagate a boolean.  And it
interacts in a tricky way with our change detection code.

But, it works and will allow people to try this out.

Other fixed problems:

- Our `rpm --verify` wrapping was broken
- Dropping privileges clashed with the default directory being `/root`,
  so `chdir(/)` too
  • Loading branch information
cgwalters committed May 14, 2021
1 parent a6da3c0 commit 312cac8
Show file tree
Hide file tree
Showing 12 changed files with 152 additions and 33 deletions.
1 change: 1 addition & 0 deletions rust/src/cliwrap/cliutil.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ pub fn run_unprivileged<T: AsRef<str>>(
proc.args(setpriv_argv);
proc.arg(real_bin);
proc.args(argv);
proc.current_dir("/");
Err(proc.exec().into())
} else {
exec_real_binary(target_bin, &argv)
Expand Down
21 changes: 19 additions & 2 deletions rust/src/cliwrap/rpm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,14 @@ fn new_rpm_app<'r>() -> App<'r, 'static> {
.bin_name(name)
.version("0.1")
.about("Wrapper for rpm")
.arg(Arg::with_name("verify").short("V"))
.arg(Arg::with_name("verify").short("V").long("verify"))
.arg(Arg::with_name("version"))
.arg(
Arg::with_name("package")
.help("package")
.takes_value(true)
.multiple(true),
)
}

// clap doesn't easily allow us to parse unknown arguments right now,
Expand Down Expand Up @@ -48,7 +54,9 @@ fn disposition(argv: &[&str]) -> Result<RunDisposition> {
{
Ok(v) => v,
Err(e) if e.kind == clap::ErrorKind::VersionDisplayed => return Ok(RunDisposition::Ok),
_ => return Ok(RunDisposition::Warn),
Err(_) => {
return Ok(RunDisposition::Warn);
}
};

if matches.is_present("verify") {
Expand Down Expand Up @@ -135,4 +143,13 @@ mod tests {
assert_eq!(disposition(&["-e", "bash"])?, RunDisposition::Warn);
Ok(())
}

#[test]
fn test_verify() -> Result<()> {
assert!(matches!(
disposition(&["--verify", "bash"])?,
RunDisposition::Notice(_)
));
Ok(())
}
}
24 changes: 21 additions & 3 deletions src/app/rpmostree-builtin-deploy.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ static gboolean opt_disallow_downgrade;
static gboolean opt_unchanged_exit_77;
static gboolean opt_bypass_driver;
static gboolean opt_skip_branch_check;
static char *opt_ex_cliwrap;

static GOptionEntry option_entries[] = {
{ "os", 0, 0, G_OPTION_ARG_STRING, &opt_osname, "Operate on provided OSNAME", "OSNAME" },
Expand All @@ -55,6 +56,7 @@ static GOptionEntry option_entries[] = {
{ "unchanged-exit-77", 0, 0, G_OPTION_ARG_NONE, &opt_unchanged_exit_77, "If no new deployment made, exit 77", NULL },
{ "register-driver", 0, 0, G_OPTION_ARG_STRING, &opt_register_driver, "Register the calling agent as the driver for updates; if REVISION is an empty string, register driver without deploying", "DRIVERNAME" },
{ "bypass-driver", 0, 0, G_OPTION_ARG_NONE, &opt_bypass_driver, "Force a deploy even if an updates driver is registered", NULL},
{ "ex-cliwrap", 0, G_OPTION_FLAG_HIDDEN, G_OPTION_ARG_STRING, &opt_ex_cliwrap, "Enable or disable wrapping binaries like /usr/bin/rpm", NULL},
{ NULL }
};

Expand Down Expand Up @@ -89,7 +91,11 @@ rpmostree_builtin_deploy (int argc,
error))
return FALSE;

if (argc < 2)
const gboolean arg_specified = argc >= 2;
// If using --ex-cliwrap or --register-driver, we don't usually
// expect them to be performing another operation.
const gboolean arg_required = !(opt_ex_cliwrap || opt_register_driver);
if (!arg_specified && arg_required)
{
rpmostree_usage_error (context, "REVISION must be specified", error);
return FALSE;
Expand All @@ -102,7 +108,10 @@ rpmostree_builtin_deploy (int argc,
return FALSE;
}

revision = argv[1];
if (arg_specified)
revision = argv[1];
else
revision = NULL;

if (!rpmostree_load_os_proxy (sysroot_proxy, opt_osname,
cancellable, &os_proxy, error))
Expand Down Expand Up @@ -139,10 +148,19 @@ rpmostree_builtin_deploy (int argc,
g_variant_dict_insert (&dict, "initiating-command-line", "s", invocation->command_line);
if (opt_register_driver)
g_variant_dict_insert (&dict, "register-driver", "s", opt_register_driver);
if (opt_ex_cliwrap)
{
gboolean cliwrap_enabled = FALSE;
if (g_str_equal (opt_ex_cliwrap, "true"))
cliwrap_enabled = TRUE;
else if (!g_str_equal (opt_ex_cliwrap, "false"))
return glnx_throw (error, "Expecting --ex-cli-wrap=true/false but found %s", opt_ex_cliwrap);
g_variant_dict_insert (&dict, "ex-cliwrap", "b", cliwrap_enabled);
}
g_autoptr(GVariant) options = g_variant_ref_sink (g_variant_dict_end (&dict));

/* Use newer D-Bus API only if we have to so we maintain coverage. */
if (install_pkgs || uninstall_pkgs)
if (install_pkgs || uninstall_pkgs || opt_ex_cliwrap)
{
if (!rpmostree_update_deployment (os_proxy,
NULL, /* refspec */
Expand Down
4 changes: 4 additions & 0 deletions src/app/rpmostree-builtin-status.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -993,6 +993,10 @@ print_one_deployment (RPMOSTreeSysroot *sysroot_proxy,
g_string_append (buf, "regenerate");
rpmostree_print_kv ("Initramfs", max_key_len, buf->str);
}

gboolean cliwrap = FALSE;
if (g_variant_dict_lookup (dict, "cliwrap", "b", &cliwrap) && cliwrap)
rpmostree_print_kv ("Cliwrap", max_key_len, "enabled");

g_autofree char **initramfs_etc_files = NULL;
g_variant_dict_lookup (dict, "initramfs-etc", "^a&s", &initramfs_etc_files);
Expand Down
12 changes: 9 additions & 3 deletions src/daemon/rpmostree-sysroot-upgrader.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ struct RpmOstreeSysrootUpgrader {

gboolean layering_initialized; /* Whether layering_type is known */
RpmOstreeSysrootUpgraderLayeringType layering_type;
gboolean cal_assembly; /* True if the txn knows that something changed, e.g. cliwrap */
gboolean layering_changed; /* Whether changes to layering should result in a new commit */
gboolean pkgs_imported; /* Whether pkgs to be layered have been downloaded & imported */
char *base_revision; /* Non-layered replicated commit */
Expand Down Expand Up @@ -985,8 +986,8 @@ prep_local_assembly (RpmOstreeSysrootUpgrader *self,
g_autoptr(RpmOstreeTreespec) treespec = generate_treespec (self);
if (treespec == NULL)
return FALSE;

rpmostree_context_set_treespec (self->ctx, treespec);

if (!rpmostree_context_setup (self->ctx, tmprootfs_abspath, tmprootfs_abspath,
cancellable, error))
return FALSE;
Expand Down Expand Up @@ -1078,11 +1079,15 @@ perform_local_assembly (RpmOstreeSysrootUpgrader *self,
{
g_clear_pointer (&self->final_revision, g_free);

/* --- override/overlay and commit --- */
/* --- override/overlay --- */
if (!rpmostree_context_assemble (self->ctx, cancellable, error))
return FALSE;
}

// TODO Unify with treefile origin handling in core
if (rpmostree_origin_get_cliwrap (self->origin))
rpmostreecxx::cliwrap_write_wrappers (self->tmprootfs_dfd);

if (!rpmostree_rootfs_postprocess_common (self->tmprootfs_dfd, cancellable, error))
return FALSE;

Expand Down Expand Up @@ -1208,7 +1213,8 @@ requires_local_assembly (RpmOstreeSysrootUpgrader *self)
* https://github.com/projectatomic/rpm-ostree/issues/753
*/

return self->overlay_packages->len > 0 ||
return rpmostree_origin_get_cliwrap (self->origin) ||
self->overlay_packages->len > 0 ||
self->override_remove_packages->len > 0 ||
self->override_replace_local_packages->len > 0 ||
g_hash_table_size (rpmostree_origin_get_local_packages (self->origin)) > 0 ||
Expand Down
1 change: 0 additions & 1 deletion src/daemon/rpmostree-sysroot-upgrader.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ rpmostree_sysroot_upgrader_dup_origin (RpmOstreeSysrootUpgrader *self);
void
rpmostree_sysroot_upgrader_set_origin (RpmOstreeSysrootUpgrader *self,
RpmOstreeOrigin *origin);

const char *
rpmostree_sysroot_upgrader_get_base (RpmOstreeSysrootUpgrader *self);

Expand Down
2 changes: 2 additions & 0 deletions src/daemon/rpmostreed-deployment-utils.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,8 @@ rpmostreed_deployment_generate_variant (OstreeSysroot *sysroot,

variant_add_from_hash_table (dict, "initramfs-etc",
rpmostree_origin_get_initramfs_etc_files (origin));
if (rpmostree_origin_get_cliwrap (origin))
g_variant_dict_insert (dict, "cliwrap", "b", TRUE);

return g_variant_dict_end (dict);
}
Expand Down
10 changes: 10 additions & 0 deletions src/daemon/rpmostreed-transaction-types.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -1140,6 +1140,16 @@ deploy_transaction_execute (RpmostreedTransaction *transaction,
changed = TRUE;
}

// Handle the --ex-cliwrap option
{
gboolean cliwrap = FALSE;
if (g_variant_dict_lookup (self->options, "ex-cliwrap", "b", &cliwrap))
{
rpmostree_origin_set_cliwrap (origin, cliwrap);
changed = TRUE;
}
}

gboolean remove_changed = FALSE;
if (no_layering)
{
Expand Down
21 changes: 20 additions & 1 deletion src/libpriv/rpmostree-origin.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,9 @@ rpmostree_origin_get_unconfigured_state (RpmOstreeOrigin *origin)
gboolean
rpmostree_origin_may_require_local_assembly (RpmOstreeOrigin *origin)
{
return rpmostree_origin_get_regenerate_initramfs (origin) ||
return
rpmostree_origin_get_cliwrap (origin) ||
rpmostree_origin_get_regenerate_initramfs (origin) ||
(g_hash_table_size (origin->cached_initramfs_etc_files) > 0) ||
(g_hash_table_size (origin->cached_packages) > 0) ||
(g_hash_table_size (origin->cached_local_packages) > 0) ||
Expand Down Expand Up @@ -529,6 +531,23 @@ rpmostree_origin_set_rojig_version (RpmOstreeOrigin *origin,
origin->cached_rojig_version = g_strdup (version);
}

gboolean
rpmostree_origin_get_cliwrap (RpmOstreeOrigin *origin)
{
return g_key_file_get_boolean (origin->kf, "rpmostree", "ex-cliwrap", NULL);
}

void
rpmostree_origin_set_cliwrap (RpmOstreeOrigin *origin, gboolean cliwrap)
{
const char k[] = "rpmostree";
const char v[] = "ex-cliwrap";
if (cliwrap)
g_key_file_set_boolean (origin->kf, k, v, TRUE);
else
g_key_file_remove_key (origin->kf, k, v, NULL);
}

/* The rojigRPM is highly special; it doesn't live in the rpmdb for example, as
* that would be fully circular. Yet, it's of critical importance to the whole
* system; we want to render it on the client. For now, what we do is stick the
Expand Down
5 changes: 5 additions & 0 deletions src/libpriv/rpmostree-origin.h
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,11 @@ void
rpmostree_origin_set_rojig_version (RpmOstreeOrigin *origin,
const char *version);

gboolean
rpmostree_origin_get_cliwrap (RpmOstreeOrigin *origin);
void
rpmostree_origin_set_cliwrap (RpmOstreeOrigin *origin, gboolean cliwrap);

gboolean
rpmostree_origin_set_rebase (RpmOstreeOrigin *origin,
const char *new_refspec,
Expand Down
61 changes: 61 additions & 0 deletions tests/kolainst/destructive/cliwrap
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
#!/bin/bash
#
# Copyright (C) 2021 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
# License as published by the Free Software Foundation; either
# version 2 of the License, or (at your option) any later version.
#
# This library is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
# Lesser General Public License for more details.
#
# You should have received a copy of the GNU Lesser General Public
# License along with this library; if not, write to the
# Free Software Foundation, Inc., 59 Temple Place - Suite 330,
# Boston, MA 02111-1307, USA.

set -euo pipefail

. ${KOLA_EXT_DATA}/libtest.sh

set -x

libtest_prepare_offline
libtest_enable_repover 0
cd $(mktemp -d)

rpm-ostree deploy --ex-cliwrap=true
rpm-ostree ex apply-live # yep it works!

wrapdir="/usr/libexec/rpm-ostree/wrapped"
if ! test -d "${wrapdir}"; then
fatal "Missing ${wrapdir}"
fi
# Test wrapped functions for rpm
rpm --version
rpm -qa > /dev/null
rpm --verify bash >out.txt
assert_file_has_content out.txt "rpm --verify is not necessary for ostree-based systems"
rm -f out.txt
if rpm -e bash 2>out.txt; then
fatal "rpm -e worked"
fi
assert_file_has_content out.txt 'Dropping privileges as `rpm` was executed with not "known safe" arguments'

if dracut --blah 2>out.txt; then
fatal "dracut worked"
fi
assert_file_has_content out.txt 'This system is rpm-ostree based'
rm -f out.txt
echo "ok cliwrap"

rpm-ostree deploy --ex-cliwrap=false
rpm-ostree ex apply-live
rpm --version
rpm -qa >/dev/null
rpm --verify bash >out.txt || true
assert_not_file_has_content "ostree-based"
echo "ok cliwrap undo"
23 changes: 0 additions & 23 deletions tests/kolainst/nondestructive/misc.sh
Original file line number Diff line number Diff line change
Expand Up @@ -46,29 +46,6 @@ assert_file_has_content err.txt 'PkgChange not allowed for user'
if runuser -u core rpm-ostree reload &>err.txt; then
assert_not_reached "Was able to reload as non-root!"
fi
echo "ok polkit"

wrapdir="/usr/libexec/rpm-ostree/wrapped"
if [ -d "${wrapdir}" ]; then
# Test wrapped functions for rpm
rpm --version
rpm -qa > /dev/null
rpm --verify >out.txt
assert_file_has_content out.txt "rpm --verify is not necessary for ostree-based systems"
rm -f out.txt
if rpm -e bash 2>out.txt; then
fatal "rpm -e worked"
fi
assert_file_has_content out.txt 'Dropping privileges as `rpm` was executed with not "known safe" arguments'

if dracut --blah 2>out.txt; then
fatal "dracut worked"
fi
assert_file_has_content out.txt 'This system is rpm-ostree based'
rm -f out.txt
else
echo "Missing ${wrapdir}; cliwrap not enabled"
fi

# StateRoot is only in --verbose, also verify we're not showing
# unlocked.
Expand Down

0 comments on commit 312cac8

Please sign in to comment.