stop auto-generating auth tokens (#3670)

Today we try to make the networks secure by default by auto-generating
an auth token if the user does not provide one. There has been a long
discussion about this that you can find
[here](https://github.com/bacalhau-project/expanso-planning/issues/518).
The summary is:
1. Adds complexity to user onboarding as they will have to go through
the logs or console output to figure out and copy the auto-generated
token
2. We are printing the generated auth token in plain text in the console
3. I prefer to decouple auto-auth from launching NATS as our transport
layer
4. While better than making the network open, token based auth is not
secure enough and we don't want to give the impression to the users that
their networks are secure be default. Reasons include:
    1. Token based auth doesn't encrypt traffic on transit
1. We are using a global token and don't identify or authorize the
compute nodes differently
    2. No easy way to rotate or expire the token
1. We are planning to add more auth options in the future that are more
secure than global tokens, and this shouldn't be the default for our
users

This PR enables users to run open networks which will simplify testing
out bacalhau, and they will need to provide their auth token to secure
their networks instead of us doing magic on their behalf and generating
a random one for them. In the future it might make more sense to fail
the network from starting if not secure instead of doing some magic

cmd/cli/serve/serve.go

@@ -217,7 +217,7 @@ func serve(cmd *cobra.Command) error {
 		return err
 	}
 
-	networkConfig, err := getNetworkConfig(nodeName)
+	networkConfig, err := getNetworkConfig()
 	if err != nil {
 		return err
 	}
@@ -521,13 +521,6 @@ func getPublicNATSOrchestratorURL(nodeConfig *node.NodeConfig) *url.URL {
 		Host:   nodeConfig.NetworkConfig.AdvertisedAddress,
 	}
 
-	// Only display the secret if the user did not set it explicitly.
-	// Else, they should already know it!
-	secret, err := config.Get[string](types.NodeNetworkAuthSecret)
-	if err == nil && secret == "" && nodeConfig.NetworkConfig.AuthSecret != "" {
-		orchestrator.User = url.User(nodeConfig.NetworkConfig.AuthSecret)
-	}
-
 	if nodeConfig.NetworkConfig.AdvertisedAddress == "" {
 		orchestrator.Host = fmt.Sprintf("127.0.0.1:%d", nodeConfig.NetworkConfig.Port)
 	}

cmd/cli/serve/util.go

@@ -3,7 +3,6 @@ package serve
 import (
 	"context"
 	"fmt"
-	"net/url"
 	"path/filepath"
 	"time"
 
@@ -25,7 +24,6 @@ import (
 	"github.com/bacalhau-project/bacalhau/pkg/config"
 	"github.com/bacalhau-project/bacalhau/pkg/config/types"
 	"github.com/bacalhau-project/bacalhau/pkg/ipfs"
-	"github.com/bacalhau-project/bacalhau/pkg/nats"
 	"github.com/bacalhau-project/bacalhau/pkg/node"
 	"github.com/bacalhau-project/bacalhau/pkg/system"
 )
@@ -207,38 +205,12 @@ func getTransportType() (string, error) {
 	return networkCfg.Type, nil
 }
 
-func getNetworkConfig(nodeID string) (node.NetworkConfig, error) {
+func getNetworkConfig() (node.NetworkConfig, error) {
 	var networkCfg types.NetworkConfig
 	if err := config.ForKey(types.NodeNetwork, &networkCfg); err != nil {
 		return node.NetworkConfig{}, err
 	}
 
-	if networkCfg.AuthSecret == "" {
-		// Generate an auth token using the user's client key. It should not be
-		// possible to compute this value by anyone other than the NATS server, and
-		// should be stable across restarts of the node.
-		secret, err := nats.CreateAuthSecret(nodeID)
-		if err != nil {
-			return node.NetworkConfig{}, err
-		}
-		networkCfg.AuthSecret = secret //nolint: gosec
-	} else {
-		// If the user supplied an auth secret and some orchestrator(s), assume
-		// they are passing a auth secret to be used as a client. Attach the
-		// secret to the orchestrator urls, ignoring any which have one already.
-		for index, orchestrator := range networkCfg.Orchestrators {
-			orchestratorURL, err := url.Parse(orchestrator)
-			if err != nil {
-				return node.NetworkConfig{}, err
-			}
-
-			if orchestratorURL.User == nil {
-				orchestratorURL.User = url.User(networkCfg.AuthSecret)
-			}
-			networkCfg.Orchestrators[index] = orchestratorURL.String()
-		}
-	}
-
 	return node.NetworkConfig{
 		Type:                     networkCfg.Type,
 		Port:                     networkCfg.Port,

pkg/devstack/devstack.go

@@ -23,7 +23,6 @@ import (
 	bac_libp2p "github.com/bacalhau-project/bacalhau/pkg/libp2p"
 	"github.com/bacalhau-project/bacalhau/pkg/logger"
 	"github.com/bacalhau-project/bacalhau/pkg/models"
-	"github.com/bacalhau-project/bacalhau/pkg/nats"
 	"github.com/bacalhau-project/bacalhau/pkg/node"
 	"github.com/bacalhau-project/bacalhau/pkg/repo"
 	"github.com/bacalhau-project/bacalhau/pkg/routing"
@@ -52,6 +51,7 @@ type DevStackOptions struct {
 	ExecutorPlugins            bool   // when true pluggable executors will be used.
 	ConfigurationRepo          string // A custom config repo
 	NetworkType                string
+	AuthSecret                 string
 }
 
 func (o *DevStackOptions) Options() []ConfigOption {
@@ -70,6 +70,7 @@ func (o *DevStackOptions) Options() []ConfigOption {
 		WithNodeInfoPublisherInterval(o.NodeInfoPublisherInterval),
 		WithExecutorPlugins(o.ExecutorPlugins),
 		WithNetworkType(o.NetworkType),
+		WithAuthSecret(o.AuthSecret),
 	}
 	return opts
 }
@@ -123,8 +124,6 @@ func Setup(
 		stackConfig.NetworkType = networkType
 	}
 
-	natsAuthSecret := ""
-
 	for i := 0; i < totalNodeCount; i++ {
 		nodeID := fmt.Sprintf("node-%d", i)
 		ctx = logger.ContextWithNodeIDLogger(ctx, nodeID)
@@ -133,14 +132,6 @@ func Setup(
 		isComputeNode := (totalNodeCount - i) <= computeNodeCount
 		log.Ctx(ctx).Debug().Msgf(`Creating Node #%d as {RequesterNode: %t, ComputeNode: %t}`, i+1, isRequesterNode, isComputeNode)
 
-		// If this is the requester node, and we are using a NATS network, we need to make sure
-		// that there is an AuthSecret set in the node config.
-		if isRequesterNode && stackConfig.NetworkType == models.NetworkTypeNATS {
-			if natsAuthSecret, err = nats.CreateAuthSecret(nodeID); err != nil {
-				return nil, err
-			}
-		}
-
 		// ////////////////////////////////////
 		// IPFS
 		// ////////////////////////////////////
@@ -180,7 +171,7 @@ func Setup(
 			Orchestrators: orchestratorAddrs,
 			Port:          swarmPort,
 			ClusterPeers:  clusterPeersAddrs,
-			AuthSecret:    natsAuthSecret,
+			AuthSecret:    stackConfig.AuthSecret,
 		}
 
 		if stackConfig.NetworkType == models.NetworkTypeNATS {
@@ -199,7 +190,7 @@ func Setup(
 				clusterConfig.StoreDir = filepath.Join(repoPath, "nats-storage")
 				clusterConfig.ClusterName = "devstack"
 				clusterConfig.ClusterPort = clusterPort
-				orchestratorAddrs = append(orchestratorAddrs, fmt.Sprintf("%s@127.0.0.1:%d", natsAuthSecret, swarmPort))
+				orchestratorAddrs = append(orchestratorAddrs, fmt.Sprintf("127.0.0.1:%d", swarmPort))
 			}
 		} else {
 			if i == 0 {

pkg/devstack/option.go

@@ -76,6 +76,7 @@ type DevStackConfig struct {
 	NodeInfoStoreTTL           time.Duration
 	TLS                        DevstackTLSSettings
 	NetworkType                string
+	AuthSecret                 string
 }
 
 func (o *DevStackConfig) MarshalZerologObject(e *zerolog.Event) {
@@ -228,6 +229,12 @@ func WithNetworkType(typ string) ConfigOption {
 	}
 }
 
+func WithAuthSecret(secret string) ConfigOption {
+	return func(c *DevStackConfig) {
+		c.AuthSecret = secret
+	}
+}
+
 func WithSelfSignedCertificate(cert string, key string) ConfigOption {
 	return func(cfg *DevStackConfig) {
 		cfg.TLS = DevstackTLSSettings{

pkg/libp2p/transport/libp2p.go

@@ -24,7 +24,6 @@ import (
 	routedhost "github.com/libp2p/go-libp2p/p2p/host/routed"
 	"github.com/libp2p/go-libp2p/p2p/protocol/identify"
 	"github.com/multiformats/go-multiaddr"
-	"github.com/rs/zerolog/log"
 )
 
 const NodeInfoTopic = "bacalhau-node-info"
@@ -128,11 +127,6 @@ func NewLibp2pTransport(ctx context.Context,
 	}, nil
 }
 
-func (t *Libp2pTransport) GetConnectionInfo(ctx context.Context) interface{} {
-	log.Ctx(ctx).Debug().Msg("libp2p transport get connection info is unsupported")
-	return nil
-}
-
 func (t *Libp2pTransport) RegisterNodeInfoConsumer(ctx context.Context, nodeInfoStore routing.NodeInfoStore) error {
 	// register consumers of node info published over gossipSub
 	nodeInfoSubscriber := pubsub.NewChainedSubscriber[models.NodeInfo](true)

pkg/nats/client.go

@@ -7,18 +7,13 @@ import (
 	"github.com/nats-io/nats.go"
 )
 
-type ClientManagerParams struct {
-	Name    string
-	Servers string
-}
-
 type ClientManager struct {
 	Client *nats.Conn
 }
 
 // NewClientManager is a helper function to create a NATS client connection with a given name and servers string
-func NewClientManager(ctx context.Context, params ClientManagerParams) (*ClientManager, error) {
-	nc, err := nats.Connect(params.Servers, nats.Name(params.Name))
+func NewClientManager(ctx context.Context, servers string, options ...nats.Option) (*ClientManager, error) {
+	nc, err := nats.Connect(servers, options...)
 	if err != nil {
 		return nil, err
 	}

pkg/nats/pubsub/pubsub_test.go

@@ -11,6 +11,7 @@ import (
 	nats_helper "github.com/bacalhau-project/bacalhau/pkg/nats"
 	"github.com/bacalhau-project/bacalhau/pkg/pubsub"
 	"github.com/nats-io/nats-server/v2/server"
+	"github.com/nats-io/nats.go"
 	"github.com/rs/zerolog/log"
 	"github.com/stretchr/testify/suite"
 )
@@ -80,10 +81,7 @@ func (s *PubSubSuite) createNatsServer() *server.Server {
 }
 
 func (s *PubSubSuite) createPubSub(ctx context.Context, subject, subscriptionSubject string, server string) *PubSub[string] {
-	clientManager, err := nats_helper.NewClientManager(ctx, nats_helper.ClientManagerParams{
-		Name:    "test",
-		Servers: server,
-	})
+	clientManager, err := nats_helper.NewClientManager(ctx, server, nats.Name("test"))
 	s.Require().NoError(err)
 
 	pubSub, err := NewPubSub[string](PubSubParams{

pkg/nats/transport/nats.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"net/url"
 	"strings"
 
 	"github.com/bacalhau-project/bacalhau/pkg/compute"
@@ -19,6 +18,7 @@ import (
 	core_transport "github.com/bacalhau-project/bacalhau/pkg/transport"
 	"github.com/hashicorp/go-multierror"
 	"github.com/nats-io/nats-server/v2/server"
+	"github.com/nats-io/nats.go"
 	"github.com/rs/zerolog/log"
 )
 
@@ -39,9 +39,9 @@ type NATSTransportConfig struct {
 	// StoreDir is the directory where the NATS server will store its data
 	StoreDir string
 
-	// AuthSecret is a secret string that clients must use to connect. It is
-	// only used by NATS servers; clients should supply the auth secret as the
-	// user part of their Orchestrator URL.
+	// AuthSecret is a secret string that clients must use to connect. NATS servers
+	// must supply this config, while clients can also supply it as the user part
+	// of their Orchestrator URL.
 	AuthSecret string
 
 	// Cluster config for requester nodes to connect with each other
@@ -63,10 +63,6 @@ func (c *NATSTransportConfig) Validate() error {
 		mErr = multierror.Append(mErr, fmt.Errorf("node ID '%s' contains one or more reserved characters: %s", c.NodeID, reservedChars))
 	}
 
-	if c.AuthSecret == "" {
-		mErr = multierror.Append(mErr, fmt.Errorf("when using NATS, an auth secret must be provided for each node connecting to the cluster"))
-	}
-
 	if c.IsRequesterNode {
 		mErr = multierror.Append(mErr, validate.IsGreaterThanZero(c.Port, "port %d must be greater than zero", c.Port))
 
@@ -84,6 +80,7 @@ func (c *NATSTransportConfig) Validate() error {
 }
 
 type NATSTransport struct {
+	Config            NATSTransportConfig
 	nodeID            string
 	natsServer        *nats_helper.ServerManager
 	natsClient        *nats_helper.ClientManager
@@ -144,23 +141,10 @@ func NewNATSTransport(ctx context.Context,
 			return nil, err
 		}
 
-		// Make sure the orchestrator URL contains the auth token so that the
-		// NATS client below can connect
-		serverURL, err := url.Parse(sm.Server.ClientURL())
-		if err != nil {
-			return nil, err
-		}
-		serverURL.User = url.User(config.AuthSecret)
-
-		config.Orchestrators = append(config.Orchestrators, serverURL.String())
+		config.Orchestrators = append(config.Orchestrators, sm.Server.ClientURL())
 	}
 
-	// create nats client
-	log.Debug().Msgf("Creating NATS client with servers: %s", strings.Join(config.Orchestrators, ","))
-	nc, err := nats_helper.NewClientManager(ctx, nats_helper.ClientManagerParams{
-		Name:    config.NodeID,
-		Servers: strings.Join(config.Orchestrators, ","),
-	})
+	nc, err := CreateClient(ctx, config)
 	if err != nil {
 		return nil, err
 	}
@@ -197,6 +181,7 @@ func NewNATSTransport(ctx context.Context,
 		nodeID:            config.NodeID,
 		natsServer:        sm,
 		natsClient:        nc,
+		Config:            config,
 		computeProxy:      computeProxy,
 		callbackProxy:     computeCallback,
 		nodeInfoPubSub:    nodeInfoPubSub,
@@ -205,8 +190,19 @@ func NewNATSTransport(ctx context.Context,
 	}, nil
 }
 
-func (t *NATSTransport) GetConnectionInfo(ctx context.Context) interface{} {
-	return t.natsClient.Client.ConnectedUrl()
+func CreateClient(ctx context.Context, config NATSTransportConfig) (*nats_helper.ClientManager, error) {
+	// create nats client
+	log.Debug().Msgf("Creating NATS client with servers: %s", strings.Join(config.Orchestrators, ","))
+	clientOptions := []nats.Option{
+		nats.Name(config.NodeID),
+	}
+	if config.AuthSecret != "" {
+		clientOptions = append(clientOptions, nats.Token(config.AuthSecret))
+	}
+	return nats_helper.NewClientManager(ctx,
+		strings.Join(config.Orchestrators, ","),
+		clientOptions...,
+	)
 }
 
 func (t *NATSTransport) RegisterNodeInfoConsumer(ctx context.Context, infostore routing.NodeInfoStore) error {

pkg/nats/transport/nats_config_test.go

@@ -113,15 +113,6 @@ func (suite *NATSTransportConfigSuite) TestValidate() {
 			},
 			expectedErrors: []string{"cluster port -1 must be greater than zero"},
 		},
-		{
-			name: "Missing Auth secret in Requester Node",
-			config: NATSTransportConfig{
-				NodeID:          "node3",
-				Port:            4222,
-				IsRequesterNode: true,
-			},
-			expectedErrors: []string{"when using NATS, an auth secret must be provided for each node connecting to the cluster"}, //nolint:lll
-		},
 	}
 
 	for _, tt := range tests {

pkg/nats/util.go

@@ -1,15 +1,12 @@
 package nats
 
 import (
-	"crypto/sha256"
-	"encoding/base64"
 	"net"
 	"net/url"
 	"regexp"
 	"strings"
 
 	"github.com/bacalhau-project/bacalhau/pkg/lib/network"
-	"github.com/bacalhau-project/bacalhau/pkg/system"
 	"github.com/pkg/errors"
 	"github.com/samber/lo"
 	"golang.org/x/exp/slices"
@@ -81,15 +78,3 @@ func removeLocalAddresses(routes []*url.URL) ([]*url.URL, error) {
 	}
 	return result, nil
 }
-
-// CreateAuthSecret will return a signed hash of the nodeID
-// provided, for use as a secret for NATS authentication.
-func CreateAuthSecret(nodeID string) (string, error) {
-	var keySig string
-	keySig, err := system.SignForClient([]byte(nodeID))
-	if err != nil {
-		return "", err
-	}
-	hash := sha256.Sum256([]byte(keySig))
-	return base64.RawURLEncoding.EncodeToString(hash[:]), nil
-}

pkg/node/config_network.go

@@ -30,9 +30,9 @@ type NetworkConfig struct {
 	// Storage directory for NATS features that require it
 	StoreDir string
 
-	// AuthSecret is a secret string that clients must use to connect. It is
-	// only used by NATS servers; clients should supply the auth secret as the
-	// user part of their Orchestrator URL.
+	// AuthSecret is a secret string that clients must use to connect. NATS servers
+	// must supply this config, while clients can also supply it as the user part
+	// of their Orchestrator URL.
 	AuthSecret string
 
 	// NATS config for requester nodes to connect with each other