Skip to content
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

Remove unused libcrypto HMAC code from s2n_prf #4094

Closed
wants to merge 2 commits into from

Conversation

goatgoose
Copy link
Contributor

@goatgoose goatgoose commented Jul 14, 2023

Description of changes:

The custom PRF implementation in s2n-tls is designed to swap the underlying HMAC implementation based on the FIPS mode. Normally, the custom HMAC implementation in s2n-tls is used, but if s2n-tls is operating in FIPS mode, the implementation is swapped to the libcrypto HMAC implementation.

However, after #4020, the PRF implementation itself is swapped to the libcrypto PRF implementation when s2n-tls is operating in FIPS mode. As such, the libcrypto HMAC implementation in s2n_prf is no longer needed, so it's removed in this PR.

Call-outs:

  • This change does impact the HMAC implementation for OpenSSL-FIPS in the PRF. Currently, OpenSSL-FIPS uses our custom PRF implementation backed with the HMAC implementation from the libcrypto. In all other usages of HMAC, OpenSSL-FIPS uses our custom HMAC implementation. Now, OpenSSL-FIPS also uses our custom HMAC implementation in the PRF.
  • hmac->cleanup and s2n_prf_wipe() currently call s2n_hmac_reset(). Resetting doesn't actually clean/wipe the state, though. Instead, it prepares the state for a future HMAC calculation with the same key and algorithm. It seemed more correct to actually wipe the state in these places instead, so I changed it.

Testing:

Existing s2n_prf unit tests.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@github-actions github-actions bot added the s2n-core team label Jul 14, 2023
@goatgoose goatgoose force-pushed the fips-hmac-clean-prf branch 2 times, most recently from 518c792 to 7c3533e Compare July 14, 2023 04:13
@goatgoose goatgoose force-pushed the fips-hmac-clean-prf branch from 7c3533e to 30a3ec4 Compare July 14, 2023 04:37
@goatgoose goatgoose marked this pull request as ready for review July 14, 2023 16:16
@goatgoose goatgoose requested review from toidiu and lrstewart July 14, 2023 16:16
@goatgoose
Copy link
Contributor Author

goatgoose commented Jul 17, 2023

Postponing this change until after the libcrypto HMAC implementation is added to s2n_hmac.

@goatgoose goatgoose closed this Jul 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant