Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

crypto/tls: no support for SSLv2 Client Hello #46899

Closed
mrutter-amzn opened this issue Jun 23, 2021 · 7 comments
Closed

crypto/tls: no support for SSLv2 Client Hello #46899

mrutter-amzn opened this issue Jun 23, 2021 · 7 comments
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.

Comments

@mrutter-amzn
Copy link

What version of Go are you using (go version)?

$ go version
go version go1.16.5 linux/amd64

Does this issue reproduce with the latest release?

Yes

What did you do?

This issue is to revisit #3930

That issue was closed because there weren't any strong use-cases that were still making use of SSLv2 Client Hello for compatibility: #3930 (comment)

We have a use-case at our company that we are unable to work around without considerable effort. We have a reverse proxy that performs TLS termination that has been deployed for thousands of services in tens of thousands of environments. We have written a new implementation of that proxy in Go.

The majority of those environments are behind hardware load balancers operating in TCP mode. We don't perform TLS termination with those load balancers because they are difficult and slow to update. The load balancers have been configured to use "ssl-ping" for health checks, where the load balancer sends an HTTPS request to /ping. We use this instead of "tcp-ping" because that mode only verifies that the load balancer can establish a TCP connection.

For health checks the hardware load balancers send an SSLv2 Client Hello to then negotiate up to TLS 1.0 in order to send the HTTPS request to /ping. Because crypto/tls does not support the SSLv2 Client Hello, our proxy rejects the request signaling to the load balancer that it is not healthy.

Options we see:

  1. Support for SSLv2 Client Hello is added to the standard library. 🎉
  2. We maintain a patch I adapted from issue 3930. Maintaining a patch that adds functionality, without external or expert review, is not ideal. This is our current stance.
  3. We update all of the hardware load balancers to use a TLS Client Hello. The team that owns the hardware load balancers is unwilling.
  4. We migrate all of the health checks from "ssl-ping" to "tcp-ping". This would be an availability risk since the ability to properly connect to the proxy will not be exercised when determining health.

What did you expect to see?

Successful TLS 1.0 handshakes from the load balancer using SSLv2 Client Hello for health checks.

What did you see instead?

TLS handshake failures.
@seankhliao
Copy link
Member

cc @FiloSottile

@seankhliao seankhliao added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Jun 23, 2021
@moldabekov
Copy link

@mrutter-amzn Michael, to be honest, by today SSLv2 and TLS 1.0 are insecure and obsolete protocols. As far as I know as a security engineer, you should upgrade to TLS 1.2.

Adding insecure/obsolete protocols to crypto/tls seems to be really bad practice.

@tmthrgd
Copy link
Contributor

tmthrgd commented Jun 24, 2021

Compare #45428.

@mrutter-amzn
Copy link
Author

@moldabekov - Yes, we totally agree on using TLS 1.2 (and 1.3 for clients that support it). We had to write logic to reject any non-ping requests that negotiate to TLS 1.0. This issue isn't to add the SSLv2 protocol to crytpo/tls, it is to add support for the SSLv2 style Client Hello which is then used to negotiate up to a higher TLS level. That means it is possible to support the SSLv2 style Client Hello while still setting the tls.Config to have a min version of TLS 1.2 and have successful negotiations to TLS 1.2 while rejecting older versions.

@tmthrgd - Thanks for pointing to that issue. As long as we can set a Config.MinVersion we are okay for proposed stages 1 and 2. We would be in a similar predicament as @ucirello when things progress to stage 3. We use and enforce higher TLS versions where possible, but we don't have that option with the specific use-case of hardware load balancers sending "ssl-ping" requests for health checks.

@networkimprov
Copy link

cc @katiehockman

@FiloSottile
Copy link
Contributor

Hi @mrutter-amzn! I can sympathize with how hard it is to upgrade legacy hardware, so I realize you’re in a tough spot.

However, your requirements for this are fundamentally different from the goal of the crypto/tls package: you need compatibility with a rare edge case regardless of its security implications (since you’d be willing to use an unencrypted connection if it was a good health indicator) while the standard library aims to provide a secure, safe, broadly useful, and modern implementation of TLS.

Although theoretically possible to implement SSLv2 Client Hellos for TLS 1.2 securely, it’s extra complexity in a critical path, and it’s not useful to the vast majority of our users, so unfortunately I can’t justify the trade-off, sorry!

@mrutter-amzn
Copy link
Author

Thanks for the consideration.

@golang golang locked and limited conversation to collaborators Jun 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@tmthrgd @networkimprov @moldabekov @FiloSottile @gopherbot @seankhliao @mrutter-amzn