feat(package): remove MD5 hashing entirely (#1262)

Author: woodruffw

changelog/1262.removal.rst

@@ -0,0 +1,7 @@
+Remove support for MD5 digests during uploads.
+
+This support was entirely vestigial, as MD5 is not a secure hash function
+and is not actually required on upload by PyPI.
+
+Indices that cross-reference the uploaded content with a digest should
+use the provided SHA-256 and/or BLAKE2 digests instead.

tests/test_package.py

@@ -294,7 +294,6 @@ def test_metadata_dictionary_values(gpg_signature, attestation):
 
 
 TWINE_1_5_0_WHEEL_HEXDIGEST = package_file.Hexdigest(
-    "1919f967e990bee7413e2a4bc35fd5d1",
     "d86b0f33f0c7df49e888b11c43b417da5520cbdbce9f20618b1494b600061e67",
     "b657a4148d05bd0098c1d6d8cc4e14e766dbe93c3a5ab6723b969da27a87bac0",
 )
@@ -308,18 +307,6 @@ def test_hash_manager():
     assert hasher.hexdigest() == TWINE_1_5_0_WHEEL_HEXDIGEST
 
 
-def test_fips_hash_manager_md5(monkeypatch):
-    """Generate hexdigest without MD5 when hashlib is using FIPS mode."""
-    replaced_md5 = pretend.raiser(ValueError("fipsmode"))
-    monkeypatch.setattr(package_file.hashlib, "md5", replaced_md5)
-
-    filename = "tests/fixtures/twine-1.5.0-py2.py3-none-any.whl"
-    hasher = package_file.HashManager(filename)
-    hasher.hash()
-    hashes = TWINE_1_5_0_WHEEL_HEXDIGEST._replace(md5=None)
-    assert hasher.hexdigest() == hashes
-
-
 @pytest.mark.parametrize("exception_class", [TypeError, ValueError])
 def test_fips_hash_manager_blake2(exception_class, monkeypatch):
     """Generate hexdigest without BLAKE2 when hashlib is using FIPS mode."""
@@ -333,21 +320,18 @@ def test_fips_hash_manager_blake2(exception_class, monkeypatch):
     assert hasher.hexdigest() == hashes
 
 
-def test_fips_metadata_excludes_md5_and_blake2(monkeypatch):
+def test_fips_metadata_excludes_blake2(monkeypatch):
     """Generate a valid metadata dictionary for Nexus 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
 
 

twine/package.py

@@ -157,7 +157,6 @@ class PackageMetadata(TypedDict, total=False):
     filetype: str
     gpg_signature: Tuple[str, bytes]
     attestations: str
-    md5_digest: str
     sha256_digest: str
     blake2_256_digest: str
 
@@ -188,7 +187,6 @@ def __init__(
         hasher.hash()
         hexdigest = hasher.hexdigest()
 
-        self.md5_digest = hexdigest.md5
         self.sha2_digest = hexdigest.sha2
         self.blake2_256_digest = hexdigest.blake2
 
@@ -274,7 +272,7 @@ def metadata_dictionary(self) -> PackageMetadata:
 
         # Additional meta-data: some of these fileds may not be set. Some
         # package repositories do not allow null values, so this only sends
-        # non-null values. In particular, FIPS disables MD5 and Blake2, making
+        # non-null values. In particular, FIPS disables Blake2, making
         # the digest values null. See https://github.com/pypa/twine/issues/775
 
         if self.comment is not None:
@@ -289,9 +287,6 @@ def metadata_dictionary(self) -> PackageMetadata:
         if self.attestations is not None:
             data["attestations"] = json.dumps(self.attestations)
 
-        if self.md5_digest:
-            data["md5_digest"] = self.md5_digest
-
         if self.blake2_256_digest:
             data["blake2_256_digest"] = self.blake2_256_digest
 
@@ -352,7 +347,6 @@ def run_gpg(cls, gpg_args: Tuple[str, ...]) -> None:
 
 
 class Hexdigest(NamedTuple):
-    md5: Optional[str]
     sha2: Optional[str]
     blake2: Optional[str]
 
@@ -367,13 +361,6 @@ def __init__(self, filename: str) -> None:
         """Initialize our manager and hasher objects."""
         self.filename = filename
 
-        self._md5_hasher = None
-        try:
-            self._md5_hasher = hashlib.md5()
-        except ValueError:
-            # FIPs mode disables MD5
-            pass
-
         self._sha2_hasher = hashlib.sha256()
 
         self._blake_hasher = None
@@ -383,15 +370,6 @@ def __init__(self, filename: str) -> None:
             # FIPS mode disables blake2
             pass
 
-    def _md5_update(self, content: bytes) -> None:
-        if self._md5_hasher is not None:
-            self._md5_hasher.update(content)
-
-    def _md5_hexdigest(self) -> Optional[str]:
-        if self._md5_hasher is not None:
-            return self._md5_hasher.hexdigest()
-        return None
-
     def _sha2_update(self, content: bytes) -> None:
         if self._sha2_hasher is not None:
             self._sha2_hasher.update(content)
@@ -414,14 +392,12 @@ def hash(self) -> None:
         """Hash the file contents."""
         with open(self.filename, "rb") as fp:
             for content in iter(lambda: fp.read(io.DEFAULT_BUFFER_SIZE), b""):
-                self._md5_update(content)
                 self._sha2_update(content)
                 self._blake_update(content)
 
     def hexdigest(self) -> Hexdigest:
         """Return the hexdigest for the file."""
         return Hexdigest(
-            self._md5_hexdigest(),
             self._sha2_hexdigest(),
             self._blake_hexdigest(),
         )