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

proposal: crypto/tls: allow registration of additional key exchanges #31520

Closed
dmjones opened this issue Apr 17, 2019 · 9 comments
Closed

proposal: crypto/tls: allow registration of additional key exchanges #31520

dmjones opened this issue Apr 17, 2019 · 9 comments
Labels
FrozenDueToAge Proposal Proposal-Crypto Proposal related to crypto packages or other security issues
Milestone

Comments

@dmjones
Copy link

dmjones commented Apr 17, 2019

The driver behind this proposal is the flurry of activity relating to post-quantum cryptography. NIST published its round 2 candidates, bringing the field down to a manageable size. Early adopters are implementing these promising candidates and even using them in production (typically in "hybrid mode" with a classical algorithm).

Allowing users to define their own TLS ciphersuites would help in this effort. Currently, forking crypto/tls would be required to register additional suites.

In terms of changes, I would propose making the keyAgreement interface public and offering a function to insert additional entries into the cipherSuites variable (as a bare minimum). I'd be happy to provide some more detailed design suggestions, if there's any appetite for this idea.

@gopherbot gopherbot added this to the Proposal milestone Apr 17, 2019
@FiloSottile FiloSottile added the Proposal-Crypto Proposal related to crypto packages or other security issues label Apr 17, 2019
@FiloSottile
Copy link
Contributor

There is no chance of doing this for TLS 1.2, because everything is way too interconnected. Adding a new key exchange method is likely to touch too much of the code. In TLS 1.3, maybe?

I'm assuming this is about key exchanges and not cipher suites, since the latter in TLS 1.3 are just stuff like "AES and SHA-2", which is not what the PQC proposals are focusing on.

I might be convinced by showing how a small, useful API would look like, but I can't think of one myself. It should come with examples of how it can be used to implement multiple unsupported key exchanges, because I do not want to have to add a new one in 6 months.

@dmjones
Copy link
Author

dmjones commented Apr 18, 2019

Yes, this is mostly about registering new key exchange algorithms. Although one still needs to declare ciphersuites that use those algorithms.

I will whip up a prototype fork of crypto/tls with a minimal set of changes, so we can review the design.

It should come with examples of how it can be used to implement multiple unsupported key exchanges, because I do not want to have to add a new one in 6 months.

I'm hopeful on this front. Projects such as liboqs (and their openssl fork) have made minor changes that subsequently support any PQ KEM. I will attempt to demonstrate my proposed changes allow the 'dropping in' of Go variants of these KEMs, were they to exist.

@FiloSottile
Copy link
Contributor

In TLS 1.3, the cipher suites are not bound to the key exchanges. That's why I would want this to focus on TLS 1.3, if it gets done.

Rather than a fully implemented fork, please start by proposing an API (with docs) here, like #30325 (comment). It keeps the discussion in one place, it's easier to review, it focuses on the important part and saves you work in the first iterations.

@FiloSottile FiloSottile changed the title proposal: crypto/tls: Allow registration of additional ciphersuites proposal: crypto/tls: allow registration of additional key exchanges Apr 18, 2019
@dmjones
Copy link
Author

dmjones commented Apr 23, 2019

@FiloSottile How about an interface to represent a key exchange mechanism for a private named group:

// A ClientShareContext stores client state relating to an ongoing key exchange.
type ClientShareContext interface{}

// A KeyExchangeHandler implements a TLS 1.3 key exchange mechanism.
type KeyExchangeHandler interface {

	// ClientShare initiates the key exchange and returns the client key
	// share. The returned context will be passed to SecretFromServerShare,
	// allowing the client to retain state (such as ephemeral keys) during the exchange.
	ClientShare() ([]byte, ClientShareContext, error)

	// SecretFromClientShare is called by the server to process the share from
	// the client. It generates (or deduces) the TLS secret and returns this, along with
	// its own share, which is sent to the client.
	SecretFromClientShare(clientShare []byte) (secret, serverShare []byte, err error)

	// SecretFromServerShare uses the server key share and the previously generated
	// context to deduce the TLS secret, which is returned.
	SecretFromServerShare(serverShare []byte, ctx ClientShareContext) ([]byte, error)

	// CurveID returns the group associated with this key exchange.
	CurveID() CurveID
}

Implementations could be provided as a new field of tls.Config. Here we take a mapping from private CurveIDs to KeyExchangeHandlers.

type Config struct {
	 // ...

	// KeyExchangeHandlers provide TLS 1.3 key exchange implementations
	// for private named groups. The CurveIDs must be from the ecdhe_private_use range
	// (see RFC 8446 section 4.2.7). To enable these private groups, include their
	// CurveID in the CurvePreferences field.
	KeyExchangeHandlers map[CurveID]KeyExchangeHandler
}

This interface is broad enough to allow the sort of KEMs being defined by the NIST competition to easily plug in. For instance, here's the blueprint for implementing a KEM:

  • ClientShare (called at the client) would call the KEM key generation function, returning the public key as the key share result and storing the secret key in the context.
  • SecretFromClientShare (called at the server) would called the KEM encapsulation function (which generates a random secret and encapsulate this using the client's public key).
  • SecretFromServerShare (called at the client) would decapsulate the encrypted secret, using the secret key stored in the context.

This approach would also allow for hybrid schemes:

  • ClientShare could generate ECDHE parameters and concatenate these with the post-quantum public key.
  • SecretFromClientShare would also generate ECDHE parameters and return these, along with the encapsulated post-quantum secret.
  • SecretFromServerShare would derive the EC key along with decapulating the post-quantum secret. How these two values are combined to create the overall secret is for the implementer to decide.

If we can agree on a suitable public API, then I can post further on how the innards of the package might need to change to support it.

Ideally we would re-write the existing ECDH code to use this interface, then we can drop the concept of a Context, because the client will store a copy of the relevant KeyExchangeHandler instead of the existing ecdheParameters. Scratch that, the ecdheParameters type is too coupled with the TLS <1.3 code.

@dmjones
Copy link
Author

dmjones commented Apr 30, 2019

Hi @FiloSottile - any thoughts on the design proposed above?

@dmjones
Copy link
Author

dmjones commented May 10, 2019

I've continued work on this, current implementation: https://github.com/thales-e-security/go/tree/go1.12.5_private_key_exchanges. As the name implies, this is forked from the go.1.12.5 tag.

Changes vs 1.12.5: go1.12.5...thales-e-security:go1.12.5_private_key_exchanges.

Versus the original design discussed above, there are minor changes. Most notable is the removal of the ClientShareContext type. Since the context would need to be stored within the handshake structs, it made more sense to just store the key exchange itself. The name of the interface and fields also changes a little.

@FiloSottile
Copy link
Contributor

While I absolutely see the value of experimenting with post-quantum key exchanges, I think such experimentation belongs in forks, and doesn't need to be first-class supported in the standard library. The proposed API surface would not be trivial, and exposes inner workings that the average application does not need to access.

As time goes on we might even directly implement some hybrids, but that can be an implementation detail hidden away from the public API.

@rsc
Copy link
Contributor

rsc commented Mar 25, 2020

Based on the discussion above and the lack of activity, this seems like a likely decline.

@rsc
Copy link
Contributor

rsc commented Apr 1, 2020

No change in consensus, so declining.

@rsc rsc closed this as completed Apr 1, 2020
@rsc rsc moved this from Likely Decline to Declined in Proposals (old) Apr 1, 2020
@golang golang locked and limited conversation to collaborators Apr 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge Proposal Proposal-Crypto Proposal related to crypto packages or other security issues
Projects
No open projects
Development

No branches or pull requests

4 participants