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

crypto/tls: support ECDHE key exchanges when ec_point_formats is missing in ClientHello extension #49126

Open
yang-wei opened this issue Oct 23, 2021 · 5 comments · May be fixed by #49127
Open

crypto/tls: support ECDHE key exchanges when ec_point_formats is missing in ClientHello extension #49126

yang-wei opened this issue Oct 23, 2021 · 5 comments · May be fixed by #49127
Assignees
Labels
NeedsInvestigation
Milestone

Comments

@yang-wei
Copy link

@yang-wei yang-wei commented Oct 23, 2021

What did you do?

Per rfc8422#section-5.1.2,

For backwards compatibility purposes, the point format list extension MAY still be included and contain exactly one value: the uncompressed point format (0). RFC 4492 specified that if this extension is missing, it means that only the uncompressed point format is supported, so interoperability with implementations that support the uncompressed format should work with or without the extension

We are seeing TLS handshake failure (client and server failed to agree on ECDHE_ECDSA key exchange algorithem) when ec_point_formats is missing because we expect it to be listed in tls/handshake_server.go

// supportsECDHE returns whether ECDHE key exchanges can be used with this
// pre-TLS 1.3 client.
func supportsECDHE(c *Config, supportedCurves []CurveID, supportedPoints []uint8) bool {
...
	supportsPointFormat := false
	for _, pointFormat := range supportedPoints {
		if pointFormat == pointFormatUncompressed {
			supportsPointFormat = true
			break
		}
	}

	return supportsCurve && supportsPointFormat
}

What did you expect to see?

If ec_point_formats is missing in ClientHello, we will allow ECDHE key exchanges because it means that only the uncompressed point format is supported

yang-wei added a commit to yang-wei/go that referenced this issue Oct 23, 2021
As describe in rfc8422 5.1.2, we will support ECDHE in the case client does not
include ec_point_formats extension in ClientHello extension. This make sure ECDHE
will work with (uncompressed point format is listed explicitly) or without extension.

rfc8422 5.1.2: https://datatracker.ietf.org/doc/html/rfc8422#section-5.1.2.

Fixes golang#49126
@gopherbot
Copy link

@gopherbot gopherbot commented Oct 23, 2021

Change https://golang.org/cl/358116 mentions this issue: crypto: support ECDHE when ec_point_formats is missing in ClientHello

@seankhliao seankhliao added the NeedsInvestigation label Oct 24, 2021
@seankhliao
Copy link
Contributor

@seankhliao seankhliao commented Oct 24, 2021

@yang-wei
Copy link
Author

@yang-wei yang-wei commented Nov 18, 2021

Hey, how is everything? Any updates on this?

@wqweto
Copy link

@wqweto wqweto commented Nov 18, 2021

This issue is affecting Windows 11 and Windows Server 2022 built-in Schannel library latest TLS 1.3 implementation.

It seems TLS1_3_CLIENT sends supported_groups (10) extension but does not send ec_point_formats (11) on ClientHello message.

Sites unreachable by TLS1_3_CLIENT:

@wqweto
Copy link

@wqweto wqweto commented Nov 18, 2021

@yang-wei Did see this part from @gopherbot reply on PR #49127?

During May-July and Nov-Jan the Go project is in a code freeze, during which little code gets reviewed or merged. If a reviewer responds with a comment like R=go1.11 or adds a tag like "wait-release", it means that this CL will be reviewed as part of the next development cycle. See https://golang.org/s/release for more details.

@FiloSottile FiloSottile self-assigned this Mar 2, 2022
@FiloSottile FiloSottile added this to the Go1.19 milestone Mar 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants