keymanager: fixes issues in keymanager API

cmd/cmd.go

@@ -162,9 +162,13 @@ func flagsToLogFields(flags *pflag.FlagSet) []z.Field {
 	return fields
 }
 
-// redact returns a redacted version of the given flag value.
-// It currently only supports redacting passwords in valid URLs provided in ".*address.*" flags.
+// redact returns a redacted version of the given flag value. It currently supports redacting
+// passwords in valid URLs provided in ".*address.*" flags and redacting auth tokens.
 func redact(flag, val string) string {
+	if strings.Contains(flag, "auth-token") {
+		return "xxxxx"
+	}
+
 	if !strings.Contains(flag, "address") {
 		return val
 	}

cmd/cmd_internal_test.go

@@ -226,6 +226,41 @@ func TestFlagsToLogFields(t *testing.T) {
 	}
 }
 
+func TestRedact(t *testing.T) {
+	tests := []struct {
+		name     string
+		flag     string
+		value    string
+		expected string
+	}{
+		{
+			name:     "redact auth tokens",
+			flag:     "keymanager-auth-token",
+			value:    "api-token-abcdef12345",
+			expected: "xxxxx",
+		},
+		{
+			name:     "redact passwords in URL addresses",
+			flag:     "api-address",
+			value:    "https://user:password@example.com/foo/bar",
+			expected: "https://user:xxxxx@example.com/foo/bar",
+		},
+		{
+			name:     "no redact",
+			flag:     "definition-file",
+			value:    "https://obol.obol.tech/dv/0x0f481bbd06a596cb3ba569b9de0cbfcf822b209c2d6877c98173df986dd3c0ec",
+			expected: "https://obol.obol.tech/dv/0x0f481bbd06a596cb3ba569b9de0cbfcf822b209c2d6877c98173df986dd3c0ec",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := redact(tt.flag, tt.value)
+			require.Equal(t, tt.expected, got)
+		})
+	}
+}
+
 // slice is a convenience function for creating string slice literals.
 func slice(strs ...string) []string {
 	return strs

cmd/createcluster.go

@@ -40,11 +40,13 @@ const (
 )
 
 type clusterConfig struct {
-	Name            string
-	ClusterDir      string
-	DefFile         string
-	KeymanagerAddrs []string
-	Clean           bool
+	Name       string
+	ClusterDir string
+	DefFile    string
+	Clean      bool
+
+	KeymanagerAddrs      []string
+	KeymanagerAuthTokens []string
 
 	NumNodes          int
 	Threshold         int
@@ -86,6 +88,7 @@ func bindClusterFlags(flags *pflag.FlagSet, config *clusterConfig) {
 	flags.StringVar(&config.ClusterDir, "cluster-dir", ".charon/cluster", "The target folder to create the cluster in.")
 	flags.StringVar(&config.DefFile, "definition-file", "", "Optional path to a cluster definition file or an HTTP URL. This overrides all other configuration flags.")
 	flags.StringSliceVar(&config.KeymanagerAddrs, "keymanager-addresses", nil, "Comma separated list of keymanager URLs to import validator key shares to. Note that multiple addresses are required, one for each node in the cluster, with node0's keyshares being imported to the first address, node1's keyshares to the second, and so on.")
+	flags.StringSliceVar(&config.KeymanagerAuthTokens, "keymanager-auth-tokens", nil, "Authentication bearer tokens to interact with the keymanager URLs. Don't include the \"Bearer\" symbol, only include the api-token.")
 	flags.IntVarP(&config.NumNodes, "nodes", "", minNodes, "The number of charon nodes in the cluster. Minimum is 4.")
 	flags.IntVarP(&config.Threshold, "threshold", "", 0, "Optional override of threshold required for signature reconstruction. Defaults to ceil(n*2/3) if zero. Warning, non-default values decrease security.")
 	flags.StringSliceVar(&config.FeeRecipientAddrs, "fee-recipient-addresses", nil, "Comma separated list of Ethereum addresses of the fee recipient for each validator. Either provide a single fee recipient address or fee recipient addresses for each validator.")
@@ -120,6 +123,11 @@ func runCreateCluster(ctx context.Context, w io.Writer, conf clusterConfig) erro
 		conf.Network = eth2util.Goerli.Name
 	}
 
+	// Ensure sufficient auth tokens are provided for the keymanager addresses
+	if len(conf.KeymanagerAddrs) != len(conf.KeymanagerAuthTokens) {
+		return errors.New("number of --keymanager-addresses do not match --keymanager-auth-tokens. Please fix configuration flags")
+	}
+
 	var def cluster.Definition
 	if conf.DefFile != "" { // Load definition from DefFile
 		def, err = loadDefinition(ctx, conf.DefFile)
@@ -209,7 +217,7 @@ func runCreateCluster(ctx context.Context, w io.Writer, conf clusterConfig) erro
 			return err
 		}
 	} else { // Or else save keys to keymanager
-		if err = writeKeysToKeymanager(ctx, conf.KeymanagerAddrs, numNodes, shareSets); err != nil {
+		if err = writeKeysToKeymanager(ctx, conf.KeymanagerAddrs, conf.KeymanagerAuthTokens, numNodes, shareSets); err != nil {
 			return err
 		}
 	}
@@ -471,11 +479,11 @@ func getValidators(dvsPubkeys []tblsv2.PublicKey, dvPrivShares [][]tblsv2.Privat
 }
 
 // writeKeysToKeymanager writes validator keys to the provided keymanager addresses.
-func writeKeysToKeymanager(ctx context.Context, addrs []string, numNodes int, shareSets [][]tblsv2.PrivateKey) error {
+func writeKeysToKeymanager(ctx context.Context, addrs, authTokens []string, numNodes int, shareSets [][]tblsv2.PrivateKey) error {
 	// Ping all keymanager addresses to check if they are accessible to avoid partial writes
 	var clients []keymanager.Client
 	for i := 0; i < numNodes; i++ {
-		cl := keymanager.New(addrs[i])
+		cl := keymanager.New(addrs[i], authTokens[i])
 		if err := cl.VerifyConnection(ctx); err != nil {
 			return err
 		}

cmd/createcluster_internal_test.go

@@ -418,6 +418,8 @@ func TestKeymanager(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 
+	const testAuthToken = "api-token-test"
+
 	// Create secret
 	secret1, err := tblsv2.GenerateSecretKey()
 	require.NoError(t, err)
@@ -431,12 +433,13 @@ func TestKeymanager(t *testing.T) {
 	results := make(chan result, minNodes) // Buffered channel
 	defer close(results)
 
-	var addrs []string
+	var addrs, authTokens []string
 	var servers []*httptest.Server
 	for i := 0; i < minNodes; i++ {
 		srv := httptest.NewServer(newKeymanagerHandler(ctx, t, i, results))
 		servers = append(servers, srv)
 		addrs = append(addrs, srv.URL)
+		authTokens = append(authTokens, testAuthToken)
 	}
 
 	defer func() {
@@ -447,16 +450,17 @@ func TestKeymanager(t *testing.T) {
 
 	// Create cluster config
 	conf := clusterConfig{
-		Name:              t.Name(),
-		SplitKeysDir:      keyDir,
-		SplitKeys:         true,
-		NumNodes:          minNodes,
-		NumDVs:            1,
-		KeymanagerAddrs:   addrs,
-		Network:           defaultNetwork,
-		WithdrawalAddrs:   []string{zeroAddress},
-		FeeRecipientAddrs: []string{zeroAddress},
-		Clean:             true,
+		Name:                 t.Name(),
+		SplitKeysDir:         keyDir,
+		SplitKeys:            true,
+		NumNodes:             minNodes,
+		NumDVs:               1,
+		KeymanagerAddrs:      addrs,
+		KeymanagerAuthTokens: authTokens,
+		Network:              defaultNetwork,
+		WithdrawalAddrs:      []string{zeroAddress},
+		FeeRecipientAddrs:    []string{zeroAddress},
+		Clean:                true,
 	}
 	conf.ClusterDir = t.TempDir()
 
@@ -495,6 +499,15 @@ func TestKeymanager(t *testing.T) {
 		}
 		require.ErrorContains(t, err, "cannot ping address")
 	})
+
+	t.Run("lengths don't match", func(t *testing.T) {
+		// Construct an incorrect config where len(KeymanagerAuthTokens) = len(KeymanagerAddresses)-1
+		incorrectConf := conf
+		incorrectConf.KeymanagerAuthTokens = incorrectConf.KeymanagerAuthTokens[1:]
+
+		err = runCreateCluster(context.Background(), nil, incorrectConf)
+		require.ErrorContains(t, err, "number of --keymanager-addresses do not match --keymanager-auth-tokens. Please fix configuration flags")
+	})
 }
 
 // TestPublish tests support for uploading the cluster lockfile to obol-api.
@@ -540,8 +553,8 @@ func TestPublish(t *testing.T) {
 
 // mockKeymanagerReq is a mock keymanager request for use in tests.
 type mockKeymanagerReq struct {
-	Keystores []keystore.Keystore `json:"keystores"`
-	Passwords []string            `json:"passwords"`
+	Keystores []string `json:"keystores"`
+	Passwords []string `json:"passwords"`
 }
 
 // decrypt returns the secret from the encrypted keystore.
@@ -579,7 +592,9 @@ func newKeymanagerHandler(ctx context.Context, t *testing.T, id int, results cha
 		require.Equal(t, len(req.Keystores), len(req.Passwords))
 		require.Equal(t, len(req.Keystores), 1) // Since we split only 1 key
 
-		secret, err := decrypt(t, req.Keystores[0], req.Passwords[0])
+		var ks keystore.Keystore
+		require.NoError(t, json.Unmarshal([]byte(req.Keystores[0]), &ks))
+		secret, err := decrypt(t, ks, req.Passwords[0])
 		require.NoError(t, err)
 
 		res := result{

cmd/dkg.go

@@ -36,7 +36,7 @@ this command at the same time.`,
 	}
 
 	bindDataDirFlag(cmd.Flags(), &config.DataDir)
-	bindKeymanagerAddrFlag(cmd.Flags(), &config.KeymanagerAddr)
+	bindKeymanagerFlags(cmd.Flags(), &config.KeymanagerAddr, &config.KeymanagerAuthToken)
 	bindDefDirFlag(cmd.Flags(), &config.DefFile)
 	bindNoVerifyFlag(cmd.Flags(), &config.NoVerify)
 	bindP2PFlags(cmd, &config.P2P)
@@ -46,8 +46,9 @@ this command at the same time.`,
 	return cmd
 }
 
-func bindKeymanagerAddrFlag(flags *pflag.FlagSet, addr *string) {
+func bindKeymanagerFlags(flags *pflag.FlagSet, addr, authToken *string) {
 	flags.StringVar(addr, "keymanager-address", "", "The keymanager URL to import validator keyshares.")
+	flags.StringVar(authToken, "keymanager-auth-token", "", "Authentication bearer token to interact with keymanager API. Don't include the \"Bearer\" symbol, only include the api-token.")
 }
 
 func bindDefDirFlag(flags *pflag.FlagSet, dataDir *string) {

dkg/disk.go

@@ -85,7 +85,7 @@ func loadDefinition(ctx context.Context, conf Config) (cluster.Definition, error
 }
 
 // writeKeysToKeymanager writes validator private keyshares for the node to the provided keymanager address.
-func writeKeysToKeymanager(ctx context.Context, keymanagerURL string, shares []share) error {
+func writeKeysToKeymanager(ctx context.Context, keymanagerURL, authToken string, shares []share) error {
 	var (
 		keystores []keystore.Keystore
 		passwords []string
@@ -106,7 +106,7 @@ func writeKeysToKeymanager(ctx context.Context, keymanagerURL string, shares []s
 		keystores = append(keystores, store)
 	}
 
-	cl := keymanager.New(keymanagerURL)
+	cl := keymanager.New(keymanagerURL, authToken)
 	err := cl.ImportKeystores(ctx, keystores, passwords)
 	if err != nil {
 		return err

dkg/dkg.go

@@ -31,12 +31,14 @@ import (
 )
 
 type Config struct {
-	DefFile        string
-	KeymanagerAddr string
-	NoVerify       bool
-	DataDir        string
-	P2P            p2p.Config
-	Log            log.Config
+	DefFile  string
+	NoVerify bool
+	DataDir  string
+	P2P      p2p.Config
+	Log      log.Config
+
+	KeymanagerAddr      string
+	KeymanagerAuthToken string
 
 	PublishAddr string
 	Publish     bool
@@ -64,9 +66,13 @@ func Run(ctx context.Context, conf Config) (err error) {
 		return err
 	}
 
+	if err := validateKeymanagerFlags(conf.KeymanagerAddr, conf.KeymanagerAuthToken); err != nil {
+		return err
+	}
+
 	// Check if keymanager address is reachable.
 	if conf.KeymanagerAddr != "" {
-		cl := keymanager.New(conf.KeymanagerAddr)
+		cl := keymanager.New(conf.KeymanagerAddr, conf.KeymanagerAuthToken)
 		if err = cl.VerifyConnection(ctx); err != nil {
 			return errors.Wrap(err, "verify keymanager address")
 		}
@@ -198,7 +204,7 @@ func Run(ctx context.Context, conf Config) (err error) {
 	// to prevent partial data writes in case of peer connection lost
 
 	if conf.KeymanagerAddr != "" { // Save to keymanager
-		if err = writeKeysToKeymanager(ctx, conf.KeymanagerAddr, shares); err != nil {
+		if err = writeKeysToKeymanager(ctx, conf.KeymanagerAddr, conf.KeymanagerAuthToken, shares); err != nil {
 			return err
 		}
 		log.Debug(ctx, "Imported keyshares to keymanager", z.Str("keymanager_address", conf.KeymanagerAddr))
@@ -681,6 +687,18 @@ func writeLockToAPI(ctx context.Context, publishAddr string, lock cluster.Lock)
 	return nil
 }
 
+// validateKeymanagerFlags returns an error if one keymanager flag is present but the other is not.
+func validateKeymanagerFlags(addr, authToken string) error {
+	if addr != "" && authToken == "" {
+		return errors.New("--keymanager-address provided but --keymanager-auth-token absent. Please fix configuration flags")
+	}
+	if addr == "" && authToken != "" {
+		return errors.New("--keymanager-auth-token provided but --keymanager-address absent. Please fix configuration flags")
+	}
+
+	return nil
+}
+
 // logPeerSummary logs peer summary with peer names and their ethereum addresses.
 func logPeerSummary(ctx context.Context, currentPeer peer.ID, peers []p2p.Peer, operators []cluster.Operator) {
 	for i, p := range peers {

dkg/dkg_internal_test.go

@@ -155,3 +155,38 @@ func TestValidSignatures(t *testing.T) {
 	_, _, err = aggLockHashSig(map[core.PubKey][]core.ParSignedData{corePubkey: getSigs(lockMsg)}, map[core.PubKey]share{corePubkey: shares}, lockMsg)
 	require.NoError(t, err)
 }
+
+func TestValidateKeymanagerFlags(t *testing.T) {
+	tests := []struct {
+		name      string
+		addr      string
+		authToken string
+		errMsg    string
+	}{
+		{
+			name:      "Both keymanager flags provided",
+			addr:      "https://keymanager@example.com",
+			authToken: "keymanager-auth-token",
+			errMsg:    "",
+		},
+		{
+			name:   "Address provided but auth token absent",
+			addr:   "https://keymanager@example.com",
+			errMsg: "--keymanager-address provided but --keymanager-auth-token absent. Please fix configuration flags",
+		},
+		{
+			name:      "Auth token provided by address absent",
+			authToken: "keymanager-auth-token",
+			errMsg:    "--keymanager-auth-token provided but --keymanager-address absent. Please fix configuration flags",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := validateKeymanagerFlags(tt.addr, tt.authToken)
+			if tt.errMsg != "" {
+				require.Equal(t, err.Error(), tt.errMsg)
+			}
+		})
+	}
+}

dkg/dkg_test.go

@@ -10,6 +10,7 @@ import (
 	"net/http/httptest"
 	"os"
 	"path"
+	"strings"
 	"testing"
 	"time"
 
@@ -110,14 +111,20 @@ func testDKG(t *testing.T, def cluster.Definition, dir string, p2pKeys []*k1.Pri
 
 	allReceivedKeystores := make(chan struct{}) // Receives struct{} for each `numNodes` keystore intercepted by the keymanager server
 	if keymanager {
+		const testAuthToken = "test-auth-token"
 		srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			bearerAuthToken := strings.Split(r.Header.Get("Authorization"), " ")
+			require.Equal(t, bearerAuthToken[0], "Bearer")
+			require.Equal(t, bearerAuthToken[1], testAuthToken)
+
 			go func() {
 				allReceivedKeystores <- struct{}{}
 			}()
 		}))
 		defer srv.Close()
 
 		conf.KeymanagerAddr = srv.URL
+		conf.KeymanagerAuthToken = testAuthToken
 	}
 
 	receivedLockfile := make(chan struct{}) // Receives string for lockfile intercepted by the obol-api server