Use persistent storage for worker auth credentials (#2139)

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.0.0-20220603214204-cead023172f3
+	github.com/hashicorp/nodeenrollment v0.0.0-20220605235506-351328414303
 )
 
 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.0.0-20220603214204-cead023172f3 h1:yGCgHpFf6g4VQKxP45r2vxQZXjequvcpm1x5XKg0oaU=
-github.com/hashicorp/nodeenrollment v0.0.0-20220603214204-cead023172f3/go.mod h1:8l3RSuZJAibZKnqX6tvLaeoFzPhDf4oCRVsyFjU5Soc=
+github.com/hashicorp/nodeenrollment v0.0.0-20220605235506-351328414303 h1:dWteo2hZzqmF7IMqIozEzLLx3J0GXGKhKU32hPERp4g=
+github.com/hashicorp/nodeenrollment v0.0.0-20220605235506-351328414303/go.mod h1:8l3RSuZJAibZKnqX6tvLaeoFzPhDf4oCRVsyFjU5Soc=
 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/commands/dev/dev.go

@@ -29,7 +29,6 @@ import (
 	"github.com/hashicorp/boundary/internal/types/scope"
 	"github.com/hashicorp/go-secure-stdlib/parseutil"
 	"github.com/hashicorp/go-secure-stdlib/strutil"
-	nodeefile "github.com/hashicorp/nodeenrollment/storage/file"
 	"github.com/mitchellh/cli"
 	"github.com/posener/complete"
 	"go.uber.org/atomic"
@@ -701,6 +700,8 @@ func (c *Command) Run(args []string) int {
 			c.Info["worker auth registration request"] = req
 			c.InfoKeys = append(c.InfoKeys, "worker auth current key id")
 			c.Info["worker auth current key id"] = c.worker.WorkerAuthCurrentKeyId
+			c.InfoKeys = append(c.InfoKeys, "worker auth storage path")
+			c.Info["worker auth storage path"] = c.worker.WorkerAuthStorage.BaseDir()
 			go func() {
 				for {
 					select {
@@ -773,9 +774,7 @@ func (c *Command) Run(args []string) int {
 
 			if !c.flagControllerOnly {
 				if !c.flagWorkerAuthStorageSkipCleanup {
-					if fileStorage, ok := c.worker.WorkerAuthStorage.(*nodeefile.FileStorage); ok {
-						fileStorage.Cleanup()
-					}
+					c.worker.WorkerAuthStorage.Cleanup()
 				}
 				if err := c.worker.Shutdown(); err != nil {
 					c.UI.Error(fmt.Errorf("Error shutting down worker: %w", err).Error())

internal/cmd/commands/server/server.go

@@ -463,22 +463,10 @@ func (c *Command) Run(args []string) int {
 			}
 			return base.CommandCliError
 		}
-		if c.WorkerAuthKms == nil {
-			req := c.worker.WorkerAuthRegistrationRequest
-			if req == "" {
-				retErr := fmt.Errorf("No worker registration request found at worker startup time")
-				if err := c.worker.Shutdown(); err != nil {
-					c.UI.Error(retErr.Error())
-					retErr = fmt.Errorf("Error shutting down worker: %w", err)
-				}
-				c.UI.Error(retErr.Error())
-				if err := c.controller.Shutdown(); err != nil {
-					c.UI.Error(fmt.Errorf("Error with controller shutdown: %w", err).Error())
-				}
-				return base.CommandCliError
-			}
+
+		if c.WorkerAuthKms == nil && c.worker.WorkerAuthRegistrationRequest != "" {
 			c.InfoKeys = append(c.InfoKeys, "worker auth registration request")
-			c.Info["worker auth registration request"] = req
+			c.Info["worker auth registration request"] = c.worker.WorkerAuthRegistrationRequest
 			c.InfoKeys = append(c.InfoKeys, "worker auth current key id")
 			c.Info["worker auth current key id"] = c.worker.WorkerAuthCurrentKeyId
 		}
@@ -648,6 +636,14 @@ 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 {

internal/daemon/worker/listeners.go

@@ -19,9 +19,9 @@ import (
 	"github.com/hashicorp/go-multierror"
 	nodee "github.com/hashicorp/nodeenrollment"
 	"github.com/hashicorp/nodeenrollment/multihop"
+	nodeenet "github.com/hashicorp/nodeenrollment/net"
 	"github.com/hashicorp/nodeenrollment/protocol"
 	"github.com/hashicorp/nodeenrollment/types"
-	"github.com/hashicorp/nodeenrollment/util/splitlistener"
 	"github.com/hashicorp/nodeenrollment/util/temperror"
 	"google.golang.org/grpc"
 )
@@ -135,7 +135,7 @@ func (w *Worker) configureForWorker(ln *base.ServerListener, logger *log.Logger)
 		return nil, fmt.Errorf("error instantiating node auth listener: %w", err)
 	}
 
-	w.workerAuthSplitListener = splitlistener.New(interceptingListener)
+	w.workerAuthSplitListener = nodeenet.NewSplitListener(interceptingListener)
 
 	downstreamServer := grpc.NewServer(
 		grpc.MaxRecvMsgSize(math.MaxInt32),

internal/daemon/worker/testing.go

@@ -32,7 +32,7 @@ type TestWorker struct {
 	name   string
 }
 
-// Worker returns the underlying controller
+// Worker returns the underlying worker
 func (tw *TestWorker) Worker() *Worker {
 	return tw.w
 }
@@ -177,6 +177,9 @@ type TestWorkerOpts struct {
 	// The worker credential storage KMS to use, or one will be created
 	WorkerAuthStorageKms wrapping.Wrapper
 
+	// The location of the worker's auth storage
+	WorkerAuthStoragePath string
+
 	// The name to use for the worker, otherwise one will be randomly
 	// generated, unless provided in a non-nil Config
 	Name string

internal/daemon/worker/worker.go

@@ -28,9 +28,9 @@ import (
 	"github.com/hashicorp/go-hclog"
 	"github.com/hashicorp/go-secure-stdlib/base62"
 	"github.com/hashicorp/go-secure-stdlib/mlock"
+	nodeenet "github.com/hashicorp/nodeenrollment/net"
 	nodeefile "github.com/hashicorp/nodeenrollment/storage/file"
 	"github.com/hashicorp/nodeenrollment/types"
-	"github.com/hashicorp/nodeenrollment/util/splitlistener"
 	ua "go.uber.org/atomic"
 	"google.golang.org/grpc/resolver"
 	"google.golang.org/grpc/resolver/manual"
@@ -75,10 +75,10 @@ type Worker struct {
 	updateTags *ua.Bool
 
 	// The storage for node enrollment
-	WorkerAuthStorage             nodeenrollment.Storage
+	WorkerAuthStorage             *nodeefile.Storage
 	WorkerAuthCurrentKeyId        string
 	WorkerAuthRegistrationRequest string
-	workerAuthSplitListener       *splitlistener.SplitListener
+	workerAuthSplitListener       *nodeenet.SplitListener
 
 	// Test-specific options
 	TestOverrideX509VerifyDnsName  string
@@ -188,26 +188,98 @@ func (w *Worker) Start() error {
 		return fmt.Errorf("error starting worker listeners: %w", err)
 	}
 
-	nodeCreds, err := types.NewNodeCredentials(w.baseContext, w.WorkerAuthStorage, nodeenrollment.WithWrapper(w.conf.WorkerAuthStorageKms))
-	if err != nil {
-		return fmt.Errorf("error generating new node creds: %w", err)
-	}
+	if w.conf.WorkerAuthKms == nil {
+		// 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
+		// startup info. These may be different because if initial creds have
+		// been generated on the worker side but not yet authorized/fetched from
+		// 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 createNodeAuthCreds bool
+		var createFetchRequest bool
+		nodeCreds, err := types.LoadNodeCredentials(w.baseContext, w.WorkerAuthStorage, nodeenrollment.CurrentId, nodeenrollment.WithWrapper(w.conf.WorkerAuthStorageKms))
+		switch {
+		case err == nil:
+			// It's unclear why this would ever happen -- it shouldn't -- so
+			// this is simply safety against panics if something goes
+			// catastrophically wrong
+			if nodeCreds == nil {
+				event.WriteSysEvent(w.baseContext, op, "no error loading worker auth creds but nil creds, creating new creds for registration")
+				createNodeAuthCreds = true
+				createFetchRequest = true
+				break
+			}
 
-	req, err := nodeCreds.CreateFetchNodeCredentialsRequest(w.baseContext)
-	if err != nil {
-		return fmt.Errorf("error creating fetch credentials request: %w", err)
-	}
-	reqBytes, err := proto.Marshal(req)
-	if err != nil {
-		return fmt.Errorf("error marshaling fetch credentials request: %w", err)
-	}
-	w.WorkerAuthRegistrationRequest = base58.FastBase58Encoding(reqBytes)
-	if err != nil {
-		return fmt.Errorf("error encoding registration request: %w", err)
-	}
-	w.WorkerAuthCurrentKeyId, err = nodeenrollment.KeyIdFromPkix(nodeCreds.CertificatePublicKeyPkix)
-	if err != nil {
-		return fmt.Errorf("error deriving key id: %w", err)
+			// Check that we have valid creds, or that we have generated creds but
+			// simply are still waiting on authentication (in which case we don't
+			// want to invalidate what we've already sent)
+			var validCreds bool
+			switch len(nodeCreds.CertificateBundles) {
+			case 0:
+				// Still waiting on initial creds, so don't invalidate the request
+				// by creating new credentials. However, we will generate and
+				// display a new valid request in case the first was lost.
+				createFetchRequest = true
+
+			default:
+				now := time.Now()
+				for _, bundle := range nodeCreds.CertificateBundles {
+					if bundle.CertificateNotBefore.AsTime().Before(now) && bundle.CertificateNotAfter.AsTime().After(now) {
+						// If we have a certificate in its validity period,
+						// everything is fine
+						validCreds = true
+						break
+					}
+				}
+
+				// Certificates are both expired, so create new credentials and
+				// output a request based on those
+				createNodeAuthCreds = !validCreds
+				createFetchRequest = !validCreds
+			}
+
+		case errors.Is(err, nodeenrollment.ErrNotFound):
+			// Nothing was found on disk, so create
+			createNodeAuthCreds = true
+			createFetchRequest = true
+
+		default:
+			// Some other type of error happened, bail out
+			return fmt.Errorf("error loading worker auth creds: %w", err)
+		}
+
+		// NOTE: this block _must_ be before the `if createFetchRequest` block
+		// or the fetch request may have no credentials to work with
+		if createNodeAuthCreds {
+			nodeCreds, err = types.NewNodeCredentials(w.baseContext, w.WorkerAuthStorage, nodeenrollment.WithWrapper(w.conf.WorkerAuthStorageKms))
+			if err != nil {
+				return fmt.Errorf("error generating new worker auth creds: %w", err)
+			}
+		}
+
+		if createFetchRequest {
+			if nodeCreds == nil {
+				return fmt.Errorf("need to create fetch request but worker auth creds are nil: %w", err)
+			}
+			req, err := nodeCreds.CreateFetchNodeCredentialsRequest(w.baseContext)
+			if err != nil {
+				return fmt.Errorf("error creating worker auth fetch credentials request: %w", err)
+			}
+			reqBytes, err := proto.Marshal(req)
+			if err != nil {
+				return fmt.Errorf("error marshaling worker auth fetch credentials request: %w", err)
+			}
+			w.WorkerAuthRegistrationRequest = base58.FastBase58Encoding(reqBytes)
+			if err != nil {
+				return fmt.Errorf("error encoding worker auth registration request: %w", err)
+			}
+			w.WorkerAuthCurrentKeyId, err = nodeenrollment.KeyIdFromPkix(nodeCreds.CertificatePublicKeyPkix)
+			if err != nil {
+				return fmt.Errorf("error deriving worker auth key id: %w", err)
+			}
+		}
 	}
 
 	if err := w.startControllerConnections(); err != nil {

internal/daemon/worker/worker_test.go

@@ -2,7 +2,10 @@ package worker
 
 import (
 	"context"
+	"crypto/rand"
+	"os"
 	"testing"
+	"time"
 
 	"github.com/hashicorp/boundary/internal/cmd/base"
 	"github.com/hashicorp/boundary/internal/cmd/config"
@@ -10,8 +13,15 @@ import (
 	"github.com/hashicorp/go-hclog"
 	"github.com/hashicorp/go-secure-stdlib/configutil/v2"
 	"github.com/hashicorp/go-secure-stdlib/listenerutil"
+	"github.com/hashicorp/nodeenrollment"
+	"github.com/hashicorp/nodeenrollment/registration"
+	"github.com/hashicorp/nodeenrollment/rotation"
+	nodeefile "github.com/hashicorp/nodeenrollment/storage/file"
+	"github.com/hashicorp/nodeenrollment/types"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"google.golang.org/protobuf/proto"
+	"google.golang.org/protobuf/types/known/timestamppb"
 )
 
 func TestWorkerNew(t *testing.T) {
@@ -127,23 +137,144 @@ func TestWorkerNew(t *testing.T) {
 	}
 }
 
-func TestSetupWorkerCredentialStorage(t *testing.T) {
+func TestSetupWorkerAuthStorage(t *testing.T) {
+	t.Parallel()
+
 	ctx := context.Background()
-	assert, require := assert.New(t), require.New(t)
+
 	ts := db.TestWrapper(t)
 	keyId, err := ts.KeyId(ctx)
+	require.NoError(t, err)
 
+	// First, just test the key ID is populated
+	tmpDir, err := os.MkdirTemp("", "")
+	require.NoError(t, err)
+	t.Cleanup(func() { require.NoError(t, os.RemoveAll(tmpDir)) })
 	tw := NewTestWorker(t, &TestWorkerOpts{
-		WorkerAuthStorageKms: ts,
-		DisableAutoStart:     true,
+		WorkerAuthStorageKms:  ts,
+		WorkerAuthStoragePath: tmpDir,
+		DisableAutoStart:      true,
 	})
-
+	t.Cleanup(tw.Shutdown)
 	err = tw.Worker().Start()
-	require.NoError(err)
-	wKeyId, err := tw.Config().WorkerAuthStorageKms.KeyId(ctx)
-	require.NoError(err)
-	assert.Equal(keyId, wKeyId)
+	require.NoError(t, err)
 
-	err = tw.Worker().Shutdown()
-	require.NoError(err)
+	wKeyId, err := tw.Config().WorkerAuthStorageKms.KeyId(ctx)
+	require.NoError(t, err)
+	assert.Equal(t, keyId, wKeyId)
+
+	// Create a fresh persistent dir for the following tests
+	tmpDir, err = os.MkdirTemp("", "")
+	require.NoError(t, err)
+	t.Cleanup(func() { require.NoError(t, os.RemoveAll(tmpDir)) })
+
+	// Get an initial set of authorized node credentials
+	initStorage, err := nodeefile.New(ctx)
+	require.NoError(t, err)
+	t.Cleanup(initStorage.Cleanup)
+	_, err = rotation.RotateRootCertificates(ctx, initStorage)
+	require.NoError(t, err)
+	initNodeCreds, err := types.NewNodeCredentials(ctx, initStorage)
+	require.NoError(t, err)
+	req, err := initNodeCreds.CreateFetchNodeCredentialsRequest(ctx)
+	require.NoError(t, err)
+	_, err = registration.AuthorizeNode(ctx, initStorage, req)
+	require.NoError(t, err)
+	fetchResp, err := registration.FetchNodeCredentials(ctx, initStorage, req)
+	require.NoError(t, err)
+	initNodeCreds, err = initNodeCreds.HandleFetchNodeCredentialsResponse(ctx, initStorage, fetchResp)
+	require.NoError(t, err)
+	initKeyId, err := nodeenrollment.KeyIdFromPkix(initNodeCreds.CertificatePublicKeyPkix)
+	require.NoError(t, err)
+
+	nonce := make([]byte, nodeenrollment.NonceSize)
+	_, err = rand.Reader.Read(nonce)
+	require.NoError(t, err)
+
+	// What's going on here: in each test we are simulating a startup of a
+	// worker that has storage in various states. The input is a function to
+	// modify the current state of node credentials by using the worker's
+	// storage, but this happens before Start so we haven't done checking yet;
+	// the assertions check what the final result is.
+	tests := []struct {
+		name                   string
+		in                     func(t *testing.T, w *Worker)
+		expKeyId               string // If set, the existing key ID to expect
+		expCreation            bool   // Whether we should expect a new key ID
+		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) {
+				// Do nothing; in this case it will have already been cleared
+			},
+			expCreation:            true,
+			expRegistrationRequest: true,
+		},
+		{
+			name: "valid creds",
+			in: func(t *testing.T, w *Worker) {
+				// Store the authorized creds
+				require.NoError(t, initNodeCreds.Store(ctx, w.WorkerAuthStorage))
+			},
+		},
+		{
+			name: "existing but not validated",
+			in: func(t *testing.T, w *Worker) {
+				creds := proto.Clone(initNodeCreds).(*types.NodeCredentials)
+				creds.CertificateBundles = nil
+				creds.RegistrationNonce = nonce
+				require.NoError(t, creds.Store(ctx, w.WorkerAuthStorage))
+			},
+			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) {
+				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))
+			},
+			expCreation:            true,
+			expRegistrationRequest: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			tw := NewTestWorker(t, &TestWorkerOpts{
+				WorkerAuthStoragePath: tmpDir,
+				DisableAutoStart:      true,
+			})
+			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)})
+
+			// Run node credentials modification
+			tt.in(t, tw.Worker())
+
+			// Start up to run logic
+			require.NoError(t, tw.Worker().Start())
+
+			// Validate state
+			if tt.expKeyId != "" {
+				assert.Equal(t, tt.expKeyId, tw.Worker().WorkerAuthCurrentKeyId)
+			} else {
+				if tt.expCreation {
+					assert.NotEmpty(t, tw.Worker().WorkerAuthCurrentKeyId)
+				} else {
+					assert.Empty(t, tw.Worker().WorkerAuthCurrentKeyId)
+				}
+			}
+			if tt.expRegistrationRequest {
+				assert.NotEmpty(t, tw.Worker().WorkerAuthRegistrationRequest)
+			} else {
+				assert.Empty(t, tw.Worker().WorkerAuthRegistrationRequest)
+			}
+		})
+	}
 }

internal/servers/repository_worker_test.go

@@ -663,7 +663,7 @@ func TestRepository_CreateWorker(t *testing.T) {
 			reader: rw,
 			fetchReq: func() *types.FetchNodeCredentialsRequest {
 				// This happens on the worker
-				fileStorage, err := file.NewFileStorage(testCtx)
+				fileStorage, err := file.New(testCtx)
 				require.NoError(t, err)
 				defer fileStorage.Cleanup()
 
@@ -707,7 +707,7 @@ func TestRepository_CreateWorker(t *testing.T) {
 			reader: rw,
 			fetchReq: func() *types.FetchNodeCredentialsRequest {
 				// This happens on the worker
-				fileStorage, err := file.NewFileStorage(testCtx)
+				fileStorage, err := file.New(testCtx)
 				require.NoError(t, err)
 				defer fileStorage.Cleanup()