Skip to content

Refactor tests for HMAC #130149

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.

Sign up for GitHub

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

Jump to bottom
Closed
picnixz opened this issue Feb 15, 2025 · 2 comments
Closed

Refactor tests for HMAC #130149

picnixz opened this issue Feb 15, 2025 · 2 comments
Assignees
@picnixz
Labels
tests Tests in the Lib/test dir type-feature A feature request or enhancement

Comments

@picnixz
Copy link
Member

picnixz commented Feb 15, 2025 •
edited by bedevere-app bot
Loading

The tests for HMAC are organized in such a way that makes the adoption of HACL* HMAC harder. More precisely, it'll be a bit harder to test the OpenSSL and the HACL* implementation of HMAC (for instance, some tests may or may not work with HACL* as they only support a subset of the hash functions supported by OpenSSL).

I am suggesting a way to make the test interface more generic using mixin classes. For instance, we have multiple ways of creating a HMAC object. It can be through hmac.HMAC or through hmac.new or through _hashlib.hmac_new. While hmac.new and hmac.HMAC are identical, _hashlib.hmac_new is different in the sense that the signature is different as well (and that the objects being returned are also different; they are not instances of hmac.HMAC but instances of _hashlib.HMAC, though they implement the same interface).

Now, once we add HACL* implementation, the interface will be similar to the Python implementation of HMAC and thus we need to test them both. Currently, it's hard to switch from the Python module implementation to the C implementation.

Linked PRs
@picnixz picnixz added tests Tests in the Lib/test dir type-feature A feature request or enhancement labels Feb 15, 2025
@picnixz picnixz self-assigned this Feb 15, 2025
@bedevere-app bedevere-app bot mentioned this issue Feb 15, 2025
Merged
1 task
picnixz added a commit that referenced this issue Mar 3, 2025
@picnixz
gh-130149: refactor tests for HMAC (#130150)
8f11af4 
Since we plan to introduce a built-in implementation for HMAC based on HACL*,
it becomes important for the HMAC tests to be flexible enough to avoid code
duplication.

In addition to the new layout based on mixin classes, we extend test coverage by
also testing the `__repr__` of HMAC objects and the HMAC one-shot functions.

We also fix the import to `_sha256` which, since gh-101924, resulted in some tests being
skipped as the module is no more available (its content was moved to the `_sha2` module).
@picnixz picnixz closed this as completed Mar 3, 2025
@picnixz
Copy link
Member Author

picnixz commented Mar 3, 2025

I won't backport this one because there will be too many conflicts. In addition, since we won't have multiple implementations of HMAC on 3.12 and 3.13, there is no need for the mixins.

@bedevere-app bedevere-app bot mentioned this issue Mar 3, 2025
Merged
picnixz added a commit that referenced this issue Mar 4, 2025
@picnixz
gh-130149: fix HMAC tests for some FIPS-only build bots (#130788)
15d926b 
Skips some HMAC tests for some FIPS-only build bots that do not have the underlying hash functions.
@picnixz picnixz reopened this Mar 16, 2025
@picnixz
Copy link
Member Author

picnixz commented Mar 16, 2025

I'm re-opening this one as I want to introduce some tweaks I did in #130157 instead of the other PR.

@bedevere-app bedevere-app bot mentioned this issue Mar 16, 2025
Merged
picnixz added a commit that referenced this issue Mar 17, 2025
@picnixz
gh-130149: cleanup refactorization of test_hmac.py (#131318)
de8890f 
New features:

* refactor `hashlib_helper.requires_hashdigest` in prevision of a future
  `hashlib_helper.requires_builtin_hashdigest` for built-in hashes only
* add `hashlib_helper.requires_openssl_hashdigest` to request OpenSSL
   hashes, assuming that `_hashlib` exists.

Refactoring:

* split hmac.copy() test by implementation
* update how algorithms are discovered for RFC test cases
* simplify how OpenSSL hash digests are requested
* refactor hexdigest tests for RFC test vectors
* typo fix: `assert_hmac_hexdigest_by_new` -> `assert_hmac_hexdigest_by_name`

Improvements:

* strengthen contract on `hmac_new_by_name` and `hmac_digest_by_name`
* rename mixin classes to better match their responsibility
@picnixz picnixz closed this as completed Mar 17, 2025
plashchynski pushed a commit to plashchynski/cpython that referenced this issue Mar 17, 2025
@picnixz @plashchynski
pythongh-130149: cleanup refactorization of test_hmac.py (python#13…
0cec3dd 
…1318)

New features:

* refactor `hashlib_helper.requires_hashdigest` in prevision of a future
  `hashlib_helper.requires_builtin_hashdigest` for built-in hashes only
* add `hashlib_helper.requires_openssl_hashdigest` to request OpenSSL
   hashes, assuming that `_hashlib` exists.

Refactoring:

* split hmac.copy() test by implementation
* update how algorithms are discovered for RFC test cases
* simplify how OpenSSL hash digests are requested
* refactor hexdigest tests for RFC test vectors
* typo fix: `assert_hmac_hexdigest_by_new` -> `assert_hmac_hexdigest_by_name`

Improvements:

* strengthen contract on `hmac_new_by_name` and `hmac_digest_by_name`
* rename mixin classes to better match their responsibility
seehwan pushed a commit to seehwan/cpython that referenced this issue Apr 16, 2025
@picnixz @seehwan80
pythongh-130149: refactor tests for HMAC (python#130150)
83690dc 
Since we plan to introduce a built-in implementation for HMAC based on HACL*,
it becomes important for the HMAC tests to be flexible enough to avoid code
duplication.

In addition to the new layout based on mixin classes, we extend test coverage by
also testing the `__repr__` of HMAC objects and the HMAC one-shot functions.

We also fix the import to `_sha256` which, since pythongh-101924, resulted in some tests being
skipped as the module is no more available (its content was moved to the `_sha2` module).
seehwan pushed a commit to seehwan/cpython that referenced this issue Apr 16, 2025
@picnixz @seehwan80
pythongh-130149: fix HMAC tests for some FIPS-only build bots (python…
dad5e92 
…#130788)

Skips some HMAC tests for some FIPS-only build bots that do not have the underlying hash functions.
seehwan pushed a commit to seehwan/cpython that referenced this issue Apr 16, 2025
@picnixz @seehwan80
pythongh-130149: cleanup refactorization of test_hmac.py (python#13…
0c129b1 
…1318)

New features:

* refactor `hashlib_helper.requires_hashdigest` in prevision of a future
  `hashlib_helper.requires_builtin_hashdigest` for built-in hashes only
* add `hashlib_helper.requires_openssl_hashdigest` to request OpenSSL
   hashes, assuming that `_hashlib` exists.

Refactoring:

* split hmac.copy() test by implementation
* update how algorithms are discovered for RFC test cases
* simplify how OpenSSL hash digests are requested
* refactor hexdigest tests for RFC test vectors
* typo fix: `assert_hmac_hexdigest_by_new` -> `assert_hmac_hexdigest_by_name`

Improvements:

* strengthen contract on `hmac_new_by_name` and `hmac_digest_by_name`
* rename mixin classes to better match their responsibility
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@picnixz picnixz

Labels
tests Tests in the Lib/test dir type-feature A feature request or enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@picnixz