Skip to content

Conversation

@Lekensteyn
Copy link
Contributor

@Lekensteyn Lekensteyn commented Apr 24, 2019

The key agreement protocols only need a source of randomness for key
generation and agreement (for KEM), and the role (client or server), so
pass these explicitly.

Make keyAgreementClient and keyAgreementServer independent of the
tls.Conn instance to allow reuse for ESNI.


In the TLS handshake, we have:

  1. Client generates an ephemeral private key and sends a derived public key share.
  2. Server takes this public key share, generates a new ephemeral private key, derives a shared secret and sends a derived public key share.
  3. Client uses its earlier private key to compute the same shared secret.

For ESNI, we have:

  1. Server generates a semi-static private key and publishes a derived public key share (say, in DNS).
  2. Client generates a new ephemeral private key, derives a shared secret and sends a derived public key share.
  3. Server uses its earlier private key to compute the shared secret.

As you can see the roles are reversed. It didn't seem correct to call keyAgreementClient as a server and vice versa. Neither am I sure if the "Alice" and "Bob" role should be reversed for SIDH. For ESNI I plan to support ECDHE only. The ecdheKex interface is adapted for the TLS handshake (with PQ support), but for ESNI, this interface will directly be used. The kex interface is maintained, instead keyAgreementXxx have been made independent of Conn. For ESNI I simply do not use the PQ algorithms, so the difference between keyAgreementClient/Server does not matter.

It is possible to specialize the "generate" method, for TLS it could receive "isClient" but in ECDHE there is no distinction so I could drop it. Thoughts?

The key agreement protocols only need a source of randomness for key
generation and agreement (for KEM), and the role (client or server), so
pass these explicitly.

Make keyAgreementClient and keyAgreementServer independent of the
tls.Conn instance to allow reuse for ESNI.
@Lekensteyn
Copy link
Contributor Author

Regarding your suggestion to split generate into generateClient/generateServer, I have decided to do that for now since it would either result in some more code duplication or a special if/else case in kexHybridSIDHp503X25519.generate. This can be done later if desired.

As for renaming "Client" -> "Initiator" (the party that generates a key share first) and "Server" -> "Responder" (the party that receives the key share first before generating a new key share and shared secret), I think it might be more confusing for the TLS handshake so I have decided against it. I'll just add a comment to the ESNI code.

type kexSIDHp503 struct{ defaultServerKEX } // Used by SIDH/P503
type kexSIKEp503 struct{} // Used by SIKE/P503
type kexHybridSIDHp503X25519 struct {
defaultServerKEX
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default implementation was not sufficient as the client and server generates keys and a shared secret using different computations. Hence the specialization below.

@kriskwiatkowski
Copy link
Contributor

LGTM

@kriskwiatkowski kriskwiatkowski merged commit ba0906b into master May 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants