Merge pull request #1028 from jlebon/pr/verify-text

ostree-ext: surface libostree signature verification text

Author: cgwalters

lib/src/cli.rs

@@ -28,7 +28,7 @@ use crate::lints;
 use crate::progress_jsonl::{ProgressWriter, RawProgressFd};
 use crate::spec::Host;
 use crate::spec::ImageReference;
-use crate::utils::sigpolicy_from_opts;
+use crate::utils::sigpolicy_from_opt;
 
 /// Shared progress options
 #[derive(Debug, Parser, PartialEq, Eq)]
@@ -111,10 +111,6 @@ pub(crate) struct SwitchOpts {
     #[clap(long)]
     pub(crate) enforce_container_sigpolicy: bool,
 
-    /// Enable verification via an ostree remote
-    #[clap(long)]
-    pub(crate) ostree_remote: Option<String>,
-
     /// Don't create a new deployment, but directly mutate the booted state.
     /// This is hidden because it's not something we generally expect to be done,
     /// but this can be used in e.g. Anaconda %post to fixup
@@ -795,10 +791,7 @@ async fn switch(opts: SwitchOpts) -> Result<()> {
         transport,
         name: opts.target.to_string(),
     };
-    let sigverify = sigpolicy_from_opts(
-        !opts.enforce_container_sigpolicy,
-        opts.ostree_remote.as_deref(),
-    );
+    let sigverify = sigpolicy_from_opt(opts.enforce_container_sigpolicy);
     let target = ostree_container::OstreeImageReference { sigverify, imgref };
     let target = ImageReference::from(target);
     let prog: ProgressWriter = opts.progress.try_into()?;

lib/src/install.rs

@@ -57,7 +57,7 @@ use crate::progress_jsonl::ProgressWriter;
 use crate::spec::ImageReference;
 use crate::store::Storage;
 use crate::task::Task;
-use crate::utils::{open_dir_noxdev, sigpolicy_from_opts};
+use crate::utils::{open_dir_noxdev, sigpolicy_from_opt};
 
 /// The toplevel boot directory
 const BOOT: &str = "boot";
@@ -112,10 +112,6 @@ pub(crate) struct InstallTargetOpts {
     #[serde(default)]
     pub(crate) enforce_container_sigpolicy: bool,
 
-    /// Enable verification via an ostree remote
-    #[clap(long)]
-    pub(crate) target_ostree_remote: Option<String>,
-
     /// By default, the accessiblity of the target image will be verified (just the manifest will be fetched).
     /// Specifying this option suppresses the check; use this when you know the issues it might find
     /// are addressed.
@@ -1216,10 +1212,7 @@ async fn prepare_install(
             "Use of --target-no-signature-verification flag which is enabled by default"
         );
     }
-    let target_sigverify = sigpolicy_from_opts(
-        !target_opts.enforce_container_sigpolicy,
-        target_opts.target_ostree_remote.as_deref(),
-    );
+    let target_sigverify = sigpolicy_from_opt(target_opts.enforce_container_sigpolicy);
     let target_imgname = target_opts
         .target_imgref
         .as_deref()

lib/src/utils.rs

@@ -172,16 +172,10 @@ pub(crate) fn spawn_editor(tmpf: &tempfile::NamedTempFile) -> Result<()> {
 }
 
 /// Convert a combination of values (likely from CLI parsing) into a signature source
-pub(crate) fn sigpolicy_from_opts(
-    disable_verification: bool,
-    ostree_remote: Option<&str>,
-) -> SignatureSource {
-    if disable_verification {
-        SignatureSource::ContainerPolicyAllowInsecure
-    } else if let Some(remote) = ostree_remote {
-        SignatureSource::OstreeRemote(remote.to_owned())
-    } else {
-        SignatureSource::ContainerPolicy
+pub(crate) fn sigpolicy_from_opt(enforce_container_verification: bool) -> SignatureSource {
+    match enforce_container_verification {
+        true => SignatureSource::ContainerPolicy,
+        false => SignatureSource::ContainerPolicyAllowInsecure,
     }
 }
 
@@ -273,20 +267,9 @@ mod tests {
 
     #[test]
     fn test_sigpolicy_from_opts() {
+        assert_eq!(sigpolicy_from_opt(true), SignatureSource::ContainerPolicy);
         assert_eq!(
-            sigpolicy_from_opts(false, None),
-            SignatureSource::ContainerPolicy
-        );
-        assert_eq!(
-            sigpolicy_from_opts(true, None),
-            SignatureSource::ContainerPolicyAllowInsecure
-        );
-        assert_eq!(
-            sigpolicy_from_opts(false, Some("foo")),
-            SignatureSource::OstreeRemote("foo".to_owned())
-        );
-        assert_eq!(
-            sigpolicy_from_opts(true, Some("foo")),
+            sigpolicy_from_opt(false),
             SignatureSource::ContainerPolicyAllowInsecure
         );
     }

ostree-ext/src/cli.rs

@@ -896,6 +896,9 @@ async fn container_store(
     {
         eprintln!("{msg}")
     }
+    if let Some(ref text) = import.verify_text {
+        println!("{text}");
+    }
     println!("Wrote: {} => {}", imgref, import.merge_commit);
     Ok(())
 }

ostree-ext/src/container/store.rs

@@ -121,12 +121,16 @@ pub struct LayeredImageState {
     pub merge_commit: String,
     /// The digest of the original manifest
     pub manifest_digest: Digest,
-    /// The image manfiest
+    /// The image manifest
     pub manifest: ImageManifest,
     /// The image configuration
     pub configuration: ImageConfiguration,
     /// Metadata for (cached, previously fetched) updates to the image, if any.
     pub cached_update: Option<CachedImageUpdate>,
+    /// The signature verification text from libostree for the base commit;
+    /// in the future we should probably instead just proxy a signature object
+    /// instead, but this is sufficient for now.
+    pub verify_text: Option<String>,
 }
 
 impl LayeredImageState {
@@ -230,6 +234,8 @@ pub struct PreparedImport {
     pub ostree_commit_layer: Option<ManifestLayerState>,
     /// Any further non-ostree (derived) layers.
     pub layers: Vec<ManifestLayerState>,
+    /// OSTree remote signature verification text, if enabled.
+    pub verify_text: Option<String>,
 }
 
 impl PreparedImport {
@@ -635,6 +641,7 @@ impl ImageImporter {
             ostree_layers: component_layers,
             ostree_commit_layer: commit_layer,
             layers: remaining_layers,
+            verify_text: None,
         };
         Ok(Box::new(imp))
     }
@@ -704,7 +711,7 @@ impl ImageImporter {
     /// Extract the base ostree commit.
     #[context("Unencapsulating base")]
     pub(crate) async fn unencapsulate_base(
-        &mut self,
+        &self,
         import: &mut store::PreparedImport,
         require_ostree: bool,
         write_refs: bool,
@@ -804,17 +811,19 @@ impl ImageImporter {
                     let blob = super::unencapsulate::decompressor(&media_type, blob)?;
                     let mut archive = tar::Archive::new(blob);
                     importer.import_commit(&mut archive, Some(cancellable))?;
-                    let commit = importer.finish_import_commit();
+                    let (commit, verify_text) = importer.finish_import_commit();
                     if write_refs {
                         repo.transaction_set_ref(None, &target_ref, Some(commit.as_str()));
                         tracing::debug!("Wrote {} => {}", target_ref, commit);
                     }
                     repo.mark_commit_partial(&commit, false)?;
                     txn.commit(Some(cancellable))?;
-                    Ok::<_, anyhow::Error>(commit)
+                    Ok::<_, anyhow::Error>((commit, verify_text))
                 });
-            let commit = super::unencapsulate::join_fetch(import_task, driver).await?;
+            let (commit, verify_text) =
+                super::unencapsulate::join_fetch(import_task, driver).await?;
             commit_layer.commit = Some(commit);
+            import.verify_text = verify_text;
             if let Some(p) = self.layer_progress.as_ref() {
                 p.send(ImportProgress::OstreeChunkCompleted(
                     commit_layer.layer.clone(),
@@ -977,7 +986,7 @@ impl ImageImporter {
             .unwrap_or_else(|| chrono::offset::Utc::now().timestamp() as u64);
         // Destructure to transfer ownership to thread
         let repo = self.repo;
-        let state = crate::tokio_util::spawn_blocking_cancellable_flatten(
+        let mut state = crate::tokio_util::spawn_blocking_cancellable_flatten(
             move |cancellable| -> Result<Box<LayeredImageState>> {
                 use rustix::fd::AsRawFd;
 
@@ -1090,6 +1099,8 @@ impl ImageImporter {
             },
         )
         .await?;
+        // We can at least avoid re-verifying the base commit.
+        state.verify_text = import.verify_text;
         Ok(state)
     }
 }
@@ -1220,6 +1231,8 @@ pub fn query_image_commit(repo: &ostree::Repo, commit: &str) -> Result<Box<Layer
         manifest,
         configuration,
         cached_update,
+        // we can't cross-reference with a remote here
+        verify_text: None,
     });
     tracing::debug!("Wrote merge commit {}", state.merge_commit);
     Ok(state)

ostree-ext/src/tar/import.rs

@@ -47,6 +47,7 @@ enum ImporterMode {
 pub(crate) struct Importer {
     repo: ostree::Repo,
     remote: Option<String>,
+    verify_text: Option<String>,
     // Cache of xattrs, keyed by their content checksum.
     xattrs: HashMap<String, glib::Variant>,
     // Reusable buffer for xattrs references. It maps a file checksum (.0)
@@ -165,6 +166,7 @@ impl Importer {
         Self {
             repo: repo.clone(),
             remote,
+            verify_text: None,
             buf: vec![0u8; 16384],
             xattrs: Default::default(),
             next_xattrs: None,
@@ -179,6 +181,7 @@ impl Importer {
         Self {
             repo: repo.clone(),
             remote: None,
+            verify_text: None,
             buf: vec![0u8; 16384],
             xattrs: Default::default(),
             next_xattrs: None,
@@ -676,14 +679,17 @@ impl Importer {
 
             // Now that we have both the commit and detached metadata in memory, verify that
             // the signatures in the detached metadata correctly sign the commit.
-            self.repo
-                .signature_verify_commit_data(
-                    remote,
-                    &commit.data_as_bytes(),
-                    &commitmeta.data_as_bytes(),
-                    ostree::RepoVerifyFlags::empty(),
-                )
-                .context("Verifying ostree commit in tar stream")?;
+            self.verify_text = Some(
+                self.repo
+                    .signature_verify_commit_data(
+                        remote,
+                        &commit.data_as_bytes(),
+                        &commitmeta.data_as_bytes(),
+                        ostree::RepoVerifyFlags::empty(),
+                    )
+                    .context("Verifying ostree commit in tar stream")?
+                    .into(),
+            );
 
             self.repo.mark_commit_partial(&checksum, true)?;
 
@@ -738,10 +744,10 @@ impl Importer {
         Ok(())
     }
 
-    pub(crate) fn finish_import_commit(self) -> String {
+    pub(crate) fn finish_import_commit(self) -> (String, Option<String>) {
         tracing::debug!("Import stats: {:?}", self.stats);
         match self.data {
-            ImporterMode::Commit(c) => c.unwrap(),
+            ImporterMode::Commit(c) => (c.unwrap(), self.verify_text),
             ImporterMode::ObjectSet(_) => unreachable!(),
         }
     }
@@ -821,7 +827,7 @@ pub async fn import_tar(
         let txn = repo.auto_transaction(Some(cancellable))?;
         let mut importer = Importer::new_for_commit(&repo, options.remote);
         importer.import_commit(&mut archive, Some(cancellable))?;
-        let checksum = importer.finish_import_commit();
+        let (checksum, _) = importer.finish_import_commit();
         txn.commit(Some(cancellable))?;
         repo.mark_commit_partial(&checksum, false)?;
         Ok::<_, anyhow::Error>(checksum)