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

Add support for strict TLS protocol version and cipher-suites #157

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mzpqnxow
Copy link

@mzpqnxow mzpqnxow commented Nov 4, 2022

This is a very poorly written patch that adds the following:

--strong-tls - Sets the TLS configuration to TLS1.3 only and sets ciphers to a list that excludes problematic cipher-suites (e.g. those with CBC-mode symmetric ciphers)
--tls1_2 - When used with --strong-tls, this enables TLS1.2, but still excludes cipher-suites with CBC-mode symmetric ciphers

I don't necessarily expect you to accept this for a few reasons:

  1. It's poorly written- I don't write go very often, you'll notice (for example) that the instantiation of http.Server now can take place in one of two places. This seems clunky, it would make more sense to conditionally set the TLSConfig and and TLSNextProto parameters. If you have an interest in accepting this I can look into how to do that properly
  2. It may be the case that gohttpserver is intended to be placed behind a reverse-proxy for TLS termination when the strength of the TLS configuration is a concern
  3. There are not any _practically exploitable issues with the default configuration

I included a few references as comments regarding the choice of cipher-suites, but the primary issue is the avoidance of CBC-mode symmetric algorithms. Historically, CBC-mode ciphers haven't fared well and many vulnerability scanners do not like finding them (particularly when TLS versions < TLS1.2 offer them) making this change mostly useful for compliance reasons within certain environments

Please feel free to reject this or suggest improvements. I'm going to keep my fork as I need it for a specific use

Thanks for your work on this project!

EDIT: As the caveat in the command-line output says- enabling this can have an impact on portability- Internet Explorer in particular may have trouble with this, as well as some older mobile/embedded devices

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

Successfully merging this pull request may close these issues.

None yet

1 participant