respect the user's security protocol preference order

config/config.go

@@ -2,6 +2,7 @@ package config
 
 import (
 	"crypto/rand"
+	"errors"
 	"fmt"
 	"time"
 
@@ -13,6 +14,7 @@ import (
 	"github.com/libp2p/go-libp2p/core/peer"
 	"github.com/libp2p/go-libp2p/core/peerstore"
 	"github.com/libp2p/go-libp2p/core/pnet"
+	"github.com/libp2p/go-libp2p/core/protocol"
 	"github.com/libp2p/go-libp2p/core/routing"
 	"github.com/libp2p/go-libp2p/core/sec"
 	"github.com/libp2p/go-libp2p/core/sec/insecure"
@@ -53,6 +55,11 @@ type AutoNATConfig struct {
 	ThrottleInterval    time.Duration
 }
 
+type Security struct {
+	ID          protocol.ID
+	Constructor interface{}
+}
+
 // Config describes a set of settings for a libp2p node
 //
 // This is *not* a stable interface. Use the options defined in the root
@@ -73,7 +80,7 @@ type Config struct {
 
 	Transports         []fx.Option
 	Muxers             []tptu.StreamMuxer
-	SecurityTransports []fx.Option
+	SecurityTransports []Security
 	Insecure           bool
 	PSK                pnet.PSK
 
@@ -171,7 +178,7 @@ func (cfg *Config) addTransports(h host.Host) error {
 
 	fxopts := []fx.Option{
 		fx.WithLogger(func() fxevent.Logger { return getFXLogger() }),
-		fx.Provide(fx.Annotate(tptu.New, fx.ParamTags(`group:"security"`))),
+		fx.Provide(fx.Annotate(tptu.New, fx.ParamTags(`name:"security"`))),
 		fx.Supply(cfg.Muxers),
 		fx.Supply(h.ID()),
 		fx.Provide(func() host.Host { return h }),
@@ -186,15 +193,50 @@ func (cfg *Config) addTransports(h host.Host) error {
 		fxopts = append(fxopts,
 			fx.Provide(
 				fx.Annotate(
-					func(id peer.ID, priv crypto.PrivKey) sec.SecureTransport {
-						return insecure.NewWithIdentity(insecure.ID, id, priv)
+					func(id peer.ID, priv crypto.PrivKey) []sec.SecureTransport {
+						return []sec.SecureTransport{insecure.NewWithIdentity(insecure.ID, id, priv)}
 					},
-					fx.ResultTags(`group:"security"`),
+					fx.ResultTags(`name:"security"`),
 				),
 			),
 		)
 	} else {
-		fxopts = append(fxopts, cfg.SecurityTransports...)
+		// fx groups are unordered, but we need to preserve the order of the security transports
+		// First of all, we construct the security transports that are needed,
+		// and save them to a group call security_unordered.
+		for _, s := range cfg.SecurityTransports {
+			fxName := fmt.Sprintf(`name:"security_%s"`, s.ID)
+			fxopts = append(fxopts, fx.Supply(fx.Annotate(s.ID, fx.ResultTags(fxName))))
+			fxopts = append(fxopts,
+				fx.Provide(fx.Annotate(
+					s.Constructor,
+					fx.ParamTags(fxName),
+					fx.As(new(sec.SecureTransport)),
+					fx.ResultTags(`group:"security_unordered"`),
+				)),
+			)
+		}
+		// Then we consume the group security_unordered, and order them by the user's preference.
+		fxopts = append(fxopts, fx.Provide(
+			fx.Annotate(
+				func(secs []sec.SecureTransport) ([]sec.SecureTransport, error) {
+					if len(secs) != len(cfg.SecurityTransports) {
+						return nil, errors.New("inconsistent length for security transports")
+					}
+					t := make([]sec.SecureTransport, 0, len(secs))
+					for _, s := range cfg.SecurityTransports {
+						for _, st := range secs {
+							if s.ID != st.ID() {
+								continue
+							}
+							t = append(t, st)
+						}
+					}
+					return t, nil
+				},
+				fx.ParamTags(`group:"security_unordered"`),
+				fx.ResultTags(`name:"security"`),
+			)))
 	}
 
 	fxopts = append(fxopts, fx.Invoke(

core/network/conn.go

@@ -57,7 +57,7 @@ type ConnSecurity interface {
 	// RemotePublicKey returns the public key of the remote peer.
 	RemotePublicKey() ic.PubKey
 
-	// Connection state info of the secured connection.
+	// ConnState returns information about the connection state.
 	ConnState() ConnectionState
 }
 

options.go

@@ -19,7 +19,6 @@ import (
 	"github.com/libp2p/go-libp2p/core/peerstore"
 	"github.com/libp2p/go-libp2p/core/pnet"
 	"github.com/libp2p/go-libp2p/core/protocol"
-	"github.com/libp2p/go-libp2p/core/sec"
 	"github.com/libp2p/go-libp2p/core/transport"
 	"github.com/libp2p/go-libp2p/p2p/host/autorelay"
 	bhost "github.com/libp2p/go-libp2p/p2p/host/basic"
@@ -73,22 +72,7 @@ func Security(name string, constructor interface{}) Option {
 		if cfg.Insecure {
 			return fmt.Errorf("cannot use security transports with an insecure libp2p configuration")
 		}
-		fxName := fmt.Sprintf(`name:"%s"`, name)
-		// provide the name of the security transport
-		cfg.SecurityTransports = append(cfg.SecurityTransports,
-			fx.Provide(fx.Annotate(
-				func() protocol.ID { return protocol.ID(name) },
-				fx.ResultTags(fxName),
-			)),
-		)
-		cfg.SecurityTransports = append(cfg.SecurityTransports,
-			fx.Provide(fx.Annotate(
-				constructor,
-				fx.ParamTags(fxName),
-				fx.As(new(sec.SecureTransport)),
-				fx.ResultTags(`group:"security"`),
-			)),
-		)
+		cfg.SecurityTransports = append(cfg.SecurityTransports, config.Security{ID: protocol.ID(name), Constructor: constructor})
 		return nil
 	}
 }

p2p/test/muxer-negotiation/muxer_test.go → p2p/test/negotiation/muxer_test.go

@@ -1,4 +1,4 @@
-package muxer_negotiation
+package negotiation
 
 import (
 	"context"

p2p/test/negotiation/security_test.go

@@ -0,0 +1,90 @@
+package negotiation
+
+import (
+	"context"
+	"crypto/rand"
+	"testing"
+
+	"github.com/libp2p/go-libp2p"
+	"github.com/libp2p/go-libp2p/core/crypto"
+	"github.com/libp2p/go-libp2p/core/peer"
+	"github.com/libp2p/go-libp2p/core/protocol"
+	"github.com/libp2p/go-libp2p/p2p/security/noise"
+	tls "github.com/libp2p/go-libp2p/p2p/security/tls"
+	"github.com/libp2p/go-libp2p/p2p/transport/tcp"
+
+	"github.com/stretchr/testify/require"
+)
+
+var (
+	noiseOpt = libp2p.Security("/noise", noise.New)
+	tlsOpt   = libp2p.Security("/tls", tls.New)
+)
+
+func TestSecurityNegotiation(t *testing.T) {
+	testcases := []testcase{
+		{
+			Name:             "server and client have the same preference",
+			ServerPreference: []libp2p.Option{tlsOpt, noiseOpt},
+			ClientPreference: []libp2p.Option{tlsOpt, noiseOpt},
+			Expected:         "/tls",
+		},
+		{
+			Name:             "client only supports one security",
+			ServerPreference: []libp2p.Option{tlsOpt, noiseOpt},
+			ClientPreference: []libp2p.Option{noiseOpt},
+			Expected:         "/noise",
+		},
+		{
+			Name:             "server only supports one security",
+			ServerPreference: []libp2p.Option{noiseOpt},
+			ClientPreference: []libp2p.Option{tlsOpt, noiseOpt},
+			Expected:         "/noise",
+		},
+		{
+			Name:             "no  overlap",
+			ServerPreference: []libp2p.Option{noiseOpt},
+			ClientPreference: []libp2p.Option{tlsOpt},
+			Error:            "failed to negotiate security protocol: protocol not supported",
+		},
+	}
+
+	clientID, _, err := crypto.GenerateEd25519Key(rand.Reader)
+	require.NoError(t, err)
+	serverID, _, err := crypto.GenerateEd25519Key(rand.Reader)
+	require.NoError(t, err)
+
+	for _, tc := range testcases {
+		tc := tc
+
+		t.Run(tc.Name, func(t *testing.T) {
+			server, err := libp2p.New(
+				libp2p.Identity(serverID),
+				libp2p.ChainOptions(tc.ServerPreference...),
+				libp2p.Transport(tcp.NewTCPTransport),
+				libp2p.ListenAddrStrings("/ip4/127.0.0.1/tcp/0"),
+			)
+			require.NoError(t, err)
+
+			client, err := libp2p.New(
+				libp2p.Identity(clientID),
+				libp2p.ChainOptions(tc.ClientPreference...),
+				libp2p.Transport(tcp.NewTCPTransport),
+				libp2p.NoListenAddrs,
+			)
+			require.NoError(t, err)
+
+			err = client.Connect(context.Background(), peer.AddrInfo{ID: server.ID(), Addrs: server.Addrs()})
+			if tc.Error != "" {
+				require.Error(t, err)
+				require.ErrorContains(t, err, tc.Error)
+				return
+			}
+
+			require.NoError(t, err)
+			conns := client.Network().ConnsToPeer(server.ID())
+			require.Len(t, conns, 1, "expected exactly one connection")
+			require.Equal(t, tc.Expected, protocol.ID(conns[0].ConnState().Security))
+		})
+	}
+}