Port securesystemslib.hash module #2815

lukpueh · 2025-03-18T13:59:59Z

securesystemslib.hash is a small wrapper around hashlib, which serves two main purposes:

provide helper function to hash a file
translate custom hash algorithm name "blake2b-256" to "blake2b" with (digest_size=32).

In preparation for the removal of securesystemslib.hash, this patch ports above behavior to tuf and uses the builtin hashlib directly where possible.

related secure-systems-lab/securesystemslib#943

lukpueh · 2025-03-18T15:01:15Z

tuf/api/_payload.py

@@ -45,6 +48,31 @@
 T = TypeVar("T", "Root", "Timestamp", "Snapshot", "Targets")


+def _hash(algo: str) -> Any:  # noqa: ANN401


Python's hashlib does not make the Hash type public, hence I'm using Any and ask the linter for forgiveness.

tuf/api/_payload.py

lukpueh · 2025-03-18T15:08:14Z

tests/repository_simulator.py

@@ -80,6 +80,8 @@

 SPEC_VER = ".".join(SPECIFICATION_VERSION)

+_DEFAULT_HASH_ALGORITHM = "sha256"


IMO it's okay to make this a local default, and not re-use the one newly added to _payload.py, so that we can keep the latter internal.

sure, I'd be fine with just a magic value as well: it's not really even a default, it's just what this repository decided to use

securesystemslib.hash is a small wrapper around hashlib, which serves two main purposes: * provide helper function to hash a file * translate custom hash algorithm name "blake2b-256" to "blake2b" with (digest_size=32). In preparation for the removal of securesystemslib.hash, this patch ports above behavior to tuf and uses the builtin hashlib directly where possible. related secure-systems-lab/securesystemslib#943 Signed-off-by: Lukas Puehringer <[email protected]>

lukpueh · 2025-03-18T15:42:09Z

tuf/api/_payload.py

+    """Returns hashed file."""
+    f.seek(0)
+    if sys.version_info >= (3, 11):
+        digest = hashlib.file_digest(f, lambda: _hash(algo))  # type: ignore[arg-type]


Mypy does not allow IO[bytes] for f here. This may be related to python/typing#659. Ignore seems fair.

jku

looks fine to me. Left some comments

jku · 2025-03-18T16:05:16Z

tuf/api/_payload.py

+                    digest_object = _hash(algorithm)
                    digest_object.update(data)
                else:
-                    digest_object = sslib_hash.digest_fileobject(
-                        data, algorithm
-                    )
-            except (
-                sslib_exceptions.UnsupportedAlgorithmError,
-                sslib_exceptions.FormatError,
-            ) as e:
+                    digest_object = _file_hash(data, algorithm)
+            except (ValueError, TypeError) as e:


the different functionality is a little annoying here (you need to call update on one digest_object but not the other)...

If the _hash and _file_hash functions directly returned the hexdigest this code would be simpler and the functions would be properly annotated

on the other hand then _hash() would need a data argument (meaning you would have to split the lambda needed by hashlib.file_digest() into another function)

I don't really know what is the simplest... up to you

Does 535a189 look better? I don't have very strong feelings about either way.

No passionate argument here either. I think I like this new version better.

jku · 2025-03-18T16:08:05Z

tests/repository_simulator.py

@@ -80,6 +80,8 @@

 SPEC_VER = ".".join(SPECIFICATION_VERSION)

+_DEFAULT_HASH_ALGORITHM = "sha256"


sure, I'd be fine with just a magic value as well: it's not really even a default, it's just what this repository decided to use

tuf/api/_payload.py

coveralls · 2025-03-18T16:45:43Z

coverage: 97.064% (-0.01%) from 97.076%
when pulling 75e83b3 on lukpueh:port-sslib-hash
into 9f873cb on theupdateframework:develop.

jku · 2025-03-19T08:00:19Z

coverage: 96.981% (-0.1%) from 97.076%

coveralls.io is down again so I'm not sure... but I'm guessing this is the "blake2b-256" special case -- otherwise we're probably exercising this code.

Remove the "default" prefix, because it's not a default but rather a fixed value. Signed-off-by: Lukas Puehringer <[email protected]>

Consolidate interface of bytes hash and file hash helpers. Signed-off-by: Lukas Puehringer <[email protected]>

lukpueh · 2025-03-19T09:14:41Z

coverage: 96.981% (-0.1%) from 97.076%

coveralls.io is down again so I'm not sure... but I'm guessing this is the "blake2b-256" special case -- otherwise we're probably exercising this code.

Yep, blake is the culprit. Let me add a test.

Signed-off-by: Lukas Puehringer <[email protected]>

lukpueh · 2025-03-19T09:58:39Z

@jku, want to re-approve after my latest changes?

jku

Looks good. The only nit I have left is that multiple docstrings still refer to "securesystemslib default hash algorithm".

Default hash sha256 is now defined locally. Signed-off-by: Lukas Puehringer <[email protected]>

lukpueh · 2025-03-19T10:30:36Z

Looks good. The only nit I have left is that multiple docstrings still refer to "securesystemslib default hash algorithm".

Good eyes! I pushed a fix and will merge when ci returns.

fixes secure-systems-lab#943 * Internal use does not need the additional features (custom blake algorithm name support and file hashing), and was replaced by direct calls to hashlib. * External users were updated to no longer require `securesystemslib.hash` (theupdateframework/python-tuf#2815, in-toto/in-toto#861) Signed-off-by: Lukas Puehringer <[email protected]>

fixes secure-systems-lab#943 * Internally, the features added by the module (i.e. custom blake name support, and file hashing) are not needed. Builtin hashlib is used now directly. * External users port the relevant features into their code base: theupdateframework/python-tuf#2815 in-toto/in-toto#861 Signed-off-by: Lukas Puehringer <[email protected]>

lukpueh requested a review from a team as a code owner March 18, 2025 13:59

lukpueh requested review from jku and removed request for jku March 18, 2025 14:00

lukpueh marked this pull request as draft March 18, 2025 14:09

lukpueh force-pushed the port-sslib-hash branch from 369642b to 7ac9f23 Compare March 18, 2025 14:57

lukpueh commented Mar 18, 2025

View reviewed changes

lukpueh force-pushed the port-sslib-hash branch from 7ac9f23 to 866409f Compare March 18, 2025 15:39

lukpueh commented Mar 18, 2025

View reviewed changes

lukpueh marked this pull request as ready for review March 18, 2025 15:42

jku approved these changes Mar 18, 2025

View reviewed changes

lukpueh added 2 commits March 19, 2025 09:28

Rename hash algo global in repo simulator

57010fb

Remove the "default" prefix, because it's not a default but rather a fixed value. Signed-off-by: Lukas Puehringer <[email protected]>

Refactor hash helpers

535a189

Consolidate interface of bytes hash and file hash helpers. Signed-off-by: Lukas Puehringer <[email protected]>

Add tests for custom blake hash

6f50998

Signed-off-by: Lukas Puehringer <[email protected]>

jku approved these changes Mar 19, 2025

View reviewed changes

docs: Remove reference to securesystemslib hash

75e83b3

Default hash sha256 is now defined locally. Signed-off-by: Lukas Puehringer <[email protected]>

lukpueh merged commit 500e8b9 into theupdateframework:develop Mar 19, 2025
17 checks passed

jku mentioned this pull request Mar 19, 2025

Annotate the library (add py.typed) secure-systems-lab/securesystemslib#970

Open

lukpueh mentioned this pull request Mar 19, 2025

Remove internal use of securesystemslib.hash secure-systems-lab/securesystemslib#977

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Port securesystemslib.hash module #2815

Port securesystemslib.hash module #2815

lukpueh commented Mar 18, 2025

lukpueh Mar 18, 2025

lukpueh Mar 18, 2025

jku Mar 18, 2025

lukpueh Mar 18, 2025

jku left a comment

jku Mar 18, 2025 •

edited

Loading

lukpueh Mar 19, 2025

jku Mar 19, 2025

jku Mar 18, 2025

coveralls commented Mar 18, 2025 •

edited

Loading

jku commented Mar 19, 2025

lukpueh commented Mar 19, 2025

lukpueh commented Mar 19, 2025

jku left a comment

lukpueh commented Mar 19, 2025

		@@ -45,6 +48,31 @@
		T = TypeVar("T", "Root", "Timestamp", "Snapshot", "Targets")


		def _hash(algo: str) -> Any: # noqa: ANN401

		@@ -80,6 +80,8 @@

		SPEC_VER = ".".join(SPECIFICATION_VERSION)

		_DEFAULT_HASH_ALGORITHM = "sha256"

Port securesystemslib.hash module #2815

Port securesystemslib.hash module #2815

Conversation

lukpueh commented Mar 18, 2025

lukpueh Mar 18, 2025

Choose a reason for hiding this comment

lukpueh Mar 18, 2025

Choose a reason for hiding this comment

jku Mar 18, 2025

Choose a reason for hiding this comment

lukpueh Mar 18, 2025

Choose a reason for hiding this comment

jku left a comment

Choose a reason for hiding this comment

jku Mar 18, 2025 • edited Loading

Choose a reason for hiding this comment

lukpueh Mar 19, 2025

Choose a reason for hiding this comment

jku Mar 19, 2025

Choose a reason for hiding this comment

jku Mar 18, 2025

Choose a reason for hiding this comment

coveralls commented Mar 18, 2025 • edited Loading

jku commented Mar 19, 2025

lukpueh commented Mar 19, 2025

lukpueh commented Mar 19, 2025

jku left a comment

Choose a reason for hiding this comment

lukpueh commented Mar 19, 2025

jku Mar 18, 2025 •

edited

Loading

coveralls commented Mar 18, 2025 •

edited

Loading