Skip to content
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

Open
cstsw opened this issue Jun 24, 2022 · 4 comments
Open

Reasonably secure TLS defaults #42

cstsw opened this issue Jun 24, 2022 · 4 comments

Comments

@cstsw
Copy link

cstsw commented Jun 24, 2022

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.

  • TLS >= 1.2
  • A set of cipher suites without known vulnerabilities (i.e. des/3des, rc4, cbc ciphers should not be offered)
@suyashkumar
Copy link
Owner

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

@cstsw
Copy link
Author

cstsw commented Jul 7, 2022

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.
Another reason for this approach was that the list of available ciphers seems to depend on other packages installed on the system, like openssl (and their respective version), and that the cipher order really matters (I once got it wrong and tools like testssl.sh could no longer negotiate a protocol with ssl-proxy, at least for some clients/browsers tested); but this aspect should also be considered in go's crypto implementation itself and not be burdened on projects using it.)

@suyashkumar
Copy link
Owner

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).

@cstsw
Copy link
Author

cstsw commented Jul 11, 2022

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:

Step 4/6 : RUN go get -u github.com/golang/dep/cmd/dep
 ---> Running in 23ea82d98a39
go: go.mod file not found in current directory or any parent directory.
        'go get' is no longer supported outside a module.
        To build and install a command, use 'go install' with a version,
        like 'go install example.com/cmd@latest'
        For more information, see https://golang.org/doc/go-get-install-deprecation
        or run 'go help get' or 'go help install'.

This currently breaks the build for me.

(A bit more detail: I use the following versions of Docker and docker-compose:

  • On Linux:
    • Docker version 20.10.6, build 370c289
    • docker-compose version 1.29.2, build 5becea4c
  • On Windows:
    • Docker version 20.10.16, build aa7e414
    • docker-compose version v2.5.1
    • both are part of the current Rancher Desktop Version 1.4.1, using dockerd/moby as the container runtime)

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).

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

No branches or pull requests

2 participants