Add ESNI client and server support #172
Conversation
The Docker image is based on Linux, be sure to build a binary that is compatible with it. Fixes interop tests on macOS. During development, one can use: `cd _dev/tris-testclient && ../go.sh build -v -i .`
|
I've addressed the review comments, the GOOS=linux change was split in a separate commit. Since it the ESNI changes touch the same context, the ESNI patch effectively depends on that. Aside from these two patches, there is also a Makefile patch to avoid unnecessary rebuilds. This is independent of ESNI, but included here for easier testing. I hope that PR #173 can be merged first. If it makes it easier for you, I could also drop that rebuild patch here and restrict PR #173 to that single change. Let me know! |
Implements https://tools.ietf.org/html/draft-ietf-tls-esni-01 Extends the tls.Config API with a ClientESNIKeys structure which must contain a valid key. If this key is not valid, the handshake will fail. A GetServerESNIKeys API is also added which allows the server to dynamically query for an appropriate ESNI key. Add a new 'esnitool' utility to generate ESNIKeys for testing purposes, this uses a short lifetime, a single curve and cipher suite. The test client and server can now be used with these keys. Additionally the test client can securely query the ESNI key from DNS (hardcoded to use 1.1.1.1:853 using DoT for now).
|
Ended up dropping the rebuild patch since it is not ready, will move that to PR #173. Now this patch is fully independent again and can be reviewed/merged independently. |
|
Can you improve tests? |
| "golang.org/x/crypto/curve25519" | ||
| ) | ||
|
|
||
| // Internal definitions, copied from common.go and esni.go |
There was a problem hiding this comment.
Might it make sense to have esni.go export them or equivalents?
| // nil, then SNI encryption is not enabled. | ||
| // | ||
| // See https://tools.ietf.org/html/draft-ietf-tls-esni-01 | ||
| ClientESNIKeys *ESNIKeys |
There was a problem hiding this comment.
This doesn't fit the pattern of a config being reused across connections since the ESNI keys are server specific. I can't think of a great way to do it, but one way would be to have a flag here to enable support, then DialWithDialer would use the resolver passed in to look up the text record.
| case "Rand": | ||
| f.Set(reflect.ValueOf(io.Reader(os.Stdin))) | ||
| case "Time", "GetCertificate", "GetConfigForClient", "VerifyPeerCertificate", "GetClientCertificate", "GetDelegatedCredential": | ||
| case "Time", "GetCertificate", "GetConfigForClient", "VerifyPeerCertificate", "GetClientCertificate", "GetDelegatedCredential", "ClientESNIKeys", "GetServerESNIKeys": |
There was a problem hiding this comment.
ClientESNIKeys isn't a function
|
I'm working with a group, https://defo.ie/ on the ESNI standard. There are some test servers there that might be useful. Also, there are some tests in our openssl fork: https://github.com/sftcd/openssl We also have working versions of nginx and lighttpd, and are building automated test suites to run in Travis CI and GitLab CI. |
|
@Lekensteyn perhaps it would be a good idea to switch to a patch on top of the upcoming Go 1.14.x. It seems to contain all non-draft TLS 1.3 features, and it would smooth the way towards eventually incorporating it into upstream Go. I also imagine that it's best to get the latest fixes from upstream's crypto packages. |
5 participants
Implements https://tools.ietf.org/html/draft-ietf-tls-esni-01
Extends the tls.Config API with a ClientESNIKeys structure which must
contain a valid key. If this key is not valid, the handshake will fail.
A GetServerESNIKeys API is also added which allows the server to
dynamically query for an appropriate ESNI key.
Add a new 'esnitool' utility to generate ESNIKeys for testing purposes,
this uses a short lifetime, a single curve and cipher suite. The test
client and server can now be used with these keys. Additionally the test
client can securely query the ESNI key from DNS (hardcoded to use
1.1.1.1:853 using DoT for now).
Fixes #138
This change depends on #171 to ensure that the shared secret can be calculated without access to a
tls.Connstructure. Therefore that commit is also included in this PR.Note: this does not implement the latest draft (-03). If so, at least these changes are necessary:
ESNIKeys.public_namein the SNI extension (and change the version number accordingly).