Skip to content

Commit

Permalink
Don't enforce container sigpolicy by default
Browse files Browse the repository at this point in the history
Closes: #218

Basically this effort has not been really successful and adds
more pain than it solves.  We need to have a solution that works
for podman too.

In many scenarios, TLS is sufficient - or at least, we're far
from the only thing that if fetched from a compromised server
would result in a compromised system (e.g. privileged containers).

Signed-off-by: Colin Walters <walters@verbum.org>
  • Loading branch information
cgwalters committed Dec 16, 2023
1 parent 9138153 commit a51ecb8
Show file tree
Hide file tree
Showing 7 changed files with 70 additions and 19 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ jobs:
run: |
set -xeuo pipefail
sudo podman run --rm -ti --privileged -v /:/target -v ./usr/bin/bootc:/usr/bin/bootc --pid=host --security-opt label=disable \
quay.io/centos-bootc/fedora-bootc-dev:eln bootc install to-filesystem --target-no-signature-verification \
quay.io/centos-bootc/fedora-bootc-dev:eln bootc install to-filesystem \
--karg=foo=bar --disable-selinux --replace=alongside /target
ls -al /boot/loader/
sudo grep foo=bar /boot/loader/entries/*.conf
2 changes: 1 addition & 1 deletion docs/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ For more information, please see [install.md](install.md).
If you have [an operating system already using ostree](https://ostreedev.github.io/ostree/#operating-systems-and-distributions-using-ostree) then you can use `bootc switch`:

```
$ bootc switch --no-signature-verification quay.io/examplecorp/custom:latest
$ bootc switch quay.io/examplecorp/custom:latest
```

This will preserve existing state in `/etc` and `/var` - for example,
Expand Down
7 changes: 2 additions & 5 deletions docs/install.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ to an existing system and install your container image. Failure to run
Here's an example of using `bootc install` (root/elevated permission required):

```bash
podman run --rm --privileged --pid=host --security-opt label=type:unconfined_t <image> bootc install to-disk --target-no-signature-verification /path/to/disk
podman run --rm --privileged --pid=host --security-opt label=type:unconfined_t <image> bootc install to-disk /path/to/disk
```

Note that while `--privileged` is used, this command will not perform any
Expand All @@ -85,10 +85,7 @@ This can be overridden via `--target_imgref`; this is handy in cases like perfor
installation in a manufacturing environment from a mirrored registry.

By default, the installation process will verify that the container (representing the target OS)
can fetch its own updates. A common cause of failure here is not changing the security settings
in `/etc/containers/policy.json` in the target OS to verify signatures.

If you are pushing an unsigned image, you **MUST** specify `bootc install --target-no-signature-verification`.
can fetch its own updates.

Additionally note that to perform an install with a target image reference set to an
authenticated registry, you must provide a pull secret. One path is to embed the pull secret into
Expand Down
33 changes: 30 additions & 3 deletions lib/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,18 @@ pub(crate) struct SwitchOpts {
#[clap(long, default_value = "registry")]
pub(crate) transport: String,

/// Explicitly opt-out of requiring any form of signature verification.
#[clap(long)]
/// This argument is deprecated and does nothing.
#[clap(long, hide = true)]
pub(crate) no_signature_verification: bool,

/// This is the inverse of the previous `--target-no-signature-verification` (which is now
/// a no-op).
///
/// Enabling this option enforces that `/etc/containers/policy.json` includes a
/// default policy which requires signatures.
#[clap(long)]
pub(crate) enforce_container_sigpolicy: bool,

/// Enable verification via an ostree remote
#[clap(long)]
pub(crate) ostree_remote: Option<String>,
Expand Down Expand Up @@ -363,7 +371,7 @@ async fn switch(opts: SwitchOpts) -> Result<()> {
name: opts.target.to_string(),
};
let sigverify = sigpolicy_from_opts(
opts.no_signature_verification,
!opts.enforce_container_sigpolicy,
opts.ostree_remote.as_deref(),
);
let target = ostree_container::OstreeImageReference { sigverify, imgref };
Expand Down Expand Up @@ -475,3 +483,22 @@ async fn run_from_opt(opt: Opt) -> Result<()> {
Opt::Man(manopts) => crate::docgen::generate_manpages(&manopts.directory),
}
}

#[test]
fn test_parse_install_args() {
// Verify we still process the legacy --target-no-signature-verification
let o = Opt::try_parse_from([
"bootc",
"install",
"to-filesystem",
"--target-no-signature-verification",
"/target",
])
.unwrap();
let o = match o {
Opt::Install(InstallOpts::ToFilesystem(fsopts)) => fsopts,
o => panic!("Expected filesystem opts, not {o:?}"),
};
assert!(o.target_opts.target_no_signature_verification);
assert_eq!(o.filesystem_opts.root_path.as_str(), "/target");
}
33 changes: 26 additions & 7 deletions lib/src/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,26 @@ pub(crate) struct InstallTargetOpts {
#[clap(long)]
pub(crate) target_imgref: Option<String>,

/// Explicitly opt-out of requiring any form of signature verification.
#[clap(long)]
/// This command line argument does nothing; it exists for compatibility.
///
/// As of newer versions of bootc, this value is enabled by default,
/// i.e. it is not enforced that a signature
/// verification policy is enabled. Hence to enable it, one can specify
/// `--target-no-signature-verification=false`.
///
/// It is likely that the functionality here will be replaced with a different signature
/// enforcement scheme in the future that integrates with `podman`.
#[clap(long, hide = true)]
#[serde(default)]
pub(crate) target_no_signature_verification: bool,

/// This is the inverse of the previous `--target-no-signature-verification` (which is now
/// a no-op). Enabling this option enforces that `/etc/containers/policy.json` includes a
/// default policy which requires signatures.
#[clap(long)]
#[serde(default)]
pub(crate) enforce_container_sigpolicy: bool,

/// Enable verification via an ostree remote
#[clap(long)]
pub(crate) target_ostree_remote: Option<String>,
Expand All @@ -80,10 +95,8 @@ pub(crate) struct InstallTargetOpts {
/// Specifying this option suppresses the check; use this when you know the issues it might find
/// are addressed.
///
/// Two main reasons this might fail:
///
/// - Forgetting `--target-no-signature-verification` if needed
/// - Using a registry which requires authentication, but not embedding the pull secret in the image.
/// A common reason this may fail is when one is using an image which requires registry authentication,
/// but not embedding the pull secret in the image so that updates can be fetched by the installed OS "day 2".
#[clap(long)]
#[serde(default)]
pub(crate) skip_fetch_check: bool,
Expand Down Expand Up @@ -916,8 +929,14 @@ async fn prepare_install(

// Parse the target CLI image reference options and create the *target* image
// reference, which defaults to pulling from a registry.
if target_opts.target_no_signature_verification {
// Perhaps log this in the future more prominently, but no reason to annoy people.
tracing::debug!(
"Use of --target-no-signature-verification flag which is enabled by default"
);
}
let target_sigverify = sigpolicy_from_opts(
target_opts.target_no_signature_verification,
!target_opts.enforce_container_sigpolicy,
target_opts.target_ostree_remote.as_deref(),
);
let target_imgname = target_opts
Expand Down
2 changes: 1 addition & 1 deletion lib/src/privtests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ fn test_install_filesystem(image: &str, blockdev: &Utf8Path) -> Result<()> {
let mountpoint: &Utf8Path = mountpoint_dir.path().try_into().unwrap();

// And run the install
cmd!(sh, "podman run --rm --privileged --pid=host --env=RUST_LOG -v /usr/bin/bootc:/usr/bin/bootc -v {mountpoint}:/target-root {image} bootc install to-filesystem --target-no-signature-verification /target-root").run()?;
cmd!(sh, "podman run --rm --privileged --pid=host --env=RUST_LOG -v /usr/bin/bootc:/usr/bin/bootc -v {mountpoint}:/target-root {image} bootc install to-filesystem /target-root").run()?;

cmd!(sh, "umount -R {mountpoint}").run()?;

Expand Down
10 changes: 9 additions & 1 deletion tests/kolainst/install
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,16 @@ case "${AUTOPKGTEST_REBOOT_MARK:-}" in
COPY usr usr
EOF
podman build -t localhost/testimage .
# Verify we do error out if signature verification is explicitly turned on
if podman run --rm -ti --privileged --pid=host \
localhost/testimage bootc install --skip-fetch-check --enforce-container-sigpolicy --karg=foo=bar ${DEV} 2>err.txt; then
echo "Performed an install with no signature verification" 1>&2; exit 1
fi
grep -E -e 'containers-policy.json.*refusing usage' err.txt

# Now do an install
podman run --rm -ti --privileged --pid=host --env RUST_LOG=error,bootc_lib::install=debug \
localhost/testimage bootc install to-disk --target-no-signature-verification --skip-fetch-check --karg=foo=bar ${DEV}
localhost/testimage bootc install to-disk --skip-fetch-check --karg=foo=bar ${DEV}
# In theory we could e.g. wipe the bootloader setup on the primary disk, then reboot;
# but for now let's just sanity test that the install command executes.
lsblk ${DEV}
Expand Down

0 comments on commit a51ecb8

Please sign in to comment.