Skip to content

Commit 113be85

Browse files
committed
cli: drop support for verifying ostree remotes
The `install` and `switch` verbs supported passing an OSTree remote to use to verify the embedded OSTree commit. This option in fact did not function correctly and no verification was actually performace. While it's possible someone was using this feature, it seems quite unlikely since the UX is more geared towards native OCI signatures. As a result, we have decided not to file a CVE for this. And in fact, since we're planning to keep moving away from ostree, instead of fixing this bug, just completely rip out support for passing and OSTree remote. Note this is distinct from the ostree-ext code, which still does support signature verification of the embedded OSTree commit. E.g. Fedora CoreOS is planning to make use of that until it can move to proper OCI signatures. While we're here, drop the negation around the container sigpolicy to make it easier to follow the logic. Signed-off-by: Jonathan Lebon <[email protected]>
1 parent 8dff320 commit 113be85

File tree

3 files changed

+10
-41
lines changed

3 files changed

+10
-41
lines changed

lib/src/cli.rs

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ use crate::lints;
2828
use crate::progress_jsonl::{ProgressWriter, RawProgressFd};
2929
use crate::spec::Host;
3030
use crate::spec::ImageReference;
31-
use crate::utils::sigpolicy_from_opts;
31+
use crate::utils::sigpolicy_from_opt;
3232

3333
/// Shared progress options
3434
#[derive(Debug, Parser, PartialEq, Eq)]
@@ -111,10 +111,6 @@ pub(crate) struct SwitchOpts {
111111
#[clap(long)]
112112
pub(crate) enforce_container_sigpolicy: bool,
113113

114-
/// Enable verification via an ostree remote
115-
#[clap(long)]
116-
pub(crate) ostree_remote: Option<String>,
117-
118114
/// Don't create a new deployment, but directly mutate the booted state.
119115
/// This is hidden because it's not something we generally expect to be done,
120116
/// but this can be used in e.g. Anaconda %post to fixup
@@ -795,10 +791,7 @@ async fn switch(opts: SwitchOpts) -> Result<()> {
795791
transport,
796792
name: opts.target.to_string(),
797793
};
798-
let sigverify = sigpolicy_from_opts(
799-
!opts.enforce_container_sigpolicy,
800-
opts.ostree_remote.as_deref(),
801-
);
794+
let sigverify = sigpolicy_from_opt(opts.enforce_container_sigpolicy);
802795
let target = ostree_container::OstreeImageReference { sigverify, imgref };
803796
let target = ImageReference::from(target);
804797
let prog: ProgressWriter = opts.progress.try_into()?;

lib/src/install.rs

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ use crate::progress_jsonl::ProgressWriter;
5757
use crate::spec::ImageReference;
5858
use crate::store::Storage;
5959
use crate::task::Task;
60-
use crate::utils::{open_dir_noxdev, sigpolicy_from_opts};
60+
use crate::utils::{open_dir_noxdev, sigpolicy_from_opt};
6161

6262
/// The toplevel boot directory
6363
const BOOT: &str = "boot";
@@ -112,10 +112,6 @@ pub(crate) struct InstallTargetOpts {
112112
#[serde(default)]
113113
pub(crate) enforce_container_sigpolicy: bool,
114114

115-
/// Enable verification via an ostree remote
116-
#[clap(long)]
117-
pub(crate) target_ostree_remote: Option<String>,
118-
119115
/// By default, the accessiblity of the target image will be verified (just the manifest will be fetched).
120116
/// Specifying this option suppresses the check; use this when you know the issues it might find
121117
/// are addressed.
@@ -1216,10 +1212,7 @@ async fn prepare_install(
12161212
"Use of --target-no-signature-verification flag which is enabled by default"
12171213
);
12181214
}
1219-
let target_sigverify = sigpolicy_from_opts(
1220-
!target_opts.enforce_container_sigpolicy,
1221-
target_opts.target_ostree_remote.as_deref(),
1222-
);
1215+
let target_sigverify = sigpolicy_from_opt(target_opts.enforce_container_sigpolicy);
12231216
let target_imgname = target_opts
12241217
.target_imgref
12251218
.as_deref()

lib/src/utils.rs

Lines changed: 6 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -172,16 +172,10 @@ pub(crate) fn spawn_editor(tmpf: &tempfile::NamedTempFile) -> Result<()> {
172172
}
173173

174174
/// Convert a combination of values (likely from CLI parsing) into a signature source
175-
pub(crate) fn sigpolicy_from_opts(
176-
disable_verification: bool,
177-
ostree_remote: Option<&str>,
178-
) -> SignatureSource {
179-
if disable_verification {
180-
SignatureSource::ContainerPolicyAllowInsecure
181-
} else if let Some(remote) = ostree_remote {
182-
SignatureSource::OstreeRemote(remote.to_owned())
183-
} else {
184-
SignatureSource::ContainerPolicy
175+
pub(crate) fn sigpolicy_from_opt(enforce_container_verification: bool) -> SignatureSource {
176+
match enforce_container_verification {
177+
true => SignatureSource::ContainerPolicy,
178+
false => SignatureSource::ContainerPolicyAllowInsecure,
185179
}
186180
}
187181

@@ -273,20 +267,9 @@ mod tests {
273267

274268
#[test]
275269
fn test_sigpolicy_from_opts() {
270+
assert_eq!(sigpolicy_from_opt(true), SignatureSource::ContainerPolicy);
276271
assert_eq!(
277-
sigpolicy_from_opts(false, None),
278-
SignatureSource::ContainerPolicy
279-
);
280-
assert_eq!(
281-
sigpolicy_from_opts(true, None),
282-
SignatureSource::ContainerPolicyAllowInsecure
283-
);
284-
assert_eq!(
285-
sigpolicy_from_opts(false, Some("foo")),
286-
SignatureSource::OstreeRemote("foo".to_owned())
287-
);
288-
assert_eq!(
289-
sigpolicy_from_opts(true, Some("foo")),
272+
sigpolicy_from_opt(false),
290273
SignatureSource::ContainerPolicyAllowInsecure
291274
);
292275
}

0 commit comments

Comments
 (0)