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

Allow verification of signatures before sending #3482

Merged
merged 8 commits into from
Sep 13, 2022
Merged

Conversation

lrstewart
Copy link
Contributor

@lrstewart lrstewart commented Sep 9, 2022

Description of changes:

Add a new API that turns on signature verification.

We already have a similar API, s2n_config_set_async_pkey_validation_mode, which only applies to signatures that use the async pkey operations. The verification is more useful for async operations because it can help detect the case where the operation is performed with the wrong key.

This new API applies to all signatures, both sync and async.

Call-outs:

  • We could deprecate s2n_config_set_async_pkey_validation_mode. However, it's not actually worse than the new API, just more narrowly focused.
    • Alternatively, we could just expand the scope of s2n_config_set_async_pkey_validation_mode to include sync signatures instead of adding a new API. Since you can actually use the async pkey callbacks to do pkey operations synchronously, it wouldn't really be a massive departure. But the the name would still be very deceptive.

Testing:

Test cases:

  • Async signing, valid signature: covered
  • Async signing, invalid signature: covered
  • Sync signing, valid signature: covered
  • Sync signing, invalid signature: NOT COVERED. I'm not sure how to test this with a unit or integ test, since it's not actually possible without libcrypto bugs or hardware issues.

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 Sep 9, 2022
@lrstewart lrstewart marked this pull request as ready for review September 9, 2022 22:44
@lrstewart lrstewart requested a review from a team as a code owner September 9, 2022 22:44
tls/s2n_async_pkey.c Show resolved Hide resolved
api/s2n.h Outdated Show resolved Hide resolved
tls/s2n_config.h Outdated Show resolved Hide resolved
api/s2n.h Outdated Show resolved Hide resolved
tls/s2n_async_pkey.c Show resolved Hide resolved
tls/s2n_async_pkey.c Show resolved Hide resolved
@lrstewart lrstewart enabled auto-merge (squash) September 12, 2022 22:17
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.

4 participants