Skip to content

Commit

Permalink
fix: downloaded layer integrity checks (#6)
Browse files Browse the repository at this point in the history
* fix: downloaded layer integrity checks

- checking that file has already been downloaded
- check file integrity after download
- change directory where layers are downloaded to no include registry like `docker` subdir

* chore: disabling nightly checks because clippy nightly is buggy

- see rust-lang/rust-clippy#13458
  • Loading branch information
appcypher authored Oct 7, 2024
1 parent d727d7e commit 4231ede
Show file tree
Hide file tree
Showing 8 changed files with 166 additions and 42 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/tests_and_checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ jobs:
matrix:
rust-toolchain:
- stable
- nightly
# - nightly
steps:
- name: Checkout Repository
uses: actions/checkout@v4
Expand Down
79 changes: 79 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions monocore/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ clap.workspace = true
dotenvy.workspace = true
futures.workspace = true
getset.workspace = true
hex = "0.4.3"
home = "0.5.9"
lazy_static = "1.5.0"
oci-spec = "0.7.0"
Expand All @@ -38,6 +39,7 @@ reqwest-middleware = "0.3.3"
reqwest-retry = "0.6.1"
serde.workspace = true
serde_json = "1.0.128"
sha2 = "0.10.8"
structstruck = "0.4.1"
thiserror.workspace = true
tokio.workspace = true
Expand Down
8 changes: 8 additions & 0 deletions monocore/lib/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,14 @@ pub enum MonocoreError {
/// An error that occurred when a join handle returned an error.
#[error("join error: {0}")]
JoinError(#[from] tokio::task::JoinError),

/// An error that occurred when an unsupported image hash algorithm was used.
#[error("unsupported image hash algorithm: {0}")]
UnsupportedImageHashAlgorithm(String),

/// An error that occurred when an image layer download failed.
#[error("image layer download failed: {0}")]
ImageLayerDownloadFailed(String),
}

/// An error that can represent any error.
Expand Down
76 changes: 37 additions & 39 deletions monocore/lib/oci/distribution/docker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use tokio::{
};

use crate::{
utils::{self, IMAGE_LAYERS_SUBDIR, MONOCORE_PATH},
utils::{self, MONOCORE_IMAGE_LAYERS_PATH, MONOCORE_PATH},
MonocoreError, MonocoreResult,
};

Expand Down Expand Up @@ -74,7 +74,7 @@ pub struct DockerRegistry {
client: ClientWithMiddleware,

/// The path to the where files are downloaded.
download_path: PathBuf,
path: PathBuf,
}

/// Stores authentication credentials obtained from the Docker registry, including tokens and expiration details.
Expand Down Expand Up @@ -130,22 +130,19 @@ impl DockerRegistry {

Self {
client,
download_path: MONOCORE_PATH.join(REGISTRY_PATH),
path: MONOCORE_PATH.join(REGISTRY_PATH),
}
}

/// Creates a new DockerRegistry instance with a custom path.
pub fn with_download_path(download_path: PathBuf) -> Self {
pub fn with_path(path: PathBuf) -> Self {
let retry_policy = ExponentialBackoff::builder().build_with_max_retries(3);
let client_builder = ClientBuilder::new(Client::new());
let client = client_builder
.with(RetryTransientMiddleware::new_with_policy(retry_policy))
.build();

Self {
client,
download_path,
}
Self { client, path }
}

/// Gets the size of a downloaded file if it exists.
Expand All @@ -158,12 +155,12 @@ impl DockerRegistry {
path.metadata().unwrap().len()
}

// TODO: Can this accept references instead.
/// Downloads a blob from the registry, supports download resumption if the file already partially exists.
async fn download_image_blob(
&self,
repository: String,
digest: Digest,
repository: &str,
digest: &Digest,
download_size: u64,
destination: PathBuf,
) -> MonocoreResult<()> {
// Ensure the destination directory exists
Expand All @@ -174,23 +171,28 @@ impl DockerRegistry {
// Get the size of the already downloaded file if it exists
let downloaded_size = self.get_downloaded_file_size(&destination);

// Create the request with a range header for resumption
let mut stream = self
.fetch_image_blob(&repository, &digest, downloaded_size..)
.await?;

// Open the file for writing, create if it doesn't exist
let mut file = if downloaded_size > 0 {
OpenOptions::new().append(true).open(&destination).await?
} else {
let mut file = if downloaded_size == 0 {
OpenOptions::new()
.create(true)
.truncate(true)
.write(true)
.open(&destination)
.await?
} else if downloaded_size < download_size {
OpenOptions::new().append(true).open(&destination).await?
} else {
tracing::info!(
"file already exists skipping download: {}",
destination.display()
);
return Ok(());
};

let mut stream = self
.fetch_image_blob(repository, digest, downloaded_size..)
.await?;

// Write the stream to the file
while let Some(chunk) = stream.next().await {
let bytes = chunk?;
Expand All @@ -199,18 +201,17 @@ impl DockerRegistry {

// TODO: Check that the downloaded file has the same digest as the one we wanted
// TODO: Use the hash method derived from the digest to verify the download
// let algorithm = digest.algorithm();
// let expected_hash = digest.to_string();
// let actual_hash = utils::get_file_hash(destination, algorithm).await?;

// if actual_hash != expected_hash {
// // Delete the file if the hash does not match
// fs::remove_file(destination).await?;
// return Err(MonocoreError::DownloadFailed(format!(
// "Downloaded file hash {} does not match expected hash {}",
// actual_hash, expected_hash
// )));
// }
let algorithm = digest.algorithm();
let expected_hash = digest.digest();
let actual_hash = hex::encode(utils::get_file_hash(&destination, algorithm).await?);

// Delete the already downloaded file if the hash does not match
if actual_hash != expected_hash {
fs::remove_file(destination).await?;
return Err(MonocoreError::ImageLayerDownloadFailed(format!(
"({repository}:{digest}) file hash {actual_hash} does not match expected hash {expected_hash}",
)));
}

Ok(())
}
Expand Down Expand Up @@ -278,13 +279,8 @@ impl OciRegistryPull for DockerRegistry {
.layers()
.iter()
.map(|layer| {
let layer_path = self
.download_path
.join(IMAGE_LAYERS_SUBDIR)
.join(layer.digest().to_string());

// TODO: Can this accept references instead.
self.download_image_blob(repository.to_string(), layer.digest().clone(), layer_path)
let layer_path = MONOCORE_IMAGE_LAYERS_PATH.join(layer.digest().to_string());
self.download_image_blob(repository, layer.digest(), layer.size(), layer_path)
})
.collect();

Expand Down Expand Up @@ -531,7 +527,9 @@ mod tests {
async fn test_pull_image() -> anyhow::Result<()> {
let registry = DockerRegistry::new();

registry.pull_image("library/alpine", None).await?;
let result = registry.pull_image("library/alpine", None).await;

assert!(result.is_ok());

Ok(())
}
Expand Down
32 changes: 32 additions & 0 deletions monocore/lib/utils/file.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
use std::path::Path;

use oci_spec::image::DigestAlgorithm;
use sha2::{Digest, Sha256, Sha384, Sha512};
use tokio::{fs::File, io::AsyncReadExt};

use crate::{MonocoreError, MonocoreResult};

//--------------------------------------------------------------------------------------------------
// Functions
//--------------------------------------------------------------------------------------------------

/// Gets the hash of a file.
pub async fn get_file_hash(path: &Path, algorithm: &DigestAlgorithm) -> MonocoreResult<Vec<u8>> {
let mut file = File::open(path).await?;
let mut buffer = Vec::new();
file.read_to_end(&mut buffer).await?;

let hash = match algorithm {
DigestAlgorithm::Sha256 => Sha256::digest(&buffer).to_vec(),
DigestAlgorithm::Sha384 => Sha384::digest(&buffer).to_vec(),
DigestAlgorithm::Sha512 => Sha512::digest(&buffer).to_vec(),
_ => {
return Err(MonocoreError::UnsupportedImageHashAlgorithm(format!(
"Unsupported algorithm: {}",
algorithm
)));
}
};

Ok(hash)
}
2 changes: 2 additions & 0 deletions monocore/lib/utils/mod.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
//! Utility functions and types.
mod conversion;
mod file;
mod path;

//--------------------------------------------------------------------------------------------------
// Exports
//--------------------------------------------------------------------------------------------------

pub use conversion::*;
pub use file::*;
pub use path::*;
Loading

0 comments on commit 4231ede

Please sign in to comment.