From 5b2313afd28dfa3028fee1416b6d2e6e4f2c3c98 Mon Sep 17 00:00:00 2001 From: Ian Stapleton Cordasco Date: Wed, 14 Jul 2021 19:25:10 -0500 Subject: [PATCH 1/2] Send only the digests we have from PackageFile Once again, we have to deal with third-party package repositories doing their own thing and users needing this to just work. Those repositories combined with FIPS mean that we will need to send the information we actually have and only that when it comes to digests. Closes gh-775 --- changelog/776.bugfix.rst | 3 +++ tests/test_package.py | 18 ++++++++++++++++++ twine/package.py | 10 ++++++++-- 3 files changed, 29 insertions(+), 2 deletions(-) create mode 100644 changelog/776.bugfix.rst diff --git a/changelog/776.bugfix.rst b/changelog/776.bugfix.rst new file mode 100644 index 00000000..222d4b67 --- /dev/null +++ b/changelog/776.bugfix.rst @@ -0,0 +1,3 @@ +Do not include md5_digest or blake2_256_digest if FIPS mode is enabled on the +host. This removes those fields from the metadata before sending the metadata +to the repository. diff --git a/tests/test_package.py b/tests/test_package.py index 45cc420b..0e261f0f 100644 --- a/tests/test_package.py +++ b/tests/test_package.py @@ -262,6 +262,24 @@ def test_fips_hash_manager_blake2(monkeypatch): assert hasher.hexdigest() == hashes +def test_fips_packagefile(monkeypatch): + """Generate a metadata dictionary when FIPS is enabled. + + See also: https://github.com/pypa/twine/issues/775 + """ + replaced_blake2b = pretend.raiser(ValueError("fipsmode")) + replaced_md5 = pretend.raiser(ValueError("fipsmode")) + monkeypatch.setattr(package_file.hashlib, "md5", replaced_md5) + monkeypatch.setattr(package_file.hashlib, "blake2b", replaced_blake2b) + + filename = "tests/fixtures/twine-1.5.0-py2.py3-none-any.whl" + pf = package_file.PackageFile.from_filename(filename, None) + + mddict = pf.metadata_dictionary() + assert "md5_digest" not in mddict + assert "blake2_256_digest" not in mddict + + def test_pkginfo_returns_no_metadata(monkeypatch): """Raise an exception when pkginfo can't interpret the metadata. diff --git a/twine/package.py b/twine/package.py index a2da7612..22c05944 100644 --- a/twine/package.py +++ b/twine/package.py @@ -152,9 +152,7 @@ def metadata_dictionary(self) -> Dict[str, MetadataValue]: "download_url": meta.download_url, "supported_platform": meta.supported_platforms, "comment": self.comment, - "md5_digest": self.md5_digest, "sha256_digest": self.sha2_digest, - "blake2_256_digest": self.blake2_256_digest, # PEP 314 "provides": meta.provides, "requires": meta.requires, @@ -174,6 +172,14 @@ def metadata_dictionary(self) -> Dict[str, MetadataValue]: if self.gpg_signature is not None: data["gpg_signature"] = self.gpg_signature + # FIPS will make us send None/null to a package repository and some of + # them don't appreciate that. FIPS disables both MD5 and Blake2 + if self.md5_digest: + data["md5_digest"] = self.md5_digest + + if self.blake2_256_digest: + data["blake2_256_digest"] = self.blake2_256_digest + return data def add_gpg_signature( From 1d235dd3ed6fd7622e4728932d8df211c15eae6c Mon Sep 17 00:00:00 2001 From: Ian Stapleton Cordasco Date: Fri, 16 Jul 2021 06:15:51 -0500 Subject: [PATCH 2/2] Update function and comments for clarity Let's be clearer about some of the bizarre logic we use to support repositories other than PyPI and distributions using FIPS Co-authored-by: Brian Rutledge --- tests/test_package.py | 4 ++-- twine/package.py | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/test_package.py b/tests/test_package.py index 0e261f0f..2e50ee9c 100644 --- a/tests/test_package.py +++ b/tests/test_package.py @@ -262,8 +262,8 @@ def test_fips_hash_manager_blake2(monkeypatch): assert hasher.hexdigest() == hashes -def test_fips_packagefile(monkeypatch): - """Generate a metadata dictionary when FIPS is enabled. +def test_fips_metadata_excludes_md5_and_blake2(monkeypatch): + """Generate a valid metadata dictionary for Nexus when FIPS is enabled. See also: https://github.com/pypa/twine/issues/775 """ diff --git a/twine/package.py b/twine/package.py index 22c05944..ca212a8e 100644 --- a/twine/package.py +++ b/twine/package.py @@ -172,8 +172,9 @@ def metadata_dictionary(self) -> Dict[str, MetadataValue]: if self.gpg_signature is not None: data["gpg_signature"] = self.gpg_signature - # FIPS will make us send None/null to a package repository and some of - # them don't appreciate that. FIPS disables both MD5 and Blake2 + # FIPS disables MD5 and Blake2, making the digest values None. Some package + # repositories don't allow null values, so this only sends non-null values. + # See also: https://github.com/pypa/twine/issues/775 if self.md5_digest: data["md5_digest"] = self.md5_digest