diff --git a/src/interchange/cjson/shims.rs b/src/interchange/cjson/shims.rs index d916318f..32b3429d 100644 --- a/src/interchange/cjson/shims.rs +++ b/src/interchange/cjson/shims.rs @@ -77,11 +77,20 @@ impl RootMetadata { ))); } + // Ignore all keys with incorrect key IDs. We should give an error if the key ID is not + // correct according to TUF spec. However, due to backward compatibility, we may receive + // metadata with key IDs generated by TUF 0.9. We simply ignore those old keys. + let keys_with_correct_key_id = self + .keys + .into_iter() + .filter(|(key_id, pkey)| key_id == pkey.key_id()) + .collect(); + metadata::RootMetadata::new( self.version, parse_datetime(&self.expires)?, self.consistent_snapshot, - self.keys.into_iter().collect(), + keys_with_correct_key_id, self.roles.root, self.roles.snapshot, self.roles.targets, diff --git a/src/metadata.rs b/src/metadata.rs index bf04a423..ba61dc2c 100644 --- a/src/metadata.rs +++ b/src/metadata.rs @@ -10,6 +10,7 @@ use std::collections::{HashMap, HashSet}; use std::fmt::{self, Debug, Display}; use std::io::Read; use std::marker::PhantomData; +use std::str; use crate::crypto::{self, HashAlgorithm, HashValue, KeyId, PrivateKey, PublicKey, Signature}; use crate::error::Error; @@ -560,9 +561,15 @@ where let canonical_bytes = D::canonicalize(&self.metadata)?; let mut signatures_needed = threshold; - for sig in &self.signatures { - match authorized_keys.get(sig.key_id()) { - Some(ref pub_key) => match pub_key.verify(&canonical_bytes, &sig) { + // Create a key_id->signature map to deduplicate the key_ids. + let signatures = self + .signatures + .iter() + .map(|sig| (sig.key_id(), sig)) + .collect::>(); + for (key_id, sig) in signatures { + match authorized_keys.get(key_id) { + Some(ref pub_key) => match pub_key.verify(&canonical_bytes, sig) { Ok(()) => { debug!("Good signature from key ID {:?}", pub_key.key_id()); signatures_needed -= 1; @@ -2355,6 +2362,21 @@ mod test { assert_eq!(jsn, encoded); } + #[test] + fn de_ser_root_metadata_wrong_key_id() { + let jsn = jsn_root_metadata_without_keyid_hash_algos(); + let mut jsn_str = str::from_utf8(&Json::canonicalize(&jsn).unwrap()) + .unwrap() + .to_owned(); + // Replace the key id to something else. + jsn_str = jsn_str.replace( + "12435b260b6172bd750aeb102f54a347c56b109e0524ab1f144593c07af66356", + "00435b260b6172bd750aeb102f54a347c56b109e0524ab1f144593c07af66356", + ); + let decoded: RootMetadata = serde_json::from_str(&jsn_str).unwrap(); + assert_eq!(3, decoded.keys.len()); + } + #[test] fn sign_and_verify_root_metadata() { let jsn = jsn_root_metadata_without_keyid_hash_algos(); @@ -2382,6 +2404,29 @@ mod test { decoded.verify(1, &[root_key.public().clone()]).unwrap(); } + #[test] + fn verify_signed_serialized_root_metadata_with_duplicate_sig() { + let jsn = json!({ + "signatures": [{ + "keyid": "a9f3ebc9b138762563a9c27b6edd439959e559709babd123e8d449ba2c18c61a", + "sig": "c4ba838e0d3f783716393a4d691f568f840733ff488bb79ac68287e97e0b31d63fcef392dbc978e878c2103ba231905af634cc651d6f0e63a35782d051ac6e00" + }, + { + "keyid": "a9f3ebc9b138762563a9c27b6edd439959e559709babd123e8d449ba2c18c61a", + "sig": "c4ba838e0d3f783716393a4d691f568f840733ff488bb79ac68287e97e0b31d63fcef392dbc978e878c2103ba231905af634cc651d6f0e63a35782d051ac6e00" + }], + "signed": jsn_root_metadata_without_keyid_hash_algos() + }); + let root_key = PrivateKey::from_pkcs8(ED25519_1_PK8, SignatureScheme::Ed25519).unwrap(); + let decoded: SignedMetadata = + serde_json::from_value(jsn).unwrap(); + assert_matches!( + decoded.verify(2, &[root_key.public().clone()]), + Err(Error::VerificationFailure(_)) + ); + decoded.verify(1, &[root_key.public().clone()]).unwrap(); + } + fn verify_signature_with_unknown_fields(mut metadata: serde_json::Value) where M: Metadata,