Skip to content

digest and hmac: drop intermediate builder and directly take slices for input #766

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Tpt wants to merge 2 commits into
base: main
Choose a base branch
from
Open

digest and hmac: drop intermediate builder and directly take slices for input #766

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
33e4f3f
digest and hmac: drop intermediate builder and directly take slices f…
Tpt Jun 18, 2026
3fed24c
Apply suggestion from @kevinjqliu
Tpt Jun 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 12 additions & 10 deletions src/aws/client.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -411,11 +411,15 @@ impl Request<'_> {
if let Some(digest) = cached_digest {
return Ok(digest);
}
let mut ctx = self.config.crypto()?.digest(DigestAlgorithm::Sha256)?;
for part in &payload {
ctx.update(part);
}
let digest = ctx.finish()?.try_into().unwrap();
let digest = self
.config
.crypto()?
.digest(
DigestAlgorithm::Sha256,
&payload.iter().map(|c| c.as_ref()).collect::<Vec<_>>(),
)?
.try_into()
.unwrap();
cached_digest = Some(digest);
Ok(digest)
};
Expand Down Expand Up @@ -582,11 +586,9 @@ impl S3Client {
}

let crypto = self.config.crypto()?;
let mut ctx = crypto.digest(DigestAlgorithm::Sha256)?;
ctx.update(body.as_ref());
let digest = ctx.finish()?;
let digest = crypto.digest(DigestAlgorithm::Sha256, &[&body])?;

builder = builder.header(SHA256_CHECKSUM, BASE64_STANDARD.encode(digest));
builder = builder.header(SHA256_CHECKSUM, BASE64_STANDARD.encode(&digest));

// S3 *requires* DeleteObjects to include a Content-MD5 header:
// https://docs.aws.amazon.com/AmazonS3/latest/API/API_DeleteObjects.html
Expand All @@ -599,7 +601,7 @@ impl S3Client {
let response = builder
.header(CONTENT_TYPE, "application/xml")
.body(body)
.with_aws_sigv4(authorizer, Some(digest))?
.with_aws_sigv4(authorizer, Some(digest.as_ref()))?
.retryable(&self.config.retry_config)
.retry_error_body(true)
.send()
Expand Down
21 changes: 10 additions & 11 deletions src/aws/credential.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,22 +105,21 @@ impl AwsCredential {
let date_string = date.format("%Y%m%d").to_string();
let secret_key = format!("AWS4{}", self.secret_key);

let mut ctx = crypto.hmac(DigestAlgorithm::Sha256, secret_key.as_bytes())?;
ctx.update(date_string.as_bytes());
let tag = crypto.hmac(
DigestAlgorithm::Sha256,
secret_key.as_bytes(),
date_string.as_bytes(),
)?;

let mut ctx = crypto.hmac(DigestAlgorithm::Sha256, ctx.finish()?)?;
ctx.update(region.as_bytes());
let tag = crypto.hmac(DigestAlgorithm::Sha256, &tag, region.as_bytes())?;

let mut ctx = crypto.hmac(DigestAlgorithm::Sha256, ctx.finish()?)?;
ctx.update(service.as_bytes());
let tag = crypto.hmac(DigestAlgorithm::Sha256, &tag, service.as_bytes())?;

let mut ctx = crypto.hmac(DigestAlgorithm::Sha256, ctx.finish()?)?;
ctx.update(b"aws4_request");
let tag = crypto.hmac(DigestAlgorithm::Sha256, &tag, b"aws4_request")?;

let mut ctx = crypto.hmac(DigestAlgorithm::Sha256, ctx.finish()?)?;
ctx.update(to_sign.as_bytes());
let tag = crypto.hmac(DigestAlgorithm::Sha256, &tag, to_sign.as_bytes())?;

Ok(hex_encode(ctx.finish()?))
Ok(hex_encode(&tag))
}
}

Expand Down
5 changes: 2 additions & 3 deletions src/azure/client.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -282,9 +282,8 @@ impl AzureEncryptionHeaders {
}

let crypto = crypto_provider(crypto)?;
let mut ctx = crypto.digest(DigestAlgorithm::Sha256)?;
ctx.update(&decoded_key);
let encryption_key_sha256 = BASE64_STANDARD.encode(ctx.finish()?);
let encryption_key_sha256 =
BASE64_STANDARD.encode(crypto.digest(DigestAlgorithm::Sha256, &[&decoded_key])?);

Ok(Self {
encryption_key: Some(encryption_key),
Expand Down
12 changes: 6 additions & 6 deletions src/azure/credential.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,9 +206,11 @@ impl AzureSigner {
),
None => string_to_sign_service_sas(url, method, &self.account, &self.start, &self.end),
};
let mut ctx = crypto.hmac(DigestAlgorithm::Sha256, &self.signing_key.0)?;
ctx.update(str_to_sign.as_bytes());
let auth = ctx.finish()?;
let auth = crypto.hmac(
DigestAlgorithm::Sha256,
&self.signing_key.0,
str_to_sign.as_bytes(),
)?;

url.query_pairs_mut().extend_pairs(query_pairs);
url.query_pairs_mut()
Expand Down Expand Up @@ -358,9 +360,7 @@ fn generate_authorization(
key: &AzureAccessKey,
) -> crate::Result<String> {
let str_to_sign = string_to_sign(h, u, method, account);
let mut ctx = crypto.hmac(DigestAlgorithm::Sha256, &key.0)?;
ctx.update(str_to_sign.as_bytes());
let auth = ctx.finish()?;
let auth = crypto.hmac(DigestAlgorithm::Sha256, &key.0, str_to_sign.as_bytes())?;
Ok(format!(
"SharedKey {}:{}",
account,
Expand Down
146 changes: 26 additions & 120 deletions src/client/crypto.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,15 @@
use crate::Result;

/// Algorithm for computing digests
#[derive(Debug, Ord, PartialOrd, Eq, PartialEq)]
#[derive(Debug, Ord, PartialOrd, Eq, PartialEq, Clone, Copy)]
#[non_exhaustive]
pub enum DigestAlgorithm {
/// SHA-256
Sha256,
}

/// Algorithm for signing payloads
#[derive(Debug, Ord, PartialOrd, Eq, PartialEq)]
#[derive(Debug, Ord, PartialOrd, Eq, PartialEq, Clone, Copy)]
#[non_exhaustive]
pub enum SigningAlgorithm {
/// RSASSA-PKCS1-v1_5 using SHA-256
Expand All @@ -35,42 +35,16 @@ pub enum SigningAlgorithm {

/// Provides cryptographic primitives
pub trait CryptoProvider: std::fmt::Debug + Send + Sync {
/// Compute a digest
fn digest(&self, algorithm: DigestAlgorithm) -> Result<Box<dyn DigestContext>>;
/// Compute the digest of `data`
fn digest(&self, algorithm: DigestAlgorithm, data: &[&[u8]]) -> Result<Vec<u8>>;

/// Compute an HMAC with the provided `secret`
fn hmac(&self, algorithm: DigestAlgorithm, secret: &[u8]) -> Result<Box<dyn HmacContext>>;
/// Compute the HMAC of `data` with the provided `secret`
fn hmac(&self, algorithm: DigestAlgorithm, secret: &[u8], data: &[u8]) -> Result<Vec<u8>>;

/// Sign a payload with the provided PEM-encoded secret
fn sign(&self, algorithm: SigningAlgorithm, pem: &[u8]) -> Result<Box<dyn Signer>>;
}

/// Incrementally compute a digest, see [`CryptoProvider::digest`]
pub trait DigestContext: Send {
/// Updates the digest with all the data in data.
///
/// It is implementation-defined behaviour to call this after calling [`Self::finish`]
fn update(&mut self, data: &[u8]);

/// Finalizes the digest calculation and returns the digest value.
///
/// It is implementation-defined behaviour to call this after calling [`Self::finish`]
fn finish(&mut self) -> Result<&[u8]>;

@alamb alamb Jun 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could possibly have a smaller API change and just allow the traits to return a Vec directly 🤔 that would also avoid the need for a copy in some cases and might be a smaller API change?

fn build(self) -> Result<Vec<u8>> { 
...
}

@Tpt Tpt Jun 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the review!

All digests are currently stored in fixed length arrays so it would not save any allocation in our case, just a possible memcopy at the cost of maybe a useless allocations.

On the breaking changes, I hoped that this changed would be reviewed before the release, seems it's not the case sadly. I just found quite sad to have a complicated API with a lot of dynamic traits where a smaller API could work. Feel free to close this MR.

}

/// Incrementally compute a HMAC, see [`CryptoProvider::hmac`]
pub trait HmacContext: Send {
/// Updates the HMAC with all the data in data.
///
/// It is implementation-defined behaviour to call this after calling [`Self::finish`]
fn update(&mut self, data: &[u8]);

/// Finalizes the HMAC calculation and returns the HMAC value.
///
/// It is implementation-defined behaviour to call this after calling [`Self::finish`]
fn finish(&mut self) -> Result<&[u8]>;
}

/// Sign a payload, see [`CryptoProvider::sign`]
pub trait Signer: Send + Sync {
/// Sign the provided payload
Expand Down Expand Up @@ -150,26 +124,24 @@ pub(crate) mod ring {
}

impl CryptoProvider for RingCryptoProvider {
fn digest(&self, algorithm: DigestAlgorithm) -> Result<Box<dyn DigestContext>> {
fn digest(&self, algorithm: DigestAlgorithm, data: &[&[u8]]) -> Result<Vec<u8>> {
let algorithm = match algorithm {
DigestAlgorithm::Sha256 => &digest::SHA256,
};
let ctx = digest::Context::new(algorithm);
Ok(Box::new(RingDigestContext {
ctx: Some(ctx),
out: None,
}))
let mut ctx = digest::Context::new(algorithm);
for chunk in data {
ctx.update(chunk);
}
Ok(ctx.finish().as_ref().to_vec())
}

fn hmac(&self, algorithm: DigestAlgorithm, secret: &[u8]) -> Result<Box<dyn HmacContext>> {
fn hmac(&self, algorithm: DigestAlgorithm, secret: &[u8], data: &[u8]) -> Result<Vec<u8>> {
let algorithm = match algorithm {
DigestAlgorithm::Sha256 => hmac::HMAC_SHA256,
};
let ctx = hmac::Context::with_key(&hmac::Key::new(algorithm, secret));
Ok(Box::new(RingHmacContext {
ctx: Some(ctx),
out: None,
}))
let mut ctx = hmac::Context::with_key(&hmac::Key::new(algorithm, secret));
ctx.update(data);
Ok(ctx.sign().as_ref().to_vec())
}

fn sign(&self, algorithm: SigningAlgorithm, pem: &[u8]) -> Result<Box<dyn Signer>> {
Expand All @@ -179,38 +151,6 @@ pub(crate) mod ring {
}
}

struct RingDigestContext {
ctx: Option<digest::Context>,
out: Option<digest::Digest>,
}

impl DigestContext for RingDigestContext {
fn update(&mut self, data: &[u8]) {
self.ctx.as_mut().unwrap().update(data);
}

fn finish(&mut self) -> Result<&[u8]> {
let digest = self.ctx.take().unwrap().finish();
Ok(digest::Digest::as_ref(self.out.insert(digest)))
}
}

struct RingHmacContext {
ctx: Option<hmac::Context>,
out: Option<hmac::Tag>,
}

impl HmacContext for RingHmacContext {
fn update(&mut self, data: &[u8]) {
self.ctx.as_mut().unwrap().update(data);
}

fn finish(&mut self) -> Result<&[u8]> {
let tag = self.ctx.take().unwrap().sign();
Ok(hmac::Tag::as_ref(self.out.insert(tag)))
}
}

/// A private RSA key for a service account
#[derive(Debug)]
pub(crate) struct RsaKeyPair(signature::RsaKeyPair);
Expand Down Expand Up @@ -302,26 +242,24 @@ pub(crate) mod aws_lc_rs {
}

impl CryptoProvider for AwsLcCryptoProvider {
fn digest(&self, algorithm: DigestAlgorithm) -> Result<Box<dyn DigestContext>> {
fn digest(&self, algorithm: DigestAlgorithm, data: &[&[u8]]) -> Result<Vec<u8>> {
let algorithm = match algorithm {
DigestAlgorithm::Sha256 => &digest::SHA256,
};
let ctx = digest::Context::new(algorithm);
Ok(Box::new(AwsLcDigestContext {
ctx: Some(ctx),
out: None,
}))
let mut ctx = digest::Context::new(algorithm);
for chunk in data {
ctx.update(chunk);
}
Ok(ctx.finish().as_ref().to_vec())
}

fn hmac(&self, algorithm: DigestAlgorithm, secret: &[u8]) -> Result<Box<dyn HmacContext>> {
fn hmac(&self, algorithm: DigestAlgorithm, secret: &[u8], data: &[u8]) -> Result<Vec<u8>> {
let algorithm = match algorithm {
DigestAlgorithm::Sha256 => hmac::HMAC_SHA256,
};
let ctx = hmac::Context::with_key(&hmac::Key::new(algorithm, secret));
Ok(Box::new(AwsLcHmacContext {
ctx: Some(ctx),
out: None,
}))
let mut ctx = hmac::Context::with_key(&hmac::Key::new(algorithm, secret));
ctx.update(data);
Ok(ctx.sign().as_ref().to_vec())
}

fn sign(&self, algorithm: SigningAlgorithm, pem: &[u8]) -> Result<Box<dyn Signer>> {
Expand All @@ -331,38 +269,6 @@ pub(crate) mod aws_lc_rs {
}
}

struct AwsLcDigestContext {
ctx: Option<digest::Context>,
out: Option<digest::Digest>,
}

impl DigestContext for AwsLcDigestContext {
fn update(&mut self, data: &[u8]) {
self.ctx.as_mut().unwrap().update(data);
}

fn finish(&mut self) -> Result<&[u8]> {
let digest = self.ctx.take().unwrap().finish();
Ok(digest::Digest::as_ref(self.out.insert(digest)))
}
}

struct AwsLcHmacContext {
ctx: Option<hmac::Context>,
out: Option<hmac::Tag>,
}

impl HmacContext for AwsLcHmacContext {
fn update(&mut self, data: &[u8]) {
self.ctx.as_mut().unwrap().update(data);
}

fn finish(&mut self) -> Result<&[u8]> {
let tag = self.ctx.take().unwrap().sign();
Ok(hmac::Tag::as_ref(self.out.insert(tag)))
}
}

/// A private RSA key for a service account
#[derive(Debug)]
pub(crate) struct RsaKeyPair(signature::RsaKeyPair);
Expand Down
21 changes: 5 additions & 16 deletions src/gcp/credential.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -927,9 +927,7 @@ impl CredentialExt for HttpRequestBuilder {
#[cfg(test)]
mod tests {
use super::*;
use crate::client::{
ClientOptions, DigestAlgorithm, DigestContext, HmacContext, StaticCredentialProvider,
};
use crate::client::{ClientOptions, DigestAlgorithm, StaticCredentialProvider};
use crate::gcp::client::{GoogleCloudStorageClient, GoogleCloudStorageConfig};

const SIGNATURE_BYTES: &[u8] = &[0x00, 0x01, 0x02, 0xab, 0xcd];
Expand All @@ -946,15 +944,16 @@ mod tests {
struct FixedCryptoProvider;

impl CryptoProvider for FixedCryptoProvider {
fn digest(&self, _algorithm: DigestAlgorithm) -> crate::Result<Box<dyn DigestContext>> {
Ok(Box::new(FixedDigestContext))
fn digest(&self, _algorithm: DigestAlgorithm, _data: &[&[u8]]) -> crate::Result<Vec<u8>> {
Ok(vec![0x12, 0x34])
}

fn hmac(
&self,
_algorithm: DigestAlgorithm,
_secret: &[u8],
) -> crate::Result<Box<dyn HmacContext>> {
_data: &[u8],
) -> crate::Result<Vec<u8>> {
panic!("GCS signed URL should not use HMAC")
}

Expand All @@ -967,16 +966,6 @@ mod tests {
}
}

struct FixedDigestContext;

impl DigestContext for FixedDigestContext {
fn update(&mut self, _data: &[u8]) {}

fn finish(&mut self) -> crate::Result<&[u8]> {
Ok(&[0x12, 0x34])
}
}

#[derive(Debug)]
struct UnusedHttpService;

Expand Down
7 changes: 4 additions & 3 deletions src/util.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,9 +307,10 @@ pub(crate) fn hex_digest(
crypto: &dyn crate::client::CryptoProvider,
bytes: &[u8],
) -> Result<String> {
let mut ctx = crypto.digest(crate::client::DigestAlgorithm::Sha256)?;
ctx.update(bytes);
Ok(hex_encode(ctx.finish()?))
Ok(hex_encode(&crypto.digest(
crate::client::DigestAlgorithm::Sha256,
&[bytes],
)?))
}

/// Returns `bytes` as a lower-case hex encoded string
Expand Down
Loading