fix(dgraph): giving users the option to control tls versions #6820
Conversation
aman-bansal
requested review from
manishrjain and
vvbalaji-dgraph
as code owners
There was a problem hiding this comment.
Reviewed 2 of 2 files at r1.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @manishrjain and @vvbalaji-dgraph)
aman-bansal
force-pushed
the
aman/tls_version_flags
branch
from
d9ecc72
to
98b2fbb
Compare
![codelingo[bot]](https://avatars.githubusercontent.com/in/14339?s=60&v=4){kind=small}
| @@ -99,7 +114,7 @@ func LoadClientTLSConfigForInternalPort(v *viper.Viper) (*tls.Config, error) { | |||
| } | |||
|
|
|||
| // LoadServerTLSConfigForInternalPort loads the TLS config for the internal ports of the cluster | |||
| func LoadServerTLSConfigForInternalPort(tlsEnabled bool, tlsDir string) (*tls.Config, error) { | |||
| func LoadServerTLSConfigForInternalPort(tlsEnabled bool, tlsDir, tlsMinVersion string) (*tls.Config, error) { | |||
There was a problem hiding this comment.
Boolean arguments can indicate low cohesion. Consider refactoring LoadServerTLSConfigForInternalPort by using a separate function for each case and helper functions for repeated code. This will make each function clearer and more modular, leading to easier maintainability.
x/tls_helper.go
Outdated
| MinVersion string | ||
| } | ||
|
|
||
| // RegisterClientTLSFlags registers the required flags to set up a TLS client. |
There was a problem hiding this comment.
Every exported function in a program should have a doc comment. The first sentence should be a summary that starts with the name (RegisterServerTLSFlags) being declared.
From effective go.
Fixes DGRAPH-2469 for patch release 20.07.
Currently Dgraph supports both tls v1.1 and tls v1.2 which introduces security concerns. This PR limits the min version to v1.2 and also enables only selected cipher suites which are more secure.
This change is
Docs Preview: