lib/tar: Pre-filter tar archive in Rust

Rather than trying to extend the ostree C code to support
arbitrary transformations, let's do pre-parsing here in safe Rust.

We keep the `/etc` → `/usr/etc` bits, but we also just completely
drop everything not in `/usr` now.

As noted in the comment, this pre-validation will hopefully also
catch any corrupt tarballs that might be exploitable in the C libarchive
codebase.

lib/src/async_util.rs

@@ -1,6 +1,6 @@
 use std::io::prelude::*;
 use std::pin::Pin;
-use tokio::io::{AsyncRead, AsyncReadExt};
+use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt};
 
 /// A [`std::io::Read`] implementation backed by an asynchronous source.
 pub(crate) struct ReadBridge<T> {
@@ -26,6 +26,35 @@ impl<T: AsyncRead> ReadBridge<T> {
     }
 }
 
+/// A [`std::io::Write`] implementation backed by an asynchronous source.
+pub(crate) struct WriteBridge<T> {
+    w: Pin<Box<T>>,
+    rt: tokio::runtime::Handle,
+}
+
+impl<T: AsyncWrite> Write for WriteBridge<T> {
+    fn write(&mut self, buf: &[u8]) -> std::io::Result<usize> {
+        let w = &mut self.w;
+        self.rt.block_on(async { w.write(buf).await })
+    }
+
+    fn flush(&mut self) -> std::io::Result<()> {
+        let w = &mut self.w;
+        self.rt.block_on(async { w.flush().await })
+    }
+}
+
+impl<T: AsyncWrite> WriteBridge<T> {
+    /// Create a [`std::io::Write`] implementation backed by an asynchronous source.
+    ///
+    /// This is useful with e.g. [`tokio::task::spawn_blocking`].
+    pub(crate) fn new(reader: T) -> Self {
+        let w = Box::pin(reader);
+        let rt = tokio::runtime::Handle::current();
+        WriteBridge { w, rt }
+    }
+}
+
 #[cfg(test)]
 mod test {
     use std::convert::TryInto;

lib/src/tar/write.rs

@@ -7,33 +7,35 @@
 //! In the future, this may also evolve into parsing the tar
 //! stream in Rust, not in C.
 
+use crate::async_util::{ReadBridge, WriteBridge};
 use crate::cmdext::CommandRedirectionExt;
 use crate::Result;
 use anyhow::{anyhow, Context};
+use camino::Utf8Path;
 use ostree::gio;
 use ostree::prelude::FileExt;
+use std::borrow::Cow;
+use std::convert::TryInto;
+use std::io::{BufWriter, Write};
 use std::os::unix::prelude::AsRawFd;
 use std::path::Path;
-use tokio::io::AsyncReadExt;
+use std::process::Stdio;
+use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite};
 use tracing::instrument;
 
 /// Configuration for tar layer commits.
 #[derive(Debug, Default)]
-pub struct WriteTarOptions<'a> {
+pub struct WriteTarOptions {
     /// Base ostree commit hash
-    pub base: Option<&'a str>,
+    pub base: Option<String>,
     /// Enable SELinux labeling from the base commit
     /// Requires the `base` option.
     pub selinux: bool,
 }
 
-struct TempSePolicy {
-    tempdir: tempfile::TempDir,
-}
-
 // Copy of logic from https://github.com/ostreedev/ostree/pull/2447
 // to avoid waiting for backport + releases
-fn sepolicy_from_base(repo: &ostree::Repo, base: &str) -> Result<TempSePolicy> {
+fn sepolicy_from_base(repo: &ostree::Repo, base: &str) -> Result<tempfile::TempDir> {
     let cancellable = gio::NONE_CANCELLABLE;
     let policypath = "usr/etc/selinux";
     let tempdir = tempfile::tempdir()?;
@@ -49,23 +51,94 @@ fn sepolicy_from_base(repo: &ostree::Repo, base: &str) -> Result<TempSePolicy> {
         };
         repo.checkout_at(Some(&opts), ostree::AT_FDCWD, policydest, base, cancellable)?;
     }
-    Ok(TempSePolicy { tempdir: tempdir })
+    Ok(tempdir)
+}
+
+/// Perform various filtering on imported tar archives.
+///  - Move /etc to /usr/etc
+///  - Entirely drop files not in /usr
+///
+/// This also acts as a Rust "pre-parser" of the tar archive, hopefully
+/// catching anything corrupt that might be exploitable from the C libarchive side.
+/// Remember that we're parsing this while we're downloading it, and in order
+/// to verify integrity we rely on the total sha256 of the blob, so all content
+/// written before then must be considered untrusted.
+fn filter_tar(src: impl std::io::Read, dest: impl std::io::Write) -> Result<()> {
+    let src = std::io::BufReader::new(src);
+    let mut src = tar::Archive::new(src);
+    let dest = BufWriter::new(dest);
+    let mut dest = tar::Builder::new(dest);
+
+    let ents = src.entries()?;
+    for entry in ents {
+        let entry = entry?;
+        let path = entry.path()?;
+        let path: &Utf8Path = (&*path).try_into()?;
+
+        if path.is_absolute() {
+            return Err(anyhow!("Invalid absolute path: {}", path));
+        }
+
+        let basename = path.file_name();
+        // Deny these special Unix paths, if we somehow find them.
+        if matches!(basename, Some("." | "..")) {
+            return Err(anyhow!("Invalid path basename: {}", path));
+        }
+        // Canonicalize by removing leading ./
+        let path = path.strip_prefix("./").unwrap_or(path);
+        // Rewrite /etc -> /usr/etc, as ostree expects it by default
+        let path = path
+            .strip_prefix("etc/")
+            .ok()
+            .map(|p| Cow::Owned(Utf8Path::new("usr/etc").join(p)))
+            .unwrap_or_else(|| Cow::Borrowed(path));
+        let path = &*path;
+
+        // Completely discard stuff outside of /usr
+        if !(matches!(path.as_str(), "/" | "/usr") || path.starts_with("usr/")) {
+            continue;
+        }
+        // Clone since this may point into the entry
+        let path = &path.to_owned();
+
+        let mut header = entry.header().clone();
+        dest.append_data(&mut header, path, entry)?;
+    }
+    dest.into_inner()?.flush()?;
+    Ok(())
+}
+
+/// Asynchronous wrapper for filter_tar()
+async fn filter_tar_async(
+    src: impl AsyncRead + Send + 'static,
+    mut dest: impl AsyncWrite + Send + Unpin,
+) -> Result<()> {
+    let (tx_buf, mut rx_buf) = tokio::io::duplex(8192);
+    let tar_transformer = tokio::task::spawn_blocking(move || -> Result<_> {
+        let src = ReadBridge::new(src);
+        let dest = WriteBridge::new(tx_buf);
+        filter_tar(src, dest)
+    });
+    let copier = tokio::io::copy(&mut rx_buf, &mut dest);
+    let (r, v) = tokio::join!(tar_transformer, copier);
+    let _v: u64 = v?;
+    Ok(r??)
 }
 
 /// Write the contents of a tarball as an ostree commit.
 #[allow(unsafe_code)] // For raw fd bits
 #[instrument(skip(repo, src))]
 pub async fn write_tar(
     repo: &ostree::Repo,
-    mut src: impl tokio::io::AsyncRead + Send + Unpin + 'static,
+    src: impl tokio::io::AsyncRead + Send + Unpin + 'static,
     refname: &str,
-    options: Option<WriteTarOptions<'_>>,
+    options: Option<WriteTarOptions>,
 ) -> Result<String> {
-    use std::process::Stdio;
+    let repo = repo.clone();
     let options = options.unwrap_or_default();
     let sepolicy = if options.selinux {
         if let Some(base) = options.base {
-            Some(sepolicy_from_base(repo, base).context("tar: Preparing sepolicy")?)
+            Some(sepolicy_from_base(&repo, &base).context("tar: Preparing sepolicy")?)
         } else {
             None
         }
@@ -84,12 +157,11 @@ pub async fn write_tar(
         c.arg("--repo=/proc/self/fd/3");
         if let Some(sepolicy) = sepolicy.as_ref() {
             c.arg("--selinux-policy");
-            c.arg(sepolicy.tempdir.path());
+            c.arg(sepolicy.path());
         }
         c.args(&[
             "--no-bindings",
             "--tar-autocreate-parents",
-            r#"--tar-pathname-filter=^etc(.*),usr/etc\1"#,
             "--tree=tar=/proc/self/fd/0",
             "--branch",
             refname,
@@ -99,15 +171,11 @@ pub async fn write_tar(
     c.kill_on_drop(true);
     let mut r = c.spawn()?;
     // Safety: We passed piped() for all of these
-    let mut child_stdin = r.stdin.take().unwrap();
+    let child_stdin = r.stdin.take().unwrap();
     let mut child_stdout = r.stdout.take().unwrap();
     let mut child_stderr = r.stderr.take().unwrap();
-    // Copy our input to child stdout
-    let input_copier = async move {
-        let _n = tokio::io::copy(&mut src, &mut child_stdin).await?;
-        drop(child_stdin);
-        Ok::<_, anyhow::Error>(())
-    };
+    // Copy the filtered tar stream to child stdin
+    let input_copier = filter_tar_async(src, child_stdin);
     // Gather stdout/stderr to buffers
     let output_copier = async move {
         let mut child_stdout_buf = String::new();
@@ -134,3 +202,33 @@ pub async fn write_tar(
     let s = child_stdout.trim();
     Ok(s.to_string())
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use std::io::Cursor;
+
+    #[tokio::test]
+    async fn tar_filter() -> Result<()> {
+        let tempd = tempfile::tempdir()?;
+        let rootfs = &tempd.path().join("rootfs");
+        std::fs::create_dir_all(rootfs.join("etc/systemd/system"))?;
+        std::fs::write(rootfs.join("etc/systemd/system/foo.service"), "fooservice")?;
+        std::fs::write(rootfs.join("blah"), "blah")?;
+        let rootfs_tar_path = &tempd.path().join("rootfs.tar");
+        let rootfs_tar = std::fs::File::create(rootfs_tar_path)?;
+        let mut rootfs_tar = tar::Builder::new(rootfs_tar);
+        rootfs_tar.append_dir_all(".", rootfs)?;
+        let _ = rootfs_tar.into_inner()?;
+        let mut dest = Vec::new();
+        let src = tokio::io::BufReader::new(tokio::fs::File::open(rootfs_tar_path).await?);
+        filter_tar_async(src, &mut dest).await?;
+        let dest = dest.as_slice();
+        let mut final_tar = tar::Archive::new(Cursor::new(dest));
+        let destdir = &tempd.path().join("destdir");
+        final_tar.unpack(destdir)?;
+        assert!(destdir.join("usr/etc/systemd/system/foo.service").exists());
+        assert!(!destdir.join("blah").exists());
+        Ok(())
+    }
+}

lib/tests/it/main.rs

@@ -263,18 +263,6 @@ async fn test_tar_import_export() -> Result<()> {
 #[tokio::test]
 async fn test_tar_write() -> Result<()> {
     let fixture = Fixture::new()?;
-    let r = ostree_ext::tar::write_tar(&fixture.destrepo, EXAMPLEOS_V0, "exampleos", None).await?;
-    // Here, we're importing a raw tarball into an ostree commit; this is a subtly different
-    // path than what we do above for the flow of "unpack tarball + ostree commit + export tar".
-    // But, they should be content-identical.
-    let (commitdata, _) = fixture.destrepo.load_commit(&r)?;
-    assert_eq!(
-        EXAMPLEOS_CONTENT_CHECKSUM,
-        ostree::commit_get_content_checksum(&commitdata)
-            .unwrap()
-            .as_str()
-    );
-
     // Test translating /etc to /usr/etc
     let tmpetc = fixture.path.join("tmproot/etc");
     let tmproot = tmpetc.parent().unwrap();