1755264 - NSS TLS 1.3 Session not aborted bugs

Status: RESOLVED FIXED

(NSS :: Libraries, enhancement)

Description

[Leander Schwarz]

[C+S] upon receiving an interleaved handshake record ("That is, if a handshake message is split over two or more records, there MUST NOT be any other records between them." RFC 8446 - 5. Record Layer) NSS does not always terminate the session

RFC 8446 - 5. Record Layer
Handshake messages MUST NOT be interleaved with other record types.

[C+S] upon receiving zero-length handshake fragments

RFC 8446 - 6.2.1 Fragmentation
Implementations MUST NOT send zero-length fragments of Handshake, Alert, or >ChangeCipherSpec content types. Zero-length fragments of Application data MAY be sent >as they are potentially useful as a traffic analysis countermeasure.

[S] upon receiving a ClientHello that negotiates TLS 1.3 but does not set the legacy version field to 0x0303

RFC 8446 - 4.1.2 Client Hello
In TLS 1.3, the client indicates its version preferences in the "supported_versions" >extension (Section 4.2.1) and the legacy_version field MUST be set to 0x0303, which is the >version number for TLS 1.2.

Comment 1

[Leander Schwarz]

Attachment: Bug 1755264 - TLS 1.3 Illegal legacy_version handling/alerts. r=djackson

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:lschwarz, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit auto_nag documentation.

Comment 3

[Leander Schwarz]

Attachment: Bug 1755264 - Added TLS 1.3 zero-length inner plaintext checks and tests, zero-length record/fragment handling tests. Enabled tls fuzzer empty alert test. r?djackson

Comment 4

[Leander Schwarz]

For all of the reported sub-bugs the RFC states that different things MUST NOT be done, but does NOT state that an alert MUST be sent and/or the connection be aborted. Following are the RFC statements cited in the reports:

[C+S] upon receiving an interleaved handshake record ("That is, if a handshake message is split over two or more records, there MUST NOT be any other records between them." RFC 8446 - 5. Record Layer) NSS does not always terminate the session.

[RFC8446, Section 5.1]:

Handshake messages MUST NOT be interleaved with other record
types. That is, if a handshake message is split over two or more
records, there MUST NOT be any other records between them.

=> No changes since the handling of this error is not specified.

[C+S] upon receiving zero-length handshake fragments.

The reporter incorrectly cited [RFC5426, Section 6.2.1], which is deprecated by the statements in
[RFC8446, Section 5.1]:

Implementations MUST NOT send zero-length fragments of Handshake
types, even if those fragments contain padding.

[RFC8446, Section 5.4]:

Implementations MUST NOT send Handshake and Alert records that have a zero-length
TLSInnerPlaintext.content; if such a message is received, the
receiving implementation MUST terminate the connection with an
"unexpected_message" alert.

=> Revision D141841 adds checks and alerts as specified in the second statement for ENCRYPTED records.

[S] upon receiving a ClientHello that negotiates TLS 1.3 but does not set the legacy version field to 0x0303.

[RFC8446, Section 4.1.2]:

In TLS 1.3, the client indicates its version preferences in the
"supported_versions" extension (Section 4.2.1) and the
legacy_version field MUST be set to 0x0303, which is the version
number for TLS 1.2.

[RFC8446, Appendix D.5]:

Implementations MUST NOT send a ClientHello.legacy_version or
ServerHello.legacy_version set to 0x0300 or less. Any endpoint
receiving a Hello message with ClientHello.legacy_version or
ServerHello.legacy_version set to 0x0300 MUST abort the handshake
with a "protocol_version" alert.

=> The RFC only specifies an alert and termination of the connection for legacy_version fields set to 0x0300 (SSL 3.0).
This was added on the client side in D138647, the server already checks this.

Comment 5

[Dennis Jackson]

https://hg.mozilla.org/projects/nss/rev/7d931c59d09f68037c30cdbb59614fde8c33e735

Comment 6

[Leander Schwarz]

https://hg.mozilla.org/projects/nss/rev/144c87accae8ac9e60946c7684cb378880a921a0