crypto/tls: support server side Encrypted Client Hello

[rolandshoemaker]

With client support for ECH landing in 1.23, we should now move on to server side support, hopefully for 1.24.

After implementing client-side ECH, server-side seems a little more complicated, mainly from the configuration standpoint.

The more I think about it, the less I think we'll want to reuse the EncryptedClientHelloConfigList field we added for server-side support, since we need a mapping between configs and keys, and we'll likely want to provide some more options.

Perhaps introducing a new struct EncryptedClientHelloKey with ID and private key fields, and a new EncryptedClientHelloKeys field in tls.Config would be sufficient, hiding the rest of the HPKE cipher suite configuration etc as an internal implementation detail.

Additionally we should probably include func MarshalEncryptedClientHelloConfigList(configs []EncryptedClientHelloConfig) ([]byte, error) to allow servers to generate the config list necessary for clients and for retry configs.

Concretely the new API would be:

type EncryptedClientHelloKey struct {
	ID         int
	PrivateKey ecdh.PrivateKey
}

type Config struct {
    ...

	// EncryptedClientHelloConfigs are the ECH configs to use when a client
	// attempts ECH. The public name is expected to be specified by ServerName.
	//
	// If EncryptedClientHelloConfigs is set, MinVersion, if set, must be
	// VersionTLS13.
	//
	// If a client attempts ECH, but it is rejected by the server, the server
	// will send the retry configs specified in
	// EncryptedClientHelloRetryConfigList, if it is initialized.
	EncryptedClientHelloKeys []EncryptedClientHelloKey

	// EncryptedClientHelloRetryConfigList specifies the ECH retry configs to be
	// sent when a client attempts ECH but it is rejected by the server.
	EncryptedClientHelloRetryConfigList []byte
}

type EncryptedClientHelloConfig struct {
	ID                uint8
	PublicKey         ecdh.PublicKey
	PublicName        string
	MaximumNameLength uint8
}

// MarshalEncryptedClientHelloConfigList serializes a list of
// EncryptedClientHelloConfigs into a ECHConfigList, suitable for usage in
// Config.EncryptedClientHelloConfigList.
func MarshalEncryptedClientHelloConfigList(configs []EncryptedClientHelloConfig) ([]byte, error)

An open question for me is how we handle SNI requests for names not included in the EncryptedClientHelloConfigs list. I think I'm erring on the side of saying we reject the handshake, and document that if the user wishes to support both ECH and non-ECH handshakes, they should use not configure EncryptedClientHelloConfigs in their top-level Config and instead use GetConfigForClient to only set EncryptedClientHelloConfigs for names they explicitly want ECH for. See Discussion below for more context, in short we want to continue supporting both ECH and non-ECH connections in a single config.

See #63369 for the client side proposal we implemented.

[AGWA]

I think most server operators who enable ECH will want to continue supporting non-ECH clients. I also think it will be desirable to use a hostname as both an ECH public name and a real SNI hostname. The proposed API makes this awkward by requiring the programmer to write a GetConfigForClient function that checks if the ECH extension is present (which requires #32936) to decide whether to return an EncryptedClientHelloConfig or not.

I'm not sure of the value in aborting a handshake if there is no ECH, since by that point, the private hostname has already been sent in the clear. There might be value in encouraging the user to use an ECH-supporting client for future connections, but that can be handled post-handshake by checking ECHAccepted in ConnectionState and sending a user-friendly error.

Note that the spec states that servers complete the handshake normally if no ECH extension is present (Section 7) and defines the ech_required alert for clients only.

To simplify configuration of a common case, and be more in line with the spec, I propose that crypto/tls complete a normal handshake if the ECH extension is absent, regardless of EncryptedClientHelloConfigs.

For simplicity, I also propose removing PublicName from EncryptedClientHelloConfig, since it seems redundant - if an operator wants to restrict the use of ECH configs to certain SNI hostnames, they can use GetConfigForClient. This is compliant with the spec, which says that checking the SNI hostname against the config's public name is "MAY" not "MUST" (Section 7.1).

This does mean that EncryptedClientHelloConfig can't be used to serialize an ECHConfigList, but the ECH config also includes a maximum_name_length that operators may want to set, so reusing the struct probably didn't make sense anyways. I'm not sure the standard library needs a function to serialize an ECHConfigList, but if it does, it could use a different struct that looks something like this:

struct {
        ID                uint8
        PublicKey         ecdh.PublicKey
        PublicName        string
        MaximumNameLength uint8
}

Finally, for some bikeshedding - having both EncryptedClientHelloConfigs and EncryptedClientHelloConfigList in tls.Config seems unfortunate. Maybe EncryptedClientHelloConfig(s) should be renamed to EncryptedClientHelloKey(s)? That would also free up the name EncryptedClientHelloConfig to be used for the above struct if desired.

[HurricanKai]

I'm not totally sure what the process is here, but wondering if there's anything that can be done to move this forward?
Firefox 129 now fairly widely supports ECH, so I'd be interested to support this on the server side too.

[rsc]

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

[rolandshoemaker]

Note that the spec states that servers complete the handshake normally if no ECH extension is present (Section 7) and defines the ech_required alert for clients only.

To simplify configuration of a common case, and be more in line with the spec, I propose that crypto/tls complete a normal handshake if the ECH extension is absent, regardless of EncryptedClientHelloConfigs.

After re-reading the spec, I agree. I had confused the mandated client and server behavior. I think it makes sense to just transparently continue with the handshake if ECH fails, but we should probably unilaterally send the retry configs as suggested in the spec.

For simplicity, I also propose removing PublicName from EncryptedClientHelloConfig, since it seems redundant - if an operator wants to restrict the use of ECH configs to certain SNI hostnames, they can use GetConfigForClient. This is compliant with the spec, which says that checking the SNI hostname against the config's public name is "MAY" not "MUST" (Section 7.1).

This seems reasonable. We still need to generate an ECHConfigList in order to send retry configs in the case of failure, but we can plausible just copy the public_name from the ServerName and use that. For multi-name setups, the user can support multiple configs using GetConfigForClient.

I think there is still a problem with how to properly set maximum_name_length for multi-name servers, since we can only be aware of one name at a time. Plausibly we could add a new top-level field to Config for this?

Finally, for some bikeshedding - having both EncryptedClientHelloConfigs and EncryptedClientHelloConfigList in tls.Config seems unfortunate. Maybe EncryptedClientHelloConfig(s) should be renamed to EncryptedClientHelloKey(s)?

Yeah I think EncryptedClientHelloKeys makes sense.

[rolandshoemaker]

@HurricanKai this is planned for 1.24, which is the next major feature release. It's tentatively scheduled for February 2025.

[rolandshoemaker]

Updated API proposal in #68500 (comment).

[rsc]

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

The proposal is #68500 (comment)

[gopherbot]

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

The proposal is #68500 (comment)