Refactor _get_metadata_file() from updater.py

Currently, the `_get_metadata_file()` from `tuf/client/updater.py`
is really large and does too many things and some of them partially.

Right now, it validates the specification version
and the metadata version in it, but doesn't validate
if the metadata has expired or if the signature could be trusted.
So, it's partially validating the metadata file
and at the same time calling  `_verify_metadata_file()` from updater.py
for the rest of the work which doesn't make sense.

It's logically better if we move the validation of the specification
and the metadata version in separate functions which would
be then called in ` _verify_metadata_file()` and that way
all of the validation will be done in ` _verify_metadata_file()`.

Signed-off-by: Martin Vrachev <[email protected]>

Author: MVrachev

tuf/client/updater.py

@@ -1412,9 +1412,81 @@ def _verify_root_self_signed(self, signable):
 
 
 
+  def _validate_metadata_version(self, expected_version, metadata_role,
+      version_downloaded):
+    """
+    Validates the metadata version number.
+    If the version number is unspecified, ensure that the version number
+    downloaded is greater than the currently trusted version number for
+    'metadata_role'.
+    """
+
+    if expected_version is not None:
+      if version_downloaded != expected_version:
+        raise tuf.exceptions.BadVersionNumberError('Downloaded'
+          ' version number: ' + repr(version_downloaded) + '. Version'
+          ' number MUST be: ' + repr(expected_version))
+
+    # The caller does not know which version to download.
+    # Verify that the downloaded version is at least greater
+    # than the one locally available.
+    else:
+      try:
+        current_version = self.metadata['current'][metadata_role]['version']
+
+        if version_downloaded < current_version:
+          raise tuf.exceptions.ReplayedMetadataError(metadata_role,
+              version_downloaded, current_version)
+
+      except KeyError:
+        logger.info(metadata_role + ' not available locally.')
+
+
+
+  def _validate_spec_version(self, metadata_spec_version):
+    """
+    Validates if the specification version number is supported.
+    It is assumed that "spec_version" is in (major.minor.fix) format,
+    (for example: "1.4.3") and that releases with the same major version
+    number maintain backward compatibility.
+    Consequently, if the major version number of new metadata equals our
+    expected major version number, the new metadata is safe to parse.
+    """
+
+    try:
+      metadata_spec_version_split = metadata_spec_version.split('.')
+      metadata_spec_major_version = int(metadata_spec_version_split[0])
+      metadata_spec_minor_version = int(metadata_spec_version_split[1])
+
+      code_spec_version_split = tuf.SPECIFICATION_VERSION.split('.')
+      code_spec_major_version = int(code_spec_version_split[0])
+      code_spec_minor_version = int(code_spec_version_split[1])
+
+      if metadata_spec_major_version != code_spec_major_version:
+        raise tuf.exceptions.UnsupportedSpecificationError(
+            'Downloaded metadata that specifies an unsupported spec_version. '
+            'This code supports major version number: ' +
+            repr(code_spec_major_version) + '; however,'
+            'metadata spec version is: ' + str(metadata_spec_version))
+
+      # report to user if minor versions do not match, continue with update
+      if metadata_spec_minor_version != code_spec_minor_version:
+        logger.info("Downloaded metadata that specifies a different minor " +
+            "spec_version.")
+        logger.info("This code has version " + tuf.SPECIFICATION_VERSION +
+            " and the metadata lists version number " +
+            str(metadata_spec_version) + ".")
+        logger.info("The update will continue as the major versions match.")
+
+    except (ValueError, TypeError) as error:
+      six.raise_from(securesystemslib.exceptions.FormatError('Improperly'
+          ' formatted spec_version, which must be in major.minor.fix format'),
+          error)
+
+
 
   def _verify_metadata_file(self, metadata_file_object,
-      metadata_role):
+      metadata_role, expected_version):
     """
     <Purpose>
       Non-public method that verifies a metadata file.  An exception is
@@ -1429,6 +1501,10 @@ def _verify_metadata_file(self, metadata_file_object,
         The role name of the metadata (e.g., 'root', 'targets',
         'unclaimed').
 
+      expected_version:
+        An integer representing the expected and required version number
+        of the 'metadata_role' file downloaded.
+
     <Exceptions>
       securesystemslib.exceptions.FormatError:
         In case the metadata file is valid JSON, but not valid TUF metadata.
@@ -1468,6 +1544,11 @@ def _verify_metadata_file(self, metadata_file_object,
       # 'securesystemslib.exceptions.FormatError' if not.
       tuf.formats.check_signable_object_format(metadata_signable)
 
+    self._validate_spec_version(metadata_signable['signed']['spec_version'])
+
+    self._validate_metadata_version(expected_version, metadata_role,
+        metadata_signable['signed']['version'])
+
     # Is 'metadata_signable' expired?
     self._ensure_not_expired(metadata_signable['signed'], metadata_role)
 
@@ -1521,8 +1602,8 @@ def _get_metadata_file(self, metadata_role, remote_filename,
         downloaded.
 
       expected_version:
-        The expected and required version number of the 'metadata_role' file
-        downloaded.  'expected_version' is an integer.
+        An integer representing the expected and required version number
+        of the 'metadata_role' file downloaded.
 
     <Exceptions>
       tuf.exceptions.NoWorkingMirrorError:
@@ -1549,85 +1630,9 @@ def _get_metadata_file(self, metadata_role, remote_filename,
       try:
         file_object = tuf.download.unsafe_download(file_mirror,
             upperbound_filelength)
-        file_object.seek(0)
-
-        # Verify 'file_object' according to the callable function.
-        # 'file_object' is also verified if decompressed above (i.e., the
-        # uncompressed version).
-        metadata_signable = \
-          securesystemslib.util.load_json_string(file_object.read().decode('utf-8'))
-
-        # Determine if the specification version number is supported.  It is
-        # assumed that "spec_version" is in (major.minor.fix) format, (for
-        # example: "1.4.3") and that releases with the same major version
-        # number maintain backwards compatibility.  Consequently, if the major
-        # version number of new metadata equals our expected major version
-        # number, the new metadata is safe to parse.
-        try:
-          metadata_spec_version = metadata_signable['signed']['spec_version']
-          metadata_spec_version_split = metadata_spec_version.split('.')
-          metadata_spec_major_version = int(metadata_spec_version_split[0])
-          metadata_spec_minor_version = int(metadata_spec_version_split[1])
-
-          code_spec_version_split = tuf.SPECIFICATION_VERSION.split('.')
-          code_spec_major_version = int(code_spec_version_split[0])
-          code_spec_minor_version = int(code_spec_version_split[1])
-
-          if metadata_spec_major_version != code_spec_major_version:
-            raise tuf.exceptions.UnsupportedSpecificationError(
-                'Downloaded metadata that specifies an unsupported '
-                'spec_version.  This code supports major version number: ' +
-                repr(code_spec_major_version) + '; however, the obtained '
-                'metadata lists version number: ' + str(metadata_spec_version))
-
-          #report to user if minor versions do not match, continue with update
-          if metadata_spec_minor_version != code_spec_minor_version:
-            logger.info("Downloaded metadata that specifies a different minor " +
-                "spec_version. This code has version " +
-                str(tuf.SPECIFICATION_VERSION) +
-                " and the metadata lists version number " +
-                str(metadata_spec_version) +
-                ". The update will continue as the major versions match.")
-
-        except (ValueError, TypeError) as error:
-          six.raise_from(securesystemslib.exceptions.FormatError('Improperly'
-              ' formatted spec_version, which must be in major.minor.fix format'),
-              error)
-
-        # If the version number is unspecified, ensure that the version number
-        # downloaded is greater than the currently trusted version number for
-        # 'metadata_role'.
-        version_downloaded = metadata_signable['signed']['version']
-
-        if expected_version is not None:
-          # Verify that the downloaded version matches the version expected by
-          # the caller.
-          if version_downloaded != expected_version:
-            raise tuf.exceptions.BadVersionNumberError('Downloaded'
-              ' version number: ' + repr(version_downloaded) + '.  Version'
-              ' number MUST be: ' + repr(expected_version))
-
-        # The caller does not know which version to download.  Verify that the
-        # downloaded version is at least greater than the one locally
-        # available.
-        else:
-          # Verify that the version number of the locally stored
-          # 'timestamp.json', if available, is less than what was downloaded.
-          # Otherwise, accept the new timestamp with version number
-          # 'version_downloaded'.
-
-          try:
-            current_version = \
-              self.metadata['current'][metadata_role]['version']
-
-            if version_downloaded < current_version:
-              raise tuf.exceptions.ReplayedMetadataError(metadata_role,
-                  version_downloaded, current_version)
-
-          except KeyError:
-            logger.info(metadata_role + ' not available locally.')
 
-        self._verify_metadata_file(file_object, metadata_role)
+        self._verify_metadata_file(file_object, metadata_role,
+            expected_version)
 
       except Exception as exception:
         # Remember the error from this mirror, and "reset" the target file.