caddyserver / caddy

Fast and extensible multi-platform HTTP/1-2-3 web server with automatic HTTPS
https://caddyserver.com
Apache License 2.0
56.83k stars 3.98k forks source link

Feature request: Option to change the Diffie–Hellman handshake curve #1117

Closed mkoppmann closed 7 years ago

mkoppmann commented 7 years ago

As far as I know, it is currently only possible to use the default P-256 curve in Caddy for the DHE handshake. To get 100% key exchange at ssllabs.com one has to use at least secp384r1.

cipherli.st also recommends secp384r1 for nginx.

I’m not suggesting to set this as new default but to have the option to change the curve. If curve25519 gets integrated (#1116) this option is also needed.

elcore commented 7 years ago

Hello @mkoppmann and @mholt,

https://golang.org/src/crypto/tls/common.go?h=defaultCurvePreferences#L542 var defaultCurvePreferences = []CurveID{CurveP256, CurveP384, CurveP521}

Golang prefers P-256 over P-384 😄

I already implemented this feature here: https://github.com/mholt/caddy/tree/CurvePreferences

mkoppmann commented 7 years ago

Hi @elcore,

thanks for posting that info. It’s probably because p256 has an assembly implementation and therefore is way faster (source).

Great that you already implemented this feature. Is there any chance to get this merged or can I help somehow with testing?

elcore commented 7 years ago

Hi @mkoppmann,

Is there any chance to get this merged

Well, I need to rewrite the code (move from 0.8.X to 0.9.X)! I could send you a modified version (for your own use) (I do not have much time until the weekend -- school etc.)

mkoppmann commented 7 years ago

For me personally, setting the curve to P384 is really just to get the higher ranking at ssllabs, so it’s definitely not high priority and I think with the current implementation in Go (and I believe OpenSSL) that P256 is more error resistant than P384.

My curve preferences are: Curve25519 > P256 > P384.

elcore commented 7 years ago

Hi @mkoppmann,

We should wait until Golang implements Curve25519 into the standard library (TLS -- CurveID).

mkoppmann commented 7 years ago

Hi @elcore,

Yes, that would be the right thing to do.

As I’m trying to get into Github and the whole open development process, what’s the right way to handle this situation, as #1116 and this issue are related? Just keeping them open or is there some “On hold” status?

elcore commented 7 years ago

Hi @mkoppmann,

Just keeping them open or is there some “On hold” status?

As far as I know, there is no "On hold" status -- @mholt needs to decide what we should do.

P.S.: Welcome to GitHub 😄

wendigo commented 7 years ago

Maybe it should be configurable just like TLS version preference is?

I've got custom caddy built with some changes deployed on my landing page (including curve preferences). Here is ssllabs score for it: https://www.ssllabs.com/ssltest/analyze.html?d=serafin.tech

mkoppmann commented 7 years ago

OT: The clients who can still connect to your site do all support the forward secrecy capable ciphers, so you should disable TLS_RSA_WITH_AES_256_CBC_SHA and TLS_RSA_WITH_AES_128_CBC_SHA.

elcore commented 7 years ago

Hello @wendigo,

it is 😄

Caddyfile:

domain.com {
   tls {
      curves ab123, cd456, ef789
   }
}
elcore commented 7 years ago

Hello @mkoppmann, @wendigo,

I implemented the feature request 😄

https://github.com/mholt/caddy/tree/CurvePreferences-0.9.X

You can test it now 😄 -- I appreciate any feedback

Supported Curves:

p256 
p384 
p521 
mholt commented 7 years ago

@elcore Make a pull request ;)

mholt commented 7 years ago

@mkoppmann Sorry to have been silent on this for a while, been a bit busy. I think we can go forward with this issue without having to wait on #1116.

mkoppmann commented 7 years ago

@elcore, @mholt, wow that’s fantastic :smile: Caddy is getting better every day :+1:

wendigo commented 7 years ago

@elcore Nice :)