-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
[crypto/4871] Add support for SHAKE128 and SHAKE256. #6203
[crypto/4871] Add support for SHAKE128 and SHAKE256. #6203
Conversation
CT Test Results 2 files 14 suites 5m 18s ⏱️ Results for commit fb86085. ♻️ This comment has been updated with latest results. To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass. See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally. Artifacts// Erlang/OTP Github Action Bot |
4f3bad6
to
9dbf022
Compare
9dbf022
to
710ef35
Compare
Great work! And complete with tests and documentation - very good! But, for certain reasons it must be compileable also with OpenSSL versions less than 1.1.1. Less that 0.9.8c is not needed. It is not necessary to return any other result than that is not supported when running given that the algorithms are not present in crypto:supported. If the PR is compiled with 1.1.0 or less the function hash_final_xof_nif is not declared. We usually solve this by another declaration in the #else part which just only returns an EXCP_NOTSUP |
Thanks :) |
If an algorithm is present in crypto:supports, it should return a value and not NOTSUP. That exception means that the algorithm is supported by Erlang/OTP, but not by the current cryptolib. The algorithm is therefore not in crypto:supports for the current cryptolib. So yes, keep the You could then |
…on SHAKExxx flags set by openssl_config.h.
Got it – I think. So Implemented with fb86085 |
Thanks for the PR! |
You're welcome. Thanks for guiding me through the details. |
Hi and great work on this particular algo! It will be very useful :) May I ask what is needed to include it in the next release and why wasn't it added in the latest one already? Is there any way to help? |
Thanks @lorenzosinisi. As it introduces addition to library API I assume it would be released with the next major version of OTP? |
Thank you! I'll take a closer look there 👍👍 yes I can imagine how the APIs would change given that shake256 would take also additional args |
This PR was merged to master and will hence be part of OTP-26. We are still adopting our ways of working with GitHub features so I guess it never got a milestone attached to it. |
This should fix #4871.
Passes tests with /lib/crypto for
openssl@3/3.0.5
andopenssl@1.1/1.1.1q
.