Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace deprecated distutils.version.StrictVersion. #433

Merged
merged 1 commit into from
Oct 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 65 additions & 14 deletions securesystemslib/gpg/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@
import binascii
import re
import logging
import dataclasses

from distutils.version import StrictVersion # pylint: disable=no-name-in-module,import-error

CRYPTO = True
NO_CRYPTO_MSG = 'gpg.utils requires the cryptography library'
Expand All @@ -29,6 +29,7 @@
except ImportError:
CRYPTO = False

# pylint: disable=wrong-import-position
from securesystemslib import exceptions
from securesystemslib import process
from securesystemslib.gpg import constants
Expand Down Expand Up @@ -297,7 +298,51 @@ def parse_subpackets(data):
return parsed_subpackets


def get_version():
@dataclasses.dataclass(order=True)
class Version:
"""A version of GPG."""

major: int
minor: int
patch: int

VERSION_RE = re.compile(r'(\d)\.(\d)\.(\d+)')
EXAMPLE = '1.3.22'

@classmethod
def from_string(cls, value: str) -> 'Version':
"""
<Purpose>
Parses `value` as a `Version`.

Expects a version in the format `major.minor.patch`. `major` and `minor`
must be one-digit numbers; `patch` can be any integer.

<Arguments>
value:
The version string to parse.

<Exceptions>
ValueError:
If the version string is invalid.

<Returns>
Version
"""
match = cls.VERSION_RE.fullmatch(value)
if not match:
raise ValueError(
f"Invalid version number '{value}'; "
f"expected MAJOR.MINOR.PATCH (e.g., '{cls.EXAMPLE}')"
)
major, minor, patch = map(int, match.groups())
return cls(major, minor, patch)

def __str__(self):
return f"{self.major}.{self.minor}.{self.patch}"


def get_version() -> Version:
"""
<Purpose>
Uses `gpg2 --version` to get the version info of the installed gpg2
Expand All @@ -309,8 +354,11 @@ def get_version():
securesystemslib.exceptions.UnsupportedLibraryError:
If the gpg command is not available

<Side Effects>
Executes a command: constants.GPG_VERSION_COMMAND.

<Returns>
Version number string, e.g. "2.1.22"
Version of GPG.

"""
if not constants.HAVE_GPG: # pragma: no cover
Expand All @@ -321,9 +369,19 @@ def get_version():
stderr=process.PIPE, universal_newlines=True)

full_version_info = gpg_process.stdout
version_string = re.search(r'(\d\.\d\.\d+)', full_version_info).group(1)
try:
match = Version.VERSION_RE.search(full_version_info)
if not match:
raise ValueError(
f"Couldn't find version number (ex. '{Version.EXAMPLE}') "
f"in the output of `{command}`:\n"
+ full_version_info
)
version = Version.from_string(match.group(0))
except ValueError as err:
raise exceptions.UnsupportedLibraryError(constants.NO_GPG_MSG) from err

return version_string
return version


def is_version_fully_supported():
Expand All @@ -337,15 +395,8 @@ def is_version_fully_supported():
constants.FULLY_SUPPORTED_MIN_VERSION, False otherwise.

"""

installed_version = get_version()
# Excluded so that coverage does not vary in different test environments
if (StrictVersion(installed_version) >=
StrictVersion(constants.FULLY_SUPPORTED_MIN_VERSION)): # pragma: no cover
return True

else: # pragma: no cover
return False
min_version = constants.FULLY_SUPPORTED_MIN_VERSION
return get_version() >= Version.from_string(min_version)


def get_hashing_class(hash_algorithm_id):
Expand Down
37 changes: 35 additions & 2 deletions tests/test_gpg.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,12 @@
import cryptography.hazmat.backends as backends
import cryptography.hazmat.primitives.hashes as hashing

from securesystemslib import exceptions
from securesystemslib import process
from securesystemslib.gpg.functions import (create_signature, export_pubkey,
verify_signature, export_pubkeys)
from securesystemslib.gpg.util import (get_version, is_version_fully_supported,
get_hashing_class, parse_packet_header, parse_subpacket_header)
get_hashing_class, parse_packet_header, parse_subpacket_header, Version)
from securesystemslib.gpg.rsa import create_pubkey as rsa_create_pubkey
from securesystemslib.gpg.dsa import create_pubkey as dsa_create_pubkey
from securesystemslib.gpg.eddsa import create_pubkey as eddsa_create_pubkey
Expand Down Expand Up @@ -73,11 +74,18 @@ def ignore_not_found_error(function, path, exc_info):
@unittest.skipIf(not HAVE_GPG, "gpg not found")
class TestUtil(unittest.TestCase):
"""Test util functions. """

def test_version_utils_return_types(self):
"""Run dummy tests for coverage. """
self.assertTrue(isinstance(get_version(), str))
self.assertTrue(isinstance(get_version(), Version))
self.assertTrue(isinstance(is_version_fully_supported(), bool))

@patch('securesystemslib.gpg.constants.GPG_VERSION_COMMAND', 'echo "bad"')
def test_version_utils_error(self):
"""Run dummy tests for coverage. """
with self.assertRaises(exceptions.UnsupportedLibraryError):
get_version()

def test_get_hashing_class(self):
# Assert return expected hashing class
expected_hashing_class = [hashing.SHA1, hashing.SHA256, hashing.SHA512]
Expand Down Expand Up @@ -809,5 +817,30 @@ def test_verify_short_signature(self):
self.assertTrue(verify_signature(signature, key, test_data))


class TestVersion(unittest.TestCase):
"""Tests for the Version utility class."""

def test_version_roundtrip_string(self):
"""Version parses and formats strings correctly."""
for value, expected in [
('1.3.0', Version(1, 3, 0)),
('1.3.1', Version(1, 3, 1)),
('1.3.22', Version(1, 3, 22)),
]:
self.assertEqual(Version.from_string(value), expected)
self.assertEqual(str(expected), value)

def test_version_from_string_invalid(self):
"""Version.from_string rejects invalid inputs."""
for value in [
'1.3',
'1.33.0',
Comment on lines +836 to +837
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are both valid for StrictVersion.

Obviously we don't need to re-implement StrictVersion here (we know this is a hack, it just has to work well enough for this specific case) but at least the latter one seems strange to me -- is this just because gpg historically hasn't used multidigit minor versions?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. Unfortunately, I can't remember if it was just an assumption based on the existing version progression (e.g. last 1.x was 1.9), or if there was a stronger indication somewhere. Happy to change, although not necessarily in this PR, which just uses the regex that was already there.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh you're right: no new bugs then :)

'1.3.-1',
'1.3.1a',
]:
with self.assertRaises(ValueError, msg=f"expected error for input '{value}'"):
Version.from_string(value)


if __name__ == "__main__":
unittest.main()