-
Notifications
You must be signed in to change notification settings - Fork 88
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Reasonably secure TLS defaults #42
Comments
Thank you for raising this! As mentioned on the PR, can we use the default safe list of ciphers instead of specifying our own? In the future this may mean updating go to stay current (or maybe specifying a list of ciphers we don't want allowed if Go hasn't updated yet), but for now it seems like using the safe defaults in go is okay and the easiest option? WDYT? I do think it's reasonable for us to go ahead and specify a min TLS version though as you suggest >=TLS 1.2 |
I'm totally fine with using go's default safe list of ciphers. (The reason I hardcoded the cipher lists currently considered safe in my patch was that I did not want to upgrade the go version. I tried upgrading to a more recent go version but there were certain build problems (and the possibility of other side effects) so I shied away from it. As I needed a timely solution in order to harden one of my servers I stayed with go 1.13 (though upgrading it to the latest patch-version) and simply hardcoded the cipherlist as a preliminary solution. |
Got it, thank you for the additional info! You mentioned you ran into build problems--was this for another program on your end, or ssl-proxy? I've went ahead and pushed an update to update ssl-proxy to 1.18 seemingly without issues, but please let me know if you run into any. Interesting that the order of the CipherSuites in the config matters! This may not be the case in go 1.18 any longer, as the comment in the Config struct indicates that order should not matter (though, in reality there may still be some order dependent behavior). For your PR #43, I think it's still reasonable to add in a restriction for >= TLS 1.2 only and rely on the built in CipherSuites as a default (feel free to update that if you would like). For your use case, it may be worth patching in your own set of CipherSuites, or we can consider more generally an optional config option if we think this is functionality that may be more widely used (though for the moment it seems pretty specific). |
Thanks for your additions, Suyash! Regarding build problems: when trying to build the current master-revision of ssl-proxy on Linux and/or Windows, via the docker-compose.build.yml, I run into the following issue:
This currently breaks the build for me. (A bit more detail: I use the following versions of Docker and docker-compose:
As soon as I'm able to build ssl-proxy on go 1.18 I'm eager to explore and test (on different servers) whether cipher order is still an issue or whether I'll have to restrict the cipher suites (my aim is not to, of course). |
In it's present state ssl-proxy is by default vulnerable to a lot of attacks (as revealed by running e.g. testssl.sh against it).
IMHO ssl-proxy should therefore use reasonably secure TLS defaults, i.e.
The text was updated successfully, but these errors were encountered: