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

@@ -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 (

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=

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
 

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 {

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
 }
 

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
 			}
 

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))

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()

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")
 	}
 

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())