Skip to content

Commit 82058b0

Browse files
benashztomhjptvoran
authored
Ensure a consistent TLS configuration (#173) (#178) - backport to 1.11 (#190)
* Ensure a consistent TLS configuration (#173) (#178) * Ensure a consistent TLS configuration for k8s API requests Previously, it was possible for the http.Client's Transport to be missing the necessary root CAs to ensure that all TLS connections between the auth engine and the Kubernetes API were validated against a configured set of CA certificates. This fix ensures that the http.Client's Transport is always consistent with the configured CA cert chain, by introducing a periodic TLS configuration checker that is started as part of the backend's initialization. Other fixes: - only update the client's transport when the CA certificate pool has changed. Co-authored-by: Tom Proctor <[email protected]> * Repo hygiene (#162) * update go and k8s versions go 1.19.1 k8s up to 1.25.0 * updated x/net and x/sys go get golang.org/x/[email protected] go get golang.org/x/[email protected] * make fmt making gofumpt happy * update chart and vault version Default to k8s 1.25.0 use chart 0.22.0 use vault 1.11.3 * Remove unused import add from cherry-pick --------- Co-authored-by: Tom Proctor <[email protected]> Co-authored-by: Theron Voran <[email protected]>
1 parent 37e92c7 commit 82058b0

File tree

14 files changed

+893
-159
lines changed

14 files changed

+893
-159
lines changed

.github/workflows/tests.yaml

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ jobs:
1515
- uses: actions/checkout@v2
1616
- uses: actions/setup-go@v2
1717
with:
18-
go-version: 1.18.0
18+
go-version: 1.19.1
1919
- uses: actions/cache@v2
2020
with:
2121
path: |
@@ -32,23 +32,23 @@ jobs:
3232
strategy:
3333
fail-fast: false
3434
matrix:
35-
kind-k8s-version: [1.21.10, 1.22.7, 1.23.6, 1.24.0]
35+
kind-k8s-version: [1.22.13, 1.23.10, 1.24.4, 1.25.0]
3636
steps:
3737
- uses: actions/checkout@v2
3838
- name: Create K8s Kind Cluster
3939
uses: helm/[email protected]
4040
with:
41-
version: v0.13.0
41+
version: v0.14.0
4242
cluster_name: vault-plugin-auth-kubernetes
4343
config: integrationtest/kind/config.yaml
4444
node_image: kindest/node:v${{ matrix.kind-k8s-version }}
4545
# Must come _after_ kind-action, because the kind step also sets up a kubectl binary.
4646
- uses: azure/[email protected]
4747
with:
48-
version: 'v1.24.0'
48+
version: 'v1.25.0'
4949
- uses: actions/setup-go@v2
5050
with:
51-
go-version: 1.18.0
51+
go-version: 1.19.1
5252
- uses: actions/cache@v2
5353
with:
5454
path: |

Makefile

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1+
TESTARGS ?= '-test.v'
12
# kind cluster name
2-
KIND_CLUSTER_NAME?=vault-plugin-auth-kubernetes
3+
KIND_CLUSTER_NAME ?= vault-plugin-auth-kubernetes
34

45
# kind k8s version
5-
KIND_K8S_VERSION?=v1.24.0
6+
KIND_K8S_VERSION ?= v1.25.0
67

78
.PHONY: default
89
default: dev
@@ -13,11 +14,11 @@ dev:
1314

1415
.PHONY: test
1516
test: fmtcheck
16-
CGO_ENABLED=0 go test ./... $(TESTARGS) -timeout=20m
17+
CGO_ENABLED=0 go test $(TESTARGS) -timeout=20m ./...
1718

1819
.PHONY: integration-test
1920
integration-test:
20-
INTEGRATION_TESTS=true CGO_ENABLED=0 go test github.com/hashicorp/vault-plugin-auth-kubernetes/integrationtest/... $(TESTARGS) -count=1 -timeout=20m
21+
INTEGRATION_TESTS=true CGO_ENABLED=0 go test $(TESTARGS) -count=1 -timeout=20m github.com/hashicorp/vault-plugin-auth-kubernetes/integrationtest/...
2122

2223
.PHONY: fmtcheck
2324
fmtcheck:
@@ -52,12 +53,13 @@ vault-image:
5253
setup-integration-test: teardown-integration-test vault-image
5354
kind --name ${KIND_CLUSTER_NAME} load docker-image hashicorp/vault:dev
5455
kubectl create namespace test
55-
helm install vault vault --repo https://helm.releases.hashicorp.com --version=0.19.0 \
56+
helm install vault vault --repo https://helm.releases.hashicorp.com --version=0.22.0 \
5657
--wait --timeout=5m \
5758
--namespace=test \
5859
--set server.dev.enabled=true \
5960
--set server.image.tag=dev \
6061
--set server.image.pullPolicy=Never \
62+
--set server.logLevel=trace \
6163
--set injector.enabled=false \
6264
--set server.extraArgs="-dev-plugin-dir=/vault/plugin_directory"
6365
kubectl patch --namespace=test statefulset vault --patch-file integrationtest/vault/hostPortPatch.yaml
@@ -69,4 +71,4 @@ setup-integration-test: teardown-integration-test vault-image
6971
.PHONY: teardown-integration-test
7072
teardown-integration-test:
7173
helm uninstall vault --namespace=test || true
72-
kubectl delete --ignore-not-found namespace test
74+
kubectl delete --ignore-not-found namespace test

backend.go

Lines changed: 208 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"crypto/tls"
77
"crypto/x509"
88
"encoding/json"
9+
"errors"
910
"fmt"
1011
"net/http"
1112
"strings"
@@ -26,6 +27,7 @@ const (
2627
aliasNameSourceSAUid = "serviceaccount_uid"
2728
aliasNameSourceSAName = "serviceaccount_name"
2829
aliasNameSourceDefault = aliasNameSourceSAUid
30+
minTLSVersion = tls.VersionTLS12
2931
)
3032

3133
var (
@@ -44,6 +46,17 @@ var (
4446
// caReloadPeriod is the time period how often the in-memory copy of local
4547
// CA cert can be used, before reading it again from disk.
4648
caReloadPeriod = 1 * time.Hour
49+
50+
// defaultHorizon provides the default duration to be used
51+
// in the tlsConfigUpdater's time.Ticker, setup in runTLSConfigUpdater()
52+
defaultHorizon = time.Second * 30
53+
54+
// defaultMinHorizon provides the minimum duration that can be specified
55+
// in the tlsConfigUpdater's time.Ticker, setup in runTLSConfigUpdater()
56+
defaultMinHorizon = time.Second * 5
57+
58+
errTLSConfigNotSet = errors.New("TLSConfig not set")
59+
errHTTPClientNotSet = errors.New("http.Client not set")
4760
)
4861

4962
// kubeAuthBackend implements logical.Backend
@@ -53,8 +66,11 @@ type kubeAuthBackend struct {
5366
// default HTTP client for connection reuse
5467
httpClient *http.Client
5568

69+
// tlsConfig is periodically updated whenever the CA certificate configuration changes.
70+
tlsConfig *tls.Config
71+
5672
// reviewFactory is used to configure the strategy for doing a token review.
57-
// Currently the only options are using the kubernetes API or mocking the
73+
// Currently, the only options are using the kubernetes API or mocking the
5874
// review. Mocks should only be used in tests.
5975
reviewFactory tokenReviewFactory
6076

@@ -71,22 +87,47 @@ type kubeAuthBackend struct {
7187
// - disable_local_ca_jwt is false
7288
localCACertReader *cachingFileReader
7389

90+
// tlsConfigUpdaterRunning is used to signal the current state of the tlsConfig updater routine.
91+
tlsConfigUpdaterRunning bool
92+
93+
// tlsConfigUpdateCancelFunc should be called in the backend's Clean(), set in initialize().
94+
tlsConfigUpdateCancelFunc context.CancelFunc
95+
7496
l sync.RWMutex
97+
98+
// tlsMu provides the lock for synchronizing updates to the tlsConfig.
99+
tlsMu sync.RWMutex
75100
}
76101

77102
// Factory returns a new backend as logical.Backend.
78103
func Factory(ctx context.Context, conf *logical.BackendConfig) (logical.Backend, error) {
79104
b := Backend()
105+
80106
if err := b.Setup(ctx, conf); err != nil {
81107
return nil, err
82108
}
109+
83110
return b, nil
84111
}
85112

113+
var getDefaultHTTPClient = cleanhttp.DefaultPooledClient
114+
115+
func getDefaultTLSConfig() *tls.Config {
116+
return &tls.Config{
117+
MinVersion: minTLSVersion,
118+
}
119+
}
120+
86121
func Backend() *kubeAuthBackend {
87122
b := &kubeAuthBackend{
88123
localSATokenReader: newCachingFileReader(localJWTPath, jwtReloadPeriod, time.Now),
89124
localCACertReader: newCachingFileReader(localCACertPath, caReloadPeriod, time.Now),
125+
// Set default HTTP client
126+
httpClient: getDefaultHTTPClient(),
127+
// Set the default TLSConfig
128+
tlsConfig: getDefaultTLSConfig(),
129+
// Set the review factory to default to calling into the kubernetes API.
130+
reviewFactory: tokenReviewAPIFactory,
90131
}
91132

92133
b.Backend = &framework.Backend{
@@ -109,46 +150,129 @@ func Backend() *kubeAuthBackend {
109150
pathsRole(b),
110151
),
111152
InitializeFunc: b.initialize,
153+
Clean: b.cleanup,
112154
}
113155

114-
// Set default HTTP client
115-
b.httpClient = cleanhttp.DefaultPooledClient()
116-
117-
// Set the review factory to default to calling into the kubernetes API.
118-
b.reviewFactory = tokenReviewAPIFactory
119-
120156
return b
121157
}
122158

123159
// initialize is used to handle the state of config values just after the K8s plugin has been mounted
124160
func (b *kubeAuthBackend) initialize(ctx context.Context, req *logical.InitializationRequest) error {
125-
// Try to load the config on initialization
126-
config, err := b.loadConfig(ctx, req.Storage)
161+
updaterCtx, cancel := context.WithCancel(context.Background())
162+
if err := b.runTLSConfigUpdater(updaterCtx, req.Storage, defaultHorizon); err != nil {
163+
cancel()
164+
return err
165+
}
166+
167+
b.tlsConfigUpdateCancelFunc = cancel
168+
169+
config, err := b.config(ctx, req.Storage)
127170
if err != nil {
128171
return err
129172
}
130-
if config == nil {
173+
174+
if config != nil {
175+
if err := b.updateTLSConfig(config); err != nil {
176+
return err
177+
}
178+
}
179+
180+
return nil
181+
}
182+
183+
func (b *kubeAuthBackend) cleanup(_ context.Context) {
184+
b.shutdownTLSConfigUpdater()
185+
}
186+
187+
// validateHTTPClientInit that the Backend's HTTPClient and TLSConfig has been properly instantiated.
188+
func (b *kubeAuthBackend) validateHTTPClientInit() error {
189+
if b.httpClient == nil {
190+
return errHTTPClientNotSet
191+
}
192+
if b.tlsConfig == nil {
193+
return errTLSConfigNotSet
194+
}
195+
196+
return nil
197+
}
198+
199+
// runTLSConfigUpdater sets up a routine that periodically calls b.updateTLSConfig(). This ensures that the
200+
// httpClient's TLS configuration is consistent with the backend's stored configuration.
201+
func (b *kubeAuthBackend) runTLSConfigUpdater(ctx context.Context, s logical.Storage, horizon time.Duration) error {
202+
b.tlsMu.Lock()
203+
defer b.tlsMu.Unlock()
204+
205+
if b.tlsConfigUpdaterRunning {
131206
return nil
132207
}
133208

134-
b.l.Lock()
135-
defer b.l.Unlock()
136-
// If we have a CA cert build the TLSConfig
137-
if len(config.CACert) > 0 {
138-
certPool := x509.NewCertPool()
139-
certPool.AppendCertsFromPEM([]byte(config.CACert))
209+
if horizon < defaultMinHorizon {
210+
return fmt.Errorf("update horizon must be equal to or greater than %s", defaultMinHorizon)
211+
}
212+
213+
if err := b.validateHTTPClientInit(); err != nil {
214+
return err
215+
}
216+
217+
updateTLSConfig := func(ctx context.Context, s logical.Storage) error {
218+
config, err := b.config(ctx, s)
219+
if err != nil {
220+
return fmt.Errorf("failed config read, err=%w", err)
221+
}
140222

141-
tlsConfig := &tls.Config{
142-
MinVersion: tls.VersionTLS12,
143-
RootCAs: certPool,
223+
if config == nil {
224+
b.Logger().Trace("Skipping TLSConfig update, no configuration set")
225+
return nil
226+
}
227+
228+
if err := b.updateTLSConfig(config); err != nil {
229+
return err
144230
}
145231

146-
b.httpClient.Transport.(*http.Transport).TLSClientConfig = tlsConfig
232+
return nil
147233
}
148234

235+
var wg sync.WaitGroup
236+
wg.Add(1)
237+
ticker := time.NewTicker(horizon)
238+
go func(ctx context.Context, s logical.Storage) {
239+
defer func() {
240+
b.tlsMu.Lock()
241+
defer b.tlsMu.Unlock()
242+
ticker.Stop()
243+
b.tlsConfigUpdaterRunning = false
244+
b.Logger().Trace("TLSConfig updater shutdown completed")
245+
}()
246+
247+
b.Logger().Trace("TLSConfig updater starting", "horizon", horizon)
248+
b.tlsConfigUpdaterRunning = true
249+
wg.Done()
250+
for {
251+
select {
252+
case <-ctx.Done():
253+
b.Logger().Trace("TLSConfig updater shutting down")
254+
return
255+
case <-ticker.C:
256+
if err := updateTLSConfig(ctx, s); err != nil {
257+
b.Logger().Warn("TLSConfig update failed, retrying",
258+
"horizon", defaultHorizon.String(), "err", err)
259+
}
260+
}
261+
}
262+
}(ctx, s)
263+
wg.Wait()
264+
149265
return nil
150266
}
151267

268+
func (b *kubeAuthBackend) shutdownTLSConfigUpdater() {
269+
if b.tlsConfigUpdateCancelFunc != nil {
270+
b.Logger().Debug("TLSConfig updater shutdown requested")
271+
b.tlsConfigUpdateCancelFunc()
272+
b.tlsConfigUpdateCancelFunc = nil
273+
}
274+
}
275+
152276
// config takes a storage object and returns a kubeConfig object.
153277
// It does not return local token and CA file which are specific to the pod we run in.
154278
func (b *kubeAuthBackend) config(ctx context.Context, s logical.Storage) (*kubeConfig, error) {
@@ -255,6 +379,70 @@ func (b *kubeAuthBackend) role(ctx context.Context, s logical.Storage, name stri
255379
return role, nil
256380
}
257381

382+
// getHTTPClient return the backend's HTTP client for connecting to the Kubernetes API.
383+
func (b *kubeAuthBackend) getHTTPClient() (*http.Client, error) {
384+
b.tlsMu.RLock()
385+
defer b.tlsMu.RUnlock()
386+
387+
if err := b.validateHTTPClientInit(); err != nil {
388+
return nil, err
389+
}
390+
391+
return b.httpClient, nil
392+
}
393+
394+
// updateTLSConfig ensures that the httpClient's TLS configuration is consistent
395+
// with the backend's stored configuration.
396+
func (b *kubeAuthBackend) updateTLSConfig(config *kubeConfig) error {
397+
b.tlsMu.Lock()
398+
defer b.tlsMu.Unlock()
399+
400+
if err := b.validateHTTPClientInit(); err != nil {
401+
return err
402+
}
403+
404+
// attempt to read the CA certificates from the config directly or from the filesystem.
405+
var caCertBytes []byte
406+
if config.CACert != "" {
407+
caCertBytes = []byte(config.CACert)
408+
} else if !config.DisableLocalCAJwt && b.localCACertReader != nil {
409+
data, err := b.localCACertReader.ReadFile()
410+
if err != nil {
411+
return err
412+
}
413+
caCertBytes = []byte(data)
414+
}
415+
416+
certPool := x509.NewCertPool()
417+
if len(caCertBytes) > 0 {
418+
if ok := certPool.AppendCertsFromPEM(caCertBytes); !ok {
419+
b.Logger().Warn("Configured CA PEM data contains no valid certificates, TLS verification will fail")
420+
}
421+
} else {
422+
// provide an empty certPool
423+
b.Logger().Warn("No CA certificates configured, TLS verification will fail")
424+
// TODO: think about supporting host root CA certificates via a configuration toggle,
425+
// in which case RootCAs should be set to nil
426+
}
427+
428+
// only refresh the Root CAs if they have changed since the last full update.
429+
if !b.tlsConfig.RootCAs.Equal(certPool) {
430+
b.Logger().Trace("Root CA certificate pool has changed, updating the client's transport")
431+
transport, ok := b.httpClient.Transport.(*http.Transport)
432+
if !ok {
433+
// should never happen
434+
return fmt.Errorf("type assertion failed for %T", b.httpClient.Transport)
435+
}
436+
437+
b.tlsConfig.RootCAs = certPool
438+
transport.TLSClientConfig = b.tlsConfig
439+
} else {
440+
b.Logger().Trace("Root CA certificate pool is unchanged, no update required")
441+
}
442+
443+
return nil
444+
}
445+
258446
func validateAliasNameSource(source string) error {
259447
for _, s := range aliasNameSources {
260448
if s == source {

0 commit comments

Comments
 (0)