Improper verification of signature threshold in tough

@@ -31,6 +31,7 @@
 
 import unittest
 import logging
+import copy
 
 import tuf
 import tuf.log
@@ -384,6 +385,68 @@ def test_verify_single_key(self):
     tuf.roledb.remove_role('Root')
 
 
+
+  def test_verify_must_not_count_duplicate_keyids_towards_threshold(self):
+    # Create and sign dummy metadata twice with same key
+    # Note that we use the non-deterministic rsassa-pss signing scheme, so
+    # creating the signature twice shows that we don't only detect duplicate
+    # signatures but also different signatures from the same key.
+    signable = {"signed" : "test", "signatures" : []}
+    signed = securesystemslib.formats.encode_canonical(
+        signable["signed"]).encode("utf-8")
+    signable["signatures"].append(
+        securesystemslib.keys.create_signature(KEYS[0], signed))
+    signable["signatures"].append(
+        securesystemslib.keys.create_signature(KEYS[0], signed))
+
+    # 'get_signature_status' uses keys from keydb for verification
+    tuf.keydb.add_key(KEYS[0])
+
+    # Assert that 'get_signature_status' returns two good signatures ...
+    status = tuf.sig.get_signature_status(
+        signable, "root", keyids=[KEYS[0]["keyid"]], threshold=2)
+    self.assertTrue(len(status["good_sigs"]) == 2)
+
+    # ... but only one counts towards the threshold
+    self.assertFalse(
+        tuf.sig.verify(signable, "root", keyids=[KEYS[0]["keyid"]], threshold=2))
+
+    # Clean-up keydb
+    tuf.keydb.remove_key(KEYS[0]["keyid"])
+
+
+
+  def test_verify_count_different_keyids_for_same_key_towards_threshold(self):
+    # Create and sign dummy metadata twice with same key but different keyids
+    signable = {"signed" : "test", "signatures" : []}
+    key_sha256 = copy.deepcopy(KEYS[0])
+    key_sha256["keyid"] = "deadbeef256"
+
+    key_sha512 = copy.deepcopy(KEYS[0])
+    key_sha512["keyid"] = "deadbeef512"
+
+    signed = securesystemslib.formats.encode_canonical(
+        signable["signed"]).encode("utf-8")
+    signable["signatures"].append(
+        securesystemslib.keys.create_signature(key_sha256, signed))
+    signable["signatures"].append(
+        securesystemslib.keys.create_signature(key_sha512, signed))
+
+    # 'get_signature_status' uses keys from keydb for verification
+    tuf.keydb.add_key(key_sha256)
+    tuf.keydb.add_key(key_sha512)
+
+    # Assert that both keys count towards threshold although its the same key
+    keyids = [key_sha256["keyid"], key_sha512["keyid"]]
+    self.assertTrue(
+        tuf.sig.verify(signable, "root", keyids=keyids, threshold=2))
+
+    # Clean-up keydb
+    tuf.keydb.remove_key(key_sha256["keyid"])
+    tuf.keydb.remove_key(key_sha512["keyid"])
+
+
+
   def test_verify_unrecognized_sig(self):
     signable = {'signed' : 'test', 'signatures' : []}
     signed = securesystemslib.formats.encode_canonical(signable['signed']).encode('utf-8')

@@ -425,6 +425,53 @@ def test_1__update_versioninfo(self):
 
 
 
+  def test_1__refresh_must_not_count_duplicate_keyids_towards_threshold(self):
+    # Update root threshold on the server repository and sign twice with 1 key
+    repository = repo_tool.load_repository(self.repository_directory)
+    repository.root.threshold = 2
+    repository.root.load_signing_key(self.role_keys['root']['private'])
+
+    # The client uses the threshold from the previous root file to verify the
+    # new root. Thus we need to make two updates so that the threshold used for
+    # verification becomes 2. I.e. we bump the version, sign twice with the
+    # same key and write to disk '2.root.json' and '3.root.json'.
+    for version in [2, 3]:
+      repository.root.version = version
+      info = tuf.roledb.get_roleinfo("root")
+      metadata = repo_lib.generate_root_metadata(
+          info["version"], info["expires"], False)
+      signed_metadata = repo_lib.sign_metadata(
+          metadata, info["keyids"], "root.json", "default")
+      signed_metadata["signatures"].append(signed_metadata["signatures"][0])
+      live_root_path = os.path.join(
+          self.repository_directory, "metadata", "root.json")
+
+      # Bypass server side verification in 'write' or 'writeall', which would
+      # catch the unmet threshold.
+      # We also skip writing to 'metadata.staged' and copying to 'metadata' and
+      # instead write directly to 'metadata'
+      repo_lib.write_metadata_file(signed_metadata, live_root_path, info["version"], True)
+
+
+    # Update from current '1.root.json' to '3.root.json' on client and assert
+    # raise of 'BadSignatureError' (caused by unmet signature threshold).
+    try:
+      self.repository_updater.refresh()
+
+    except tuf.exceptions.NoWorkingMirrorError as e:
+      mirror_errors = list(e.mirror_errors.values())
+      self.assertTrue(len(mirror_errors) == 1)
+      self.assertTrue(
+          isinstance(mirror_errors[0],
+          securesystemslib.exceptions.BadSignatureError))
+      self.assertEqual(
+          str(mirror_errors[0]),
+          repr("root") + " metadata has bad signature.")
+
+    else:
+      self.fail(
+          "Expected a NoWorkingMirrorError composed of one BadSignatureError")
+
 
   def test_1__update_fileinfo(self):
       # Tests

@@ -71,11 +71,23 @@ def get_signature_status(signable, role=None, repository_name='default',
   """
   <Purpose>
     Return a dictionary representing the status of the signatures listed in
-    'signable'.  Given an object conformant to SIGNABLE_SCHEMA, a set of public
-    keys in 'tuf.keydb', a set of roles in 'tuf.roledb', and a role,
-    the status of these signatures can be determined.  This method will iterate
-    the signatures in 'signable' and enumerate all the keys that are valid,
-    invalid, unrecognized, or unauthorized.
+    'signable'. Signatures in the returned dictionary are identified by the
+    signature keyid and can have a status of either:
+
+    * bad -- Invalid signature
+    * good -- Valid signature from key that is available in 'tuf.keydb', and is
+      authorized for the passed role as per 'tuf.roledb' (authorization may be
+      overwritten by passed 'keyids').
+    * unknown -- Signature from key that is not available in 'tuf.keydb', or if
+      'role' is None.
+    * unknown signing schemes -- Signature from key with unknown signing
+      scheme.
+    * untrusted -- Valid signature from key that is available in 'tuf.keydb',
+      but is not trusted for the passed role as per 'tuf.roledb' or the passed
+      'keyids'.
+
+    NOTE: The result may contain duplicate keyids or keyids that reference the
+    same key, if 'signable' lists multiple signatures from the same key.
 
   <Arguments>
     signable:
@@ -87,7 +99,7 @@ def get_signature_status(signable, role=None, repository_name='default',
       Conformant to tuf.formats.SIGNABLE_SCHEMA.
 
     role:
-      TUF role (e.g., 'root', 'targets', 'snapshot').
+      TUF role string (e.g. 'root', 'targets', 'snapshot' or timestamp).
 
     threshold:
       Rather than reference the role's threshold as set in tuf.roledb.py, use
@@ -133,22 +145,6 @@ def get_signature_status(signable, role=None, repository_name='default',
 
   # The signature status dictionary returned.
   signature_status = {}
-
-  # The fields of the signature_status dict, where each field stores keyids.  A
-  # description of each field:
-  #
-  # good_sigs = keys confirmed to have produced 'sig' using 'signed', which are
-  # associated with 'role';
-  #
-  # bad_sigs = negation of good_sigs;
-  #
-  # unknown_sigs = keys not found in the 'keydb' database;
-  #
-  # untrusted_sigs = keys that are not in the list of keyids associated with
-  # 'role';
-  #
-  # unknown_signing_scheme = signing schemes specified in keys that are
-  # unsupported;
   good_sigs = []
   bad_sigs = []
   unknown_sigs = []
@@ -240,18 +236,26 @@ def verify(signable, role, repository_name='default', threshold=None,
     keyids=None):
   """
   <Purpose>
-    Verify whether the authorized signatures of 'signable' meet the minimum
-    required by 'role'.  Authorized signatures are those with valid keys
-    associated with 'role'.  'signable' must conform to SIGNABLE_SCHEMA
-    and 'role' must not equal 'None' or be less than zero.
+    Verify that 'signable' has a valid threshold of authorized signatures
+    identified by unique keyids. The threshold and whether a keyid is
+    authorized is determined by querying the 'threshold' and 'keyids' info for
+    the passed 'role' in 'tuf.roledb'. Both values can be overwritten by
+    passing the 'threshold' or 'keyids' arguments.
+
+    NOTE:
+    - Signatures with identical authorized keyids only count towards the
+      threshold once.
+    - Signatures with different authorized keyids each count towards the
+      threshold, even if the keyids identify the same key.
 
   <Arguments>
     signable:
-      A dictionary containing a list of signatures and a 'signed' identifier.
+      A dictionary containing a list of signatures and a 'signed' identifier
+      that conforms to SIGNABLE_SCHEMA, e.g.:
       signable = {'signed':, 'signatures': [{'keyid':, 'method':, 'sig':}]}
 
     role:
-      TUF role (e.g., 'root', 'targets', 'snapshot').
+      TUF role string (e.g. 'root', 'targets', 'snapshot' or timestamp).
 
     threshold:
       Rather than reference the role's threshold as set in tuf.roledb.py, use
@@ -278,8 +282,8 @@ def verify(signable, role, repository_name='default', threshold=None,
     get_signature_status() will be caught here and re-raised.
 
   <Returns>
-    Boolean.  True if the number of good signatures >= the role's threshold,
-    False otherwise.
+    Boolean.  True if the number of good unique (by keyid) signatures >= the
+    role's threshold, False otherwise.
   """
 
   tuf.formats.SIGNABLE_SCHEMA.check_match(signable)
@@ -303,7 +307,7 @@ def verify(signable, role, repository_name='default', threshold=None,
   if threshold is None or threshold <= 0: #pragma: no cover
     raise securesystemslib.exceptions.Error("Invalid threshold: " + repr(threshold))
 
-  return len(good_sigs) >= threshold
+  return len(set(good_sigs)) >= threshold