diff --git a/go.mod b/go.mod index 4ce1d09e1..955176dfc 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.0.0-20220603214204-cead023172f3 + github.com/hashicorp/nodeenrollment v0.0.0-20220605235506-351328414303 ) require ( diff --git a/go.sum b/go.sum index 9e8efeefa..62345953f 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.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= diff --git a/internal/cmd/commands/dev/dev.go b/internal/cmd/commands/dev/dev.go index 70055bd7d..f33611e0d 100644 --- a/internal/cmd/commands/dev/dev.go +++ b/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()) diff --git a/internal/cmd/commands/server/server.go b/internal/cmd/commands/server/server.go index a62cb8e9e..70bb3a651 100644 --- a/internal/cmd/commands/server/server.go +++ b/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 { diff --git a/internal/daemon/worker/listeners.go b/internal/daemon/worker/listeners.go index 1aac3dfba..7a8a9a5c6 100644 --- a/internal/daemon/worker/listeners.go +++ b/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), diff --git a/internal/daemon/worker/testing.go b/internal/daemon/worker/testing.go index 5025f9791..5fe5387eb 100644 --- a/internal/daemon/worker/testing.go +++ b/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 diff --git a/internal/daemon/worker/worker.go b/internal/daemon/worker/worker.go index a8cee0726..85fdf7ad7 100644 --- a/internal/daemon/worker/worker.go +++ b/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 { diff --git a/internal/daemon/worker/worker_test.go b/internal/daemon/worker/worker_test.go index 6c18a4f3f..fcd57d79a 100644 --- a/internal/daemon/worker/worker_test.go +++ b/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) + } + }) + } } diff --git a/internal/servers/repository_worker_test.go b/internal/servers/repository_worker_test.go index 673a7aeb9..7daaffb5e 100644 --- a/internal/servers/repository_worker_test.go +++ b/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()