dotnet / dotnet/yarp

Parse more of the ClientHello when using SSL3 in the record layer

Open
#2,821 14 comments 0 reactions 0 assignees View on GitHub
Type: Bug
Dominant language
C#
Stars
9.6k
Forks
933
Avg merge
12d 18h
Merged PRs (30d)
2

Description

### Describe the bug

Using the `TlsFrameHelper` to parse ClientHello doesn't populate very much information if the record layer version is SSL3.0. Specifically, the `TlsFrameInfo.SupportedVersions` isn't populated with the ClientHello version (which is likely TLS1.2 or higher). This means application level logic can't easily do early SSL/TLS version checks to reject requests.

This is due to the following check
https://github.com/dotnet/yarp/blob/68f81c455e34bd1ab47bd09ca8ec3590af060a32/src/ReverseProxy/Utilities/TlsFrameHelper.cs#L325
SSL3.0 is less than TLS1.0 so the rest of the ClientHello parsing is skipped.

From the TLS1.2 Protocol RFC
https://www.rfc-editor.org/rfc/rfc5246#appendix-E
> TLS versions 1.0, 1.1, and 1.2, and SSL 3.0 are very similar, and use
compatible ClientHello messages; thus, supporting all of them is
relatively easy.

> Earlier versions of the TLS specification were not fully clear on
what the record layer version number (TLSPlaintext.version) should
contain when sending ClientHello (i.e., before it is known which
version of the protocol will be employed). Thus, TLS servers
compliant with this specification MUST accept any value {03,XX} as
the record layer version number for ClientHello.
> TLS clients that wish to negotiate with older servers MAY send any
value {03,XX} as the record layer version number. Typical values
would be {03,00}

It seems like the check could easily be changed to `>= SslProtocols.Ssl3`. I did a quick test with this and it looks like it correctly parsed the data and showed `Ssl3 | Tls12` in `SupportedVersions`.

### To Reproduce

Send a TLS request with SSL3 as the version in the record layer and TLS1.2 as the ClientHello version.

### Further technical details

N/A

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.