crypto/tls: ECH decodeInnerClientHello incorrectly rejects ClientHello with GREASE values in supportedVersions

[gdy666]

Problem Description

In Golang's crypto/tls implementation, the function decodeInnerClientHello contains the following code:

if len(inner.supportedVersions) != 1 || (len(inner.supportedVersions) >= 1 && inner.supportedVersions[0] != VersionTLS13) {
    return nil, errors.New("tls: client sent encrypted_client_hello extension and offered incompatible versions")
}

This logic requires supportedVersions to contain exactly one version, which must be TLS 1.3, otherwise, the connection is rejected.

However, Chrome and Edge use the GREASE mechanism, which means supportedVersions may contain multiple values, such as:

supported_versions: [GREASE, TLS 1.3]

Since len(inner.supportedVersions) != 1, Golang incorrectly rejects these connections, causing Chrome and Edge to fail when accessing a Golang Web server with ECH enabled, while Firefox works fine.

Steps to Reproduce

Start a Golang crypto/tls server with ECH enabled.
Try accessing the server using Chrome or Edge.
The connection fails with a TLS error in Chrome.
Try accessing the server using Firefox.
The connection works fine.

Expected Behavior

The server should accept connections as long as supportedVersions contains TLS 1.3, rather than rejecting them due to the GREASE mechanism.

Suggested Fix

Modify decodeInnerClientHello logic to:

Ignore GREASE values (i.e., values with the format 0xXAXA).
Allow multiple versions in supportedVersions, as long as TLS 1.3 is included.
Proposed Code Change

hasTLS13 := false
  for _, v := range inner.supportedVersions {
      if v == VersionTLS13 {
          hasTLS13 = true
          break
      }
  }

  if !hasTLS13 {
      return nil, errors.New("tls: client sent encrypted_client_hello extension but did not offer TLS 1.3")
  }

This ensures compatibility with Chrome and Edge while maintaining ECH security.

Related Fix Proposal
🔗 Gerrit CL: https://go-review.googlesource.com/c/go/+/648015

[gdy666]

@rolandshoemaker

[gdy666]

Ensure ECH processing allows multiple versions while enforcing TLS 1.3+ requirement

	hasTLS13 := false
	for _, v := range inner.supportedVersions {
		if v == VersionTLS13 {
			hasTLS13 = true
		} else if v < VersionTLS13 {
			return nil, errors.New("tls: client sent encrypted_client_hello extension with unsupported versions")
		}
	}

	if !hasTLS13 {
		return nil, errors.New("tls: client sent encrypted_client_hello extension but did not offer TLS 1.3")
	}

[rolandshoemaker]

Ah, right, GREASE.

Checking for < VersionTLS13 is probably fine, although I guess skipping values > 0x0A0A would be more explicit. Either way we should have a comment indicating why we do this.

[gdy666]

啊，对，GREASE。

检查 可能没问题，尽管我想跳过值会更明确。无论哪种方式，我们都应该有一个注释来说明我们为什么要这样做。< VersionTLS13``> 0x0A0A

Thanks for the feedback!
I’ve updated the code to explicitly skip GREASE values (0x?A0A) and added a detailed comment explaining why we do this.
Let me know if any further refinements are needed!

[gopherbot]

Change https://go.dev/cl/648015 mentions this issue: crypto/tls: fix ECH compatibility

[gdy666]

@gopherbot please backport to go 1.24

[gopherbot]

Backport issue(s) opened: #73118 (for 1.24).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.

[gopherbot]

Change https://go.dev/cl/661936 mentions this issue: crypto/tls: fix ECH compatibility