Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

gh-99108: Initial import of HACL-SHA3 into Python#103597

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.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
gpshead merged 15 commits intopython:mainfrommsprotz:protz_sha3
May 8, 2023

Conversation

msprotz
Copy link
Contributor

@msprotzmsprotz commentedApr 17, 2023
edited by bedevere-bot
Loading

I have a PR that's close to landing upstream (hacl-star/hacl-star#803). If there are any comments here, I'll be happy to tweak the PR upstream.

This replaces the in-tree SHA3 implementation for hashlib with a verified version from the HACL* project.

A few remarks:

  • just like the original implementation, the HACL implementation uses a single API for all six Keccak variants
  • I did not understand why there was a lock in sha3module.c, along with critical sections (ENTER_HASHLIB, etc.) when none of the other hashing modules have them -- I have removed this part

@gpshead please let me know what you think! I'm around, and can help make sure this gets into 3.12

Eclips4, thinkwelltwd, and erlend-aasland reacted with rocket emoji
@gpsheadgpshead self-assigned thisApr 19, 2023
@msprotz
Copy link
ContributorAuthor

I've refreshed this PR with a newer upstream.

  • Eliminates an unused variable.
  • Switches to a saner API of finish (SHA3) and squeeze (extra length argument, only for Shake).

@gpshead the upstream PR will land soon, let me know if there's anything I can do to help make sure this gets into 3.12!

@arhadthedevarhadthedev added stdlibPython modules in the Lib dir topic-SSL labelsMay 3, 2023
@msprotz
Copy link
ContributorAuthor

I've landed the upstream PR, meaning that this is now pulling from HACL*'s main branch. I see fromhttps://peps.python.org/pep-0693/ that the first 3.12 beta is in just a few days. I'll keep an eye on this PR in case any change or fixup is needed. Thanks!

gpshead reacted with thumbs up emoji

@gpsheadgpshead added the 🔨 test-with-buildbotsTest PR w/ buildbots; report in status section labelMay 5, 2023
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by@gpshead for commitb15b524 🤖

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

@bedevere-botbedevere-bot removed the 🔨 test-with-buildbotsTest PR w/ buildbots; report in status section labelMay 5, 2023
@gpshead
Copy link
Member

At first glance I believe this looks good, I'll get it in. I'm running it through the buildbots to see if anything unusual build-wise falls out to be addressed.(expect some flaky infrastructure or pre-existing issue noise among those results as well)

msprotz reacted with thumbs up emoji

@msprotz
Copy link
ContributorAuthor

msprotz commentedMay 5, 2023
edited
Loading

Thanks, so far only the refleaks test seems relevant -- it says hashlib leaked. Is there any way to get a trace of the allocations that weren't freed? Nothing jumps out at me looking at the code right now, and I don't believe anything was done differently compared to the other hashes (for which there were no leaks issues).

EDIT: used OSX'sleaks tool in conjunction with MallocStackLogging, but I'm getting allocation traces from within the interpreter, which isn't super useful. I'll keep trying to figure this out, but of course any pointers are appreciated. Thanks.

EDIT2: tried tracing the calls to malloc performed within HACL, but the addresses don't seem to match whatleaks tells me has leaked... maybe the leaked objects are Python objects?

gpshead reacted with thumbs up emoji

@msprotz
Copy link
ContributorAuthor

I merged master in and I think I found the missing PyBuffer_Release. Can you run buildbot again?

gpshead reacted with thumbs up emoji

@gpsheadgpshead added the 🔨 test-with-buildbotsTest PR w/ buildbots; report in status section labelMay 6, 2023
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by@gpshead for commit43f4ba7 🤖

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

@bedevere-botbedevere-bot removed the 🔨 test-with-buildbotsTest PR w/ buildbots; report in status section labelMay 6, 2023
@msprotz
Copy link
ContributorAuthor

This is the only error that shows up (WASI build):

ERROR: test_with_segments (test.test_pathlib.PathSubclassTest.test_with_segments)ERROR: test_with_segments (test.test_pathlib.PathTest.test_with_segments)ERROR: test_with_segments (test.test_pathlib.PosixPathTest.test_with_segments)

This doesn't seem to be related, does it?

gpshead reacted with thumbs up emoji

@msprotz
Copy link
ContributorAuthor

msprotz commentedMay 6, 2023
edited
Loading

@AA-Turner I pushed a commit to enforce better ordering.

I'm not sure how the list is ordered, though: from_abc.c to_weakref.c are files starting with an underscore, correctly sorted. Then, there is a mix of underscore-prefixed files and non-underscore-prefixed files, sorted alphabetically,after removing the initial underscore:arraymodule.c, ...,_operator.c,posixmodule.c, and so on.

I did my best to try to make the list look sensible. I hope this addresses your comments, let me know if it doesn't. Thanks!

@AA-Turner
Copy link
Member

AA-Turner commentedMay 6, 2023
edited
Loading

@zooba would likely be the best to comment on whatshould be the case, though your changes seem alright to me -- thank you.

A

msprotz reacted with thumbs up emoji

@msprotz
Copy link
ContributorAuthor

Great, thanks. I'm also happy to do a followup PR to fix things up after this one lands, if need be.

@gpsheadgpshead merged commit15665d8 intopython:mainMay 8, 2023
@msprotzmsprotz deleted the protz_sha3 branchMay 8, 2023 05:04
jbower-fb pushed a commit to jbower-fb/cpython that referenced this pull requestMay 8, 2023
)Replaces our built-in SHA3 implementation with a verified one from the HACL* project.This implementation is used when OpenSSL does not provide SHA3 or is not present.3.11 shiped with a very slow tiny sha3 implementation to get off of the <=3.10 reference implementation that wound up having serious bugs. This brings us back to a reasonably performing built-in implementation consistent with what we've just replaced our other guaranteed available standard hash algorithms with: code from the HACL* project.---------Co-authored-by: Gregory P. Smith <greg@krypto.org>
carljm added a commit to carljm/cpython that referenced this pull requestMay 9, 2023
* main: (47 commits)pythongh-97696 Remove unnecessary check for eager_start kwarg (python#104188)pythonGH-104308: socket.getnameinfo should release the GIL (python#104307)pythongh-104310: Add importlib.util.allowing_all_extensions() (pythongh-104311)pythongh-99113: A Per-Interpreter GIL! (pythongh-104210)pythonGH-104284: Fix documentation gettext build (python#104296)pythongh-89550: Buffer GzipFile.write to reduce execution time by ~15% (python#101251)pythongh-104223: Fix issues with inheriting from buffer classes (python#104227)pythongh-99108: fix typo in Modules/Setup (python#104293)pythonGH-104145: Use fully-qualified cross reference types for the bisect module (python#104172)pythongh-103193: Improve `getattr_static` test coverage (python#104286)  Trim trailing whitespace and test on CI (python#104275)pythongh-102500: Remove mention of bytes shorthand (python#104281)pythongh-97696: Improve and fix documentation for asyncio eager tasks (python#104256)pythongh-99108: Replace SHA3 implementation HACL* version (python#103597)pythongh-104273: Remove redundant len() calls in argparse function (python#104274)pythongh-64660: Don't hardcode Argument Clinic return converter result variable name (python#104200)pythongh-104265 Disallow instantiation of `_csv.Reader` and `_csv.Writer` (python#104266)pythonGH-102613: Improve performance of `pathlib.Path.rglob()` (pythonGH-104244)pythongh-103650: Fix perf maps address format (python#103651)pythonGH-89812: Churn `pathlib.Path` methods (pythonGH-104243)  ...
carljm added a commit to carljm/cpython that referenced this pull requestMay 9, 2023
* main: (29 commits)pythongh-104276: Make `_struct.unpack_iterator` type use type flag instead of custom constructor (python#104277)pythongh-97696: Move around and update the whatsnew entry for asyncio eager task factory (python#104298)pythongh-103193: Fix refleaks in `test_inspect` and `test_typing` (python#104320)  require-pr-label.yml: Add missing "permissions:" (python#104309)pythongh-90656: Add platform triplets for 64-bit LoongArch (LA64) (python#30939)pythongh-104180: Read SOCKS proxies from macOS System Configuration (python#104181)pythongh-97696 Remove unnecessary check for eager_start kwarg (python#104188)pythonGH-104308: socket.getnameinfo should release the GIL (python#104307)pythongh-104310: Add importlib.util.allowing_all_extensions() (pythongh-104311)pythongh-99113: A Per-Interpreter GIL! (pythongh-104210)pythonGH-104284: Fix documentation gettext build (python#104296)pythongh-89550: Buffer GzipFile.write to reduce execution time by ~15% (python#101251)pythongh-104223: Fix issues with inheriting from buffer classes (python#104227)pythongh-99108: fix typo in Modules/Setup (python#104293)pythonGH-104145: Use fully-qualified cross reference types for the bisect module (python#104172)pythongh-103193: Improve `getattr_static` test coverage (python#104286)  Trim trailing whitespace and test on CI (python#104275)pythongh-102500: Remove mention of bytes shorthand (python#104281)pythongh-97696: Improve and fix documentation for asyncio eager tasks (python#104256)pythongh-99108: Replace SHA3 implementation HACL* version (python#103597)  ...
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@asottileasottileasottile left review comments

@AA-TurnerAA-TurnerAA-Turner left review comments

@gpsheadgpsheadgpshead approved these changes

@tirantiranAwaiting requested review from tirantiran is a code owner

Assignees

@gpsheadgpshead

Labels
stdlibPython modules in the Lib dirtopic-SSL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

6 participants
@msprotz@bedevere-bot@gpshead@AA-Turner@asottile@arhadthedev

[8]ページ先頭

©2009-2025 Movatter.jp