golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.3k stars 17.58k forks source link

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

Closed dmjones closed 4 years ago

dmjones commented 5 years ago

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.

FiloSottile commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 https://github.com/golang/go/issues/30325#issuecomment-483863377. 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.

dmjones commented 5 years ago

@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:

This approach would also allow for hybrid schemes:

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 commented 5 years ago

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

dmjones commented 5 years ago

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: https://github.com/golang/go/compare/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 commented 4 years ago

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 commented 4 years ago

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

rsc commented 4 years ago

No change in consensus, so declining.