Skip to content

fix(ktls): Validate record type in s2n_ktls_recvmsg - #6030

Open
alexw91 wants to merge 1 commit into
aws:mainfrom
alexw91:ktls-record-validation
Open

fix(ktls): Validate record type in s2n_ktls_recvmsg#6030
alexw91 wants to merge 1 commit into
aws:mainfrom
alexw91:ktls-record-validation

Conversation

@alexw91

@alexw91 alexw91 commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Goal

Validate the TLS ContentType received via kTLS ancillary data in s2n_ktls_recvmsg.

Why

RFC 8446 Section 5: "If a TLS implementation receives an unexpected record type, it MUST terminate the connection with an 'unexpected_message' alert."

How

Range check after extracting the record type from the control message. Only values 21–23 (alert, handshake, application_data) are accepted. All others fail with S2N_ERR_BAD_MESSAGE.

Testing

Exhaustive test iterating all 256 byte values as the record type. Values {21, 22, 23} succeed; all others are rejected.

Related

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

@jmayclin
jmayclin self-requested a review August 10, 2026 21:49
@alexw91
alexw91 force-pushed the ktls-record-validation branch from dc8f4fc to b501a6b Compare August 10, 2026 22:05
Reject unexpected TLS ContentType values received via kTLS ancillary
data. Only alert (21), handshake (22), and application_data (23) are
valid post-handshake record types. All other values now fail with
S2N_ERR_BAD_MESSAGE per RFC 8446 Section 5.

Add an exhaustive unit test that injects all 256 possible byte values
as the record type and verifies the accept/reject behavior.
@alexw91
alexw91 force-pushed the ktls-record-validation branch from b501a6b to a063ced Compare August 10, 2026 23:06

@jmayclin jmayclin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This kind feels like the wrong place to handle this. All of the record types eventually bubble up through s2n_read_full_record, so I'd think that any validation we're doing should probably just be done there?

Also it might be easier to pause this until #6017, since that actually does some similar assertion cleanup?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants