From 32d04b9ce7f9cb2f28ea235669aa28c00bfeb018 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Wed, 15 Jun 2022 22:39:52 -0400 Subject: [PATCH] Allow a worker to use KMS auth but accept PKI for proxy (#2206) This essentially worked for non-dev mode before but would create storage directories that weren't actually used or needed, and it didn't work in dev mode because it made assumptions about the presence of a worker auth kms or not. This fixes those behaviors. --- go.mod | 2 +- go.sum | 4 ++-- internal/cmd/base/servers.go | 4 ++++ internal/cmd/commands/dev/dev.go | 21 +++++++++--------- internal/cmd/commands/server/server.go | 18 +++++++-------- internal/daemon/worker/auth_rotation.go | 11 +++++----- .../daemon/worker/controller_connection.go | 2 +- internal/daemon/worker/testing.go | 4 ++++ internal/daemon/worker/worker.go | 18 +++++++-------- internal/daemon/worker/worker_test.go | 22 ++++++++++--------- 10 files changed, 58 insertions(+), 48 deletions(-) diff --git a/go.mod b/go.mod index 99b0ebca12..b8fa62ab11 100644 --- a/go.mod +++ b/go.mod @@ -92,7 +92,7 @@ require github.com/hashicorp/go-dbw v0.0.0-20220412153211-c470aec9369f // this i require ( github.com/DATA-DOG/go-sqlmock v1.5.0 github.com/hashicorp/go-kms-wrapping/extras/kms/v2 v2.0.0-20220515130442-cac0b5ac133b - github.com/hashicorp/nodeenrollment v0.1.0 + github.com/hashicorp/nodeenrollment v0.1.1 ) require ( diff --git a/go.sum b/go.sum index ec430c6cbc..fea4afdbad 100644 --- a/go.sum +++ b/go.sum @@ -740,8 +740,8 @@ github.com/hashicorp/golang-lru v0.5.4 h1:YDjusn29QI/Das2iO9M0BHnIbxPeyuCHsjMW+l github.com/hashicorp/golang-lru v0.5.4/go.mod h1:iADmTwqILo4mZ8BN3D2Q6+9jd8WM5uGBxy+E8yxSoD4= github.com/hashicorp/hcl v1.0.0 h1:0Anlzjpi4vEasTeNFn2mLJgTSwt0+6sfsiTG8qcWGx4= github.com/hashicorp/hcl v1.0.0/go.mod h1:E5yfLk+7swimpb2L/Alb/PJmXilQ/rhwaUYs4T20WEQ= -github.com/hashicorp/nodeenrollment v0.1.0 h1:mhMVbrHM6NTDcpte/mOilP3Fd/HrcctshGs4/82EO6U= -github.com/hashicorp/nodeenrollment v0.1.0/go.mod h1:LIPKi+g0g/vl3xhpbzugCalHSxX1PMeqnatkAsxRgyM= +github.com/hashicorp/nodeenrollment v0.1.1 h1:tDR6UPJKrMDHAwLyeyFMRX5G2E1Hu6gi9/m1qrdXET0= +github.com/hashicorp/nodeenrollment v0.1.1/go.mod h1:LIPKi+g0g/vl3xhpbzugCalHSxX1PMeqnatkAsxRgyM= github.com/hashicorp/vault/api v1.3.1 h1:pkDkcgTh47PRjY1NEFeofqR4W/HkNUi9qIakESO2aRM= github.com/hashicorp/vault/api v1.3.1/go.mod h1:QeJoWxMFt+MsuWcYhmwRLwKEXrjwAFFywzhptMsTIUw= github.com/hashicorp/vault/sdk v0.1.13/go.mod h1:B+hVj7TpuQY1Y/GPbCpffmgd+tSEwvhkWnjtSYCaS2M= diff --git a/internal/cmd/base/servers.go b/internal/cmd/base/servers.go index 91cb44566d..dc1ed2e381 100644 --- a/internal/cmd/base/servers.go +++ b/internal/cmd/base/servers.go @@ -126,6 +126,10 @@ type Server struct { DevTargetSessionConnectionLimit int DevLoopbackHostPluginId string + // DevUsePkiForUpstream is a hint that we are in dev mode and have a worker + // auth KMS but want to use PKI for upstream connections + DevUsePkiForUpstream bool + EnabledPlugins []EnabledPlugin HostPlugins map[string]plgpb.HostPluginServiceClient diff --git a/internal/cmd/commands/dev/dev.go b/internal/cmd/commands/dev/dev.go index 458cc89e36..8d073ee6d2 100644 --- a/internal/cmd/commands/dev/dev.go +++ b/internal/cmd/commands/dev/dev.go @@ -577,7 +577,7 @@ func (c *Command) Run(args []string) int { if c.flagRecoveryKey != "" { c.Config.DevRecoveryKey = c.flagRecoveryKey } - if err := c.SetupKMSes(c.Context, c.UI, c.Config, base.WithSkipWorkerAuthKmsInstantiation(!c.flagUseKmsWorkerAuthMethod)); err != nil { + if err := c.SetupKMSes(c.Context, c.UI, c.Config); err != nil { c.UI.Error(err.Error()) return base.CommandUserError } @@ -585,14 +585,15 @@ func (c *Command) Run(args []string) int { c.UI.Error("Controller KMS not found after parsing KMS blocks") return base.CommandUserError } - if c.flagUseKmsWorkerAuthMethod { - if c.WorkerAuthKms == nil { - c.UI.Error("Worker Auth KMS not found after parsing KMS blocks") - return base.CommandUserError - } - c.InfoKeys = append(c.InfoKeys, "[Worker-Auth] AEAD Key Bytes") - c.Info["[Worker-Auth] AEAD Key Bytes"] = c.Config.DevWorkerAuthKey - } else { + if c.WorkerAuthKms == nil { + c.UI.Error("Worker Auth KMS not found after parsing KMS blocks") + return base.CommandUserError + } + c.InfoKeys = append(c.InfoKeys, "[Worker-Auth] AEAD Key Bytes") + c.Info["[Worker-Auth] AEAD Key Bytes"] = c.Config.DevWorkerAuthKey + + if !c.flagUseKmsWorkerAuthMethod { + c.DevUsePkiForUpstream = true // These must be unset for PKI c.Config.Worker.Name = "" c.Config.Worker.Description = "" @@ -809,7 +810,7 @@ func (c *Command) Run(args []string) int { } if !c.flagControllerOnly { - if !c.flagWorkerAuthStorageSkipCleanup { + if !c.flagWorkerAuthStorageSkipCleanup && c.worker.WorkerAuthStorage != nil { c.worker.WorkerAuthStorage.Cleanup() } if err := c.worker.Shutdown(); err != nil { diff --git a/internal/cmd/commands/server/server.go b/internal/cmd/commands/server/server.go index 52c6762404..5013282e32 100644 --- a/internal/cmd/commands/server/server.go +++ b/internal/cmd/commands/server/server.go @@ -494,7 +494,7 @@ func (c *Command) Run(args []string) int { return base.CommandCliError } - if c.WorkerAuthKms == nil && c.worker.WorkerAuthRegistrationRequest != "" { + if c.worker.WorkerAuthRegistrationRequest != "" { c.InfoKeys = append(c.InfoKeys, "worker auth registration request") c.Info["worker auth registration request"] = c.worker.WorkerAuthRegistrationRequest c.InfoKeys = append(c.InfoKeys, "worker auth current key id") @@ -677,14 +677,6 @@ func (c *Command) StartWorker() error { return fmt.Errorf("Error initializing worker: %w", err) } - if c.WorkerAuthKms == nil { - if c.worker.WorkerAuthStorage == nil { - return fmt.Errorf("No worker auth KMS specified and no worker auth storage found") - } - c.InfoKeys = append(c.InfoKeys, "worker auth storage path") - c.Info["worker auth storage path"] = c.worker.WorkerAuthStorage.BaseDir() - } - if err := c.worker.Start(); err != nil { retErr := fmt.Errorf("Error starting worker: %w", err) if err := c.worker.Shutdown(); err != nil { @@ -694,6 +686,14 @@ func (c *Command) StartWorker() error { return retErr } + if c.WorkerAuthKms == nil || c.DevUsePkiForUpstream { + if c.worker.WorkerAuthStorage == nil { + return fmt.Errorf("No worker auth storage found") + } + c.InfoKeys = append(c.InfoKeys, "worker auth storage path") + c.Info["worker auth storage path"] = c.worker.WorkerAuthStorage.BaseDir() + } + return nil } diff --git a/internal/daemon/worker/auth_rotation.go b/internal/daemon/worker/auth_rotation.go index cdf3f38b77..f1270f42c3 100644 --- a/internal/daemon/worker/auth_rotation.go +++ b/internal/daemon/worker/auth_rotation.go @@ -14,9 +14,8 @@ import ( func (w *Worker) startAuthRotationTicking(cancelCtx context.Context) { const op = "worker.(Worker).startAuthRotationTicking" - if w.conf.WorkerAuthKms != nil { - // Nothing to do here, just immediately exit - // FIXME: Write a system event + if w.conf.WorkerAuthKms != nil && !w.conf.DevUsePkiForUpstream { + event.WriteSysEvent(cancelCtx, op, "using kms worker authentication; pki auth rotation ticking not running") return } @@ -38,7 +37,7 @@ func (w *Worker) startAuthRotationTicking(cancelCtx context.Context) { select { case <-cancelCtx.Done(): - event.WriteSysEvent(cancelCtx, op, "auth rotation ticking shutting down") + event.WriteSysEvent(cancelCtx, op, "pki auth rotation ticking shutting down") return case <-timer.C: @@ -56,7 +55,7 @@ func (w *Worker) startAuthRotationTicking(cancelCtx context.Context) { } if currentNodeCreds == nil { - event.WriteSysEvent(cancelCtx, op, "no error loading worker auth creds but nil creds, skipping rotation") + event.WriteSysEvent(cancelCtx, op, "no error loading worker pki auth creds but nil creds, skipping rotation") continue } @@ -83,7 +82,7 @@ func (w *Worker) startAuthRotationTicking(cancelCtx context.Context) { if earliestValid.After(now) || latestValid.Before(now) || earliestValid.After(latestValid) { // We basically have no valid creds so we can't rotate. // TODO (maybe): Have this trigger a new set of creds and request? - event.WriteSysEvent(cancelCtx, op, "not within node creds validity period, unsure what to do, not rotating") + event.WriteSysEvent(cancelCtx, op, "not within worker creds validity period, unsure what to do, not rotating") continue } diff --git a/internal/daemon/worker/controller_connection.go b/internal/daemon/worker/controller_connection.go index 29eaa3bec9..1779784a3c 100644 --- a/internal/daemon/worker/controller_connection.go +++ b/internal/daemon/worker/controller_connection.go @@ -84,7 +84,7 @@ func (w *Worker) controllerDialerFunc() func(context.Context, string) (net.Conn, var conn net.Conn var err error switch { - case w.conf.WorkerAuthKms != nil: + case w.conf.WorkerAuthKms != nil && !w.conf.DevUsePkiForUpstream: conn, err = w.v1KmsAuthDialFn(ctx, addr) default: conn, err = protocol.Dial(ctx, w.WorkerAuthStorage, addr, nodeenrollment.WithWrapper(w.conf.WorkerAuthStorageKms)) diff --git a/internal/daemon/worker/testing.go b/internal/daemon/worker/testing.go index dddf8b408b..5a6e38d79e 100644 --- a/internal/daemon/worker/testing.go +++ b/internal/daemon/worker/testing.go @@ -258,6 +258,10 @@ func NewTestWorker(t testing.TB, opts *TestWorkerOpts) *TestWorker { Name: opts.Name, } } + if opts.WorkerAuthStoragePath != "" { + opts.Config.Worker.AuthStoragePath = opts.WorkerAuthStoragePath + tw.b.DevUsePkiForUpstream = true + } tw.name = opts.Config.Worker.Name serverName, err := os.Hostname() diff --git a/internal/daemon/worker/worker.go b/internal/daemon/worker/worker.go index d7dfaabcbc..cb25b22002 100644 --- a/internal/daemon/worker/worker.go +++ b/internal/daemon/worker/worker.go @@ -163,13 +163,6 @@ func New(conf *Config) (*Worker, error) { return nil, fmt.Errorf("exactly one proxy listener is required") } - var err error - w.WorkerAuthStorage, err = nodeefile.New(w.baseContext, - nodeefile.WithBaseDirectory(w.conf.RawConfig.Worker.AuthStoragePath)) - if err != nil { - return nil, err - } - return w, nil } @@ -187,7 +180,7 @@ func (w *Worker) Start() error { controllerResolver := manual.NewBuilderWithScheme(scheme) w.controllerResolver.Store(controllerResolver) - if w.conf.WorkerAuthKms == nil { + if w.conf.WorkerAuthKms == nil || w.conf.DevUsePkiForUpstream { // In this section, we look for existing worker credentials. The two // variables below store whether to create new credentials and whether // to create a fetch request so it can be displayed in the worker @@ -196,6 +189,13 @@ func (w *Worker) Start() error { // the controller, we don't want to invalidate that request on restart // by generating a new set of credentials. However it's safe to output a // new fetch request so we do in fact do that. + var err error + w.WorkerAuthStorage, err = nodeefile.New(w.baseContext, + nodeefile.WithBaseDirectory(w.conf.RawConfig.Worker.AuthStoragePath)) + if err != nil { + return fmt.Errorf("error loading worker auth storage directory: %w", err) + } + var createNodeAuthCreds bool var createFetchRequest bool nodeCreds, err := types.LoadNodeCredentials(w.baseContext, w.WorkerAuthStorage, nodeenrollment.CurrentId, nodeenrollment.WithWrapper(w.conf.WorkerAuthStorageKms)) @@ -489,7 +489,7 @@ func (w *Worker) getSessionTls(hello *tls.ClientHelloInfo) (*tls.Config, error) } if sessionId == "" { - event.WriteSysEvent(ctx, op, "session_id not found in either SNI or ALPN protos", "server_name", hello.ServerName, "alpn_protos", hello.SupportedProtos) + event.WriteSysEvent(ctx, op, "session_id not found in either SNI or ALPN protos", "server_name", hello.ServerName) return nil, fmt.Errorf("could not find session ID in SNI or ALPN protos") } diff --git a/internal/daemon/worker/worker_test.go b/internal/daemon/worker/worker_test.go index 028c4e152f..31e77dbeb0 100644 --- a/internal/daemon/worker/worker_test.go +++ b/internal/daemon/worker/worker_test.go @@ -198,44 +198,44 @@ func TestSetupWorkerAuthStorage(t *testing.T) { // the assertions check what the final result is. tests := []struct { name string - in func(t *testing.T, w *Worker) + in func(*testing.T, nodeenrollment.Storage, *Worker) expKeyId string // If set, the existing key ID to expect expRegistrationRequest bool // Whether we should have seen a registration request generated expError string // Some other error }{ { name: "no creds", - in: func(t *testing.T, w *Worker) { + in: func(t *testing.T, storage nodeenrollment.Storage, w *Worker) { // Do nothing; in this case it will have already been cleared }, expRegistrationRequest: true, }, { name: "valid creds", - in: func(t *testing.T, w *Worker) { + in: func(t *testing.T, storage nodeenrollment.Storage, w *Worker) { // Store the authorized creds - require.NoError(t, initNodeCreds.Store(ctx, w.WorkerAuthStorage)) + require.NoError(t, initNodeCreds.Store(ctx, storage)) }, expKeyId: initKeyId, }, { name: "existing but not validated", - in: func(t *testing.T, w *Worker) { + in: func(t *testing.T, storage nodeenrollment.Storage, w *Worker) { creds := proto.Clone(initNodeCreds).(*types.NodeCredentials) creds.CertificateBundles = nil creds.RegistrationNonce = nonce - require.NoError(t, creds.Store(ctx, w.WorkerAuthStorage)) + require.NoError(t, creds.Store(ctx, storage)) }, expKeyId: initKeyId, expRegistrationRequest: true, }, { name: "existing and outside cert times", // Note that cert from next CA will already not be valid - in: func(t *testing.T, w *Worker) { + in: func(t *testing.T, storage nodeenrollment.Storage, w *Worker) { creds := proto.Clone(initNodeCreds).(*types.NodeCredentials) creds.CertificateBundles[0].CertificateNotBefore = timestamppb.New(time.Time{}) creds.CertificateBundles[0].CertificateNotAfter = timestamppb.New(time.Time{}) - require.NoError(t, creds.Store(ctx, w.WorkerAuthStorage)) + require.NoError(t, creds.Store(ctx, storage)) }, expRegistrationRequest: true, }, @@ -250,10 +250,12 @@ func TestSetupWorkerAuthStorage(t *testing.T) { t.Cleanup(tw.Shutdown) // Always clear out storage that was there before, ignore errors - _ = tw.Worker().WorkerAuthStorage.Remove(ctx, &types.NodeCredentials{Id: string(nodeenrollment.CurrentId)}) + storage, err := nodeefile.New(tw.Context(), nodeefile.WithBaseDirectory(tmpDir)) + require.NoError(t, err) + _ = storage.Remove(ctx, &types.NodeCredentials{Id: string(nodeenrollment.CurrentId)}) // Run node credentials modification - tt.in(t, tw.Worker()) + tt.in(t, storage, tw.Worker()) // Start up to run logic require.NoError(t, tw.Worker().Start())