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 Encrypted Client Hello in clients #63369

Closed
dennisjackson opened this issue Oct 4, 2023 · 24 comments
Closed

crypto/tls: support Encrypted Client Hello in clients #63369

dennisjackson opened this issue Oct 4, 2023 · 24 comments
Assignees
Labels
Proposal Proposal-Accepted Proposal-Crypto Proposal related to crypto packages or other security issues
Milestone

Comments

@dennisjackson
Copy link

Encrypted Client Hello is a new privacy feature that is finishing standardization at the IETF. ECH is a big step forward for the internet, because it removes one of the last places where the websites users visit is leaked to the network in plaintext.

ECH has already been implemented in NSS and BoringSSL, and an experimental fork is available for OpenSSL.

Firefox has launched support for ECH, Chrome is expected to launch support imminently and Cloudflare have rolled out server-side support globally.

Support in crypto/tls would need to be both for servers and clients. Lack of support is currently blocking Caddy from shipping ECH.

cc @FiloSottile

@gopherbot gopherbot added this to the Proposal milestone Oct 4, 2023
@seankhliao seankhliao added the Proposal-Crypto Proposal related to crypto packages or other security issues label Oct 4, 2023
@dennisjackson
Copy link
Author

I should add that Cloudflare's fork of crypto/tls has ECH support in case that's of interest to the official Golang maintainers.

@ianlancetaylor ianlancetaylor moved this to Incoming in Proposals Oct 4, 2023
@ianlancetaylor
Copy link
Contributor

CC @golang/security

@rolandshoemaker
Copy link
Member

We were just talking about this today. It seems like a reasonable thing to implement given the direction the ecosystem is going. I think we're likely to prioritize client support first, but given the existence of stuff like Caddy, server support also seems reasonable.

I think we'll want to design the API such that we can internally wire up the net/http client to automatically populate the necessary information via e.g. the DNS discovery mechanism (or manually configure it by passing a tls.Config).

@mholt
Copy link

mholt commented Oct 14, 2023

Thank you Roland 😁 Thousands of users appreciate that!

@pitapita012

This comment was marked as spam.

@DeltaLaboratory
Copy link

Is there work being done somewhere on this proposal or will start working on this proposal after ECH has been fully standardized to RFC?

@rolandshoemaker
Copy link
Member

I think ideally we'd like to get at least the client half of this in for 1.23, but we still need to decide on what the API will look like.

To just throw something out there, perhaps a very simple first option, which would support clients and leave the door open for sever support, would be to just add a field to tls.Config containing an already serialized ECHConfigList.

e.g.

type Config struct {
    ...

    // EncryptedClientHelloConfigList is a serialized ECHConfigList. If
    // provided, clients will attempt to connect to servers using Encrypted
    // Client Hello (ECH) using one of the provided ECHConfigs. Servers
    // currently ignore this field.
    EncryptedClientHelloConfigList []byte
}

A client that populates this would then populate Config.ServerName as usual, and the internals would take care of the rest. One open question is how it would handle ECH failures, i.e. do we want to offer a knob to control whether we allow fallback or not (I would default to just failing closed, but others might have strong opinions on this).

This would hide away most of the internals of the ECH configuration, allowing direct passthrough for supported discovery mechanisms, and allow us to be as opinionated as we want in terms of picking HPKE cipher suites and such (which is generally a goal of crypto/tls).

For server support we'd then either reuse the EncryptedClientHelloConfigList field with an additional field for private keys, or a map between ECHConfig and private keys.

@rolandshoemaker rolandshoemaker self-assigned this Feb 20, 2024
@DeltaLaboratory
Copy link

I have some questions that I don't know about:

  1. is there an existing HPKE implementation in go?
  2. if we set the EncryptedClientHelloConfigList in serialized form, how do we generate the serialized config? Or would it be more convenient to receive a non-serialized structure? Would it be too much of a performance issue?
  3. if we use serialized config, how do we validate/handle error for invalid config?

@rolandshoemaker
Copy link
Member

  1. Not yet. I suspect we'll use this as a learning experience to develop an internal-only HPKE API and then based on that experience propose a public one.
  2. ECH configs will be fetched from the various discovery mechanisms. It seems unlikely to me (based on a survey of the current ecosystem) that (client) users will ever be generating them themselves, nor should they likely be altering configs they get from elsewhere
  3. I'm not entirely convinced that users should be attempting to validate ECH configs themselves. In the case of an error the config should be re-fetched using one of the discovery methods, and if that continues to fail, should stop trying to use ECH for that particular host.

@rsc
Copy link
Contributor

rsc commented Feb 28, 2024

Let's make this proposal about the client side, which the discussion has focused on, and we can revisit the server part in a separate proposal when we have ideas about what the API looks like.

@rsc rsc changed the title proposal: crypto/tls: Support Encrypted Client Hello proposal: crypto/tls: support Encrypted Client Hello in clients Feb 28, 2024
@alexleach
Copy link

  1. is there an existing HPKE implementation in go?
  1. Not yet. I suspect we'll use this as a learning experience to develop an internal-only HPKE API and then based on that experience propose a public one.

Actually, I saw on Wikipedia's article about Server Name Indication that cloudflare/go implemented ECH support in this commit from July, last year. This was merged on Feb 9th '24. You asked specifically about HPKE; I'm no expert, but there's a hpke.go file and a separate folder called hpke as well.

In the commit note, it does mention an external dependency requirement, to cloudflare/circl. That hpke folder looks like it is duplicated in the circl repository in fact.

Adds support for draft 13 of the Encrypted ClientHello (ECH) extension
for TLS. This requires CIRCL to implement draft 08 or later of the HPKE
specification (draft-irtf-cfrg-hpke-08).

Maybe cloudflare's licenses are compatible with golang's and their library could be merged into the standard library? It looks like a lot of work was put into that initial commit (38 changed files with 7,335 additions and 35 deletions)!

@rsc
Copy link
Contributor

rsc commented Mar 1, 2024

Maybe cloudflare's licenses are compatible with golang's and their library could be merged into the standard library?

FWIW, license compatibility is not enough; we require a CLA so that all code can be distributed under the simple Go license, not an amalgamation of many licenses.

@rsc
Copy link
Contributor

rsc commented Mar 1, 2024

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc rsc moved this from Incoming to Active in Proposals Mar 1, 2024
@rsc
Copy link
Contributor

rsc commented Mar 8, 2024

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

The proposal details are in #63369 (comment).

@rsc
Copy link
Contributor

rsc commented Mar 15, 2024

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

The proposal details are in #63369 (comment).

@rsc rsc changed the title proposal: crypto/tls: support Encrypted Client Hello in clients crypto/tls: support Encrypted Client Hello in clients Mar 15, 2024
@rsc rsc moved this from Likely Accept to Accepted in Proposals Mar 15, 2024
@rsc rsc modified the milestones: Proposal, Backlog Mar 15, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/578575 mentions this issue: crypto/tls: add ech client support

@crrodriguez
Copy link

Is there any hope for the server side part ? caddy really needs this feature and its authors defer to the standard library for an implementation..

@rolandshoemaker rolandshoemaker modified the milestones: Backlog, Go1.23 Apr 29, 2024
@rolandshoemaker
Copy link
Member

Not for 1.23. We're planning on server support in 1.24.

@rolandshoemaker
Copy link
Member

When implementing this I realized we never defined the new error type in the proposal, and that we probably also want to introduce a ECHAccepted bool field in the ConnectionState struct. Assuming there are no objections, I'm extending the proposal to include the following:

type ECHRejectionError struct {
    RetryConfigList []byte
}
func (*ECHRejectionError) Error() string

type ConnectionState struct {
    ...

    // ECHAccepted indicates if ECH was offered by the client, and accepted by the server.
    ECHAccepted bool
}

@rolandshoemaker
Copy link
Member

rolandshoemaker commented May 14, 2024

One final change (I promise) which allows us to decouple outer server certificate verification vs inner certificate verification in the case of ECH rejection.

type Config {
	...

	// EncryptedClientHelloRejectionVerify, if not nil, is called when ECH is
	// rejected, in order to verify the ECH provider certificate in the outer
	// client hello. If it returns a non-nil error, the handshake is aborted and
	// that error results.
	//
	// Unlike VerifyPeerCertificate and VerifyConnection, normal certificate
	// verification will not be performed before calling
	// EncryptedClientHelloRejectionVerify.
	//
	// If EncryptedClientHelloRejectionVerify is nil, and ECH is rejected the
	// roots in RootCAs will be used to verify the ECH providers public
	// certificate. VerifyPeerCertificate and VerifyConnection are not called
	// when ECH is rejected, even if set.
	EncryptedClientHelloRejectionVerify func(ConnectionState) error
}

@terrytw
Copy link

terrytw commented May 23, 2024

It seems that this issue was changed from implementing ECH to solely implementing client side of ECH, where can one follow the progression of server side implementation? I can't find any issue or PR on it.

@rolandshoemaker
Copy link
Member

There isn't one yet. Once we're a bit deeper into the freeze I'll open an issue for tracking progress on it for 1.24.

@rugk
Copy link

rugk commented Jul 16, 2024

@rolandshoemaker just as a heads up as I see no issue yet, did you create one?

@rolandshoemaker
Copy link
Member

Thanks for the nudge, filed #68500.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Proposal Proposal-Accepted Proposal-Crypto Proposal related to crypto packages or other security issues
Projects
Status: Accepted
Development

No branches or pull requests