Implement curve X25519 (Golang 1.8) #1376

Merged
merged 3 commits into from Feb 17, 2017

Projects

None yet

3 participants

@elcore
Collaborator
elcore commented Jan 25, 2017 edited

I promised to implement X25519 ๐Ÿ˜„ (even though I have no time)

@elcore elcore Implement curve X25519
1832981
@mholt mholt referenced this pull request Jan 25, 2017
Open

Update Caddy to use Go 1.8 #1375

9 of 11 tasks complete
@lhecker
Collaborator
lhecker commented Jan 29, 2017

@mholt @elcore What about the tls.Config.CurvePreferences? Cloudflare strongly recommends setting that value and I believe this PR might be a good fit for doing so. What do you think?

@elcore
Collaborator
elcore commented Jan 29, 2017 edited

@lhecker

What about the tls.Config.CurvePreferences? Cloudflare strongly recommends setting that value and I believe this PR might be a good fit for doing so. What do you think?

We are using the default configuration until the user decided to modify the curves.

Default:

tls.Config.CurvePreferences -> empty -> default curves []CurveID{X25519, CurveP256, CurveP384, CurveP521}

Custom Configuration:

tls.Config.CurvePreferences -> value -> []CurveID{value}

https://github.com/mholt/caddy/blob/master/caddytls/config.go#L274-L280

https://github.com/golang/go/blob/master/src/crypto/tls/common.go#L486-L488

https://github.com/golang/go/blob/master/src/crypto/tls/common.go#L693

@lhecker
Collaborator
lhecker commented Jan 29, 2017 edited

Yeah I'm aware of that - I didn't even ask what we currently use...
What I was saying is that the Go defaults are not safe for public internet services. It's about this folder not having optimized assembly code for CurveP384 and CurveP521 (and sadly everything besides x86_64, but luckily almost all use that architecture for hosting their sites nowadays). This indeed causes caddy to take up to one second of CPU time per handshake when the client forces it's use.
Since caddy is surely trying to be a "serious" project I figured that we should also be serious and thus wary about possible implementation details inside the Go code.

@elcore
Collaborator
elcore commented Jan 29, 2017

@lhecker

Feel free to implement it, I am currently too sick to do it. I have 39ยฐC fever!!

@lhecker
Collaborator
lhecker commented Jan 29, 2017

@elcore Sure! Get well soon!

@elcore
Collaborator
elcore commented Jan 29, 2017

@lhecker thank you

lhecker and others added some commits Jan 29, 2017
@lhecker lhecker caddytls: Added a default curves list
b2860dc
@elcore elcore caddytls: Improve tests
49873be
@mholt
mholt approved these changes Feb 17, 2017 View changes

Thanks @elcore for getting this made up so early! Now we will merge it in. ๐ŸŽ‰

@mholt mholt merged commit 91ff734 into master Feb 17, 2017

1 of 5 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
continuous-integration/travis-ci/push The Travis CI build could not complete due to an error
Details
continuous-integration/appveyor/branch AppVeyor build failed
Details
continuous-integration/appveyor/pr AppVeyor build failed
Details
licence/cla Contributor License Agreement is signed.
Details
@mholt mholt deleted the X25519 branch Feb 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment