Skip to content

Commit

Permalink
Fix the behavior of complete_verified_as function
Browse files Browse the repository at this point in the history
`complete_verified_as` finishes writing the blob, verifying its digest
and size against the expected descriptor, and then writing the contents
to a file with a completely different sha256 digest. The reason is that
both `complete_verified_as` and `complete` call self.hash.finish(), but
this function can only be called once, because after the first call it
transitions into the Finalized state [1]. The second time it gets called
it realizes it's in the Finalized state and then it calls self.init,
resetting the hasher. This is a bad API design.

Fix this by only calling self.hash.finish() once and then passing the
result to a new internal function `complete_as`, which does the same
thing as `complete` but avoids calling self.hash.finish() again.

Add a test to ensure `complete_verified_as` behaves as expected.

[1] https://docs.rs/openssl/0.10.66/src/openssl/hash.rs.html#295-297

Signed-off-by: Ariel Miculas-Trif <amiculas@cisco.com>
  • Loading branch information
ariel-miculas committed Sep 18, 2024
1 parent aa0e9c1 commit b319dfd
Showing 1 changed file with 30 additions and 6 deletions.
36 changes: 30 additions & 6 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -614,20 +614,25 @@ impl<'a> BlobWriter<'a> {
found: self.size,
});
}
self.complete()
self.complete_as(&found_digest)
}

/// Finish writing this blob object.
pub fn complete(mut self) -> Result<Blob> {
let sha256 = hex::encode(self.hash.finish()?);
let destname = &format!("{}/{}", BLOBDIR, sha256);
/// Finish writing this blob object with the supplied name
fn complete_as(mut self, sha256_digest: &str) -> Result<Blob> {
let destname = &format!("{}/{}", BLOBDIR, sha256_digest);
let target = self.target.take().unwrap();
target.replace(destname)?;
Ok(Blob {
sha256: Sha256Digest::from_str(&sha256).unwrap(),
sha256: Sha256Digest::from_str(sha256_digest).unwrap(),
size: self.size,
})
}

/// Finish writing this blob object.
pub fn complete(mut self) -> Result<Blob> {
let sha256 = hex::encode(self.hash.finish()?);
self.complete_as(&sha256)
}
}

impl<'a> std::io::Write for BlobWriter<'a> {
Expand Down Expand Up @@ -814,4 +819,23 @@ mod tests {
assert_eq!(w.fsck().unwrap(), 6);
Ok(())
}

#[test]
fn test_complete_verified_as() -> Result<()> {
let td = cap_tempfile::tempdir(cap_std::ambient_authority())?;
let oci_dir = OciDir::ensure(&td)?;
let empty_json_digest = oci_image::DescriptorBuilder::default()
.media_type(MediaType::EmptyJSON)
.size(2_u32)
.digest(Sha256Digest::from_str(
"44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a",
)?)
.build()?;

let mut empty_json_blob = oci_dir.create_blob()?;
empty_json_blob.write("{}".as_bytes())?;
let blob = empty_json_blob.complete_verified_as(&empty_json_digest)?;
assert_eq!(blob.sha256().digest(), empty_json_digest.digest().digest());
Ok(())
}
}

0 comments on commit b319dfd

Please sign in to comment.