docs: clarify integrity protection requirements for connection serialization#5782
Merged
CarolYeh910 merged 1 commit intomainfrom Mar 19, 2026
Merged
docs: clarify integrity protection requirements for connection serialization#5782CarolYeh910 merged 1 commit intomainfrom
CarolYeh910 merged 1 commit intomainfrom
Conversation
alexw91
approved these changes
Mar 11, 2026
CarolYeh910
approved these changes
Mar 18, 2026
| * are trusted after basic format validation. Callers must verify the integrity of the | ||
| * buffer before calling this function. See the s2n_connection_deserialize API docs. | ||
| */ | ||
|
|
Contributor
There was a problem hiding this comment.
Maybe move above the function definition to highlight it?
Contributor
Author
There was a problem hiding this comment.
I've got the main comment on the function declaration in s2n.h for users actually integrating with it, this one is more for those poking around inside the implementation so I think its ok here
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Goal
Strengthen the documentation around connection serialization to make the lack of built-in integrity protection explicit and actionable.
Why
The serialized connection blob contains cryptographic secrets and security-critical parameters (cipher suite, sequence numbers, protocol version) but does not include a MAC or signature. The existing documentation mentioned that users should "MAC and encrypt" the blob, but did not explain why integrity protection matters or what happens when it's missing. This could lead users to overlook the integrity requirement, especially if they only read one of the three documentation surfaces (usage guide, serialize API doc, or deserialize API doc).
How
docs/usage-guide/topics/ch14-connection-serialization.md: Rewrote the warning block as a single cohesive piece that explains what the blob contains, that it has no MAC or signature, and that users must provide both secrecy and integrity.api/s2n.h(s2n_connection_serializeands2n_connection_deserialize): Updated the warning to note that the blob has no MAC or signature and that a modified buffer will be deserialized as-is. Upgraded the recommendation from "it is recommended" to "callers MUST" to match the serialize doc's tone. Added encrypt-then-MAC as the recommended approach.tls/s2n_connection_serialize.c: Added a brief comment at thes2n_connection_deserializeentry point noting the trust assumption and pointing to the API docs.Testing
Documentation-only change.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.