GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-99108: Implement HACL* HMAC by picnixz · Pull Request #130157 · python/cpython

Conversation

@picnixz

@picnixz picnixz commented Feb 15, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

This supersedes #126359.

Ideally, I'd like this to be part of 3.14 but we only have two alpha releases until the first beta.

Note that the HACL* HMAC implementation does not support truncated SHA-2-512/224 (which is different from SHA-2/224) so we need to either ask HACL* to do it or document it in the hmac module documentation.

For reviews, I would strongly advise review commit by commit instead of the entire file. Each commit should compile separately (assuming preceeding ones are present, maybe except the first few commits are not compiling due to some configure/build I forgot to put it at that time, but otherwise the interface was written incrementally).

cc @msprotz @gpshead

  • Issue: Replace built-in hashlib with verified implementations from HACL* #99108

📚 Documentation preview 📚: https://cpython-previews--130157.org.readthedocs.build/

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@bedevere-app bedevere-app Bot mentioned this pull request Feb 15, 2025
Replace built-in hashlib with verified implementations from HACL* #99108
Closed
@picnixz
picnixz force-pushed the feat/hmac/hacl-99108 branch 4 times, most recently from eb4fd4a to 62abfac Compare February 15, 2025 14:07
@picnixz picnixz mentioned this pull request Feb 15, 2025
gh-99108: add HACL*-based 1-shot HMAC implementation #126359
Closed
@picnixz
picnixz force-pushed the feat/hmac/hacl-99108 branch from 62abfac to 3381ac7 Compare February 15, 2025 14:11
@picnixz
Merge branch 'main' into feat/hmac/hacl-99108
20ce770
@picnixz
picnixz force-pushed the feat/hmac/hacl-99108 branch 2 times, most recently from 21fcd9a to 583c1f4 Compare February 15, 2025 14:15
@picnixz

picnixz commented Feb 15, 2025

Copy link
Copy Markdown
Member Author

@msprotz I'm not sure if it's an HACL issue or not, but Lib_IntVector_Intrinsics_vec256 in libintvector.h is conditionnally exposed, yet it's unconditionally used in _hacl/Hacl_Streaming_Types.h.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@msprotz

msprotz commented Feb 17, 2025

Copy link
Copy Markdown
Contributor

You are correct -- there is one additional step to do to handle this case. Note that HMAC is the first time we have a file that contains references to vec128/vec256 yet may be compiled on a system that has no such types. (Previously, Blake2b_256 was only ever built if vec256 was known at build-time to exist, and likewise with Blake2s_128.)

One option is to conditional includes, but that leads to other difficulties, such as having to hide cases of the agile hmac state union behind #ifdef, and then propagating #ifdefs everywhere in this file. This is error-prone and a lot of maintenance.

The other option we've done is simply to #define Lib_IntVector_Intrinsics_vec256 void * in case the build determines that HACL_CAN_COMPILE_VEC256 is false (and likewise with vec128). This solves all of the problems above and avoids a complicated include dance.

What I don't understand / remember, though, is why this isn't done directly inside libintvector.h. (We currently do it in our mini-configure for CI purposes and emit those #ifdefs in config.h.)

I'd be curious to see if you could manually patch libintvector.h to define those types to void* (in the #else case of #ifdef HACL_CAN_COMPILE_VEC256, and likewise for VEC128) and then see if we get a good build? I would then be happy to upstream this fix. Thank you!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@msprotz msprotz mentioned this pull request Feb 18, 2025
blake2module.c needs to be compiled with libhacl SIMD flags #130213
Closed
@picnixz picnixz mentioned this pull request Feb 25, 2025
gh-130213: Check availability of Intel SIMD types #130332
Closed
@picnixz
Merge remote-tracking branch 'upstream/main' into feat/hmac/hacl-99108
713fe1b
@picnixz
picnixz force-pushed the feat/hmac/hacl-99108 branch 2 times, most recently from a993e69 to 8ce1524 Compare March 12, 2025 12:04
@picnixz
Merge remote-tracking branch 'upstream/main' into feat/hmac/hacl-99108
bc2a2f8
@picnixz
picnixz force-pushed the feat/hmac/hacl-99108 branch from 8ce1524 to dcb89f5 Compare March 12, 2025 12:22
@picnixz

picnixz commented Mar 12, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

I'll wait for #130960 to be merged so that I don't need to re-update the SBOMs etc.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@picnixz picnixz mentioned this pull request Mar 15, 2025
Undefined references to HACL symbols when statically linking Python 3.12 #131298
Closed
@picnixz
Merge remote-tracking branch 'upstream/main' into feat/hmac/hacl-99108
9fc5a71
@picnixz
picnixz force-pushed the feat/hmac/hacl-99108 branch 7 times, most recently from 22e3003 to 56b3c8f Compare March 16, 2025 10:41
@picnixz picnixz mentioned this pull request Mar 16, 2025
Refactor tests for HMAC #130149
Closed
@gpshead

gpshead commented Mar 29, 2025

Copy link
Copy Markdown
Member

I'd (1) leave the SIMD detection for a followup PR and (2) keep the pedantic checks for the return code.

similarly, documentation updates we're not clear on the need for can be considered separately.

picnixz and chris-eibl reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@msprotz

msprotz commented Mar 29, 2025

Copy link
Copy Markdown
Contributor

Agreed that SIMD can be left for later -- this should be transparent from the point of view of the clients.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

chris-eibl
chris-eibl approved these changes Mar 29, 2025
View reviewed changes

@chris-eibl chris-eibl left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM. Thank you @picnixz!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@picnixz picnixz mentioned this pull request Mar 29, 2025
Unify recognization of message digest names for _hashlib and _hmac #131876
Open
hugovk
hugovk reviewed Mar 29, 2025
View reviewed changes
Comment thread Doc/whatsnew/3.14.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Lib/test/support/hashlib_helper.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

picnixz and others added 2 commits March 29, 2025 17:18
@picnixz @hugovk
narrow except guard
a5fc810
Co-authored-by: Hugo van Kemenade <[email protected]>
@picnixz
put hmac section before http section
b58073d
@picnixz
picnixz requested a review from hugovk March 30, 2025 08:29
AA-Turner
AA-Turner approved these changes Apr 1, 2025
View reviewed changes
Comment thread Lib/hmac.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Lib/hmac.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread PC/config.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Doc/whatsnew/3.14.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

picnixz added 5 commits April 1, 2025 11:44
@picnixz
cosmetic changes in hmac.py
6f84a82
- add two blank lines for separating functions
- use consistent truthy checks
@picnixz
group cryptographic primitives modules
de7ad74
@picnixz
better group cryptographic primitives
674af07
@picnixz
update naming
26cfeb0
@picnixz
update naming
258aa20
@picnixz picnixz added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Apr 1, 2025
@bedevere-bot

bedevere-bot commented Apr 1, 2025

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @picnixz for commit 258aa20 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F130157%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Apr 1, 2025
@picnixz picnixz added the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Apr 1, 2025
@bedevere-bot

bedevere-bot commented Apr 1, 2025

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @picnixz for commit 258aa20 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F130157%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot bedevere-bot removed the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Apr 1, 2025
@picnixz

picnixz commented Apr 1, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

I expect failures on some FIPS-only build bots but that's fine because the test suite is already failing on them. And some failures on other build bots as well (like aarch64 which is known to be flaky these past days). Once all build bots are fine, I'll merge it (but only on Friday, as I'm leaving in a few hours) [so please don't merge it since I want to write the commit message, TiA]

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@picnixz
Merge branch 'main' into feat/hmac/hacl-99108
629cbce
@picnixz
picnixz merged commit 0a97427 into python:main Apr 4, 2025
@bedevere-app bedevere-app Bot removed the awaiting merge label Apr 4, 2025
@picnixz
picnixz deleted the feat/hmac/hacl-99108 branch April 4, 2025 17:04
@picnixz

picnixz commented Apr 4, 2025

Copy link
Copy Markdown
Member Author

Thank you all for the feedback, especially @msprotz for the upstream work! The next step is to enable the SIMD support.

chris-eibl reacted with rocket emoji
All reactions
  • 🚀 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

picnixz added a commit to picnixz/cpython that referenced this pull request Apr 5, 2025
@picnixz
pythongh-99108: Implement HACL* HMAC (python#130157)
e115ce3
A new extension module, `_hmac`, now exposes the HACL* HMAC (formally verified) implementation.

The HACL* implementation is used as a fallback implementation when the OpenSSL implementation of HMAC
is not available or disabled. For now, only named hash algorithms are recognized and SIMD support provided
by HACL* for the BLAKE2 hash functions is not yet used.
@zanieb zanieb mentioned this pull request Apr 9, 2025
Bump to 3.14.0a7 astral-sh/python-build-standalone#580
Merged
@picnixz picnixz mentioned this pull request Apr 12, 2025
gh-131298: eliminate HACL* static libraries for cryptographic modules #132438
Merged
zanieb added a commit to astral-sh/python-build-standalone that referenced this pull request May 17, 2025
@zanieb @geofft
Bump to 3.14.0a7 (#580)
0964f0f
The big changes here are:

- Switching to zlib-ng on Windows
(python/cpython#131438)
- Using hmac for hashing functions
(python/cpython#130157)

---------

Co-authored-by: Geoffrey Thomas <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gpshead gpshead gpshead approved these changes
@AA-Turner AA-Turner AA-Turner approved these changes
@chris-eibl chris-eibl chris-eibl approved these changes
@sethmlarson sethmlarson Awaiting requested review from sethmlarson sethmlarson is a code owner
@tiran tiran Awaiting requested review from tiran
@ericsnowcurrently ericsnowcurrently Awaiting requested review from ericsnowcurrently ericsnowcurrently is a code owner
@erlend-aasland erlend-aasland Awaiting requested review from erlend-aasland erlend-aasland is a code owner
@corona10 corona10 Awaiting requested review from corona10 corona10 is a code owner
@hugovk hugovk Awaiting requested review from hugovk

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@picnixz picnixz

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

7 participants

@picnixz @msprotz @bedevere-bot @gpshead @hugovk @AA-Turner @chris-eibl
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.