-
-
Notifications
You must be signed in to change notification settings - Fork 30.4k
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
bpo-43279: Update code taken from Keccak Code Package #24601
Conversation
There were some updates to the Keccak package since it was integrated in CPython initially. History can be found in https://github.com/XKCP/XKCP. XKCP's contributors did some refactoring. In particular, they replaced `UINT64` and `UINT8` with `uint64_t` and `uint8_t`. Also, they added support for 64-bit big-endian platforms. The changes are reflected in sha3module.c. I replaced files in the kcp folder with ones generated with `generic64lc/libXKCP.a.pack` and `generic32lc/libXKCP.a.pack` targets. And removed PlSnP-Fallback.inc because it is not used.
Thanks for your patch, but I'm going to remove the entire |
Nice. BTW, it looks like a title of the PEP misses one digit in the OpenSSL version, it is "1.1 or newer" instead of "1.1.1 or newer" as mentioned later in the proposal. Also, I noticed a weird behavior related to SHA-3 in Python 3.8 that uses OpenSSL 1.1.1: >>> import hashlib
>>> hashlib.new("sha3_224")
<_sha3.sha3_224 object at 0x7f5a57e2ca30>
>>> hashlib.new("sha3-224")
<sha3_224 HASH object @ 0x7f5a57dc6c10> In the first case, it uses the built-in implementation. And in the second one, it uses an implementation from OpenSSL. I am not sure whether this is a feature or a bug that should be fixed 🙂. |
This PR is stale because it has been open for 30 days with no activity. |
@tiran congrats on landing PEP 644! 🎉 I can see that the |
@pablogsal What's your opinion on this patch? Ot updates the internal code base of _sha3 module. The code is only used when Python is compiled without OpenSSL support. I initially planned to remove the internal _sha3 module, but I won't have time to finish the task until tomorrow. I'm a bit worried to land the change. It's a fairly large change. The current code works and as (AFAIK) no problems. |
Unless is strictly necessary I would recommend to not do it, specially given the size of the diff (unless someone can review it thoroughly). We are still before beta so this could theoretically be merged but if you are going to remove the Ultimately is up to you although my recommendation is to at least do it after you remove the |
The update won't be needed when the |
Do you think the module can be removed at the current stage? |
There were some updates to the Keccak package since it was integrated in CPython initially.
History can be found in https://github.com/XKCP/XKCP.
XKCP's contributors did some refactoring. In particular, they replaced
UINT64
andUINT8
withuint64_t
anduint8_t
.Also, they added support for 64-bit big-endian platforms.
The changes are reflected in sha3module.c.
I replaced files in the kcp folder with ones generated with
generic64lc/libXKCP.a.pack
andgeneric32lc/libXKCP.a.pack
targets.And removed PlSnP-Fallback.inc because it is not used.
https://bugs.python.org/issue43279