On Mon, 22 Sep 2025 19:48:20 GMT, Alice Pellegrini <[email protected]> wrote:
> According to RFC 8446 section 5.1
>> Handshake messages MUST NOT span key changes. Implementations
>> MUST verify that all messages immediately preceding a key change
>> align with a record boundary; if not, then they MUST terminate the
>> connection with an "unexpected_message" alert. Because the
>> ClientHello, EndOfEarlyData, ServerHello, Finished, and KeyUpdate
>> messages can immediately precede a key change, implementations
>> MUST send these messages in alignment with a record boundary.
>
> The TLS implementation does not fail with alert(fatal, unexpected_message)
> when a KeyUpdate record is not on a record boundary, but this is required by
> the specification (as a key change happens immediately after a key update
> record)
>
>
> Since the data on whether a message aligns with a record boundary is only
> known in the implementations of `InputRecord` (as even incomplete parts of
> other handshake messages, if coming after one of the mentioned handshakes
> records, would require a failure, making checking that said message is the
> last complete one of that record insufficient), and the fact that, **if the
> negotiated protocol is TLS13** _(or even DTLS13 in the future)_, knowing that
> any of the mentioned messages did not align with the record boundary is
> enough to fail the connection, I am proposing to add this as a method of
> `InputRecord`;
>
> In addition, even if the handshake context was accessible from within
> `InputRecord`, for both ServerHello and ClientHello the negotiated protocol
> version is not known when the input record is decoded.
>
> The change mentions the name of the message currently being consumed in the
> exception because (since the messages are consumed in the order in which they
> appear in the record's body, and the groups of messages contained in each
> record are consumed in the order in which said records were delivered) it can
> be shown that if that flag is set, the first consumer that calls
> `tls13keyChangeHsExceedsRecordBoundary` will correspond to the first message
> to violate the boundary requirement, among the messages in the record it was
> found in.
>
> <br/><br/>
>
> I would appreciate suggestions on how to make the code better, especially in
> terms of where and how to store the fact that the violation might (if the
> negotiated protocol is or will be TLS13) have happened, and where to put the
> comments mentioning the specification RFC8446, for example in the
> `InputRecord` base class or the TLS13 Consumers that were modified.
test/jdk/sun/security/ssl/SSLEngineImpl/TLS13UnalignedKeyChangeHSMessage.java
line 67:
> 65:
> 66: private ContextParameters getContextParameters() {
> 67: return new ContextParameters("TLSv1.3", "PKIX", "SunX509");
Would be helpful to have a run of this test using TLSv1.2 protocol to verify
nothing fails there.
test/jdk/sun/security/ssl/SSLEngineImpl/TLS13UnalignedKeyChangeHSMessage.java
line 95:
> 93: if (sTOc.remaining() > 5
> 94: && (serverHelloMsg =
> 95: extractHandshakeMsg(sTOc, SERVER_HELLO_ID,
> false)
It would be nice to also test other messages like KeyUpdate and ClientHello.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27437#discussion_r2392783194
PR Review Comment: https://git.openjdk.org/jdk/pull/27437#discussion_r2392780458
