Merge pull request #283 from aabbaabb/develop

Deduplicate signatures with the same key_id.

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,

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::<HashMap<&KeyId, &Signature>>();
+        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<crate::interchange::cjson::Json, RootMetadata> =
+            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<M>(mut metadata: serde_json::Value)
     where
         M: Metadata,