[PATCH proxmox 0/3] oci: refactor code

[PATCH proxmox 0/3] oci: refactor code

[Filip Schauer]

Refactor the proxmox-oci code in preparation for OverlayFS support.

Filip Schauer (3):
  oci: add a digest helper module
  oci: reduce redundant layer decompression during extraction
  oci: factor out parsing and extraction into modules

 proxmox-oci/src/digest.rs  |  94 +++++++++
 proxmox-oci/src/extract.rs | 270 ++++++++++++++++++++++++++
 proxmox-oci/src/lib.rs     | 386 +------------------------------------
 proxmox-oci/src/parse.rs   |  51 +++++
 4 files changed, 422 insertions(+), 379 deletions(-)
 create mode 100644 proxmox-oci/src/digest.rs
 create mode 100644 proxmox-oci/src/extract.rs
 create mode 100644 proxmox-oci/src/parse.rs

-- 
2.47.3

[PATCH proxmox 1/3] oci: add a digest helper module

[Filip Schauer]

Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
---
 proxmox-oci/src/digest.rs | 94 +++++++++++++++++++++++++++++++++++++++
 proxmox-oci/src/lib.rs    |  2 +
 2 files changed, 96 insertions(+)
 create mode 100644 proxmox-oci/src/digest.rs

diff --git a/proxmox-oci/src/digest.rs b/proxmox-oci/src/digest.rs
new file mode 100644
index 00000000..b2170b87
--- /dev/null
+++ b/proxmox-oci/src/digest.rs
@@ -0,0 +1,94 @@
+use std::io::{self, Read};
+use std::str::FromStr;
+
+use oci_spec::image::{Digest, DigestAlgorithm};
+use sha2::Digest as _;
+use sha2::digest::DynDigest;
+use thiserror::Error;
+
+#[derive(Debug, Error)]
+pub enum DigestError {
+    #[error("Unsupported digest algorithm: {0}")]
+    UnsupportedAlgorithm(String),
+    #[error("Digest mismatch. Expected {expected}, got {actual}")]
+    Mismatch { expected: Digest, actual: Digest },
+}
+
+struct Digester(Box<dyn DynDigest>);
+
+impl Digester {
+    fn new(digest_algorithm: &DigestAlgorithm) -> Result<Self, DigestError> {
+        let hasher: Box<dyn DynDigest> = match digest_algorithm {
+            DigestAlgorithm::Sha256 => Box::new(sha2::Sha256::new()),
+            DigestAlgorithm::Sha384 => Box::new(sha2::Sha384::new()),
+            DigestAlgorithm::Sha512 => Box::new(sha2::Sha512::new()),
+            other => return Err(DigestError::UnsupportedAlgorithm(other.to_string())),
+        };
+        Ok(Self(hasher))
+    }
+
+    fn update(&mut self, data: &[u8]) {
+        self.0.update(data);
+    }
+
+    fn finalize(&mut self) -> Digest {
+        let sha_bit_count = self.0.output_size() * 8;
+        let digest_str = self
+            .0
+            .finalize_reset()
+            .iter()
+            .map(|b| format!("{b:02x}"))
+            .collect::<String>();
+        Digest::from_str(&format!("sha{sha_bit_count}:{digest_str}")).expect("valid digest")
+    }
+}
+
+pub struct VerifyingReader<R: Read> {
+    inner: R,
+    digester: Digester,
+    expected_digest: Digest,
+    eof: bool,
+}
+
+impl<R: Read> VerifyingReader<R> {
+    pub fn new(inner: R, expected_digest: Digest) -> Result<Self, DigestError> {
+        Ok(Self {
+            inner,
+            digester: Digester::new(expected_digest.algorithm())?,
+            expected_digest,
+            eof: false,
+        })
+    }
+
+    fn verify(&mut self) -> io::Result<()> {
+        let actual = self.digester.finalize();
+
+        if actual != self.expected_digest {
+            return Err(io::Error::other(DigestError::Mismatch {
+                expected: self.expected_digest.clone(),
+                actual,
+            }));
+        }
+
+        Ok(())
+    }
+}
+
+impl<R: Read> Read for VerifyingReader<R> {
+    fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
+        if buf.is_empty() {
+            return Ok(0);
+        }
+
+        let n = self.inner.read(buf)?;
+        if n == 0 {
+            if !self.eof {
+                self.eof = true;
+                self.verify()?;
+            }
+            return Ok(0);
+        }
+        self.digester.update(&buf[..n]);
+        Ok(n)
+    }
+}
diff --git a/proxmox-oci/src/lib.rs b/proxmox-oci/src/lib.rs
index 78045f49..71970cd8 100644
--- a/proxmox-oci/src/lib.rs
+++ b/proxmox-oci/src/lib.rs
@@ -15,6 +15,8 @@ use sha2::{Digest, Sha256};
 use tar::{Archive, EntryType};
 use thiserror::Error;
 
+mod digest;
+
 mod oci_tar_image;
 use oci_tar_image::{OciTarImage, OciTarImageBlob};
 
-- 
2.47.3

[PATCH proxmox 2/3] oci: reduce redundant layer decompression during extraction

[Filip Schauer]

According to the OCI Image Specification, the order of layers in the
image manifest matches the order of the rootfs.diff_ids in the image
configuration.

Instead of redundantly decompressing and hashing every layer upfront to
build a lookup map, pair layer blobs directly with their corresponding
diff_ids and verify the digests on the fly.

Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
---
 proxmox-oci/src/lib.rs | 112 ++++++++---------------------------------
 1 file changed, 21 insertions(+), 91 deletions(-)

diff --git a/proxmox-oci/src/lib.rs b/proxmox-oci/src/lib.rs
index 71970cd8..7182b008 100644
--- a/proxmox-oci/src/lib.rs
+++ b/proxmox-oci/src/lib.rs
@@ -1,4 +1,3 @@
-use std::collections::HashMap;
 use std::ffi::OsStr;
 use std::fs::{File, read_dir, remove_dir_all, remove_file};
 use std::io::{self, ErrorKind, Read, Seek, SeekFrom};
@@ -10,12 +9,11 @@ use flate2::read::GzDecoder;
 use oci_spec::OciSpecError;
 pub use oci_spec::image::{Arch, Config};
 use oci_spec::image::{ImageConfiguration, ImageManifest, MediaType};
-use sha2::digest::generic_array::GenericArray;
-use sha2::{Digest, Sha256};
 use tar::{Archive, EntryType};
 use thiserror::Error;
 
 mod digest;
+use digest::{DigestError, VerifyingReader};
 
 mod oci_tar_image;
 use oci_tar_image::{OciTarImage, OciTarImageBlob};
@@ -23,80 +21,6 @@ use oci_tar_image::{OciTarImage, OciTarImageBlob};
 const WHITEOUT_PREFIX: &[u8] = b".wh.";
 const OPAQUE_WHITEOUT_NAME: &str = ".wh..wh..opq";
 
-fn compute_digest<R: Read, H: Digest>(
-    mut reader: R,
-    mut hasher: H,
-) -> io::Result<GenericArray<u8, H::OutputSize>> {
-    let mut buf = proxmox_io::boxed::zeroed(32768);
-
-    loop {
-        let bytes_read = reader.read(&mut buf)?;
-        if bytes_read == 0 {
-            break Ok(hasher.finalize());
-        }
-
-        hasher.update(&buf[..bytes_read]);
-    }
-}
-
-fn compute_sha256<R: Read>(reader: R) -> io::Result<oci_spec::image::Sha256Digest> {
-    let digest = compute_digest(reader, Sha256::new())?;
-    Ok(oci_spec::image::Sha256Digest::from_str(&format!("{digest:x}")).expect("valid digest"))
-}
-
-/// Build a mapping from uncompressed layer digests (as found in the image config's `rootfs.diff_ids`)
-/// to their corresponding compressed-layer digests (i.e. the filenames under `blobs/<algorithm>/<digest>`)
-fn build_layer_map<R: Read + Seek>(
-    mut oci_tar_image: OciTarImage<R>,
-    image_manifest: &ImageManifest,
-) -> Result<
-    (
-        OciTarImage<R>,
-        HashMap<oci_spec::image::Digest, oci_spec::image::Descriptor>,
-    ),
-    ExtractError,
-> {
-    let mut layer_mapping = HashMap::new();
-
-    for layer in image_manifest.layers() {
-        let digest = match layer.media_type() {
-            MediaType::ImageLayer | MediaType::ImageLayerNonDistributable => layer.digest().clone(),
-            MediaType::ImageLayerGzip | MediaType::ImageLayerNonDistributableGzip => {
-                let mut compressed_blob = oci_tar_image
-                    .open_blob(layer.digest())
-                    .ok_or(ExtractError::MissingLayerFile(layer.digest().clone()))?;
-                let decoder = GzDecoder::new(&mut compressed_blob);
-                let hash = compute_sha256(decoder)?.into();
-                oci_tar_image = compressed_blob.into_oci_tar_image();
-                hash
-            }
-            MediaType::ImageLayerZstd | MediaType::ImageLayerNonDistributableZstd => {
-                let mut compressed_blob = oci_tar_image
-                    .open_blob(layer.digest())
-                    .ok_or(ExtractError::MissingLayerFile(layer.digest().clone()))?;
-                let decoder = zstd::Decoder::new(&mut compressed_blob)?;
-                let hash = compute_sha256(decoder)?.into();
-                oci_tar_image = compressed_blob.into_oci_tar_image();
-                hash
-            }
-            // Skip any other non-ImageLayer related media types.
-            // Match explicitly to avoid missing new image layer types when oci-spec updates.
-            MediaType::Descriptor
-            | MediaType::LayoutHeader
-            | MediaType::ImageManifest
-            | MediaType::ImageIndex
-            | MediaType::ImageConfig
-            | MediaType::ArtifactManifest
-            | MediaType::EmptyJSON
-            | MediaType::Other(_) => continue,
-        };
-
-        layer_mapping.insert(digest, layer.clone());
-    }
-
-    Ok((oci_tar_image, layer_mapping))
-}
-
 #[derive(Debug, Error)]
 pub enum ProxmoxOciError {
     #[error("Error while parsing OCI image: {0}")]
@@ -177,8 +101,8 @@ fn parse_image<P: AsRef<Path>>(
 pub enum ExtractError {
     #[error("Incorrectly formatted digest: \"{0}\"")]
     InvalidDigest(String),
-    #[error("Unknown layer digest {0} found in rootfs.diff_ids")]
-    UnknownLayerDigest(oci_spec::image::Digest),
+    #[error(transparent)]
+    DigestError(#[from] DigestError),
     #[error("Layer file {0} mentioned in image manifest is missing")]
     MissingLayerFile(oci_spec::image::Digest),
     #[error("IO error: {0}")]
@@ -188,23 +112,23 @@ pub enum ExtractError {
 }
 
 fn extract_image_rootfs<R: Read + Seek, P: AsRef<Path>>(
-    oci_tar_image: OciTarImage<R>,
+    mut oci_tar_image: OciTarImage<R>,
     image_manifest: &ImageManifest,
     image_config: &ImageConfiguration,
     target_path: P,
 ) -> Result<(), ExtractError> {
-    let (mut oci_tar_image, layer_map) = build_layer_map(oci_tar_image, image_manifest)?;
     let target_path = target_path.as_ref().canonicalize()?;
 
-    for layer in image_config.rootfs().diff_ids() {
-        let layer_digest = oci_spec::image::Digest::from_str(layer)
-            .map_err(|_| ExtractError::InvalidDigest(layer.to_string()))?;
-        let layer_descriptor = layer_map
-            .get(&layer_digest)
-            .ok_or(ExtractError::UnknownLayerDigest(layer_digest.clone()))?;
-        let mut layer_file = oci_tar_image
-            .open_blob(layer_descriptor.digest())
-            .ok_or(ExtractError::MissingLayerFile(layer_digest))?;
+    for (layer_descriptor, diffid_str) in image_manifest
+        .layers()
+        .iter()
+        .zip(image_config.rootfs().diff_ids())
+    {
+        let layer_diffid = oci_spec::image::Digest::from_str(diffid_str)
+            .map_err(|_| ExtractError::InvalidDigest(diffid_str.to_string()))?;
+        let mut layer_file = oci_tar_image.open_blob(layer_descriptor.digest()).ok_or(
+            ExtractError::MissingLayerFile(layer_descriptor.digest().clone()),
+        )?;
 
         type DecodeFn<T> = Box<dyn for<'a> Fn(&'a mut T) -> io::Result<Box<dyn Read + 'a>>>;
         let decode_fn: DecodeFn<OciTarImageBlob<R>> = match layer_descriptor.media_type() {
@@ -231,7 +155,13 @@ fn extract_image_rootfs<R: Read + Seek, P: AsRef<Path>>(
             }
         };
 
-        remove_replaced_paths(&mut decode_fn(&mut layer_file)?, &target_path)?;
+        let mut verified = VerifyingReader::new(decode_fn(&mut layer_file)?, layer_diffid)?;
+        remove_replaced_paths(&mut verified, &target_path)?;
+
+        // Force an EOF so VerifyingReader actually verifies.
+        io::copy(&mut verified, &mut io::sink())?;
+        drop(verified);
+
         layer_file.seek(SeekFrom::Start(0))?;
         extract_archive(&mut decode_fn(&mut layer_file)?, &target_path)?;
 
-- 
2.47.3

[PATCH proxmox 3/3] oci: factor out parsing and extraction into modules

[Filip Schauer]

Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
---
 proxmox-oci/src/extract.rs | 270 +++++++++++++++++++++++++++++++
 proxmox-oci/src/lib.rs     | 318 +------------------------------------
 proxmox-oci/src/parse.rs   |  51 ++++++
 3 files changed, 328 insertions(+), 311 deletions(-)
 create mode 100644 proxmox-oci/src/extract.rs
 create mode 100644 proxmox-oci/src/parse.rs

diff --git a/proxmox-oci/src/extract.rs b/proxmox-oci/src/extract.rs
new file mode 100644
index 00000000..d4aa9e4b
--- /dev/null
+++ b/proxmox-oci/src/extract.rs
@@ -0,0 +1,270 @@
+use std::ffi::OsStr;
+use std::fs::{read_dir, remove_dir_all, remove_file};
+use std::io::{self, ErrorKind, Read, Seek, SeekFrom};
+use std::os::unix::ffi::OsStrExt;
+use std::path::{Path, PathBuf};
+use std::str::FromStr;
+
+use flate2::read::GzDecoder;
+use oci_spec::image::{ImageConfiguration, ImageManifest, MediaType};
+use tar::{Archive, EntryType};
+use thiserror::Error;
+
+use crate::digest::{DigestError, VerifyingReader};
+use crate::oci_tar_image::{OciTarImage, OciTarImageBlob};
+
+const WHITEOUT_PREFIX: &[u8] = b".wh.";
+const OPAQUE_WHITEOUT_NAME: &str = ".wh..wh..opq";
+
+#[derive(Debug, Error)]
+pub enum ExtractError {
+    #[error("Incorrectly formatted digest: \"{0}\"")]
+    InvalidDigest(String),
+    #[error(transparent)]
+    DigestError(#[from] DigestError),
+    #[error("Layer file {0} mentioned in image manifest is missing")]
+    MissingLayerFile(oci_spec::image::Digest),
+    #[error(transparent)]
+    Io(#[from] io::Error),
+    #[error("Layer has wrong media type: {0}")]
+    WrongMediaType(String),
+}
+
+pub(crate) fn extract_image_rootfs<R: Read + Seek, P: AsRef<Path>>(
+    mut oci_tar_image: OciTarImage<R>,
+    image_manifest: &ImageManifest,
+    image_config: &ImageConfiguration,
+    target_path: P,
+) -> Result<(), ExtractError> {
+    let target_path = target_path.as_ref().canonicalize()?;
+
+    for (layer_descriptor, diffid_str) in image_manifest
+        .layers()
+        .iter()
+        .zip(image_config.rootfs().diff_ids())
+    {
+        let layer_diffid = oci_spec::image::Digest::from_str(diffid_str)
+            .map_err(|_| ExtractError::InvalidDigest(diffid_str.to_string()))?;
+        let mut layer_file = oci_tar_image.open_blob(layer_descriptor.digest()).ok_or(
+            ExtractError::MissingLayerFile(layer_descriptor.digest().clone()),
+        )?;
+
+        type DecodeFn<T> = Box<dyn for<'a> Fn(&'a mut T) -> io::Result<Box<dyn Read + 'a>>>;
+        let decode_fn: DecodeFn<OciTarImageBlob<R>> = match layer_descriptor.media_type() {
+            MediaType::ImageLayer | MediaType::ImageLayerNonDistributable => {
+                Box::new(|file| Ok(Box::new(file)))
+            }
+            MediaType::ImageLayerGzip | MediaType::ImageLayerNonDistributableGzip => {
+                Box::new(|file| Ok(Box::new(GzDecoder::new(file))))
+            }
+            MediaType::ImageLayerZstd | MediaType::ImageLayerNonDistributableZstd => {
+                Box::new(|file| Ok(Box::new(zstd::Decoder::new(file)?)))
+            }
+            // Error on any other non-ImageLayer related media types.
+            // Match explicitly to avoid missing new image layer types when oci-spec updates.
+            media_type @ (MediaType::Descriptor
+            | MediaType::LayoutHeader
+            | MediaType::ImageManifest
+            | MediaType::ImageIndex
+            | MediaType::ImageConfig
+            | MediaType::ArtifactManifest
+            | MediaType::EmptyJSON
+            | MediaType::Other(_)) => {
+                return Err(ExtractError::WrongMediaType(media_type.to_string()));
+            }
+        };
+
+        let mut verified = VerifyingReader::new(decode_fn(&mut layer_file)?, layer_diffid)?;
+        remove_replaced_paths(&mut verified, &target_path)?;
+
+        // Force an EOF so VerifyingReader actually verifies.
+        io::copy(&mut verified, &mut io::sink())?;
+        drop(verified);
+
+        layer_file.seek(SeekFrom::Start(0))?;
+        extract_archive(&mut decode_fn(&mut layer_file)?, &target_path)?;
+
+        oci_tar_image = layer_file.into_oci_tar_image();
+    }
+
+    Ok(())
+}
+
+/// Validate a path previously received from a `tar::Entry`.
+///
+/// Tar files may contain arbitrary paths, so reject absolute paths and paths containing parent
+/// directory references, as joining them onto the target directory would lead outside of it.
+pub(crate) fn validate_path(path: &Path) -> io::Result<()> {
+    use std::path::Component;
+
+    if !path
+        .components()
+        .all(|c| matches!(c, Component::Normal(_) | Component::CurDir))
+    {
+        return Err(io::Error::other(format!(
+            "illegal path {path:?} in layer - absolute paths and '..' components are not allowed"
+        )));
+    }
+
+    Ok(())
+}
+
+/// Join the `path` of a layer entry onto the resolved `target_path`, resolving its parent only.
+///
+/// Fails if the parent leads outside of the target through a symlink, like the tar crate checks
+/// before unpacking, assuming that nothing else modifies the target meanwhile. Returns `None` if
+/// the parent does not exist or `path` has no last component.
+fn resolve_in_target(target_path: &Path, path: &Path) -> io::Result<Option<PathBuf>> {
+    let (Some(parent), Some(name)) = (path.parent(), path.file_name()) else {
+        return Ok(None);
+    };
+    Ok(resolve_dir_in_target(target_path, parent)?.map(|dir| dir.join(name)))
+}
+
+/// Resolve the directory `dir` of a layer entry within the resolved `target_path`, see
+/// [`resolve_in_target`].
+fn resolve_dir_in_target(target_path: &Path, dir: &Path) -> io::Result<Option<PathBuf>> {
+    let resolved = match target_path.join(dir).canonicalize() {
+        Ok(resolved) => resolved,
+        Err(err) if matches!(err.kind(), ErrorKind::NotFound | ErrorKind::NotADirectory) => {
+            return Ok(None);
+        }
+        Err(err) => return Err(err),
+    };
+
+    if !resolved.starts_with(target_path) {
+        return Err(io::Error::other(format!(
+            "illegal path {dir:?} in layer - a symlink leads outside of the target directory"
+        )));
+    }
+
+    Ok(Some(resolved))
+}
+
+/// Remove what a layer replaces with directories or whites out from previous layers.
+fn remove_replaced_paths<R: Read>(reader: &mut R, target_path: &Path) -> io::Result<()> {
+    let mut archive = Archive::new(reader);
+
+    let mut directories = Vec::new();
+    let mut whiteouts = Vec::new();
+    for entry in archive.entries()? {
+        let entry = entry?;
+        let path = entry.path()?;
+        validate_path(&path)?;
+
+        match entry.header().entry_type() {
+            EntryType::Directory => directories.push(path.into_owned()),
+            EntryType::Regular if is_whiteout(&path) => whiteouts.push(path.into_owned()),
+            _ => {}
+        }
+    }
+
+    // Parents first and before whiteouts, so nothing resolves through a symlink being replaced.
+    directories.sort();
+    for dir in directories {
+        if let Some(dir_path_abs) = resolve_in_target(target_path, &dir)?
+            && dir_path_abs.symlink_metadata().is_ok_and(|m| !m.is_dir())
+        {
+            remove_file(dir_path_abs)?;
+        }
+    }
+
+    for whiteout in whiteouts {
+        apply_whiteout(target_path, &whiteout)?;
+    }
+
+    Ok(())
+}
+
+fn is_whiteout(path: &Path) -> bool {
+    path.file_name()
+        .is_some_and(|name| name.as_bytes().starts_with(WHITEOUT_PREFIX))
+}
+
+/// Apply the whiteout file at `path` on previous layers.
+fn apply_whiteout(target_path: &Path, path: &Path) -> io::Result<()> {
+    let Some(filename) = path.file_name() else {
+        return Ok(());
+    };
+
+    if filename == OPAQUE_WHITEOUT_NAME {
+        if let Some(parent) = path.parent()
+            && let Some(whiteout_abs_path) = resolve_dir_in_target(target_path, parent)?
+        {
+            for direntry in read_dir(whiteout_abs_path)? {
+                remove_path(&direntry?.path())?;
+            }
+        }
+    } else if let Some(filename_stripped) = filename.as_bytes().strip_prefix(WHITEOUT_PREFIX) {
+        let whiteout_path = match filename_stripped {
+            b"." => match path.parent() {
+                Some(p) if p.parent().is_some() => p,
+                _ => return Ok(()), // Prevent whiteout of root directory
+            },
+            b".." => return Ok(()), // Prevent whiteout of grandparent directory
+            fname => &path.with_file_name(OsStr::from_bytes(fname)),
+        };
+        if let Some(whiteout_abs_path) = resolve_in_target(target_path, whiteout_path)? {
+            remove_path(&whiteout_abs_path)?;
+        }
+    }
+
+    Ok(())
+}
+
+fn extract_archive<R: Read>(reader: &mut R, target_path: &Path) -> io::Result<()> {
+    let mut archive = Archive::new(reader);
+    archive.set_preserve_ownerships(true);
+    archive.set_preserve_permissions(true);
+    archive.set_unpack_xattrs(true);
+
+    // Delay directory entries until the end (they will be created if needed by descendants),
+    // to ensure that directory permissions do not interfere with descendant extraction.
+    let mut directories = Vec::new();
+    for entry in archive.entries()? {
+        let mut file = entry?;
+
+        let filepath = file.path()?;
+        validate_path(&filepath)?;
+
+        if file.header().entry_type() == EntryType::Directory {
+            directories.push(file);
+            continue;
+        } else if file.header().entry_type() == EntryType::Regular && is_whiteout(&filepath) {
+            continue;
+        }
+
+        // If a file or directory already exists at this path, remove it first.
+        if let Some(file_path_abs) = resolve_in_target(target_path, &filepath)? {
+            remove_path(&file_path_abs)?;
+        }
+
+        file.unpack_in(target_path)?;
+    }
+
+    // Apply the directories in reverse topological order,
+    // to avoid failure on restrictive parent directory permissions.
+    directories.sort_by(|a, b| b.path_bytes().cmp(&a.path_bytes()));
+    for mut dir in directories {
+        // Skip a directory replaced by a later entry, also to not apply it through a symlink.
+        if let Some(dir_path_abs) = resolve_in_target(target_path, &dir.path()?)?
+            && dir_path_abs.symlink_metadata().is_ok_and(|m| !m.is_dir())
+        {
+            continue;
+        }
+
+        dir.unpack_in(target_path)?;
+    }
+
+    Ok(())
+}
+
+/// Remove whatever exists at `path`, without following a symlink there.
+fn remove_path(path: &Path) -> io::Result<()> {
+    match path.symlink_metadata() {
+        Ok(metadata) if metadata.is_dir() => remove_dir_all(path),
+        Ok(_) => remove_file(path),
+        Err(err) if matches!(err.kind(), ErrorKind::NotFound | ErrorKind::NotADirectory) => Ok(()),
+        Err(err) => Err(err),
+    }
+}
diff --git a/proxmox-oci/src/lib.rs b/proxmox-oci/src/lib.rs
index 7182b008..d5814915 100644
--- a/proxmox-oci/src/lib.rs
+++ b/proxmox-oci/src/lib.rs
@@ -1,25 +1,17 @@
-use std::ffi::OsStr;
-use std::fs::{File, read_dir, remove_dir_all, remove_file};
-use std::io::{self, ErrorKind, Read, Seek, SeekFrom};
-use std::os::unix::ffi::OsStrExt;
-use std::path::{Path, PathBuf};
-use std::str::FromStr;
+use std::path::Path;
 
-use flate2::read::GzDecoder;
-use oci_spec::OciSpecError;
 pub use oci_spec::image::{Arch, Config};
-use oci_spec::image::{ImageConfiguration, ImageManifest, MediaType};
-use tar::{Archive, EntryType};
 use thiserror::Error;
 
 mod digest;
-use digest::{DigestError, VerifyingReader};
 
-mod oci_tar_image;
-use oci_tar_image::{OciTarImage, OciTarImageBlob};
+mod extract;
+use extract::{ExtractError, extract_image_rootfs};
+
+mod parse;
+use parse::{ParseError, parse_image};
 
-const WHITEOUT_PREFIX: &[u8] = b".wh.";
-const OPAQUE_WHITEOUT_NAME: &str = ".wh..wh..opq";
+mod oci_tar_image;
 
 #[derive(Debug, Error)]
 pub enum ProxmoxOciError {
@@ -53,299 +45,3 @@ pub fn parse_and_extract_image<P: AsRef<Path>>(
 
     Ok(image_config.config().clone())
 }
-
-#[derive(Debug, Error)]
-pub enum ParseError {
-    #[error("OCI spec error: {0}")]
-    OciSpec(#[from] OciSpecError),
-    #[error("Wrong media type")]
-    WrongMediaType,
-    #[error("IO error: {0}")]
-    Io(#[from] io::Error),
-    #[error("Unsupported CPU architecture")]
-    UnsupportedArchitecture,
-    #[error("Missing image config")]
-    MissingImageConfig,
-}
-
-fn parse_image<P: AsRef<Path>>(
-    oci_tar_path: P,
-    arch: Option<&Arch>,
-) -> Result<(OciTarImage<File>, ImageManifest, ImageConfiguration), ParseError> {
-    let oci_tar_file = File::open(oci_tar_path)?;
-    let mut oci_tar_image = OciTarImage::new(oci_tar_file)?;
-
-    let image_manifest = oci_tar_image
-        .image_manifest(arch)
-        .ok_or(ParseError::UnsupportedArchitecture)??;
-
-    let image_config_descriptor = image_manifest.config();
-
-    if image_config_descriptor.media_type() != &MediaType::ImageConfig {
-        return Err(ParseError::WrongMediaType);
-    }
-
-    let mut image_config_file = oci_tar_image
-        .open_blob(image_config_descriptor.digest())
-        .ok_or(ParseError::MissingImageConfig)?;
-    let image_config = ImageConfiguration::from_reader(&mut image_config_file)?;
-
-    Ok((
-        image_config_file.into_oci_tar_image(),
-        image_manifest,
-        image_config,
-    ))
-}
-
-#[derive(Debug, Error)]
-pub enum ExtractError {
-    #[error("Incorrectly formatted digest: \"{0}\"")]
-    InvalidDigest(String),
-    #[error(transparent)]
-    DigestError(#[from] DigestError),
-    #[error("Layer file {0} mentioned in image manifest is missing")]
-    MissingLayerFile(oci_spec::image::Digest),
-    #[error("IO error: {0}")]
-    Io(#[from] io::Error),
-    #[error("Layer has wrong media type: {0}")]
-    WrongMediaType(String),
-}
-
-fn extract_image_rootfs<R: Read + Seek, P: AsRef<Path>>(
-    mut oci_tar_image: OciTarImage<R>,
-    image_manifest: &ImageManifest,
-    image_config: &ImageConfiguration,
-    target_path: P,
-) -> Result<(), ExtractError> {
-    let target_path = target_path.as_ref().canonicalize()?;
-
-    for (layer_descriptor, diffid_str) in image_manifest
-        .layers()
-        .iter()
-        .zip(image_config.rootfs().diff_ids())
-    {
-        let layer_diffid = oci_spec::image::Digest::from_str(diffid_str)
-            .map_err(|_| ExtractError::InvalidDigest(diffid_str.to_string()))?;
-        let mut layer_file = oci_tar_image.open_blob(layer_descriptor.digest()).ok_or(
-            ExtractError::MissingLayerFile(layer_descriptor.digest().clone()),
-        )?;
-
-        type DecodeFn<T> = Box<dyn for<'a> Fn(&'a mut T) -> io::Result<Box<dyn Read + 'a>>>;
-        let decode_fn: DecodeFn<OciTarImageBlob<R>> = match layer_descriptor.media_type() {
-            MediaType::ImageLayer | MediaType::ImageLayerNonDistributable => {
-                Box::new(|file| Ok(Box::new(file)))
-            }
-            MediaType::ImageLayerGzip | MediaType::ImageLayerNonDistributableGzip => {
-                Box::new(|file| Ok(Box::new(GzDecoder::new(file))))
-            }
-            MediaType::ImageLayerZstd | MediaType::ImageLayerNonDistributableZstd => {
-                Box::new(|file| Ok(Box::new(zstd::Decoder::new(file)?)))
-            }
-            // Error on any other non-ImageLayer related media types.
-            // Match explicitly to avoid missing new image layer types when oci-spec updates.
-            media_type @ (MediaType::Descriptor
-            | MediaType::LayoutHeader
-            | MediaType::ImageManifest
-            | MediaType::ImageIndex
-            | MediaType::ImageConfig
-            | MediaType::ArtifactManifest
-            | MediaType::EmptyJSON
-            | MediaType::Other(_)) => {
-                return Err(ExtractError::WrongMediaType(media_type.to_string()));
-            }
-        };
-
-        let mut verified = VerifyingReader::new(decode_fn(&mut layer_file)?, layer_diffid)?;
-        remove_replaced_paths(&mut verified, &target_path)?;
-
-        // Force an EOF so VerifyingReader actually verifies.
-        io::copy(&mut verified, &mut io::sink())?;
-        drop(verified);
-
-        layer_file.seek(SeekFrom::Start(0))?;
-        extract_archive(&mut decode_fn(&mut layer_file)?, &target_path)?;
-
-        oci_tar_image = layer_file.into_oci_tar_image();
-    }
-
-    Ok(())
-}
-
-/// Validate a path previously received from a `tar::Entry`.
-///
-/// Tar files may contain arbitrary paths, so reject absolute paths and paths containing parent
-/// directory references, as joining them onto the target directory would lead outside of it.
-pub(crate) fn validate_path(path: &Path) -> io::Result<()> {
-    use std::path::Component;
-
-    if !path
-        .components()
-        .all(|c| matches!(c, Component::Normal(_) | Component::CurDir))
-    {
-        return Err(io::Error::other(format!(
-            "illegal path {path:?} in layer - absolute paths and '..' components are not allowed"
-        )));
-    }
-
-    Ok(())
-}
-
-/// Join the `path` of a layer entry onto the resolved `target_path`, resolving its parent only.
-///
-/// Fails if the parent leads outside of the target through a symlink, like the tar crate checks
-/// before unpacking, assuming that nothing else modifies the target meanwhile. Returns `None` if
-/// the parent does not exist or `path` has no last component.
-fn resolve_in_target(target_path: &Path, path: &Path) -> io::Result<Option<PathBuf>> {
-    let (Some(parent), Some(name)) = (path.parent(), path.file_name()) else {
-        return Ok(None);
-    };
-    Ok(resolve_dir_in_target(target_path, parent)?.map(|dir| dir.join(name)))
-}
-
-/// Resolve the directory `dir` of a layer entry within the resolved `target_path`, see
-/// [`resolve_in_target`].
-fn resolve_dir_in_target(target_path: &Path, dir: &Path) -> io::Result<Option<PathBuf>> {
-    let resolved = match target_path.join(dir).canonicalize() {
-        Ok(resolved) => resolved,
-        Err(err) if matches!(err.kind(), ErrorKind::NotFound | ErrorKind::NotADirectory) => {
-            return Ok(None);
-        }
-        Err(err) => return Err(err),
-    };
-
-    if !resolved.starts_with(target_path) {
-        return Err(io::Error::other(format!(
-            "illegal path {dir:?} in layer - a symlink leads outside of the target directory"
-        )));
-    }
-
-    Ok(Some(resolved))
-}
-
-/// Remove what a layer replaces with directories or whites out from previous layers.
-fn remove_replaced_paths<R: Read>(reader: &mut R, target_path: &Path) -> io::Result<()> {
-    let mut archive = Archive::new(reader);
-
-    let mut directories = Vec::new();
-    let mut whiteouts = Vec::new();
-    for entry in archive.entries()? {
-        let entry = entry?;
-        let path = entry.path()?;
-        validate_path(&path)?;
-
-        match entry.header().entry_type() {
-            EntryType::Directory => directories.push(path.into_owned()),
-            EntryType::Regular if is_whiteout(&path) => whiteouts.push(path.into_owned()),
-            _ => {}
-        }
-    }
-
-    // Parents first and before whiteouts, so nothing resolves through a symlink being replaced.
-    directories.sort();
-    for dir in directories {
-        if let Some(dir_path_abs) = resolve_in_target(target_path, &dir)?
-            && dir_path_abs.symlink_metadata().is_ok_and(|m| !m.is_dir())
-        {
-            remove_file(dir_path_abs)?;
-        }
-    }
-
-    for whiteout in whiteouts {
-        apply_whiteout(target_path, &whiteout)?;
-    }
-
-    Ok(())
-}
-
-fn is_whiteout(path: &Path) -> bool {
-    path.file_name()
-        .is_some_and(|name| name.as_bytes().starts_with(WHITEOUT_PREFIX))
-}
-
-/// Apply the whiteout file at `path` on previous layers.
-fn apply_whiteout(target_path: &Path, path: &Path) -> io::Result<()> {
-    let Some(filename) = path.file_name() else {
-        return Ok(());
-    };
-
-    if filename == OPAQUE_WHITEOUT_NAME {
-        if let Some(parent) = path.parent()
-            && let Some(whiteout_abs_path) = resolve_dir_in_target(target_path, parent)?
-        {
-            for direntry in read_dir(whiteout_abs_path)? {
-                remove_path(&direntry?.path())?;
-            }
-        }
-    } else if let Some(filename_stripped) = filename.as_bytes().strip_prefix(WHITEOUT_PREFIX) {
-        let whiteout_path = match filename_stripped {
-            b"." => match path.parent() {
-                Some(p) if p.parent().is_some() => p,
-                _ => return Ok(()), // Prevent whiteout of root directory
-            },
-            b".." => return Ok(()), // Prevent whiteout of grandparent directory
-            fname => &path.with_file_name(OsStr::from_bytes(fname)),
-        };
-        if let Some(whiteout_abs_path) = resolve_in_target(target_path, whiteout_path)? {
-            remove_path(&whiteout_abs_path)?;
-        }
-    }
-
-    Ok(())
-}
-
-fn extract_archive<R: Read>(reader: &mut R, target_path: &Path) -> io::Result<()> {
-    let mut archive = Archive::new(reader);
-    archive.set_preserve_ownerships(true);
-    archive.set_preserve_permissions(true);
-    archive.set_unpack_xattrs(true);
-
-    // Delay directory entries until the end (they will be created if needed by descendants),
-    // to ensure that directory permissions do not interfere with descendant extraction.
-    let mut directories = Vec::new();
-    for entry in archive.entries()? {
-        let mut file = entry?;
-
-        let filepath = file.path()?;
-        validate_path(&filepath)?;
-
-        if file.header().entry_type() == EntryType::Directory {
-            directories.push(file);
-            continue;
-        } else if file.header().entry_type() == EntryType::Regular && is_whiteout(&filepath) {
-            continue;
-        }
-
-        // If a file or directory already exists at this path, remove it first.
-        if let Some(file_path_abs) = resolve_in_target(target_path, &filepath)? {
-            remove_path(&file_path_abs)?;
-        }
-
-        file.unpack_in(target_path)?;
-    }
-
-    // Apply the directories in reverse topological order,
-    // to avoid failure on restrictive parent directory permissions.
-    directories.sort_by(|a, b| b.path_bytes().cmp(&a.path_bytes()));
-    for mut dir in directories {
-        // Skip a directory replaced by a later entry, also to not apply it through a symlink.
-        if let Some(dir_path_abs) = resolve_in_target(target_path, &dir.path()?)?
-            && dir_path_abs.symlink_metadata().is_ok_and(|m| !m.is_dir())
-        {
-            continue;
-        }
-
-        dir.unpack_in(target_path)?;
-    }
-
-    Ok(())
-}
-
-/// Remove whatever exists at `path`, without following a symlink there.
-fn remove_path(path: &Path) -> io::Result<()> {
-    match path.symlink_metadata() {
-        Ok(metadata) if metadata.is_dir() => remove_dir_all(path),
-        Ok(_) => remove_file(path),
-        Err(err) if matches!(err.kind(), ErrorKind::NotFound | ErrorKind::NotADirectory) => Ok(()),
-        Err(err) => Err(err),
-    }
-}
diff --git a/proxmox-oci/src/parse.rs b/proxmox-oci/src/parse.rs
new file mode 100644
index 00000000..681bd2d8
--- /dev/null
+++ b/proxmox-oci/src/parse.rs
@@ -0,0 +1,51 @@
+use std::fs::File;
+use std::path::Path;
+
+use oci_spec::OciSpecError;
+use oci_spec::image::{Arch, ImageConfiguration, ImageManifest, MediaType};
+use thiserror::Error;
+
+use crate::oci_tar_image::OciTarImage;
+
+#[derive(Debug, Error)]
+pub enum ParseError {
+    #[error(transparent)]
+    OciSpec(#[from] OciSpecError),
+    #[error("Wrong media type")]
+    WrongMediaType,
+    #[error(transparent)]
+    Io(#[from] std::io::Error),
+    #[error("Unsupported CPU architecture")]
+    UnsupportedArchitecture,
+    #[error("Missing image config")]
+    MissingImageConfig,
+}
+
+pub(crate) fn parse_image<P: AsRef<Path>>(
+    oci_tar_path: P,
+    arch: Option<&Arch>,
+) -> Result<(OciTarImage<File>, ImageManifest, ImageConfiguration), ParseError> {
+    let oci_tar_file = File::open(oci_tar_path)?;
+    let mut oci_tar_image = OciTarImage::new(oci_tar_file)?;
+
+    let image_manifest = oci_tar_image
+        .image_manifest(arch)
+        .ok_or(ParseError::UnsupportedArchitecture)??;
+
+    let image_config_descriptor = image_manifest.config();
+
+    if image_config_descriptor.media_type() != &MediaType::ImageConfig {
+        return Err(ParseError::WrongMediaType);
+    }
+
+    let mut image_config_file = oci_tar_image
+        .open_blob(image_config_descriptor.digest())
+        .ok_or(ParseError::MissingImageConfig)?;
+    let image_config = ImageConfiguration::from_reader(&mut image_config_file)?;
+
+    Ok((
+        image_config_file.into_oci_tar_image(),
+        image_manifest,
+        image_config,
+    ))
+}
-- 
2.47.3