Checking version after verifying signatures

In section 5.3.1 and section 5.4.1 of the spec, the first part of
verifying a new snapshot, targets, and delegated targets role requires
checking the new role's version number before we've checked that these
new roles were signed by the root role. This exposes TUF clients to
potential parser bugs which allows an attacker to potentially compromise
the system.

For example, consider a man-in-the-middle attacker that has a parser bug
which allows for executing arbitrary code, such as [CVE-2017-18349]. An
attacker could exploit this in a few manners:

1. [POUF-1] inlines the metadata signatures in with the role JSON
   metadata file. While snapshot, targets, and delegated targets
   can have their hashes listed in trusted metadata (and thus be
   validated before parsing), the timestamp role must be parsed in order
   to verify it was signed by the trusted root role.
2. TUF-1.0.5 Section 5.3.1 of the workflow states the snapshot should be
   verified by hash and version number before checking the signature.
   While we could verify the hash before parsing, the spec doesn't state
   that the hash should be checked first. It's possible then for a
   conformant TUF client to check the version number before hash, which
   would expose the client to the parser bug.
3. TUF-1.0.5 Section 4.4 and 5.4.1 make it optional for the snapshot
   role to contain the hashes of the targets and any delegated targets
   role. If left out, we need to still parse these files to check the
   version number before validating the signatures.

This patch addresses (2) and (3) by moving the verification of the
version number to after we've verified a role was properly signed by the
trusted root role. This would enable a future POUF that's addressed (1)
to avoid future parser exploits.

[POUF-1]: https://github.com/theupdateframework/taps/blob/master/POUFs/reference-POUF/pouf1.md
[CVE-2017-18349]: https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-18349

Author: erickt

tuf-spec.md

@@ -2,7 +2,7 @@
 
 Last modified: **30 September 2020**
 
-Version: **1.0.9**
+Version: **1.0.10**
 
 We strive to make the specification easy to implement, so if you come across
 any inconsistencies or experience any difficulty, do let us know by sending an
@@ -1191,19 +1191,25 @@ of the form VERSION_NUMBER.FILENAME.EXT (e.g., 42.snapshot.json), where
 VERSION_NUMBER is the version number of the snapshot metadata file listed in
 the timestamp metadata file.
 
-  * **5.3.1**. **Check against timestamp metadata.** The hashes and version
-  number of the new snapshot metadata file MUST match the hashes, if any, and
-  version number listed in the trusted timestamp metadata.  If hashes and
-  version do not match, discard the new snapshot metadata, abort the update
-  cycle, and report the failure.
+  * **5.3.1**. **Check against timestamp role's snapshot hash.** The hashes
+  of the new snapshot metadata file MUST match the hashes, if any, listed in
+  the trusted timestamp metadata.  This is done, in part, to prevent a
+  mix-and-match attack by man-in-the-middle attackers.  If the hashes do not
+  match, discard the new snapshot metadata, abort the update cycle, and report
+  the failure.
 
   * **5.3.2**. **Check for an arbitrary software attack.** The new snapshot
   metadata file MUST have been signed by a threshold of keys specified in the
   trusted root metadata file.  If the new snapshot metadata file is not signed
   as required, discard it, abort the update cycle, and report the signature
   failure.
 
-  * **5.3.3**. **Check for a rollback attack.** The version number of the targets
+  * **5.3.3**. **Check against timestamp role's snapshot version.** The version
+  number of the new snapshot metadata file MUST match the version number listed
+  in the trusted timestamp metadata.  If the versions do not match, discard the
+  new snapshot metadata, abort the update cycle, and report the failure.
+
+  * **5.3.4**. **Check for a rollback attack.** The version number of the targets
   metadata file, and all delegated targets metadata files, if any, in the
   trusted snapshot metadata file, if any, MUST be less than or equal to its
   version number in the new snapshot metadata file. Furthermore, any targets
@@ -1212,13 +1218,14 @@ the timestamp metadata file.
   these conditions are not met, discard the new snapshot metadata file, abort
   the update cycle, and report the failure.
 
-  * **5.3.4**. **Check for a freeze attack.** The latest known time MUST be
+  * **5.3.5**. **Check for a freeze attack.** The latest known time MUST be
   lower than the expiration timestamp in the new snapshot metadata file.  If
   so, the new snapshot metadata file becomes the trusted snapshot metadata
   file. If the new snapshot metadata file is expired, discard it, abort the
   update cycle, and report the potential freeze attack.
 
-  * **5.3.5**. **Persist snapshot metadata.** The client MUST write the file to
+
+  * **5.3.6**. **Persist snapshot metadata.** The client MUST write the file to
   non-volatile storage as FILENAME.EXT (e.g. snapshot.json).
 
 **5.4**. **Download the top-level targets metadata file**, up to either the
@@ -1231,51 +1238,56 @@ of the form VERSION_NUMBER.FILENAME.EXT (e.g., 42.targets.json), where
 VERSION_NUMBER is the version number of the targets metadata file listed in the
 snapshot metadata file.
 
-  * **5.4.1**. **Check against snapshot metadata.** The hashes and version
-  number of the new targets metadata file MUST match the hashes, if any, and
-  version number listed in the trusted snapshot metadata.  This is done, in
-  part, to prevent a mix-and-match attack by man-in-the-middle attackers.  If
-  the new targets metadata file does not match, discard it, abort the update
-  cycle, and report the failure.
+  * **5.4.1**. **Check against snapshot role's targets hash.** The hashes
+  of the new targets metadata file MUST match the hashes, if any, listed in the
+  trusted snapshot metadata.  This is done, in part, to prevent a mix-and-match
+  attack by man-in-the-middle attackers.  If the new targets metadata file does
+  not match, discard the new target metadata, abort the update cycle, and
+  report the failure.
 
   * **5.4.2**. **Check for an arbitrary software attack.** The new targets
   metadata file MUST have been signed by a threshold of keys specified in the
   trusted root metadata file.  If the new targets metadata file is not signed
   as required, discard it, abort the update cycle, and report the failure.
 
-  * **5.4.3**. **Check for a freeze attack.** The latest known time MUST be
+  * **5.4.3**. **Check against snapshot role's targets version.** The version
+  number of the new targets metadata file MUST match the version number listed
+  in the trusted snapshot metadata.  If the versions do not match, discard it,
+  abort the update cycle, and report the failure.
+
+  * **5.4.4**. **Check for a freeze attack.** The latest known time MUST be
   lower than the expiration timestamp in the new targets metadata file.  If so,
   the new targets metadata file becomes the trusted targets metadata file.  If
   the new targets metadata file is expired, discard it, abort the update cycle,
   and report the potential freeze attack.
 
-  * **5.4.4**. **Persist targets metadata.** The client MUST write the file to
+  * **5.4.5**. **Persist targets metadata.** The client MUST write the file to
   non-volatile storage as FILENAME.EXT (e.g. targets.json).
 
-  * **5.4.5**. **Perform a pre-order depth-first search for metadata about the
+  * **5.4.6**. **Perform a pre-order depth-first search for metadata about the
   desired target, beginning with the top-level targets role.**  Note: If
-  any metadata requested in steps 5.4.5.1 - 5.4.5.2 cannot be downloaded nor
+  any metadata requested in steps 5.4.6.1 - 5.4.6.2 cannot be downloaded nor
   validated, end the search and report that the target cannot be found.
 
-    * **5.4.5.1**. If this role has been visited before, then skip this role
+    * **5.4.6.1**. If this role has been visited before, then skip this role
     (so that cycles in the delegation graph are avoided).  Otherwise, if an
     application-specific maximum number of roles have been visited, then go to
     step 5.5 (so that attackers cannot cause the client to waste excessive
     bandwidth or time).  Otherwise, if this role contains metadata about the
     desired target, then go to step 5.5.
 
-    * **5.4.5.2**. Otherwise, recursively search the list of delegations in
+    * **5.4.6.2**. Otherwise, recursively search the list of delegations in
     order of appearance.
 
-      * **5.4.5.2.1**. If the current delegation is a multi-role delegation,
+      * **5.4.6.2.1**. If the current delegation is a multi-role delegation,
       recursively visit each role, and check that each has signed exactly the
       same non-custom metadata (i.e., length and hashes) about the target (or
       the lack of any such metadata).
 
-      * **5.4.5.2.2**. If the current delegation is a terminating delegation,
+      * **5.4.6.2.2**. If the current delegation is a terminating delegation,
       then jump to step 5.5.
 
-      * **5.4.5.2.3**. Otherwise, if the current delegation is a
+      * **5.4.6.2.3**. Otherwise, if the current delegation is a
       non-terminating delegation, continue processing the next delegation, if
       any. Stop the search, and jump to step 5.5 as soon as a delegation
       returns a result.