Skip to content

Commit

Permalink
Merge pull request #230 from cgwalters/no-sigverify
Browse files Browse the repository at this point in the history
Don't enforce container sigpolicy by default
  • Loading branch information
cgwalters authored Dec 18, 2023
2 parents 57a97cd + 20d2d6c commit f194c46
Show file tree
Hide file tree
Showing 11 changed files with 172 additions and 24 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 @@ -364,7 +372,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 @@ -476,3 +484,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");
}
26 changes: 26 additions & 0 deletions lib/src/fixtures/spec-ostree-remote.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# This one drops the now-optional signature schema
apiVersion: org.containers.bootc/v1alpha1
kind: BootcHost
metadata:
name: host
spec:
image:
image: quay.io/fedora/fedora-coreos:stable
transport: registry
signature: !ostreeRemote "fedora"
status:
booted:
image:
image:
image: quay.io/otherexample/otherimage:latest
transport: registry
version: 20231230.1
timestamp: 2023-12-30T16:10:11Z
imageDigest: sha256:b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c
incompatible: false
pinned: false
ostree:
checksum: 41af286dc0b172ed2f1ca934fd2278de4a1192302ffa07087cea2682e7d372e3
deploySerial: 0
rollback: null
isContainer: false
25 changes: 25 additions & 0 deletions lib/src/fixtures/spec-v1.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# This one drops the now-optional signature schema
apiVersion: org.containers.bootc/v1alpha1
kind: BootcHost
metadata:
name: host
spec:
image:
image: quay.io/otherexample/otherimage:latest
transport: registry
status:
booted:
image:
image:
image: quay.io/otherexample/otherimage:latest
transport: registry
version: 20231230.1
timestamp: 2023-12-30T16:10:11Z
imageDigest: sha256:b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c
incompatible: false
pinned: false
ostree:
checksum: 41af286dc0b172ed2f1ca934fd2278de4a1192302ffa07087cea2682e7d372e3
deploySerial: 0
rollback: null
isContainer: false
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 @@ -914,8 +927,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
28 changes: 25 additions & 3 deletions lib/src/spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,9 @@ pub struct ImageReference {
pub image: String,
/// The container image transport
pub transport: String,
/// Disable signature verification
pub signature: ImageSignature,
/// Signature verification type
#[serde(skip_serializing_if = "Option::is_none")]
pub signature: Option<ImageSignature>,
}

/// The status of the booted image
Expand Down Expand Up @@ -132,12 +133,33 @@ mod tests {
use super::*;

#[test]
fn test_parse_spec() {
fn test_parse_spec_v0() {
const SPEC_FIXTURE: &str = include_str!("fixtures/spec.yaml");
let host: Host = serde_yaml::from_str(SPEC_FIXTURE).unwrap();
assert_eq!(
host.spec.image.as_ref().unwrap().image.as_str(),
"quay.io/example/someimage:latest"
);
}

#[test]
fn test_parse_spec_v1() {
const SPEC_FIXTURE: &str = include_str!("fixtures/spec-v1.yaml");
let host: Host = serde_yaml::from_str(SPEC_FIXTURE).unwrap();
assert_eq!(
host.spec.image.as_ref().unwrap().image.as_str(),
"quay.io/otherexample/otherimage:latest"
);
assert_eq!(host.spec.image.as_ref().unwrap().signature, None);
}

#[test]
fn test_parse_ostreeremote() {
const SPEC_FIXTURE: &str = include_str!("fixtures/spec-ostree-remote.yaml");
let host: Host = serde_yaml::from_str(SPEC_FIXTURE).unwrap();
assert_eq!(
host.spec.image.as_ref().unwrap().signature,
Some(ImageSignature::OstreeRemote("fedora".into()))
);
}
}
36 changes: 34 additions & 2 deletions lib/src/status.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,12 @@ fn transport_to_string(transport: ostree_container::Transport) -> String {

impl From<OstreeImageReference> for ImageReference {
fn from(imgref: OstreeImageReference) -> Self {
let signature = match imgref.sigverify {
ostree_container::SignatureSource::ContainerPolicyAllowInsecure => None,
v => Some(v.into()),
};
Self {
signature: imgref.sigverify.into(),
signature,
transport: transport_to_string(imgref.imgref.transport),
image: imgref.imgref.name,
}
Expand All @@ -61,8 +65,12 @@ impl From<OstreeImageReference> for ImageReference {

impl From<ImageReference> for OstreeImageReference {
fn from(img: ImageReference) -> Self {
let sigverify = match img.signature {
Some(v) => v.into(),
None => ostree_container::SignatureSource::ContainerPolicyAllowInsecure,
};
Self {
sigverify: img.signature.into(),
sigverify,
imgref: ostree_container::ImageReference {
/// SAFETY: We validated the schema in kube-rs
transport: img.transport.as_str().try_into().unwrap(),
Expand Down Expand Up @@ -284,3 +292,27 @@ pub(crate) async fn status(opts: super::cli::StatusOpts) -> Result<()> {

Ok(())
}

#[test]
fn test_convert_signatures() {
use std::str::FromStr;
let ir_unverified = &OstreeImageReference::from_str(
"ostree-unverified-registry:quay.io/someexample/foo:latest",
)
.unwrap();
let ir_ostree = &OstreeImageReference::from_str(
"ostree-remote-registry:fedora:quay.io/fedora/fedora-coreos:stable",
)
.unwrap();

let ir = ImageReference::from(ir_unverified.clone());
assert_eq!(ir.image, "quay.io/someexample/foo:latest");
assert_eq!(ir.signature, None);

let ir = ImageReference::from(ir_ostree.clone());
assert_eq!(ir.image, "quay.io/fedora/fedora-coreos:stable");
assert_eq!(
ir.signature,
Some(ImageSignature::OstreeRemote("fedora".into()))
);
}
2 changes: 1 addition & 1 deletion tests/kolainst/install
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ case "${AUTOPKGTEST_REBOOT_MARK:-}" in
EOF
podman build -t localhost/testimage .
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 f194c46

Please sign in to comment.