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

TLS server options #302

Merged
merged 8 commits into from
Oct 21, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
## Unreleased

Improvements:
* Added options for setting the TLS minimum version (default 1.2) and supported cipher suites: [GH-302](https://github.com/hashicorp/vault-k8s/pull/302)

## 0.13.1 (September 29, 2021)

Changes:
Expand Down
5 changes: 5 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ go 1.17
require (
github.com/cenkalti/backoff/v4 v4.1.1
github.com/hashicorp/go-hclog v0.9.2
github.com/hashicorp/go-secure-stdlib/tlsutil v0.1.1
github.com/hashicorp/vault/sdk v0.1.14-0.20191205220236-47cffd09f972
github.com/kelseyhightower/envconfig v1.4.0
github.com/kr/text v0.2.0
Expand Down Expand Up @@ -36,12 +37,16 @@ require (
github.com/googleapis/gnostic v0.5.5 // indirect
github.com/hashicorp/errwrap v1.0.0 // indirect
github.com/hashicorp/go-multierror v1.0.0 // indirect
github.com/hashicorp/go-secure-stdlib/parseutil v0.1.1 // indirect
github.com/hashicorp/go-secure-stdlib/strutil v0.1.1 // indirect
github.com/hashicorp/go-sockaddr v1.0.2 // indirect
github.com/hashicorp/golang-lru v0.5.4 // indirect
github.com/imdario/mergo v0.3.12 // indirect
github.com/json-iterator/go v1.1.11 // indirect
github.com/mattn/go-colorable v0.1.8 // indirect
github.com/mattn/go-isatty v0.0.12 // indirect
github.com/matttproud/golang_protobuf_extensions v1.0.2-0.20181231171920-c182affec369 // indirect
github.com/mitchellh/mapstructure v1.4.1 // indirect
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
github.com/modern-go/reflect2 v1.0.1 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
Expand Down
9 changes: 9 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,14 @@ github.com/hashicorp/go-multierror v1.0.0/go.mod h1:dHtQlpGsu+cZNNAkkCN/P3hoUDHh
github.com/hashicorp/go-plugin v1.0.1/go.mod h1:++UyYGoz3o5w9ZzAdZxtQKrWWP+iqPBn3cQptSMzBuY=
github.com/hashicorp/go-retryablehttp v0.5.3/go.mod h1:9B5zBasrRhHXnJnui7y6sL7es7NDiJgTc6Er0maI1Xs=
github.com/hashicorp/go-rootcerts v1.0.0/go.mod h1:K6zTfqpRlCUIjkwsN4Z+hiSfzSTQa6eBIzfwKfwNnHU=
github.com/hashicorp/go-secure-stdlib/parseutil v0.1.1 h1:78ki3QBevHwYrVxnyVeaEz+7WtifHhauYF23es/0KlI=
github.com/hashicorp/go-secure-stdlib/parseutil v0.1.1/go.mod h1:QmrqtbKuxxSWTN3ETMPuB+VtEiBJ/A9XhoYGv8E1uD8=
github.com/hashicorp/go-secure-stdlib/strutil v0.1.1 h1:nd0HIW15E6FG1MsnArYaHfuw9C2zgzM8LxkG5Ty/788=
github.com/hashicorp/go-secure-stdlib/strutil v0.1.1/go.mod h1:gKOamz3EwoIoJq7mlMIRBpVTAUn8qPCrEclOKKWhD3U=
github.com/hashicorp/go-secure-stdlib/tlsutil v0.1.1 h1:Yc026VyMyIpq1UWRnakHRG01U8fJm+nEfEmjoAb00n8=
github.com/hashicorp/go-secure-stdlib/tlsutil v0.1.1/go.mod h1:l8slYwnJA26yBz+ErHpp2IRCLr0vuOMGBORIz4rRiAs=
github.com/hashicorp/go-sockaddr v1.0.0/go.mod h1:7Xibr9yA9JjQq1JpNB2Vw7kxv8xerXegt+ozgdvDeDU=
github.com/hashicorp/go-sockaddr v1.0.2 h1:ztczhD1jLxIRjVejw8gFomI1BQZOe2WoVOu0SyteCQc=
github.com/hashicorp/go-sockaddr v1.0.2/go.mod h1:rB4wwRAUzs07qva3c5SdrY/NEtAUjGlgmH/UkBUC97A=
github.com/hashicorp/go-syslog v1.0.0/go.mod h1:qPfqrKkXGihmCqbJM2mZgkZGvKG1dFdvsLplgctolz4=
github.com/hashicorp/go-uuid v1.0.0/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro=
Expand Down Expand Up @@ -334,6 +341,8 @@ github.com/mitchellh/gox v0.4.0/go.mod h1:Sd9lOJ0+aimLBi73mGofS1ycjY8lL3uZM3JPS4
github.com/mitchellh/iochan v1.0.0/go.mod h1:JwYml1nuB7xOzsp52dPpHFffvOCDupsG0QubkSMEySY=
github.com/mitchellh/mapstructure v0.0.0-20160808181253-ca63d7c062ee/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y=
github.com/mitchellh/mapstructure v1.1.2/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y=
github.com/mitchellh/mapstructure v1.4.1 h1:CpVNEelQCZBooIPDn+AR3NpivK/TIKU8bDxdASFVQag=
github.com/mitchellh/mapstructure v1.4.1/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo=
github.com/mitchellh/reflectwalk v1.0.0/go.mod h1:mSTlrgnPZtwu0c4WaC2kGObEpuNDbx0jmZXqmk4esnw=
github.com/moby/spdystream v0.2.0/go.mod h1:f7i0iNDQJ059oMTcWxx8MA/zKFIuD/lY+0GqbN2Wy8c=
github.com/moby/term v0.0.0-20201216013528-df9cb8a40635/go.mod h1:FBS0z0QWA44HXygs7VXDUOGoN/1TV3RuWkLO04am3wc=
Expand Down
41 changes: 37 additions & 4 deletions subcommand/injector/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"time"

"github.com/hashicorp/go-hclog"
"github.com/hashicorp/go-secure-stdlib/tlsutil"
agentInject "github.com/hashicorp/vault-k8s/agent-inject"
"github.com/hashicorp/vault-k8s/helper/cert"
"github.com/hashicorp/vault-k8s/leader"
Expand Down Expand Up @@ -62,6 +63,8 @@ type Command struct {
flagResourceRequestMem string // Set Memory request in the injected containers
flagResourceLimitCPU string // Set CPU limit in the injected containers
flagResourceLimitMem string // Set Memory limit in the injected containers
flagTLSMinVersion string // Minimum TLS version supported by the webhook server
flagTLSCipherSuites string // Comma-separated list of supported cipher suites

flagSet *flag.FlagSet

Expand Down Expand Up @@ -120,7 +123,8 @@ func (c *Command) Run(args []string) int {
logger := hclog.New(&hclog.LoggerOptions{
Name: "handler",
Level: level,
JSONFormat: (c.flagLogFormat == "json")})
JSONFormat: (c.flagLogFormat == "json"),
})

namespace := getNamespace()
var secrets informerv1.SecretInformer
Expand Down Expand Up @@ -206,10 +210,15 @@ func (c *Command) Run(args []string) int {
}

var handler http.Handler = mux
tlsConfig, err := c.makeTLSConfig()
if err != nil {
c.UI.Error(fmt.Sprintf("Failed to configure TLS: %s", err))
return 1
}
server := &http.Server{
Addr: c.flagListen,
Handler: handler,
TLSConfig: &tls.Config{GetCertificate: c.getCertificate},
TLSConfig: tlsConfig,
ErrorLog: logger.StandardLogger(&hclog.StandardLoggerOptions{ForceLevel: hclog.Error}),
}

Expand Down Expand Up @@ -265,6 +274,28 @@ func (c *Command) handleReady(rw http.ResponseWriter, req *http.Request) {
rw.WriteHeader(204)
}

func (c *Command) makeTLSConfig() (*tls.Config, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

minTLSVersion, ok := tlsutil.TLSLookup[c.flagTLSMinVersion]
if !ok {
return nil, fmt.Errorf("invalid or unsupported TLS version %q", c.flagTLSMinVersion)
}

ciphers, err := tlsutil.ParseCiphers(c.flagTLSCipherSuites)
if err != nil {
return nil, fmt.Errorf("failed to parse TLS cipher suites list %q: %s", c.flagTLSCipherSuites, err)
}

tlsConfig := &tls.Config{
GetCertificate: c.getCertificate,
MinVersion: minTLSVersion,
}
if len(ciphers) > 0 {
tlsConfig.CipherSuites = ciphers
}

return tlsConfig, nil
}

func (c *Command) getCertificate(*tls.ClientHelloInfo) (*tls.Certificate, error) {
certRaw := c.cert.Load()
if certRaw == nil {
Expand Down Expand Up @@ -369,8 +400,10 @@ func (c *Command) Help() string {
return c.help
}

const synopsis = "Vault Agent injector service"
const help = `
const (
synopsis = "Vault Agent injector service"
help = `
Usage: vault-k8s agent-inject [options]
Run the Admission Webhook server for injecting Vault Agent containers into pods.
`
)
61 changes: 61 additions & 0 deletions subcommand/injector/command_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
package injector

import (
"fmt"
"strings"
"testing"

"github.com/stretchr/testify/assert"
)

func TestTLSConfig(t *testing.T) {
tests := map[string]struct {
tlsVersion string
suites string
expectedError error
}{
"defaults": {
tlsVersion: defaultTLSMinVersion,
suites: "",
expectedError: nil,
},
"bad tls": {
tlsVersion: "tls1000",
suites: "",
expectedError: fmt.Errorf(`invalid or unsupported TLS version "tls1000"`),
},
"non-default tls": {
tlsVersion: "tls13",
suites: "",
expectedError: nil,
},
"suites specified": {
tlsVersion: defaultTLSMinVersion,
suites: "TLS_RSA_WITH_AES_128_GCM_SHA256,TLS_RSA_WITH_AES_256_GCM_SHA384",
expectedError: nil,
},
"invalid suites specified": {
tlsVersion: defaultTLSMinVersion,
suites: "suite1,suite2,suite3",
expectedError: fmt.Errorf(`failed to parse TLS cipher suites list "suite1,suite2,suite3": unsupported cipher "suite1"`),
},
}

for name, tc := range tests {
t.Run(name, func(t *testing.T) {
c := &Command{}
c.flagTLSMinVersion = tc.tlsVersion
c.flagTLSCipherSuites = tc.suites
result, err := c.makeTLSConfig()
assert.Equal(t, tc.expectedError, err)
if tc.expectedError == nil {
assert.NotZero(t, result.MinVersion)
if len(tc.suites) == 0 {
assert.Nil(t, result.CipherSuites)
} else {
assert.Len(t, result.CipherSuites, len(strings.Split(tc.suites, ",")))
}
}
})
}
}
32 changes: 30 additions & 2 deletions subcommand/injector/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,21 @@ package injector
import (
"flag"
"fmt"
"sort"
"strconv"
"strings"

"github.com/hashicorp/go-hclog"
"github.com/hashicorp/go-secure-stdlib/tlsutil"
"github.com/hashicorp/vault-k8s/agent-inject/agent"
"github.com/hashicorp/vault-k8s/helper/flags"
"github.com/kelseyhightower/envconfig"
)

const (
DefaultLogLevel = "info"
DefaultLogFormat = "standard"
DefaultLogLevel = "info"
DefaultLogFormat = "standard"
defaultTLSMinVersion = "tls12"
)

// Specification are the supported environment variables, prefixed with
Expand Down Expand Up @@ -100,6 +103,12 @@ 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"`
}

func (c *Command) init() {
Expand Down Expand Up @@ -160,6 +169,17 @@ 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))

tlsVersions := []string{}
for v := range tlsutil.TLSLookup {
tlsVersions = append(tlsVersions, v)
}
sort.Strings(tlsVersions)
tlsStr := strings.Join(tlsVersions, ", ")
c.flagSet.StringVar(&c.flagTLSMinVersion, "tls-min-version", defaultTLSMinVersion,
fmt.Sprintf(`Minimum supported version of TLS. Defaults to %s. Accepted values are %s.`, defaultTLSMinVersion, tlsStr))
c.flagSet.StringVar(&c.flagTLSCipherSuites, "tls-cipher-suites", "",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should include the env variable name in the usage?

"Comma-separated list of supported cipher suites for TLS 1.0-1.2")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I follow, does this mean we cannot set the cipher suites for any TLS version above 1.2?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the docs, Note that TLS 1.3 ciphersuites are not configurable.

When testing this, it seems to just ignore that flag when TLS 1.3 is set. No errors thrown or anything.

Copy link
Contributor

@benashz benashz Oct 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very interesting...

In that case, perhaps rephrasing it a bit to be:

Suggested change
"Comma-separated list of supported cipher suites for TLS 1.0-1.2")
"Comma-separated list of supported cipher suites (ignored for tls13 and higher)")

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, though I think we'll keep this phrasing for now.


c.help = flags.Usage(help, c.flagSet)
}

Expand Down Expand Up @@ -311,5 +331,13 @@ func (c *Command) parseEnvs() error {
c.flagResourceLimitMem = envs.ResourceLimitMem
}

if envs.TLSMinVersion != "" {
c.flagTLSMinVersion = envs.TLSMinVersion
}

if envs.TLSCipherSuites != "" {
c.flagTLSCipherSuites = envs.TLSCipherSuites
}

return nil
}
2 changes: 2 additions & 0 deletions subcommand/injector/flags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,8 @@ func TestCommandEnvs(t *testing.T) {
{env: "AGENT_INJECT_CPU_LIMIT", value: "1000m", cmdPtr: &cmd.flagResourceLimitCPU},
{env: "AGENT_INJECT_MEM_LIMIT", value: "256m", cmdPtr: &cmd.flagResourceLimitMem},
{env: "AGENT_INJECT_TEMPLATE_STATIC_SECRET_RENDER_INTERVAL", value: "12s", cmdPtr: &cmd.flagStaticSecretRenderInterval},
{env: "AGENT_INJECT_TLS_MIN_VERSION", value: "tls13", cmdPtr: &cmd.flagTLSMinVersion},
{env: "AGENT_INJECT_TLS_CIPHER_SUITES", value: "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA,TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA", cmdPtr: &cmd.flagTLSCipherSuites},
}

for _, tt := range tests {
Expand Down