api: add key update request functionality #4453
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
camshaft
approved these changes
Mar 8, 2024
camshaft
reviewed
Mar 8, 2024
| POSIX_ENSURE_REF(conn); | ||
| /* s2n-tls does not currently support requesting key updates from peers */ | ||
| POSIX_ENSURE(!peer_update_requested, S2N_ERR_T_USAGE); | ||
| s2n_atomic_flag_set(&conn->key_update_pending); |
There was a problem hiding this comment.
Maybe we should have another check to make sure that the handshake is complete?
There was a problem hiding this comment.
Theoretically not necessary? We just wouldn't send the update until after the handshake
There was a problem hiding this comment.
Well it'd be nice to be able to have the compliance comment about not sending this until the finished message has been sent, so I'll go ahead and add it.
There was a problem hiding this comment.
But this method doesn't send anything, so that'd be the wrong place for that compliance comment
There was a problem hiding this comment.
Yup, I'd add it as a test case in s2n_key_update_test.c
lrstewart
reviewed
Mar 8, 2024
lrstewart
reviewed
Mar 8, 2024
- return unimplemented err rather than generic type - explicitly mention that the key update message is not sent until s2n_send is called - explicitly mention that the method will fail is peer_update is set to false.
lrstewart
reviewed
Mar 8, 2024
- switch err to a usage type - switch to uint8_t for the input
- use enum intead of uint8_t for the toggle parameter
lrstewart
approved these changes
Mar 8, 2024
- cleanup singular vs plural nouns in documentation
camshaft
approved these changes
Mar 8, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of changes:
This commit adds a new API which allows a customer to signal their desire for a key update.
Testing:
Unit tests were added to cover new functionality.
Testing was done locally with other implementations to confirm that they receive the new key update. at the signaled time.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.