TLS server options #302
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,8 +13,10 @@ import ( | |
| ) | ||
|
|
||
| const ( | ||
| DefaultLogLevel = "info" | ||
| DefaultLogFormat = "standard" | ||
| defaultTLSMinVersion = "tls12" | ||
kalafut marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| defaultTLSPreferServerCipherSuites = true | ||
|
There was a problem hiding this comment. Is this flag/option needed? Reading from https://pkg.go.dev/crypto/tls#Config, it looks like this deprecated and is now completely ignored. There was a problem hiding this comment. Looks like the deprecation happened recently, in go1.17 so this flag will still be used by the server since we're on 1.16. Having said that, I think that we should consider updating the go version we use to compile rather than introduce a parameter that will be ignored shortly after. There was a problem hiding this comment. Right, we can remove the option if we update our builds to go 1.17; they're on 1.16 currently. There was a problem hiding this comment. main is now building with go 1.17. I merged that into this branch, and removed the |
||
| ) | ||
|
|
||
| // Specification are the supported environment variables, prefixed with | ||
|
|
@@ -100,6 +102,15 @@ type Specification struct { | |
|
|
||
| // ResourceLimitMem is the AGENT_INJECT_MEM_LIMIT environment variable. | ||
| ResourceLimitMem string `envconfig:"AGENT_INJECT_MEM_LIMIT"` | ||
|
|
||
| // TLSMinVersion is the AGENT_INJECT_TLS_MIN_VERSION environment variable | ||
| TLSMinVersion string `envconfig:"tls_min_version"` | ||
|
|
||
| // TLSCipherSuites is the AGENT_INJECT_TLS_CIPHER_SUITES environment variable | ||
| TLSCipherSuites string `envconfig:"tls_cipher_suites"` | ||
|
|
||
| // TLSPreferServerCipherSuites is the AGENT_INJECT_TLS_PREFER_SERVER_CIPHER_SUITES environment variable | ||
| TLSPreferServerCipherSuites string `envconfig:"tls_prefer_server_cipher_suites"` | ||
| } | ||
|
|
||
| func (c *Command) init() { | ||
|
|
@@ -160,6 +171,13 @@ func (c *Command) init() { | |
| c.flagSet.StringVar(&c.flagResourceLimitMem, "memory-limit", agent.DefaultResourceLimitMem, | ||
| fmt.Sprintf("Memory resource limit set in injected containers. Defaults to %s", agent.DefaultResourceLimitMem)) | ||
|
|
||
| c.flagSet.StringVar(&c.flagTLSMinVersion, "tls-min-version", defaultTLSMinVersion, | ||
| fmt.Sprintf(`Minimum supported version of TLS. Defaults to %s. Accepted values are "tls10", "tls11", "tls12" or "tls13"`, defaultTLSMinVersion)) | ||
benashz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| c.flagSet.StringVar(&c.flagTLSCipherSuites, "tls-cipher-suites", "", | ||
|
There was a problem hiding this comment. I wonder if we should include the |
||
| "Comma-separated list of supported cipher suites") | ||
|
There was a problem hiding this comment. Is it worth mentioning that the order will not be respected? I think Golang will automatically reorder right? Maybe that's quite an advanced detail to include in quite high level help text though. There was a problem hiding this comment. Perhaps, but it is really a matter of which go version was used during the build. I suppose that was one advantage of the previous flag. The server preferred order would be enabled regardless of version of go that was used. There was a problem hiding this comment. Yeah, in 1.17 the order is ignored, and it's also only honored for TLS 1.0-1.2 (I added that last part to the usage string in 50c85b8). |
||
| c.flagSet.BoolVar(&c.flagTLSPreferServerCipherSuites, "tls-prefer-server-cipher-suites", defaultTLSPreferServerCipherSuites, | ||
| fmt.Sprintf("Prefer the server's cipher suites over the client cipher suites. Defaults to %v", defaultTLSPreferServerCipherSuites)) | ||
|
|
||
| c.help = flags.Usage(help, c.flagSet) | ||
| } | ||
|
|
||
|
|
@@ -311,5 +329,20 @@ func (c *Command) parseEnvs() error { | |
| c.flagResourceLimitMem = envs.ResourceLimitMem | ||
| } | ||
|
|
||
| if envs.TLSMinVersion != "" { | ||
| c.flagTLSMinVersion = envs.TLSMinVersion | ||
| } | ||
|
|
||
| if envs.TLSCipherSuites != "" { | ||
| c.flagTLSCipherSuites = envs.TLSCipherSuites | ||
| } | ||
|
|
||
| if envs.TLSPreferServerCipherSuites != "" { | ||
| c.flagTLSPreferServerCipherSuites, err = strconv.ParseBool(envs.TLSPreferServerCipherSuites) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message mentions parsing, but really we received an unsupported TLS version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, changed in 05aeaaf to
invalid or unsupported TLS version