Skip to content

Commit c48711e

Browse files
committed
fix(web-bot-auth): reject duplicate covered components
RFC 9421 Section 2.5, step 2.1: each component identifier occurs only once. Parsing collected components into an IndexMap, silently dropping repeats, so a Signature-Input with a duplicated identifier verified against the signature over the de-duplicated base.
1 parent bc27a88 commit c48711e

1 file changed

Lines changed: 31 additions & 6 deletions

File tree

‎crates/web-bot-auth/src/message_signatures.rs‎

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use std::fmt::Write as _;
22
use std::sync::LazyLock;
33
use std::time::Duration;
44

5-
use indexmap::IndexMap;
5+
use indexmap::{IndexMap, IndexSet};
66
use regex::bytes::Regex;
77
use sfv::FieldType;
88
use time::UtcDateTime;
@@ -144,12 +144,19 @@ impl TryFrom<sfv::InnerList> for SignatureBaseBuilder {
144144
type Error = ImplementationError;
145145

146146
fn try_from(value: sfv::InnerList) -> Result<Self, Self::Error> {
147+
let components = value
148+
.items
149+
.iter()
150+
.map(|item| (*item).clone().try_into())
151+
.collect::<Result<Vec<CoveredComponent>, ImplementationError>>()?;
152+
// RFC 9421 Section 2.5, step 2.1: each component identifier occurs only once.
153+
if IndexSet::<&CoveredComponent>::from_iter(&components).len() != components.len() {
154+
return Err(ImplementationError::ParsingError(
155+
"duplicate covered component".to_string(),
156+
));
157+
}
147158
Ok(SignatureBaseBuilder {
148-
components: value
149-
.items
150-
.iter()
151-
.map(|item| (*item).clone().try_into())
152-
.collect::<Result<Vec<CoveredComponent>, ImplementationError>>()?,
159+
components,
153160
// Note: it is the responsibility of higher layers to check whether the message is
154161
// expired, down here we just parse.
155162
parameters: value.params.into(),
@@ -747,6 +754,24 @@ mod tests {
747754
assert!(matches!(err, ImplementationError::FailedToVerify(_)));
748755
}
749756

757+
#[test]
758+
fn test_parsing_rejects_duplicate_components() {
759+
struct Duplicated;
760+
impl SignedMessage for Duplicated {
761+
fn lookup_component(&self, name: &CoveredComponent) -> Vec<String> {
762+
(StandardTestVector {})
763+
.lookup_component(name)
764+
.into_iter()
765+
.map(|value| {
766+
value.replace(r#"("@authority")"#, r#"("@authority" "@authority")"#)
767+
})
768+
.collect()
769+
}
770+
}
771+
let err = MessageVerifier::parse(&Duplicated, |(_, _)| true).unwrap_err();
772+
assert!(matches!(err, ImplementationError::ParsingError(_)));
773+
}
774+
750775
#[test]
751776
fn test_verifying_unsupported_algorithm() {
752777
let mut keyring = KeyRing::default();

0 commit comments

Comments
 (0)