[crypto/4871] Add support for SHAKE128 and SHAKE256.

[marcellanz]

This should fix #4871. Passes tests with /lib/crypto for openssl@3/3.0.5 and [email protected]/1.1.1q.

[github-actions[bot]]

CT Test Results

    2 files    14 suites   5m 18s :stopwatch: 183 tests 169 :heavy_check_mark:   14 :zzz: 0 :x: 462 runs  329 :heavy_check_mark: 133 :zzz: 0 :x:

Results for commit fb86085f.

:recycle: 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

• Complete CT logs (Download Logs)
• HTML Documentation (Download HTML Docs)
• Windows Installer

// Erlang/OTP Github Action Bot

[HansN]

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

[marcellanz]

Thanks :) I missed that case and will adapt to have returned EXCP_NOTSUP for OPENSSL_VERSION_NUMBER < 1.0. With returning EXCP_NOTSUP, consequently, I would not remove shake128 and shake256 from crypto:supports(hashs) at all when compiled with OpenSSL < 1.0, right?

[HansN]

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 #ifdef HAVE_SHAKExxx in algorithms.c.

You could then #undef HAVE_SHAKExxx in openssl_config.h for versions you do not want to support with old function calls.

[marcellanz]

Got it – I think. So hash_final_xof_nif is defined unconditionally in hash.h as OTP could support XOF functions. HAVE_SHAKExxx flags are defined as soon as openssl 1.1.1 or above is choosen for the build and their implementation in hash.c depends on the two HAVE_SHAKExxx flags, with one falling back implementation returning EXCP_NOTSUP if called but not supported.

Implemented with fb86085

[HansN]

Thanks for the PR!

[marcellanz]

Thanks for the PR!

You're welcome. Thanks for guiding me through the details.

[lorenzosinisi]

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?

[marcellanz]

Thanks @lorenzosinisi. As it introduces addition to library API I assume it would be released with the next major version of OTP? I think it would be listed here https://github.com/erlang/otp/milestone/162?closed=1 but its not so far.

[lorenzosinisi]

Thanks @lorenzosinisi. As it introduces addition to library API I assume it would be released with the next major version of OTP?

I think it would be listed here https://github.com/erlang/otp/milestone/162?closed=1 but its not so far.

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

[IngelaAndin]

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.