Skip to content

install: Add a config knob for container sigpolicy #2116

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jbtrystram wants to merge 1 commit into
base: main
Choose a base branch
from
Open

install: Add a config knob for container sigpolicy #2116

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 28 additions & 21 deletions crates/lib/src/install.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1536,7 +1536,7 @@ async fn verify_target_fetch(
async fn prepare_install(
mut config_opts: InstallConfigOpts,
source_opts: InstallSourceOpts,
target_opts: InstallTargetOpts,
mut target_opts: InstallTargetOpts,
mut composefs_options: InstallComposefsOpts,
target_fs: Option<FilesystemEnum>,
) -> Result<Arc<State>> {
Expand Down Expand Up @@ -1584,6 +1584,33 @@ async fn prepare_install(
}
};

// Load install configuration from TOML drop-in files early, so that
// config values are available when constructing the target image reference.
let install_config = config::load_config()?;
if let Some(ref config) = install_config {
tracing::debug!("Loaded install configuration");
// Merge config file values into config_opts (CLI takes precedence)
// Only apply config file value if CLI didn't explicitly set it
if !config_opts.bootupd_skip_boot_uuid {
config_opts.bootupd_skip_boot_uuid = config
.bootupd
.as_ref()
.and_then(|b| b.skip_boot_uuid)
.unwrap_or(false);
}

if config_opts.bootloader.is_none() {
config_opts.bootloader = config.bootloader.clone();
}

if !target_opts.enforce_container_sigpolicy {
target_opts.enforce_container_sigpolicy =
config.enforce_container_sigpolicy.unwrap_or(false);
}
Comment on lines +1594 to +1609
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The logic for merging boolean options from the configuration file can be made more explicit to improve readability and avoid potentially redundant assignments.

When a boolean configuration option is not present, the current implementation with unwrap_or(false) assigns false to a variable that is already known to be false inside the if !... block.

Using if let to check for the presence of the config value before assigning makes the intent clearer and avoids this no-op assignment.

        if !config_opts.bootupd_skip_boot_uuid {
            if let Some(skip) = config.bootupd.as_ref().and_then(|b| b.skip_boot_uuid) {
                config_opts.bootupd_skip_boot_uuid = skip;
            }
        }

        if config_opts.bootloader.is_none() {
            config_opts.bootloader = config.bootloader.clone();
        }

        if !target_opts.enforce_container_sigpolicy {
            if let Some(enforce) = config.enforce_container_sigpolicy {
                target_opts.enforce_container_sigpolicy = enforce;
            }
        }

} else {
tracing::debug!("No install configuration found");
}

// 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 {
Expand Down Expand Up @@ -1671,26 +1698,6 @@ async fn prepare_install(
println!("Digest: {digest}");
}

let install_config = config::load_config()?;
if let Some(ref config) = install_config {
tracing::debug!("Loaded install configuration");
// Merge config file values into config_opts (CLI takes precedence)
// Only apply config file value if CLI didn't explicitly set it
if !config_opts.bootupd_skip_boot_uuid {
config_opts.bootupd_skip_boot_uuid = config
.bootupd
.as_ref()
.and_then(|b| b.skip_boot_uuid)
.unwrap_or(false);
}

if config_opts.bootloader.is_none() {
config_opts.bootloader = config.bootloader.clone();
}
} else {
tracing::debug!("No install configuration found");
}

let root_filesystem = target_fs
.or(install_config
.as_ref()
Expand Down
51 changes: 51 additions & 0 deletions crates/lib/src/install/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,9 @@ pub(crate) struct InstallConfiguration {
/// Interface (systemd-boot always does, GRUB needs the `bli` module).
/// Defaults to false for broad compatibility.
pub(crate) discoverable_partitions: Option<bool>,
/// Enforce that the containers-storage stack has a non-default
/// (i.e. not `insecureAcceptAnything`) container image signature policy.
pub(crate) enforce_container_sigpolicy: Option<bool>,
}

fn merge_basic<T>(s: &mut Option<T>, o: Option<T>, _env: &EnvProperties) {
Expand Down Expand Up @@ -218,6 +221,11 @@ impl Mergeable for InstallConfiguration {
other.discoverable_partitions,
env,
);
merge_basic(
&mut self.enforce_container_sigpolicy,
other.enforce_container_sigpolicy,
env,
);
if let Some(other_kargs) = other.kargs {
self.kargs
.get_or_insert_with(Default::default)
Expand Down Expand Up @@ -940,3 +948,46 @@ root-fs-type = "xfs"
.unwrap();
assert_eq!(c.install.unwrap().discoverable_partitions, None);
}

#[test]
fn test_parse_enforce_container_sigpolicy() {
let env = EnvProperties {
sys_arch: "x86_64".to_string(),
};

// Test parsing true and false
for (input, expected) in [("true", true), ("false", false)] {
let toml_str = format!(
r#"[install]
enforce-container-sigpolicy = {input}
"#
);
let c: InstallConfigurationToplevel = toml::from_str(&toml_str).unwrap();
assert_eq!(
c.install.unwrap().enforce_container_sigpolicy.unwrap(),
expected
);
}

// Default (not specified) is None
let c: InstallConfigurationToplevel = toml::from_str(
r#"[install]
root-fs-type = "xfs"
"#,
)
.unwrap();
assert!(c.install.unwrap().enforce_container_sigpolicy.is_none());

// Test merging: last write wins
let mut install: InstallConfiguration = toml::from_str(
r#"enforce-container-sigpolicy = false
"#,
)
.unwrap();
let other = InstallConfiguration {
enforce_container_sigpolicy: Some(true),
..Default::default()
};
install.merge(other, &env);
assert_eq!(install.enforce_container_sigpolicy.unwrap(), true);
}
6 changes: 6 additions & 0 deletions docs/src/man/bootc-install-config.5.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ The `install` section supports these subfields:
implement the Boot Loader Interface (BLI); systemd-boot always does, GRUB
needs the `bli` module (available in newer builds). Defaults to `true`
when using systemd-boot, `false` otherwise.
- `enforce-container-sigpolicy`: A boolean that controls whether to enforce that
`/etc/containers/policy.json` includes a default policy which requires signatures.
When `true`, image pulls will be rejected if the policy file specifies
`insecureAcceptAnything` as the default. Defaults to `false`.
This is equivalent to the `--enforce-container-sigpolicy` CLI flag.

# filesystem

Expand Down Expand Up @@ -79,6 +84,7 @@ kargs = ["nosmt", "console=tty0"]
stateroot = "myos"
root-mount-spec = "LABEL=rootfs"
boot-mount-spec = "UUID=abcd-1234"
enforce-container-sigpolicy = true

[install.ostree]
bls-append-except-default = 'grub_users=""'
Expand Down
Loading