test(worker): Remove test-only hooks from actual code

This change introduces a `nonceFn` field that can be changed
by a test but at runtime is transparent to the code that calls it.

The current nonce-generating logic defaults to a test-only field which
opens us up to this value inadvertently being set.
With this change, the default behavior becomes using `base62` to
generate a random nonce, and any tests have to explicitly change that.

internal/servers/worker/controller_connection.go

@@ -138,17 +138,13 @@ func (w *Worker) createClientConn(addr string) error {
 func (w *Worker) workerAuthTLSConfig() (*tls.Config, *base.WorkerAuthInfo, error) {
 	var err error
 	info := &base.WorkerAuthInfo{
-		Name:            w.conf.RawConfig.Worker.Name,
-		Description:     w.conf.RawConfig.Worker.Description,
-		ConnectionNonce: w.testReusedAuthNonce,
+		Name:        w.conf.RawConfig.Worker.Name,
+		Description: w.conf.RawConfig.Worker.Description,
 	}
-	if info.ConnectionNonce == "" {
-		if info.ConnectionNonce, err = base62.Random(20); err != nil {
-			return nil, nil, err
-		}
-		if w.testReuseAuthNonces {
-			w.testReusedAuthNonce = info.ConnectionNonce
-		}
+
+	info.ConnectionNonce, err = w.nonceFn(20)
+	if err != nil {
+		return nil, nil, err
 	}
 
 	pubKey, privKey, err := ed25519.GenerateKey(w.conf.SecureRandomReader)

internal/servers/worker/testing.go

@@ -183,8 +183,9 @@ type TestWorkerOpts struct {
 	// connection cannot be made back to the controller
 	StatusGracePeriodDuration time.Duration
 
-	// Whether to set the replay value
-	EnableAuthReplay bool
+	// Overrides worker's nonceFn, for cases where we want to have control
+	// over the nonce we send to the Controller
+	NonceFn randFn
 }
 
 func NewTestWorker(t *testing.T, opts *TestWorkerOpts) *TestWorker {
@@ -280,8 +281,8 @@ func NewTestWorker(t *testing.T, opts *TestWorkerOpts) *TestWorker {
 		t.Fatal(err)
 	}
 
-	if opts.EnableAuthReplay {
-		tw.w.testReuseAuthNonces = true
+	if opts.NonceFn != nil {
+		tw.w.nonceFn = opts.NonceFn
 	}
 
 	if !opts.DisableAutoStart {

internal/servers/worker/worker.go

@@ -21,12 +21,15 @@ import (
 	"github.com/hashicorp/boundary/internal/servers"
 	"github.com/hashicorp/boundary/internal/servers/worker/session"
 	"github.com/hashicorp/go-hclog"
+	"github.com/hashicorp/go-secure-stdlib/base62"
 	"github.com/hashicorp/go-secure-stdlib/mlock"
 	ua "go.uber.org/atomic"
 	"google.golang.org/grpc/resolver"
 	"google.golang.org/grpc/resolver/manual"
 )
 
+type randFn func(length int) (string, error)
+
 type Worker struct {
 	conf   *Config
 	logger hclog.Logger
@@ -48,6 +51,9 @@ type Worker struct {
 
 	listeners []*base.ServerListener
 
+	// Used to generate a random nonce for Controller connections
+	nonceFn randFn
+
 	// We store the current set in an atomic value so that we can add
 	// reload-on-sighup behavior later
 	tags *atomic.Value
@@ -55,10 +61,6 @@ type Worker struct {
 	// request. It can be set via startup in New below, or (eventually) via
 	// SIGHUP.
 	updateTags ua.Bool
-
-	// Test-related values
-	testReuseAuthNonces bool
-	testReusedAuthNonce string
 }
 
 func New(conf *Config) (*Worker, error) {
@@ -72,6 +74,7 @@ func New(conf *Config) (*Worker, error) {
 		controllerSessionConn: new(atomic.Value),
 		sessionInfoMap:        new(sync.Map),
 		tags:                  new(atomic.Value),
+		nonceFn:               base62.Random,
 	}
 
 	w.lastStatusSuccess.Store((*LastStatusInformation)(nil))

internal/servers/worker/worker_test.go

@@ -11,7 +11,7 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func TestWorkerNewListenerConfig(t *testing.T) {
+func TestWorkerNew(t *testing.T) {
 	tests := []struct {
 		name       string
 		in         *Config
@@ -76,6 +76,20 @@ func TestWorkerNewListenerConfig(t *testing.T) {
 				require.Len(t, w.listeners, 2)
 			},
 		},
+		{
+			name: "worker nonce func is set",
+			in: &Config{
+				Server: &base.Server{
+					Listeners: []*base.ServerListener{
+						{Config: &listenerutil.ListenerConfig{Purpose: []string{"proxy"}}},
+					},
+				},
+			},
+			expErr: false,
+			assertions: func(t *testing.T, w *Worker) {
+				require.NotNil(t, w.nonceFn)
+			},
+		},
 	}
 
 	for _, tt := range tests {

internal/tests/cluster/worker_replay_test.go

@@ -13,7 +13,6 @@ import (
 	"github.com/hashicorp/boundary/internal/servers/controller"
 	"github.com/hashicorp/boundary/internal/servers/worker"
 	"github.com/hashicorp/go-hclog"
-	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
@@ -41,24 +40,49 @@ func TestWorkerReplay(t *testing.T) {
 		Config:             conf,
 		WorkerAuthKms:      c1.Config().WorkerAuthKms,
 		InitialControllers: c1.ClusterAddrs(),
-		EnableAuthReplay:   true,
+		NonceFn: func(length int) (string, error) {
+			return "test_noncetest_nonce", nil
+		},
 	})
 
 	// Give time for it to connect
 	time.Sleep(10 * time.Second)
+
+	// Assert that the worker has connected
+	logBuf, err := os.ReadFile(ec.AllEvents.Name())
+	require.NoError(t, err)
+	require.Equal(t, 1, strings.Count(string(logBuf), "worker successfully authed"))
+
+	// Assert we have the expected nonce in the DB
+	nonces, err := c1.ServersRepo().ListNonces(c1.Context(), servers.NoncePurposeWorkerAuth)
+	require.NoError(t, err)
+	require.Len(t, nonces, 1)
+	require.Equal(t, &servers.Nonce{Nonce: "test_noncetest_nonce", Purpose: servers.NoncePurposeWorkerAuth}, nonces[0])
+
 	require.NoError(t, w1.Worker().Shutdown())
 
-	// Now, start up again
-	require.NoError(t, w1.Worker().Start())
+	// Now, start up again with the same nonce
+	w1 = worker.NewTestWorker(t, &worker.TestWorkerOpts{
+		Config:             conf,
+		WorkerAuthKms:      c1.Config().WorkerAuthKms,
+		InitialControllers: c1.ClusterAddrs(),
+		NonceFn: func(length int) (string, error) {
+			return "test_noncetest_nonce", nil
+		},
+	})
+
+	// Give time for it to connect
 	time.Sleep(10 * time.Second)
 
 	// We should find only one nonce, and one successful worker authentication,
 	// both in the output and in the database
 	ec.AllEvents.Close()
-	logBuf, err := os.ReadFile(ec.AllEvents.Name())
+	logBuf, err = os.ReadFile(ec.AllEvents.Name())
 	require.NoError(t, err)
-	assert.Equal(t, 1, strings.Count(string(logBuf), "worker successfully authed"))
-	nonces, err := c1.ServersRepo().ListNonces(c1.Context(), servers.NoncePurposeWorkerAuth)
+	require.Equal(t, 1, strings.Count(string(logBuf), "worker successfully authed"))
+
+	nonces, err = c1.ServersRepo().ListNonces(c1.Context(), servers.NoncePurposeWorkerAuth)
 	require.NoError(t, err)
-	assert.Len(t, nonces, 1)
+	require.Len(t, nonces, 1)
+	require.Equal(t, &servers.Nonce{Nonce: "test_noncetest_nonce", Purpose: servers.NoncePurposeWorkerAuth}, nonces[0])
 }