Skip to content

Commit 5533f59

Browse files
authored
Merge pull request #15 from junkurihara/fix/constant-time-content-digest
Fix: Security hardening: constant-time digest comparison + regression tests
2 parents 65cbd19 + 669614d commit 5533f59

File tree

3 files changed

+132
-6
lines changed

3 files changed

+132
-6
lines changed

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ resolver = "2"
44

55
[workspace.package]
66
edition = "2021"
7-
version = "0.0.22"
7+
version = "0.0.23"
88
authors = ["Jun Kurihara"]
99
homepage = "https://github.com/junkurihara/httpsig-rs"
1010
repository = "https://github.com/junkurihara/httpsig-rs"

httpsig-hyper/Cargo.toml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ rsa-signature = ["httpsig/rsa-signature"]
1919

2020

2121
[dependencies]
22-
httpsig = { path = "../httpsig", version = "0.0.22" }
22+
httpsig = { path = "../httpsig", version = "0.0.23" }
2323

2424
thiserror = { version = "2.0.18" }
2525
tracing = { version = "0.1.44" }
@@ -28,6 +28,7 @@ futures = { version = "0.3.31", default-features = false, features = [
2828
"async-await",
2929
] }
3030
indexmap = { version = "2.11.4" }
31+
subtle = { version = "2.6.1", default-features = false }
3132

3233
# content digest with rfc8941 structured field values
3334
sha2 = { version = "0.10.9", default-features = false }

httpsig-hyper/src/hyper_content_digest.rs

Lines changed: 129 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use http_body_util::{combinators::BoxBody, BodyExt, Full};
88
use sha2::Digest;
99
use std::future::Future;
1010
use std::str::FromStr;
11+
use subtle::ConstantTimeEq;
1112

1213
// hyper's http specific extension to generate and verify http signature
1314

@@ -133,15 +134,16 @@ where
133134
Self: Sized,
134135
{
135136
let header_map = self.headers();
136-
let (cd_type, _expected_digest) = extract_content_digest(header_map).await?;
137+
let (cd_type, expected_digest) = extract_content_digest(header_map).await?;
137138
let (header, body) = self.into_parts();
138139
let body_bytes = body
139140
.into_bytes()
140141
.await
141142
.map_err(|_e| HyperDigestError::HttpBodyError("Failed to get body bytes".to_string()))?;
142143
let digest = derive_digest(&body_bytes, &cd_type);
143144

144-
if digest == _expected_digest {
145+
// Use constant time equality check to prevent timing attacks
146+
if is_equal_digest(&digest, &expected_digest) {
145147
let new_body = Full::new(body_bytes).map_err(|never| match never {}).boxed();
146148
let res = Request::from_parts(header, new_body);
147149
Ok(res)
@@ -184,15 +186,16 @@ where
184186
Self: Sized,
185187
{
186188
let header_map = self.headers();
187-
let (cd_type, _expected_digest) = extract_content_digest(header_map).await?;
189+
let (cd_type, expected_digest) = extract_content_digest(header_map).await?;
188190
let (header, body) = self.into_parts();
189191
let body_bytes = body
190192
.into_bytes()
191193
.await
192194
.map_err(|_e| HyperDigestError::HttpBodyError("Failed to get body bytes".to_string()))?;
193195
let digest = derive_digest(&body_bytes, &cd_type);
194196

195-
if digest == _expected_digest {
197+
// Use constant time equality check to prevent timing attacks
198+
if is_equal_digest(&digest, &expected_digest) {
196199
let new_body = Full::new(body_bytes).map_err(|never| match never {}).boxed();
197200
let res = Response::from_parts(header, new_body);
198201
Ok(res)
@@ -204,6 +207,16 @@ where
204207
}
205208
}
206209

210+
// Constant time equality check for digest verification to prevent timing attacks
211+
fn is_equal_digest(digest1: &[u8], digest2: &[u8]) -> bool {
212+
// Early return if the lengths are different to prevent unnecessary computation,
213+
// which is not a security risk in this context since the digest lengths are fixed for each algorithm.
214+
if digest1.len() != digest2.len() {
215+
return false;
216+
}
217+
digest1.ct_eq(digest2).into()
218+
}
219+
207220
async fn extract_content_digest(header_map: &http::HeaderMap) -> HyperDigestResult<(ContentDigestType, Vec<u8>)> {
208221
let content_digest_header = header_map
209222
.get(CONTENT_DIGEST_HEADER)
@@ -301,4 +314,116 @@ mod tests {
301314
let verified = res.verify_content_digest().await;
302315
assert!(verified.is_ok());
303316
}
317+
318+
#[tokio::test]
319+
async fn hyper_request_digest_mismatch_by_body_tamper_should_fail() {
320+
// 1) Create a request and set a correct Content-Digest for the original body
321+
let body = Full::new(&b"{\"hello\": \"world\"}"[..]);
322+
let req = Request::builder()
323+
.method("GET")
324+
.uri("https://example.com/")
325+
.header("date", "Sun, 09 May 2021 18:30:00 GMT")
326+
.header("content-type", "application/json")
327+
.body(body)
328+
.unwrap();
329+
330+
let req = req.set_content_digest(&ContentDigestType::Sha256).await.unwrap();
331+
assert!(req.headers().contains_key(CONTENT_DIGEST_HEADER));
332+
333+
// 2) Tamper the body while keeping the digest header unchanged
334+
let (parts, _old_body) = req.into_parts();
335+
let tampered_body = Full::new(&b"{\"hello\": \"pwned\"}"[..]).boxed();
336+
let tampered_req = Request::from_parts(parts, tampered_body);
337+
338+
// 3) Verification must fail
339+
let verified = tampered_req.verify_content_digest().await;
340+
assert!(verified.is_err());
341+
match verified.err().unwrap() {
342+
HyperDigestError::InvalidContentDigest(_) => {}
343+
e => panic!("unexpected error: {e:?}"),
344+
}
345+
}
346+
347+
#[tokio::test]
348+
async fn hyper_response_digest_mismatch_by_header_tamper_should_fail() {
349+
// 1) Create a response and set a correct Content-Digest
350+
let body = Full::new(&b"{\"hello\": \"world\"}"[..]);
351+
let res = Response::builder()
352+
.status(200)
353+
.header("date", "Sun, 09 May 2021 18:30:00 GMT")
354+
.header("content-type", "application/json")
355+
.body(body)
356+
.unwrap();
357+
358+
let res = res.set_content_digest(&ContentDigestType::Sha256).await.unwrap();
359+
let (mut parts, body) = res.into_parts();
360+
361+
// 2) Tamper the Content-Digest header (keep it syntactically valid)
362+
// Expected digest is: X48E9qOokqqrvdts8nOJRJN3OWDUoyWxBf7kbu9DBPE=
363+
// Change the first character to another valid base64 character.
364+
parts.headers.insert(
365+
CONTENT_DIGEST_HEADER,
366+
"sha-256=:Y48E9qOokqqrvdts8nOJRJN3OWDUoyWxBf7kbu9DBPE=:".parse().unwrap(),
367+
);
368+
369+
let tampered_res = Response::from_parts(parts, body);
370+
371+
// 3) Verification must fail
372+
let verified = tampered_res.verify_content_digest().await;
373+
assert!(verified.is_err());
374+
match verified.err().unwrap() {
375+
HyperDigestError::InvalidContentDigest(_) => {}
376+
e => panic!("unexpected error: {e:?}"),
377+
}
378+
}
379+
380+
#[tokio::test]
381+
async fn hyper_request_missing_content_digest_header_should_fail() {
382+
let body = Full::new(&b"{\"hello\": \"world\"}"[..]);
383+
let req = Request::builder()
384+
.method("GET")
385+
.uri("https://example.com/")
386+
.header("date", "Sun, 09 May 2021 18:30:00 GMT")
387+
.header("content-type", "application/json")
388+
.body(body)
389+
.unwrap();
390+
391+
// No set_content_digest() call => header missing
392+
let verified = req.verify_content_digest().await;
393+
assert!(verified.is_err());
394+
match verified.err().unwrap() {
395+
HyperDigestError::NoDigestHeader(_) => {}
396+
e => panic!("unexpected error: {e:?}"),
397+
}
398+
}
399+
400+
#[tokio::test]
401+
async fn hyper_request_digest_length_mismatch_should_fail() {
402+
// 1) Create a request and attach a valid Content-Digest header
403+
let body = Full::new(&b"{\"hello\": \"world\"}"[..]);
404+
let req = Request::builder()
405+
.method("GET")
406+
.uri("https://example.com/")
407+
.header("date", "Sun, 09 May 2021 18:30:00 GMT")
408+
.header("content-type", "application/json")
409+
.body(body)
410+
.unwrap();
411+
412+
let req = req.set_content_digest(&ContentDigestType::Sha256).await.unwrap();
413+
414+
// 2) Extract parts and replace the Content-Digest header
415+
// with a syntactically valid but length-mismatched base64 value.
416+
// This ensures that length mismatches are properly rejected.
417+
let (mut parts, body) = req.into_parts();
418+
419+
parts
420+
.headers
421+
.insert(CONTENT_DIGEST_HEADER, "sha-256=:AAAA=:".parse().unwrap());
422+
423+
let tampered_req = Request::from_parts(parts, body);
424+
425+
// 3) Verification must fail due to digest length mismatch
426+
let verified = tampered_req.verify_content_digest().await;
427+
assert!(verified.is_err());
428+
}
304429
}

0 commit comments

Comments
 (0)