From 8962b2ae336d94627f2f4361f96799ee3a5bd9e4 Mon Sep 17 00:00:00 2001 From: Marco Munizaga Date: Mon, 14 Nov 2022 13:25:11 -0800 Subject: [PATCH 01/19] transport.Listener returns a list of multiaddrs --- core/transport/transport.go | 2 +- p2p/host/autonat/dialpolicy_test.go | 6 +-- p2p/net/swarm/swarm_addr.go | 2 +- p2p/net/swarm/swarm_listen.go | 23 +++++++--- p2p/net/upgrader/listener.go | 5 ++ p2p/net/upgrader/listener_test.go | 34 +++++++------- p2p/net/upgrader/upgrader_test.go | 10 ++-- p2p/transport/quic/cmd/server/main.go | 2 +- p2p/transport/quic/conn_test.go | 32 ++++++------- p2p/transport/quic/listener.go | 4 +- p2p/transport/quic/listener_test.go | 4 +- p2p/transport/tcp/tcp_test.go | 12 ++--- p2p/transport/testsuite/stream_suite.go | 6 +-- p2p/transport/testsuite/transport_suite.go | 10 ++-- p2p/transport/websocket/listener.go | 4 ++ p2p/transport/websocket/websocket_test.go | 16 +++---- p2p/transport/webtransport/listener.go | 6 +-- p2p/transport/webtransport/transport_test.go | 48 ++++++++++---------- 18 files changed, 122 insertions(+), 104 deletions(-) diff --git a/core/transport/transport.go b/core/transport/transport.go index ad2ee66496..2a7537738c 100644 --- a/core/transport/transport.go +++ b/core/transport/transport.go @@ -91,7 +91,7 @@ type Listener interface { Accept() (CapableConn, error) Close() error Addr() net.Addr - Multiaddr() ma.Multiaddr + Multiaddrs() []ma.Multiaddr } // TransportNetwork is an inet.Network with methods for managing transports. diff --git a/p2p/host/autonat/dialpolicy_test.go b/p2p/host/autonat/dialpolicy_test.go index 75731ae9cf..e56a862b67 100644 --- a/p2p/host/autonat/dialpolicy_test.go +++ b/p2p/host/autonat/dialpolicy_test.go @@ -47,9 +47,9 @@ func (l *mockL) Accept() (transport.CapableConn, error) { <-l.ctx.Done() return nil, errors.New("expected in mocked test") } -func (l *mockL) Close() error { return nil } -func (l *mockL) Addr() net.Addr { return nil } -func (l *mockL) Multiaddr() multiaddr.Multiaddr { return l.addr } +func (l *mockL) Close() error { return nil } +func (l *mockL) Addr() net.Addr { return nil } +func (l *mockL) Multiaddrs() []multiaddr.Multiaddr { return []multiaddr.Multiaddr{l.addr} } func TestSkipDial(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) diff --git a/p2p/net/swarm/swarm_addr.go b/p2p/net/swarm/swarm_addr.go index 8d088e76df..aa8b808084 100644 --- a/p2p/net/swarm/swarm_addr.go +++ b/p2p/net/swarm/swarm_addr.go @@ -18,7 +18,7 @@ func (s *Swarm) ListenAddresses() []ma.Multiaddr { func (s *Swarm) listenAddressesNoLock() []ma.Multiaddr { addrs := make([]ma.Multiaddr, 0, len(s.listeners.m)) for l := range s.listeners.m { - addrs = append(addrs, l.Multiaddr()) + addrs = append(addrs, l.Multiaddrs()...) } return addrs } diff --git a/p2p/net/swarm/swarm_listen.go b/p2p/net/swarm/swarm_listen.go index 044da2e8de..59407162bb 100644 --- a/p2p/net/swarm/swarm_listen.go +++ b/p2p/net/swarm/swarm_listen.go @@ -43,7 +43,7 @@ func (s *Swarm) ListenClose(addrs ...ma.Multiaddr) { s.listeners.Lock() for l := range s.listeners.m { - if !containsMultiaddr(addrs, l.Multiaddr()) { + if !containsSomeMultiaddr(addrs, l.Multiaddrs()) { continue } @@ -92,11 +92,13 @@ func (s *Swarm) AddListenAddr(a ma.Multiaddr) error { s.listeners.cacheEOL = time.Time{} s.listeners.Unlock() - maddr := list.Multiaddr() + maddrs := list.Multiaddrs() // signal to our notifiees on listen. s.notifyAll(func(n network.Notifiee) { - n.Listen(s, maddr) + for _, maddr := range maddrs { + n.Listen(s, maddr) + } }) go func() { @@ -116,7 +118,9 @@ func (s *Swarm) AddListenAddr(a ma.Multiaddr) error { // signal to our notifiees on listen close. s.notifyAll(func(n network.Notifiee) { - n.ListenClose(s, maddr) + for _, maddr := range maddrs { + n.ListenClose(s, maddr) + } }) s.refs.Done() }() @@ -147,11 +151,16 @@ func (s *Swarm) AddListenAddr(a ma.Multiaddr) error { return nil } -func containsMultiaddr(addrs []ma.Multiaddr, addr ma.Multiaddr) bool { - for _, a := range addrs { - if addr == a { +func containsSomeMultiaddr(hayStack []ma.Multiaddr, needles []ma.Multiaddr) bool { + seenSet := make(map[string]struct{}, len(needles)) + for _, a := range needles { + seenSet[string(a.Bytes())] = struct{}{} + } + for _, a := range hayStack { + if _, found := seenSet[string(a.Bytes())]; found { return true } } return false + } diff --git a/p2p/net/upgrader/listener.go b/p2p/net/upgrader/listener.go index c07299c1a5..0a3ab557e8 100644 --- a/p2p/net/upgrader/listener.go +++ b/p2p/net/upgrader/listener.go @@ -10,6 +10,7 @@ import ( logging "github.com/ipfs/go-log/v2" tec "github.com/jbenet/go-temp-err-catcher" + "github.com/multiformats/go-multiaddr" manet "github.com/multiformats/go-multiaddr/net" ) @@ -175,4 +176,8 @@ func (l *listener) String() string { return fmt.Sprintf("", l.Multiaddr()) } +func (l *listener) Multiaddrs() []multiaddr.Multiaddr { + return []multiaddr.Multiaddr{l.Multiaddr()} +} + var _ transport.Listener = (*listener)(nil) diff --git a/p2p/net/upgrader/listener_test.go b/p2p/net/upgrader/listener_test.go index 5b5410753a..0d872d1d12 100644 --- a/p2p/net/upgrader/listener_test.go +++ b/p2p/net/upgrader/listener_test.go @@ -56,7 +56,7 @@ func TestAcceptSingleConn(t *testing.T) { ln := createListener(t, u) defer ln.Close() - cconn, err := dial(t, u, ln.Multiaddr(), id, &network.NullScope{}) + cconn, err := dial(t, u, ln.Multiaddrs()[0], id, &network.NullScope{}) require.NoError(err) sconn, err := ln.Accept() @@ -80,7 +80,7 @@ func TestAcceptMultipleConns(t *testing.T) { }() for i := 0; i < 10; i++ { - cconn, err := dial(t, u, ln.Multiaddr(), id, &network.NullScope{}) + cconn, err := dial(t, u, ln.Multiaddrs()[0], id, &network.NullScope{}) require.NoError(err) toClose = append(toClose, cconn) @@ -104,7 +104,7 @@ func TestConnectionsClosedIfNotAccepted(t *testing.T) { ln := createListener(t, u) defer ln.Close() - conn, err := dial(t, u, ln.Multiaddr(), id, &network.NullScope{}) + conn, err := dial(t, u, ln.Multiaddrs()[0], id, &network.NullScope{}) require.NoError(err) errCh := make(chan error) @@ -143,7 +143,7 @@ func TestFailedUpgradeOnListen(t *testing.T) { errCh <- err }() - _, err := dial(t, u, ln.Multiaddr(), id, &network.NullScope{}) + _, err := dial(t, u, ln.Multiaddrs()[0], id, &network.NullScope{}) require.Error(err) // close the listener. @@ -177,7 +177,7 @@ func TestListenerClose(t *testing.T) { require.Contains(err.Error(), "use of closed network connection") // doesn't accept new connections when it is closed - _, err = dial(t, u, ln.Multiaddr(), peer.ID("1"), &network.NullScope{}) + _, err = dial(t, u, ln.Multiaddrs()[0], peer.ID("1"), &network.NullScope{}) require.Error(err) } @@ -189,7 +189,7 @@ func TestListenerCloseClosesQueued(t *testing.T) { var conns []transport.CapableConn for i := 0; i < 10; i++ { - conn, err := dial(t, upgrader, ln.Multiaddr(), id, &network.NullScope{}) + conn, err := dial(t, upgrader, ln.Multiaddrs()[0], id, &network.NullScope{}) require.NoError(err) conns = append(conns, conn) } @@ -249,7 +249,7 @@ func TestConcurrentAccept(t *testing.T) { go func() { defer wg.Done() - conn, err := dial(t, u, ln.Multiaddr(), id, &network.NullScope{}) + conn, err := dial(t, u, ln.Multiaddrs()[0], id, &network.NullScope{}) if err != nil { errCh <- err return @@ -279,7 +279,7 @@ func TestAcceptQueueBacklogged(t *testing.T) { // setup AcceptQueueLength connections, but don't accept any of them var counter int32 // to be used atomically doDial := func() { - conn, err := dial(t, u, ln.Multiaddr(), id, &network.NullScope{}) + conn, err := dial(t, u, ln.Multiaddrs()[0], id, &network.NullScope{}) require.NoError(err) atomic.AddInt32(&counter, 1) t.Cleanup(func() { conn.Close() }) @@ -315,7 +315,7 @@ func TestListenerConnectionGater(t *testing.T) { defer ln.Close() // no gating. - conn, err := dial(t, u, ln.Multiaddr(), id, &network.NullScope{}) + conn, err := dial(t, u, ln.Multiaddrs()[0], id, &network.NullScope{}) require.NoError(err) require.False(conn.IsClosed()) _ = conn.Close() @@ -323,28 +323,28 @@ func TestListenerConnectionGater(t *testing.T) { // rejecting after handshake. testGater.BlockSecured(true) testGater.BlockAccept(false) - conn, err = dial(t, u, ln.Multiaddr(), "invalid", &network.NullScope{}) + conn, err = dial(t, u, ln.Multiaddrs()[0], "invalid", &network.NullScope{}) require.Error(err) require.Nil(conn) // rejecting on accept will trigger firupgrader. testGater.BlockSecured(true) testGater.BlockAccept(true) - conn, err = dial(t, u, ln.Multiaddr(), "invalid", &network.NullScope{}) + conn, err = dial(t, u, ln.Multiaddrs()[0], "invalid", &network.NullScope{}) require.Error(err) require.Nil(conn) // rejecting only on acceptance. testGater.BlockSecured(false) testGater.BlockAccept(true) - conn, err = dial(t, u, ln.Multiaddr(), "invalid", &network.NullScope{}) + conn, err = dial(t, u, ln.Multiaddrs()[0], "invalid", &network.NullScope{}) require.Error(err) require.Nil(conn) // back to normal testGater.BlockSecured(false) testGater.BlockAccept(false) - conn, err = dial(t, u, ln.Multiaddr(), id, &network.NullScope{}) + conn, err = dial(t, u, ln.Multiaddrs()[0], id, &network.NullScope{}) require.NoError(err) require.False(conn.IsClosed()) _ = conn.Close() @@ -360,13 +360,13 @@ func TestListenerResourceManagement(t *testing.T) { connScope := mocknetwork.NewMockConnManagementScope(ctrl) gomock.InOrder( - rcmgr.EXPECT().OpenConnection(network.DirInbound, true, gomock.Not(ln.Multiaddr())).Return(connScope, nil), + rcmgr.EXPECT().OpenConnection(network.DirInbound, true, gomock.Not(ln.Multiaddrs()[0])).Return(connScope, nil), connScope.EXPECT().PeerScope(), connScope.EXPECT().SetPeer(id), connScope.EXPECT().PeerScope(), ) - cconn, err := dial(t, upgrader, ln.Multiaddr(), id, &network.NullScope{}) + cconn, err := dial(t, upgrader, ln.Multiaddrs()[0], id, &network.NullScope{}) require.NoError(t, err) defer cconn.Close() @@ -383,8 +383,8 @@ func TestListenerResourceManagementDenied(t *testing.T) { id, upgrader := createUpgraderWithResourceManager(t, rcmgr) ln := createListener(t, upgrader) - rcmgr.EXPECT().OpenConnection(network.DirInbound, true, gomock.Not(ln.Multiaddr())).Return(nil, errors.New("nope")) - _, err := dial(t, upgrader, ln.Multiaddr(), id, &network.NullScope{}) + rcmgr.EXPECT().OpenConnection(network.DirInbound, true, gomock.Not(ln.Multiaddrs()[0])).Return(nil, errors.New("nope")) + _, err := dial(t, upgrader, ln.Multiaddrs()[0], id, &network.NullScope{}) require.Error(t, err) done := make(chan struct{}) diff --git a/p2p/net/upgrader/upgrader_test.go b/p2p/net/upgrader/upgrader_test.go index 106752ab6a..ccbd6ffac3 100644 --- a/p2p/net/upgrader/upgrader_test.go +++ b/p2p/net/upgrader/upgrader_test.go @@ -134,21 +134,21 @@ func TestOutboundConnectionGating(t *testing.T) { testGater := &testGater{} _, dialUpgrader := createUpgraderWithConnGater(t, testGater) - conn, err := dial(t, dialUpgrader, ln.Multiaddr(), id, &network.NullScope{}) + conn, err := dial(t, dialUpgrader, ln.Multiaddrs()[0], id, &network.NullScope{}) require.NoError(err) require.NotNil(conn) _ = conn.Close() // blocking accepts doesn't affect the dialling side, only the listener. testGater.BlockAccept(true) - conn, err = dial(t, dialUpgrader, ln.Multiaddr(), id, &network.NullScope{}) + conn, err = dial(t, dialUpgrader, ln.Multiaddrs()[0], id, &network.NullScope{}) require.NoError(err) require.NotNil(conn) _ = conn.Close() // now let's block all connections after being secured. testGater.BlockSecured(true) - conn, err = dial(t, dialUpgrader, ln.Multiaddr(), id, &network.NullScope{}) + conn, err = dial(t, dialUpgrader, ln.Multiaddrs()[0], id, &network.NullScope{}) require.Error(err) require.Contains(err.Error(), "gater rejected connection") require.Nil(conn) @@ -169,7 +169,7 @@ func TestOutboundResourceManagement(t *testing.T) { connScope.EXPECT().PeerScope().Return(&network.NullScope{}), ) _, dialUpgrader := createUpgrader(t) - conn, err := dial(t, dialUpgrader, ln.Multiaddr(), id, connScope) + conn, err := dial(t, dialUpgrader, ln.Multiaddrs()[0], id, connScope) require.NoError(t, err) require.NotNil(t, conn) connScope.EXPECT().Done() @@ -191,7 +191,7 @@ func TestOutboundResourceManagement(t *testing.T) { connScope.EXPECT().Done(), ) _, dialUpgrader := createUpgrader(t) - _, err := dial(t, dialUpgrader, ln.Multiaddr(), id, connScope) + _, err := dial(t, dialUpgrader, ln.Multiaddrs()[0], id, connScope) require.Error(t, err) }) diff --git a/p2p/transport/quic/cmd/server/main.go b/p2p/transport/quic/cmd/server/main.go index e6585137f2..7122de6058 100644 --- a/p2p/transport/quic/cmd/server/main.go +++ b/p2p/transport/quic/cmd/server/main.go @@ -48,7 +48,7 @@ func run(port string) error { if err != nil { return err } - fmt.Printf("Listening. Now run: go run cmd/client/main.go %s %s\n", ln.Multiaddr(), peerID) + fmt.Printf("Listening. Now run: go run cmd/client/main.go %s %s\n", ln.Multiaddrs()[0], peerID) for { conn, err := ln.Accept() if err != nil { diff --git a/p2p/transport/quic/conn_test.go b/p2p/transport/quic/conn_test.go index 047c30347e..6bca67da4a 100644 --- a/p2p/transport/quic/conn_test.go +++ b/p2p/transport/quic/conn_test.go @@ -84,7 +84,7 @@ func testHandshake(t *testing.T, tc *connTestCase) { clientTransport, err := NewTransport(clientKey, nil, nil, nil, tc.Options...) require.NoError(t, err) defer clientTransport.(io.Closer).Close() - conn, err := clientTransport.Dial(context.Background(), ln.Multiaddr(), serverID) + conn, err := clientTransport.Dial(context.Background(), ln.Multiaddrs()[0], serverID) require.NoError(t, err) defer conn.Close() serverConn, err := ln.Accept() @@ -146,7 +146,7 @@ func testResourceManagerSuccess(t *testing.T, tc *connTestCase) { connChan := make(chan tpt.CapableConn) serverConnScope := mocknetwork.NewMockConnManagementScope(ctrl) go func() { - serverRcmgr.EXPECT().OpenConnection(network.DirInbound, false, gomock.Not(ln.Multiaddr())).Return(serverConnScope, nil) + serverRcmgr.EXPECT().OpenConnection(network.DirInbound, false, gomock.Not(ln.Multiaddrs()[0])).Return(serverConnScope, nil) serverConnScope.EXPECT().SetPeer(clientID) serverConn, err := ln.Accept() require.NoError(t, err) @@ -154,9 +154,9 @@ func testResourceManagerSuccess(t *testing.T, tc *connTestCase) { }() connScope := mocknetwork.NewMockConnManagementScope(ctrl) - clientRcmgr.EXPECT().OpenConnection(network.DirOutbound, false, ln.Multiaddr()).Return(connScope, nil) + clientRcmgr.EXPECT().OpenConnection(network.DirOutbound, false, ln.Multiaddrs()[0]).Return(connScope, nil) connScope.EXPECT().SetPeer(serverID) - conn, err := clientTransport.Dial(context.Background(), ln.Multiaddr(), serverID) + conn, err := clientTransport.Dial(context.Background(), ln.Multiaddrs()[0], serverID) require.NoError(t, err) serverConn := <-connChan t.Log("received conn") @@ -238,12 +238,12 @@ func testResourceManagerAcceptDenied(t *testing.T, tc *connTestCase) { }() clientConnScope := mocknetwork.NewMockConnManagementScope(ctrl) - clientRcmgr.EXPECT().OpenConnection(network.DirOutbound, false, ln.Multiaddr()).Return(clientConnScope, nil) + clientRcmgr.EXPECT().OpenConnection(network.DirOutbound, false, ln.Multiaddrs()[0]).Return(clientConnScope, nil) clientConnScope.EXPECT().SetPeer(serverID) // In rare instances, the connection gating error will already occur on Dial. // In that case, Done is called on the connection scope. clientConnScope.EXPECT().Done().MaxTimes(1) - conn, err := clientTransport.Dial(context.Background(), ln.Multiaddr(), serverID) + conn, err := clientTransport.Dial(context.Background(), ln.Multiaddrs()[0], serverID) // In rare instances, the connection gating error will already occur on Dial. if err == nil { _, err = conn.AcceptStream() @@ -277,7 +277,7 @@ func testStreams(t *testing.T, tc *connTestCase) { clientTransport, err := NewTransport(clientKey, nil, nil, nil, tc.Options...) require.NoError(t, err) defer clientTransport.(io.Closer).Close() - conn, err := clientTransport.Dial(context.Background(), ln.Multiaddr(), serverID) + conn, err := clientTransport.Dial(context.Background(), ln.Multiaddrs()[0], serverID) require.NoError(t, err) defer conn.Close() serverConn, err := ln.Accept() @@ -317,7 +317,7 @@ func testHandshakeFailPeerIDMismatch(t *testing.T, tc *connTestCase) { clientTransport, err := NewTransport(clientKey, nil, nil, nil, tc.Options...) require.NoError(t, err) // dial, but expect the wrong peer ID - _, err = clientTransport.Dial(context.Background(), ln.Multiaddr(), thirdPartyID) + _, err = clientTransport.Dial(context.Background(), ln.Multiaddrs()[0], thirdPartyID) require.Error(t, err) require.Contains(t, err.Error(), "CRYPTO_ERROR") defer clientTransport.(io.Closer).Close() @@ -374,7 +374,7 @@ func testConnectionGating(t *testing.T, tc *connTestCase) { require.NoError(t, err) defer clientTransport.(io.Closer).Close() // make sure that connection attempts fails - conn, err := clientTransport.Dial(context.Background(), ln.Multiaddr(), serverID) + conn, err := clientTransport.Dial(context.Background(), ln.Multiaddrs()[0], serverID) // In rare instances, the connection gating error will already occur on Dial. // In most cases, it will be returned by AcceptStream. if err == nil { @@ -386,7 +386,7 @@ func testConnectionGating(t *testing.T, tc *connTestCase) { cg.EXPECT().InterceptAccept(gomock.Any()).Return(true) cg.EXPECT().InterceptSecured(gomock.Any(), gomock.Any(), gomock.Any()).Return(true) clientTransport.(*transport).clientConfig.HandshakeIdleTimeout = 2 * time.Second - conn, err = clientTransport.Dial(context.Background(), ln.Multiaddr(), serverID) + conn, err = clientTransport.Dial(context.Background(), ln.Multiaddrs()[0], serverID) require.NoError(t, err) defer conn.Close() require.Eventually(t, func() bool { @@ -414,14 +414,14 @@ func testConnectionGating(t *testing.T, tc *connTestCase) { defer clientTransport.(io.Closer).Close() // make sure that connection attempts fails - _, err = clientTransport.Dial(context.Background(), ln.Multiaddr(), serverID) + _, err = clientTransport.Dial(context.Background(), ln.Multiaddrs()[0], serverID) require.Error(t, err) require.Contains(t, err.Error(), "connection gated") // now allow the peerId and make sure the connection goes through cg.EXPECT().InterceptSecured(gomock.Any(), gomock.Any(), gomock.Any()).Return(true) clientTransport.(*transport).clientConfig.HandshakeIdleTimeout = 2 * time.Second - conn, err := clientTransport.Dial(context.Background(), ln.Multiaddr(), serverID) + conn, err := clientTransport.Dial(context.Background(), ln.Multiaddrs()[0], serverID) require.NoError(t, err) conn.Close() }) @@ -474,10 +474,10 @@ func testDialTwo(t *testing.T, tc *connTestCase) { clientTransport, err := NewTransport(clientKey, nil, nil, nil, tc.Options...) require.NoError(t, err) defer clientTransport.(io.Closer).Close() - c1, err := clientTransport.Dial(context.Background(), ln1.Multiaddr(), serverID) + c1, err := clientTransport.Dial(context.Background(), ln1.Multiaddrs()[0], serverID) require.NoError(t, err) defer c1.Close() - c2, err := clientTransport.Dial(context.Background(), ln2.Multiaddr(), serverID2) + c2, err := clientTransport.Dial(context.Background(), ln2.Multiaddrs()[0], serverID2) require.NoError(t, err) defer c2.Close() @@ -632,7 +632,7 @@ func TestHolePunching(t *testing.T) { go func() { conn, err := t2.Dial( network.WithSimultaneousConnect(context.Background(), false, ""), - ln1.Multiaddr(), + ln1.Multiaddrs()[0], serverID, ) require.NoError(t, err) @@ -650,7 +650,7 @@ func TestHolePunching(t *testing.T) { conn1, err := t1.Dial( network.WithSimultaneousConnect(context.Background(), true, ""), - ln2.Multiaddr(), + ln2.Multiaddrs()[0], clientID, ) require.NoError(t, err) diff --git a/p2p/transport/quic/listener.go b/p2p/transport/quic/listener.go index 949a3e7db6..6cad6fa58b 100644 --- a/p2p/transport/quic/listener.go +++ b/p2p/transport/quic/listener.go @@ -164,6 +164,6 @@ func (l *listener) Addr() net.Addr { } // Multiaddr returns the multiaddress of this listener. -func (l *listener) Multiaddr() ma.Multiaddr { - return l.localMultiaddr +func (l *listener) Multiaddrs() []ma.Multiaddr { + return []ma.Multiaddr{l.localMultiaddr} } diff --git a/p2p/transport/quic/listener_test.go b/p2p/transport/quic/listener_test.go index 7c8494ae91..7f5fe8458e 100644 --- a/p2p/transport/quic/listener_test.go +++ b/p2p/transport/quic/listener_test.go @@ -66,7 +66,7 @@ func TestListenAddr(t *testing.T) { defer ln.Close() port := ln.Addr().(*net.UDPAddr).Port require.NotZero(t, port) - require.Equal(t, ln.Multiaddr().String(), fmt.Sprintf("/ip4/127.0.0.1/udp/%d/quic", port)) + require.Equal(t, ln.Multiaddrs()[0].String(), fmt.Sprintf("/ip4/127.0.0.1/udp/%d/quic", port)) }) t.Run("for IPv6", func(t *testing.T) { @@ -76,7 +76,7 @@ func TestListenAddr(t *testing.T) { defer ln.Close() port := ln.Addr().(*net.UDPAddr).Port require.NotZero(t, port) - require.Equal(t, ln.Multiaddr().String(), fmt.Sprintf("/ip6/::/udp/%d/quic", port)) + require.Equal(t, ln.Multiaddrs()[0].String(), fmt.Sprintf("/ip6/::/udp/%d/quic", port)) }) } diff --git a/p2p/transport/tcp/tcp_test.go b/p2p/transport/tcp/tcp_test.go index 9204046328..b320c5a40b 100644 --- a/p2p/transport/tcp/tcp_test.go +++ b/p2p/transport/tcp/tcp_test.go @@ -86,10 +86,10 @@ func TestResourceManager(t *testing.T) { t.Run("success", func(t *testing.T) { scope := mocknetwork.NewMockConnManagementScope(ctrl) - rcmgr.EXPECT().OpenConnection(network.DirOutbound, true, ln.Multiaddr()).Return(scope, nil) + rcmgr.EXPECT().OpenConnection(network.DirOutbound, true, ln.Multiaddrs()[0]).Return(scope, nil) scope.EXPECT().SetPeer(peerA) scope.EXPECT().PeerScope().Return(&network.NullScope{}).AnyTimes() // called by the upgrader - conn, err := tb.Dial(context.Background(), ln.Multiaddr(), peerA) + conn, err := tb.Dial(context.Background(), ln.Multiaddrs()[0], peerA) require.NoError(t, err) scope.EXPECT().Done() defer conn.Close() @@ -97,18 +97,18 @@ func TestResourceManager(t *testing.T) { t.Run("connection denied", func(t *testing.T) { rerr := errors.New("nope") - rcmgr.EXPECT().OpenConnection(network.DirOutbound, true, ln.Multiaddr()).Return(nil, rerr) - _, err = tb.Dial(context.Background(), ln.Multiaddr(), peerA) + rcmgr.EXPECT().OpenConnection(network.DirOutbound, true, ln.Multiaddrs()[0]).Return(nil, rerr) + _, err = tb.Dial(context.Background(), ln.Multiaddrs()[0], peerA) require.ErrorIs(t, err, rerr) }) t.Run("peer denied", func(t *testing.T) { scope := mocknetwork.NewMockConnManagementScope(ctrl) - rcmgr.EXPECT().OpenConnection(network.DirOutbound, true, ln.Multiaddr()).Return(scope, nil) + rcmgr.EXPECT().OpenConnection(network.DirOutbound, true, ln.Multiaddrs()[0]).Return(scope, nil) rerr := errors.New("nope") scope.EXPECT().SetPeer(peerA).Return(rerr) scope.EXPECT().Done() - _, err = tb.Dial(context.Background(), ln.Multiaddr(), peerA) + _, err = tb.Dial(context.Background(), ln.Multiaddrs()[0], peerA) require.ErrorIs(t, err, rerr) }) } diff --git a/p2p/transport/testsuite/stream_suite.go b/p2p/transport/testsuite/stream_suite.go index b139976b91..3170b72c40 100644 --- a/p2p/transport/testsuite/stream_suite.go +++ b/p2p/transport/testsuite/stream_suite.go @@ -197,7 +197,7 @@ func SubtestStress(t *testing.T, ta, tb transport.Transport, maddr ma.Multiaddr, serve(t, l) }() - c, err := tb.Dial(context.Background(), l.Multiaddr(), peerA) + c, err := tb.Dial(context.Background(), l.Multiaddrs()[0], peerA) if err != nil { t.Error(err) return @@ -259,7 +259,7 @@ func SubtestStreamOpenStress(t *testing.T, ta, tb transport.Transport, maddr ma. connA, err = l.Accept() accepted <- err }() - connB, err = tb.Dial(context.Background(), l.Multiaddr(), peerA) + connB, err = tb.Dial(context.Background(), l.Multiaddrs()[0], peerA) if err != nil { t.Fatal(err) } @@ -373,7 +373,7 @@ func SubtestStreamReset(t *testing.T, ta, tb transport.Transport, maddr ma.Multi }() - muxb, err := tb.Dial(context.Background(), l.Multiaddr(), peerA) + muxb, err := tb.Dial(context.Background(), l.Multiaddrs()[0], peerA) if err != nil { t.Fatal(err) } diff --git a/p2p/transport/testsuite/transport_suite.go b/p2p/transport/testsuite/transport_suite.go index bd8892e807..f8d0ff1113 100644 --- a/p2p/transport/testsuite/transport_suite.go +++ b/p2p/transport/testsuite/transport_suite.go @@ -111,11 +111,11 @@ func SubtestBasic(t *testing.T, ta, tb transport.Transport, maddr ma.Multiaddr, } }() - if !tb.CanDial(list.Multiaddr()) { + if !tb.CanDial(list.Multiaddrs()[0]) { t.Error("CanDial should have returned true") } - connA, err = tb.Dial(ctx, list.Multiaddr(), peerA) + connA, err = tb.Dial(ctx, list.Multiaddrs()[0], peerA) if err != nil { t.Fatal(err) } @@ -232,11 +232,11 @@ func SubtestPingPong(t *testing.T, ta, tb transport.Transport, maddr ma.Multiadd sWg.Wait() }() - if !tb.CanDial(list.Multiaddr()) { + if !tb.CanDial(list.Multiaddrs()[0]) { t.Error("CanDial should have returned true") } - connB, err = tb.Dial(ctx, list.Multiaddr(), peerA) + connB, err = tb.Dial(ctx, list.Multiaddrs()[0], peerA) if err != nil { t.Fatal(err) } @@ -297,7 +297,7 @@ func SubtestCancel(t *testing.T, ta, tb transport.Transport, maddr ma.Multiaddr, ctx, cancel := context.WithCancel(context.Background()) cancel() - c, err := tb.Dial(ctx, list.Multiaddr(), peerA) + c, err := tb.Dial(ctx, list.Multiaddrs()[0], peerA) if err == nil { c.Close() t.Fatal("dial should have failed") diff --git a/p2p/transport/websocket/listener.go b/p2p/transport/websocket/listener.go index 128fdf5eb5..ee6810807b 100644 --- a/p2p/transport/websocket/listener.go +++ b/p2p/transport/websocket/listener.go @@ -136,3 +136,7 @@ func (l *listener) Close() error { func (l *listener) Multiaddr() ma.Multiaddr { return l.laddr } + +func (l *listener) Multiaddrs() []ma.Multiaddr { + return []ma.Multiaddr{l.laddr} +} diff --git a/p2p/transport/websocket/websocket_test.go b/p2p/transport/websocket/websocket_test.go index 016dbe59b9..051c8064b6 100644 --- a/p2p/transport/websocket/websocket_test.go +++ b/p2p/transport/websocket/websocket_test.go @@ -190,7 +190,7 @@ func testWSSServer(t *testing.T, listenAddr ma.Multiaddr) (ma.Multiaddr, peer.ID close(errChan) }() - return l.Multiaddr(), id, errChan + return l.Multiaddrs()[0], id, errChan } func getTLSConf(t *testing.T, ip net.IP, start, end time.Time) *tls.Config { @@ -330,9 +330,9 @@ func connectAndExchangeData(t *testing.T, laddr ma.Multiaddr, secure bool) { l, err := tpt.Listen(laddr) require.NoError(t, err) if secure { - require.Contains(t, l.Multiaddr().String(), "tls") + require.Contains(t, l.Multiaddrs()[0].String(), "tls") } else { - require.Equal(t, lastComponent(t, l.Multiaddr()), wsComponent) + require.Equal(t, lastComponent(t, l.Multiaddrs()[0]), wsComponent) } defer l.Close() @@ -346,7 +346,7 @@ func connectAndExchangeData(t *testing.T, laddr ma.Multiaddr, secure bool) { _, u := newUpgrader(t) tpt, err := New(u, &network.NullResourceManager{}, opts...) require.NoError(t, err) - c, err := tpt.Dial(context.Background(), l.Multiaddr(), server) + c, err := tpt.Dial(context.Background(), l.Multiaddrs()[0], server) require.NoError(t, err) str, err := c.OpenStream(context.Background()) require.NoError(t, err) @@ -401,14 +401,14 @@ func TestWebsocketListenSecureAndInsecure(t *testing.T) { require.NoError(t, err) // dialing the insecure address should succeed - conn, err := client.Dial(context.Background(), lnInsecure.Multiaddr(), serverID) + conn, err := client.Dial(context.Background(), lnInsecure.Multiaddrs()[0], serverID) require.NoError(t, err) defer conn.Close() require.Equal(t, lastComponent(t, conn.RemoteMultiaddr()).String(), wsComponent.String()) require.Equal(t, lastComponent(t, conn.LocalMultiaddr()).String(), wsComponent.String()) // dialing the secure address should fail - _, err = client.Dial(context.Background(), lnSecure.Multiaddr(), serverID) + _, err = client.Dial(context.Background(), lnSecure.Multiaddrs()[0], serverID) require.NoError(t, err) }) @@ -418,14 +418,14 @@ func TestWebsocketListenSecureAndInsecure(t *testing.T) { require.NoError(t, err) // dialing the insecure address should succeed - conn, err := client.Dial(context.Background(), lnSecure.Multiaddr(), serverID) + conn, err := client.Dial(context.Background(), lnSecure.Multiaddrs()[0], serverID) require.NoError(t, err) defer conn.Close() require.Equal(t, lastComponent(t, conn.RemoteMultiaddr()), wssComponent) require.Equal(t, lastComponent(t, conn.LocalMultiaddr()), wssComponent) // dialing the insecure address should fail - _, err = client.Dial(context.Background(), lnInsecure.Multiaddr(), serverID) + _, err = client.Dial(context.Background(), lnInsecure.Multiaddrs()[0], serverID) require.NoError(t, err) }) } diff --git a/p2p/transport/webtransport/listener.go b/p2p/transport/webtransport/listener.go index a5fae40da3..9a93d014b1 100644 --- a/p2p/transport/webtransport/listener.go +++ b/p2p/transport/webtransport/listener.go @@ -218,11 +218,11 @@ func (l *listener) Addr() net.Addr { return l.addr } -func (l *listener) Multiaddr() ma.Multiaddr { +func (l *listener) Multiaddrs() []ma.Multiaddr { if l.transport.certManager == nil { - return l.multiaddr + return []ma.Multiaddr{l.multiaddr} } - return l.multiaddr.Encapsulate(l.transport.certManager.AddrComponent()) + return []ma.Multiaddr{l.multiaddr.Encapsulate(l.transport.certManager.AddrComponent())} } func (l *listener) Close() error { diff --git a/p2p/transport/webtransport/transport_test.go b/p2p/transport/webtransport/transport_test.go index 70d4ec4b3d..3e856b2569 100644 --- a/p2p/transport/webtransport/transport_test.go +++ b/p2p/transport/webtransport/transport_test.go @@ -114,7 +114,7 @@ func TestTransport(t *testing.T) { require.NoError(t, err) defer tr2.(io.Closer).Close() - conn, err := tr2.Dial(context.Background(), ln.Multiaddr(), serverID) + conn, err := tr2.Dial(context.Background(), ln.Multiaddrs()[0], serverID) require.NoError(t, err) str, err := conn.OpenStream(context.Background()) require.NoError(t, err) @@ -123,7 +123,7 @@ func TestTransport(t *testing.T) { require.NoError(t, str.Close()) // check RemoteMultiaddr - _, addr, err := manet.DialArgs(ln.Multiaddr()) + _, addr, err := manet.DialArgs(ln.Multiaddrs()[0]) require.NoError(t, err) _, port, err := net.SplitHostPort(addr) require.NoError(t, err) @@ -167,14 +167,14 @@ func TestHashVerification(t *testing.T) { t.Run("fails using only a wrong hash", func(t *testing.T) { // replace the certificate hash in the multiaddr with a fake hash - addr := stripCertHashes(ln.Multiaddr()).Encapsulate(foobarHash) + addr := stripCertHashes(ln.Multiaddrs()[0]).Encapsulate(foobarHash) _, err := tr2.Dial(context.Background(), addr, serverID) require.Error(t, err) require.Contains(t, err.Error(), "CRYPTO_ERROR (0x12a): cert hash not found") }) t.Run("fails when adding a wrong hash", func(t *testing.T) { - _, err := tr2.Dial(context.Background(), ln.Multiaddr().Encapsulate(foobarHash), serverID) + _, err := tr2.Dial(context.Background(), ln.Multiaddrs()[0].Encapsulate(foobarHash), serverID) require.Error(t, err) }) @@ -248,9 +248,9 @@ func TestListenerAddrs(t *testing.T) { require.NoError(t, err) ln2, err := tr.Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/quic/webtransport")) require.NoError(t, err) - hashes1 := extractCertHashes(ln1.Multiaddr()) + hashes1 := extractCertHashes(ln1.Multiaddrs()[0]) require.Len(t, hashes1, 2) - hashes2 := extractCertHashes(ln2.Multiaddr()) + hashes2 := extractCertHashes(ln2.Multiaddrs()[0]) require.Equal(t, hashes1, hashes2) } @@ -304,7 +304,7 @@ func TestResourceManagerListening(t *testing.T) { return nil, errors.New("denied") }) - _, err = cl.Dial(context.Background(), ln.Multiaddr(), serverID) + _, err = cl.Dial(context.Background(), ln.Multiaddrs()[0], serverID) require.EqualError(t, err, "received status 503") }) @@ -326,7 +326,7 @@ func TestResourceManagerListening(t *testing.T) { scope.EXPECT().Done().Do(func() { close(serverDone) }) // The handshake will complete, but the server will immediately close the connection. - conn, err := cl.Dial(context.Background(), ln.Multiaddr(), serverID) + conn, err := cl.Dial(context.Background(), ln.Multiaddrs()[0], serverID) require.NoError(t, err) defer conn.Close() clientDone := make(chan struct{}) @@ -365,13 +365,13 @@ func TestConnectionGaterDialing(t *testing.T) { defer ln.Close() connGater.EXPECT().InterceptSecured(network.DirOutbound, serverID, gomock.Any()).Do(func(_ network.Direction, _ peer.ID, addrs network.ConnMultiaddrs) { - require.Equal(t, stripCertHashes(ln.Multiaddr()), addrs.RemoteMultiaddr()) + require.Equal(t, stripCertHashes(ln.Multiaddrs()[0]), addrs.RemoteMultiaddr()) }) _, key := newIdentity(t) cl, err := libp2pwebtransport.New(key, connGater, &network.NullResourceManager{}) require.NoError(t, err) defer cl.(io.Closer).Close() - _, err = cl.Dial(context.Background(), ln.Multiaddr(), serverID) + _, err = cl.Dial(context.Background(), ln.Multiaddrs()[0], serverID) require.EqualError(t, err, "secured connection gated") } @@ -389,15 +389,15 @@ func TestConnectionGaterInterceptAccept(t *testing.T) { defer ln.Close() connGater.EXPECT().InterceptAccept(gomock.Any()).Do(func(addrs network.ConnMultiaddrs) { - require.Equal(t, stripCertHashes(ln.Multiaddr()), addrs.LocalMultiaddr()) - require.NotEqual(t, stripCertHashes(ln.Multiaddr()), addrs.RemoteMultiaddr()) + require.Equal(t, stripCertHashes(ln.Multiaddrs()[0]), addrs.LocalMultiaddr()) + require.NotEqual(t, stripCertHashes(ln.Multiaddrs()[0]), addrs.RemoteMultiaddr()) }) _, key := newIdentity(t) cl, err := libp2pwebtransport.New(key, nil, &network.NullResourceManager{}) require.NoError(t, err) defer cl.(io.Closer).Close() - _, err = cl.Dial(context.Background(), ln.Multiaddr(), serverID) + _, err = cl.Dial(context.Background(), ln.Multiaddrs()[0], serverID) require.EqualError(t, err, "received status 403") } @@ -421,11 +421,11 @@ func TestConnectionGaterInterceptSecured(t *testing.T) { connGater.EXPECT().InterceptAccept(gomock.Any()).Return(true) connGater.EXPECT().InterceptSecured(network.DirInbound, clientID, gomock.Any()).Do(func(_ network.Direction, _ peer.ID, addrs network.ConnMultiaddrs) { - require.Equal(t, stripCertHashes(ln.Multiaddr()), addrs.LocalMultiaddr()) - require.NotEqual(t, stripCertHashes(ln.Multiaddr()), addrs.RemoteMultiaddr()) + require.Equal(t, stripCertHashes(ln.Multiaddrs()[0]), addrs.LocalMultiaddr()) + require.NotEqual(t, stripCertHashes(ln.Multiaddrs()[0]), addrs.RemoteMultiaddr()) }) // The handshake will complete, but the server will immediately close the connection. - conn, err := cl.Dial(context.Background(), ln.Multiaddr(), serverID) + conn, err := cl.Dial(context.Background(), ln.Multiaddrs()[0], serverID) require.NoError(t, err) defer conn.Close() done := make(chan struct{}) @@ -479,7 +479,7 @@ func TestStaticTLSConf(t *testing.T) { ln, err := tr.Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/quic/webtransport")) require.NoError(t, err) defer ln.Close() - require.Empty(t, extractCertHashes(ln.Multiaddr()), "listener address shouldn't contain any certhash") + require.Empty(t, extractCertHashes(ln.Multiaddrs()[0]), "listener address shouldn't contain any certhash") t.Run("fails when the certificate is invalid", func(t *testing.T) { _, key := newIdentity(t) @@ -487,7 +487,7 @@ func TestStaticTLSConf(t *testing.T) { require.NoError(t, err) defer cl.(io.Closer).Close() - _, err = cl.Dial(context.Background(), ln.Multiaddr(), serverID) + _, err = cl.Dial(context.Background(), ln.Multiaddrs()[0], serverID) require.Error(t, err) if !strings.Contains(err.Error(), "certificate is not trusted") && !strings.Contains(err.Error(), "certificate signed by unknown authority") { @@ -501,7 +501,7 @@ func TestStaticTLSConf(t *testing.T) { require.NoError(t, err) defer cl.(io.Closer).Close() - addr := ln.Multiaddr().Encapsulate(getCerthashComponent(t, []byte("foo"))) + addr := ln.Multiaddrs()[0].Encapsulate(getCerthashComponent(t, []byte("foo"))) _, err = cl.Dial(context.Background(), addr, serverID) require.Error(t, err) require.Contains(t, err.Error(), "cert hash not found") @@ -516,8 +516,8 @@ func TestStaticTLSConf(t *testing.T) { require.NoError(t, err) defer cl.(io.Closer).Close() - require.True(t, cl.CanDial(ln.Multiaddr())) - conn, err := cl.Dial(context.Background(), ln.Multiaddr(), serverID) + require.True(t, cl.CanDial(ln.Multiaddrs()[0])) + conn, err := cl.Dial(context.Background(), ln.Multiaddrs()[0], serverID) require.NoError(t, err) defer conn.Close() }) @@ -538,7 +538,7 @@ func TestAcceptQueueFilledUp(t *testing.T) { cl, err := libp2pwebtransport.New(key, nil, &network.NullResourceManager{}) require.NoError(t, err) defer cl.(io.Closer).Close() - return cl.Dial(context.Background(), ln.Multiaddr(), serverID) + return cl.Dial(context.Background(), ln.Multiaddrs()[0], serverID) } for i := 0; i < 16; i++ { @@ -577,7 +577,7 @@ func TestSNIIsSent(t *testing.T) { require.NoError(t, err) defer tr.(io.Closer).Close() - beforeQuicMa, withQuicMa := ma.SplitFunc(ln1.Multiaddr(), func(c ma.Component) bool { + beforeQuicMa, withQuicMa := ma.SplitFunc(ln1.Multiaddrs()[0], func(c ma.Component) bool { return c.Protocol().Code == ma.P_QUIC }) @@ -663,7 +663,7 @@ func TestFlowControlWindowIncrease(t *testing.T) { defer tr2.(io.Closer).Close() var addr ma.Multiaddr - for _, comp := range ma.Split(ln.Multiaddr()) { + for _, comp := range ma.Split(ln.Multiaddrs()[0]) { if _, err := comp.ValueForProtocol(ma.P_UDP); err == nil { addr = addr.Encapsulate(ma.StringCast(fmt.Sprintf("/udp/%d", proxy.LocalPort()))) continue From 4d7b72acac3ecc057d824109ad96857b89eac49e Mon Sep 17 00:00:00 2001 From: Marco Munizaga Date: Tue, 15 Nov 2022 12:50:00 -0800 Subject: [PATCH 02/19] Support both QUIC versions in QUIC transport --- p2p/transport/quic/conn_test.go | 127 +++++++++++++++++++--- p2p/transport/quic/listener.go | 60 ++++++---- p2p/transport/quic/listener_test.go | 12 +- p2p/transport/quic/options.go | 8 ++ p2p/transport/quic/quic_multiaddr.go | 58 +++++++--- p2p/transport/quic/quic_multiaddr_test.go | 27 ++++- p2p/transport/quic/transport.go | 51 +++++++-- 7 files changed, 277 insertions(+), 66 deletions(-) diff --git a/p2p/transport/quic/conn_test.go b/p2p/transport/quic/conn_test.go index 6bca67da4a..42d56293e3 100644 --- a/p2p/transport/quic/conn_test.go +++ b/p2p/transport/quic/conn_test.go @@ -20,6 +20,7 @@ import ( tpt "github.com/libp2p/go-libp2p/core/transport" "github.com/golang/mock/gomock" + "github.com/lucas-clemente/quic-go" quicproxy "github.com/lucas-clemente/quic-go/integrationtests/tools/proxy" ma "github.com/multiformats/go-multiaddr" "github.com/stretchr/testify/require" @@ -33,8 +34,8 @@ type connTestCase struct { } var connTestCases = []*connTestCase{ - {"reuseport_on", []Option{}}, - {"reuseport_off", []Option{DisableReuseport()}}, + {"reuseport_on", []Option{DisableDraft29()}}, + {"reuseport_off", []Option{DisableReuseport(), DisableDraft29()}}, } func createPeer(t *testing.T) (peer.ID, ic.PrivKey) { @@ -103,13 +104,13 @@ func testHandshake(t *testing.T, tc *connTestCase) { } t.Run("on IPv4", func(t *testing.T) { - ln := runServer(t, serverTransport, "/ip4/127.0.0.1/udp/0/quic") + ln := runServer(t, serverTransport, "/ip4/127.0.0.1/udp/0/quic-v1") defer ln.Close() handshake(t, ln) }) t.Run("on IPv6", func(t *testing.T) { - ln := runServer(t, serverTransport, "/ip6/::1/udp/0/quic") + ln := runServer(t, serverTransport, "/ip6/::1/udp/0/quic-v1") defer ln.Close() handshake(t, ln) }) @@ -134,7 +135,7 @@ func testResourceManagerSuccess(t *testing.T, tc *connTestCase) { serverTransport, err := NewTransport(serverKey, nil, nil, serverRcmgr, tc.Options...) require.NoError(t, err) defer serverTransport.(io.Closer).Close() - ln, err := serverTransport.Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/quic")) + ln, err := serverTransport.Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/quic-v1")) require.NoError(t, err) defer ln.Close() @@ -185,7 +186,7 @@ func testResourceManagerDialDenied(t *testing.T, tc *connTestCase) { defer clientTransport.(io.Closer).Close() connScope := mocknetwork.NewMockConnManagementScope(ctrl) - target := ma.StringCast("/ip4/127.0.0.1/udp/1234/quic") + target := ma.StringCast("/ip4/127.0.0.1/udp/1234/quic-v1") rcmgr.EXPECT().OpenConnection(network.DirOutbound, false, target).Return(connScope, nil) rerr := errors.New("nope") @@ -228,7 +229,7 @@ func testResourceManagerAcceptDenied(t *testing.T, tc *connTestCase) { serverTransport, err := NewTransport(serverKey, nil, nil, serverRcmgr, tc.Options...) require.NoError(t, err) defer serverTransport.(io.Closer).Close() - ln, err := serverTransport.Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/quic")) + ln, err := serverTransport.Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/quic-v1")) require.NoError(t, err) defer ln.Close() connChan := make(chan tpt.CapableConn) @@ -271,7 +272,7 @@ func testStreams(t *testing.T, tc *connTestCase) { serverTransport, err := NewTransport(serverKey, nil, nil, nil, tc.Options...) require.NoError(t, err) defer serverTransport.(io.Closer).Close() - ln := runServer(t, serverTransport, "/ip4/127.0.0.1/udp/0/quic") + ln := runServer(t, serverTransport, "/ip4/127.0.0.1/udp/0/quic-v1") defer ln.Close() clientTransport, err := NewTransport(clientKey, nil, nil, nil, tc.Options...) @@ -312,7 +313,7 @@ func testHandshakeFailPeerIDMismatch(t *testing.T, tc *connTestCase) { serverTransport, err := NewTransport(serverKey, nil, nil, nil, tc.Options...) require.NoError(t, err) defer serverTransport.(io.Closer).Close() - ln := runServer(t, serverTransport, "/ip4/127.0.0.1/udp/0/quic") + ln := runServer(t, serverTransport, "/ip4/127.0.0.1/udp/0/quic-v1") clientTransport, err := NewTransport(clientKey, nil, nil, nil, tc.Options...) require.NoError(t, err) @@ -358,7 +359,7 @@ func testConnectionGating(t *testing.T, tc *connTestCase) { serverTransport, err := NewTransport(serverKey, nil, cg, nil, tc.Options...) defer serverTransport.(io.Closer).Close() require.NoError(t, err) - ln := runServer(t, serverTransport, "/ip4/127.0.0.1/udp/0/quic") + ln := runServer(t, serverTransport, "/ip4/127.0.0.1/udp/0/quic-v1") defer ln.Close() cg.EXPECT().InterceptAccept(gomock.Any()) @@ -403,7 +404,7 @@ func testConnectionGating(t *testing.T, tc *connTestCase) { serverTransport, err := NewTransport(serverKey, nil, nil, nil, tc.Options...) require.NoError(t, err) defer serverTransport.(io.Closer).Close() - ln := runServer(t, serverTransport, "/ip4/127.0.0.1/udp/0/quic") + ln := runServer(t, serverTransport, "/ip4/127.0.0.1/udp/0/quic-v1") defer ln.Close() cg := NewMockConnectionGater(mockCtrl) @@ -443,12 +444,12 @@ func testDialTwo(t *testing.T, tc *connTestCase) { serverTransport, err := NewTransport(serverKey, nil, nil, nil, tc.Options...) require.NoError(t, err) defer serverTransport.(io.Closer).Close() - ln1 := runServer(t, serverTransport, "/ip4/127.0.0.1/udp/0/quic") + ln1 := runServer(t, serverTransport, "/ip4/127.0.0.1/udp/0/quic-v1") defer ln1.Close() serverTransport2, err := NewTransport(serverKey2, nil, nil, nil, tc.Options...) require.NoError(t, err) defer serverTransport2.(io.Closer).Close() - ln2 := runServer(t, serverTransport2, "/ip4/127.0.0.1/udp/0/quic") + ln2 := runServer(t, serverTransport2, "/ip4/127.0.0.1/udp/0/quic-v1") defer ln2.Close() data := bytes.Repeat([]byte{'a'}, 5*1<<20) // 5 MB @@ -533,7 +534,7 @@ func testStatelessReset(t *testing.T, tc *connTestCase) { serverTransport, err := NewTransport(serverKey, nil, nil, nil, tc.Options...) require.NoError(t, err) defer serverTransport.(io.Closer).Close() - ln := runServer(t, serverTransport, "/ip4/127.0.0.1/udp/0/quic") + ln := runServer(t, serverTransport, "/ip4/127.0.0.1/udp/0/quic-v1") var drop uint32 serverPort := ln.Addr().(*net.UDPAddr).Port @@ -550,7 +551,7 @@ func testStatelessReset(t *testing.T, tc *connTestCase) { clientTransport, err := NewTransport(clientKey, nil, nil, nil, tc.Options...) require.NoError(t, err) defer clientTransport.(io.Closer).Close() - proxyAddr, err := toQuicMultiaddr(proxy.LocalAddr()) + proxyAddr, err := toQuicMultiaddr(proxy.LocalAddr(), quic.Version1) require.NoError(t, err) conn, err := clientTransport.Dial(context.Background(), proxyAddr, serverID) require.NoError(t, err) @@ -560,6 +561,8 @@ func testStatelessReset(t *testing.T, tc *connTestCase) { require.NoError(t, err) str, err := conn.OpenStream(context.Background()) require.NoError(t, err) + _, err = conn.LocalMultiaddr().ValueForProtocol(ma.P_QUIC_V1) + require.NoError(t, err) str.Write([]byte("foobar")) connChan <- conn }() @@ -579,7 +582,7 @@ func testStatelessReset(t *testing.T, tc *connTestCase) { // Retry starting the listener until we're successful (with a 3s timeout). require.Eventually(t, func() bool { var err error - ln, err = serverTransport.Listen(ma.StringCast(fmt.Sprintf("/ip4/127.0.0.1/udp/%d/quic", serverPort))) + ln, err = serverTransport.Listen(ma.StringCast(fmt.Sprintf("/ip4/127.0.0.1/udp/%d/quic-v1", serverPort))) return err == nil }, 3*time.Second, 50*time.Millisecond) defer ln.Close() @@ -606,7 +609,7 @@ func TestHolePunching(t *testing.T) { t1, err := NewTransport(serverKey, nil, nil, nil) require.NoError(t, err) defer t1.(io.Closer).Close() - laddr, err := ma.NewMultiaddr("/ip4/127.0.0.1/udp/0/quic") + laddr, err := ma.NewMultiaddr("/ip4/127.0.0.1/udp/0/quic-v1") require.NoError(t, err) ln1, err := t1.Listen(laddr) require.NoError(t, err) @@ -672,3 +675,93 @@ func TestHolePunching(t *testing.T) { <-done1 <-done2 } + +func TestGetErrorWhenListeningWithDraft29WhenDisabled(t *testing.T) { + _, serverKey := createPeer(t) + + t1, err := NewTransport(serverKey, nil, nil, nil, DisableDraft29()) + require.NoError(t, err) + defer t1.(io.Closer).Close() + laddr, err := ma.NewMultiaddr("/ip4/127.0.0.1/udp/0/quic") + require.NoError(t, err) + _, err = t1.Listen(laddr) + require.Error(t, err) +} + +func TestClientCanDialDifferentQUICVersions(t *testing.T) { + type testCase struct { + name string + serverDisablesDraft29 bool + } + + testCases := []testCase{ + { + name: "Client dials quic-v1 on a quic-v1 only server", + serverDisablesDraft29: true, + }, + { + name: "Client dials both draft 29 and v1 on server that supports both", + serverDisablesDraft29: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + serverID, serverKey := createPeer(t) + _, clientKey := createPeer(t) + + var serverOpts []Option + if tc.serverDisablesDraft29 { + serverOpts = append(serverOpts, DisableDraft29()) + } + + t1, err := NewTransport(serverKey, nil, nil, nil, serverOpts...) + require.NoError(t, err) + defer t1.(io.Closer).Close() + laddr, err := ma.NewMultiaddr("/ip4/127.0.0.1/udp/0/quic-v1") + require.NoError(t, err) + ln1, err := t1.Listen(laddr) + require.NoError(t, err) + + t2, err := NewTransport(clientKey, nil, nil, nil) + require.NoError(t, err) + defer t2.(io.Closer).Close() + + ctx := context.Background() + + for _, a := range ln1.Multiaddrs() { + _, v, err := fromQuicMultiaddr(a) + require.NoError(t, err) + + done := make(chan struct{}) + go func() { + defer close(done) + conn, err := ln1.Accept() + require.NoError(t, err) + defer conn.Close() + + _, versionConnLocal, err := fromQuicMultiaddr(conn.LocalMultiaddr()) + require.NoError(t, err) + _, versionConnRemote, err := fromQuicMultiaddr(conn.RemoteMultiaddr()) + require.NoError(t, err) + + require.Equal(t, v, versionConnLocal) + require.Equal(t, v, versionConnRemote) + }() + + conn, err := t2.Dial(ctx, a, serverID) + require.NoError(t, err) + _, versionConnLocal, err := fromQuicMultiaddr(conn.LocalMultiaddr()) + require.NoError(t, err) + _, versionConnRemote, err := fromQuicMultiaddr(conn.RemoteMultiaddr()) + require.NoError(t, err) + + require.Equal(t, v, versionConnLocal) + require.Equal(t, v, versionConnRemote) + + <-done + conn.Close() + } + }) + } +} diff --git a/p2p/transport/quic/listener.go b/p2p/transport/quic/listener.go index 6cad6fa58b..d5d9e7712a 100644 --- a/p2p/transport/quic/listener.go +++ b/p2p/transport/quic/listener.go @@ -3,6 +3,7 @@ package libp2pquic import ( "context" "crypto/tls" + "errors" "net" ic "github.com/libp2p/go-libp2p/core/crypto" @@ -19,18 +20,18 @@ var quicListen = quic.Listen // so we can mock it in tests // A listener listens for QUIC connections. type listener struct { - quicListener quic.Listener - conn pConn - transport *transport - rcmgr network.ResourceManager - privKey ic.PrivKey - localPeer peer.ID - localMultiaddr ma.Multiaddr + quicListener quic.Listener + conn pConn + transport *transport + rcmgr network.ResourceManager + privKey ic.PrivKey + localPeer peer.ID + localMultiaddrs map[quic.VersionNumber]ma.Multiaddr } var _ tpt.Listener = &listener{} -func newListener(pconn pConn, t *transport, localPeer peer.ID, key ic.PrivKey, identity *p2ptls.Identity, rcmgr network.ResourceManager) (tpt.Listener, error) { +func newListener(pconn pConn, t *transport, localPeer peer.ID, key ic.PrivKey, identity *p2ptls.Identity, rcmgr network.ResourceManager, enableDraft29 bool) (tpt.Listener, error) { var tlsConf tls.Config tlsConf.GetConfigForClient = func(_ *tls.ClientHelloInfo) (*tls.Config, error) { // return a tls.Config that verifies the peer's certificate chain. @@ -44,18 +45,30 @@ func newListener(pconn pConn, t *transport, localPeer peer.ID, key ic.PrivKey, i if err != nil { return nil, err } - localMultiaddr, err := toQuicMultiaddr(ln.Addr()) + localMultiaddr, err := toQuicMultiaddr(ln.Addr(), quic.Version1) if err != nil { return nil, err } + + localMultiaddrs := map[quic.VersionNumber]ma.Multiaddr{} + localMultiaddrs[quic.Version1] = localMultiaddr + + if enableDraft29 { + localMultiaddr, err := toQuicMultiaddr(ln.Addr(), quic.VersionDraft29) + if err != nil { + return nil, err + } + localMultiaddrs[quic.VersionDraft29] = localMultiaddr + } + return &listener{ - conn: pconn, - quicListener: ln, - transport: t, - rcmgr: rcmgr, - privKey: key, - localPeer: localPeer, - localMultiaddr: localMultiaddr, + conn: pconn, + quicListener: ln, + transport: t, + rcmgr: rcmgr, + privKey: key, + localPeer: localPeer, + localMultiaddrs: localMultiaddrs, }, nil } @@ -97,7 +110,7 @@ func (l *listener) Accept() (tpt.CapableConn, error) { } func (l *listener) setupConn(qconn quic.Connection) (*conn, error) { - remoteMultiaddr, err := toQuicMultiaddr(qconn.RemoteAddr()) + remoteMultiaddr, err := toQuicMultiaddr(qconn.RemoteAddr(), qconn.ConnectionState().Version) if err != nil { return nil, err } @@ -127,6 +140,11 @@ func (l *listener) setupConn(qconn quic.Connection) (*conn, error) { return nil, err } + localMultiaddr, found := l.localMultiaddrs[qconn.ConnectionState().Version] + if !found { + return nil, errors.New("unknown quic version:" + qconn.ConnectionState().Version.String()) + } + l.conn.IncreaseCount() return &conn{ quicConn: qconn, @@ -134,7 +152,7 @@ func (l *listener) setupConn(qconn quic.Connection) (*conn, error) { transport: l.transport, scope: connScope, localPeer: l.localPeer, - localMultiaddr: l.localMultiaddr, + localMultiaddr: localMultiaddr, privKey: l.privKey, remoteMultiaddr: remoteMultiaddr, remotePeerID: remotePeerID, @@ -165,5 +183,9 @@ func (l *listener) Addr() net.Addr { // Multiaddr returns the multiaddress of this listener. func (l *listener) Multiaddrs() []ma.Multiaddr { - return []ma.Multiaddr{l.localMultiaddr} + mas := make([]ma.Multiaddr, 0, len(l.localMultiaddrs)) + for _, a := range l.localMultiaddrs { + mas = append(mas, a) + } + return mas } diff --git a/p2p/transport/quic/listener_test.go b/p2p/transport/quic/listener_test.go index 7f5fe8458e..ac7b2af638 100644 --- a/p2p/transport/quic/listener_test.go +++ b/p2p/transport/quic/listener_test.go @@ -66,7 +66,11 @@ func TestListenAddr(t *testing.T) { defer ln.Close() port := ln.Addr().(*net.UDPAddr).Port require.NotZero(t, port) - require.Equal(t, ln.Multiaddrs()[0].String(), fmt.Sprintf("/ip4/127.0.0.1/udp/%d/quic", port)) + var multiaddrsStrings []string + for _, a := range ln.Multiaddrs() { + multiaddrsStrings = append(multiaddrsStrings, a.String()) + } + require.Contains(t, multiaddrsStrings, fmt.Sprintf("/ip4/127.0.0.1/udp/%d/quic", port)) }) t.Run("for IPv6", func(t *testing.T) { @@ -76,7 +80,11 @@ func TestListenAddr(t *testing.T) { defer ln.Close() port := ln.Addr().(*net.UDPAddr).Port require.NotZero(t, port) - require.Equal(t, ln.Multiaddrs()[0].String(), fmt.Sprintf("/ip6/::/udp/%d/quic", port)) + var multiaddrsStrings []string + for _, a := range ln.Multiaddrs() { + multiaddrsStrings = append(multiaddrsStrings, a.String()) + } + require.Contains(t, multiaddrsStrings, fmt.Sprintf("/ip6/::/udp/%d/quic", port)) }) } diff --git a/p2p/transport/quic/options.go b/p2p/transport/quic/options.go index 75b2dbca09..857480778d 100644 --- a/p2p/transport/quic/options.go +++ b/p2p/transport/quic/options.go @@ -4,6 +4,7 @@ type Option func(opts *config) error type config struct { disableReuseport bool + disableDraft29 bool metrics bool } @@ -24,6 +25,13 @@ func DisableReuseport() Option { } } +func DisableDraft29() Option { + return func(cfg *config) error { + cfg.disableDraft29 = true + return nil + } +} + // WithMetrics enables Prometheus metrics collection. func WithMetrics() Option { return func(cfg *config) error { diff --git a/p2p/transport/quic/quic_multiaddr.go b/p2p/transport/quic/quic_multiaddr.go index 81b66af8aa..adf5a71b4f 100644 --- a/p2p/transport/quic/quic_multiaddr.go +++ b/p2p/transport/quic/quic_multiaddr.go @@ -1,30 +1,62 @@ package libp2pquic import ( + "errors" "net" + "github.com/lucas-clemente/quic-go" ma "github.com/multiformats/go-multiaddr" manet "github.com/multiformats/go-multiaddr/net" ) -var quicMA ma.Multiaddr +var quicV1MA ma.Multiaddr = ma.StringCast("/quic-v1") +var quicDraft29MA ma.Multiaddr = ma.StringCast("/quic") -func init() { - var err error - quicMA, err = ma.NewMultiaddr("/quic") - if err != nil { - panic(err) - } -} - -func toQuicMultiaddr(na net.Addr) (ma.Multiaddr, error) { +func toQuicMultiaddr(na net.Addr, version quic.VersionNumber) (ma.Multiaddr, error) { udpMA, err := manet.FromNetAddr(na) if err != nil { return nil, err } - return udpMA.Encapsulate(quicMA), nil + switch version { + case quic.VersionDraft29: + return udpMA.Encapsulate(quicDraft29MA), nil + case quic.Version1: + return udpMA.Encapsulate(quicV1MA), nil + default: + return nil, errors.New("unknown quic version") + } } -func fromQuicMultiaddr(addr ma.Multiaddr) (net.Addr, error) { - return manet.ToNetAddr(addr.Decapsulate(quicMA)) +func fromQuicMultiaddr(addr ma.Multiaddr) (net.Addr, quic.VersionNumber, error) { + var version quic.VersionNumber + var partsBeforeQuic ma.Multiaddr + ma.ForEach(addr, func(c ma.Component) bool { + switch c.Protocol().Code { + case ma.P_QUIC: + version = quic.VersionDraft29 + return false + case ma.P_QUIC_V1: + version = quic.Version1 + return false + default: + if partsBeforeQuic == nil { + partsBeforeQuic = &c + } else { + partsBeforeQuic = partsBeforeQuic.Encapsulate(&c) + } + return true + } + }) + if partsBeforeQuic == nil { + return nil, version, errors.New("no addr before quic component") + } + if version == 0 { + // Not found + return nil, version, errors.New("unknown quic version") + } + netAddr, err := manet.ToNetAddr(partsBeforeQuic) + if err != nil { + return nil, version, err + } + return netAddr, version, err } diff --git a/p2p/transport/quic/quic_multiaddr_test.go b/p2p/transport/quic/quic_multiaddr_test.go index db7cdb34cd..34d971f812 100644 --- a/p2p/transport/quic/quic_multiaddr_test.go +++ b/p2p/transport/quic/quic_multiaddr_test.go @@ -4,24 +4,45 @@ import ( "net" "testing" + "github.com/lucas-clemente/quic-go" ma "github.com/multiformats/go-multiaddr" "github.com/stretchr/testify/require" ) func TestConvertToQuicMultiaddr(t *testing.T) { addr := &net.UDPAddr{IP: net.IPv4(192, 168, 0, 42), Port: 1337} - maddr, err := toQuicMultiaddr(addr) + maddr, err := toQuicMultiaddr(addr, quic.VersionDraft29) require.NoError(t, err) require.Equal(t, maddr.String(), "/ip4/192.168.0.42/udp/1337/quic") } -func TestConvertFromQuicMultiaddr(t *testing.T) { +func TestConvertToQuicV1Multiaddr(t *testing.T) { + addr := &net.UDPAddr{IP: net.IPv4(192, 168, 0, 42), Port: 1337} + maddr, err := toQuicMultiaddr(addr, quic.Version1) + require.NoError(t, err) + require.Equal(t, maddr.String(), "/ip4/192.168.0.42/udp/1337/quic-v1") +} + +func TestConvertFromQuicDraft29Multiaddr(t *testing.T) { maddr, err := ma.NewMultiaddr("/ip4/192.168.0.42/udp/1337/quic") require.NoError(t, err) - addr, err := fromQuicMultiaddr(maddr) + addr, v, err := fromQuicMultiaddr(maddr) + require.NoError(t, err) + udpAddr, ok := addr.(*net.UDPAddr) + require.True(t, ok) + require.Equal(t, udpAddr.IP, net.IPv4(192, 168, 0, 42)) + require.Equal(t, udpAddr.Port, 1337) + require.Equal(t, v, quic.VersionDraft29) +} + +func TestConvertFromQuicV1Multiaddr(t *testing.T) { + maddr, err := ma.NewMultiaddr("/ip4/192.168.0.42/udp/1337/quic-v1") + require.NoError(t, err) + addr, v, err := fromQuicMultiaddr(maddr) require.NoError(t, err) udpAddr, ok := addr.(*net.UDPAddr) require.True(t, ok) require.Equal(t, udpAddr.IP, net.IPv4(192, 168, 0, 42)) require.Equal(t, udpAddr.Port, 1337) + require.Equal(t, v, quic.Version1) } diff --git a/p2p/transport/quic/transport.go b/p2p/transport/quic/transport.go index 29a22c20f2..08d313fb9f 100644 --- a/p2p/transport/quic/transport.go +++ b/p2p/transport/quic/transport.go @@ -149,6 +149,8 @@ type transport struct { holePunchingMx sync.Mutex holePunching map[holePunchKey]*activeHolePunch + enableDraft29 bool + connMx sync.Mutex conns map[quic.Connection]*conn } @@ -192,6 +194,10 @@ func NewTransport(key ic.PrivKey, psk pnet.PSK, gater connmgr.ConnectionGater, r rcmgr = &network.NullResourceManager{} } qconfig := quicConfig.Clone() + if cfg.disableDraft29 { + qconfig.Versions = []quic.VersionNumber{quic.Version1} + } + keyBytes, err := key.Raw() if err != nil { return nil, err @@ -214,14 +220,15 @@ func NewTransport(key ic.PrivKey, psk pnet.PSK, gater connmgr.ConnectionGater, r } tr := &transport{ - privKey: key, - localPeer: localPeer, - identity: identity, - connManager: connManager, - gater: gater, - rcmgr: rcmgr, - conns: make(map[quic.Connection]*conn), - holePunching: make(map[holePunchKey]*activeHolePunch), + privKey: key, + localPeer: localPeer, + identity: identity, + connManager: connManager, + gater: gater, + rcmgr: rcmgr, + conns: make(map[quic.Connection]*conn), + holePunching: make(map[holePunchKey]*activeHolePunch), + enableDraft29: !cfg.disableDraft29, } qconfig.AllowConnectionWindowIncrease = tr.allowWindowIncrease tr.serverConfig = qconfig @@ -231,6 +238,10 @@ func NewTransport(key ic.PrivKey, psk pnet.PSK, gater connmgr.ConnectionGater, r // Dial dials a new QUIC connection func (t *transport) Dial(ctx context.Context, raddr ma.Multiaddr, p peer.ID) (tpt.CapableConn, error) { + _, v, err := fromQuicMultiaddr(raddr) + if err != nil { + return nil, err + } netw, host, err := manet.DialArgs(raddr) if err != nil { return nil, err @@ -239,7 +250,7 @@ func (t *transport) Dial(ctx context.Context, raddr ma.Multiaddr, p peer.ID) (tp if err != nil { return nil, err } - remoteMultiaddr, err := toQuicMultiaddr(addr) + remoteMultiaddr, err := toQuicMultiaddr(addr, v) if err != nil { return nil, err } @@ -262,7 +273,15 @@ func (t *transport) Dial(ctx context.Context, raddr ma.Multiaddr, p peer.ID) (tp if err != nil { return nil, err } - qconn, err := quicDialContext(ctx, pconn, addr, host, tlsConf, t.clientConfig) + + clientConfig := t.clientConfig + if v == quic.Version1 { + // The endpoint has explicit support for version 1, so we'll only use that version. + clientConfig = t.clientConfig.Clone() + clientConfig.Versions = []quic.VersionNumber{quic.Version1} + } + + qconn, err := quicDialContext(ctx, pconn, addr, host, tlsConf, clientConfig) if err != nil { scope.Done() pconn.DecreaseCount() @@ -280,7 +299,7 @@ func (t *transport) Dial(ctx context.Context, raddr ma.Multiaddr, p peer.ID) (tp return nil, errors.New("p2p/transport/quic BUG: expected remote pub key to be set") } - localMultiaddr, err := toQuicMultiaddr(pconn.LocalAddr()) + localMultiaddr, err := toQuicMultiaddr(pconn.LocalAddr(), v) if err != nil { qconn.CloseWithError(0, "") return nil, err @@ -403,6 +422,14 @@ func (t *transport) CanDial(addr ma.Multiaddr) bool { // Listen listens for new QUIC connections on the passed multiaddr. func (t *transport) Listen(addr ma.Multiaddr) (tpt.Listener, error) { + _, v, err := fromQuicMultiaddr(addr) + if err != nil { + return nil, err + } + if v == quic.VersionDraft29 && !t.enableDraft29 { + return nil, errors.New("can't listen on `/quic` multiaddr (QUIC draft 29 version) when draft 29 support is disabled") + } + lnet, host, err := manet.DialArgs(addr) if err != nil { return nil, err @@ -415,7 +442,7 @@ func (t *transport) Listen(addr ma.Multiaddr) (tpt.Listener, error) { if err != nil { return nil, err } - ln, err := newListener(conn, t, t.localPeer, t.privKey, t.identity, t.rcmgr) + ln, err := newListener(conn, t, t.localPeer, t.privKey, t.identity, t.rcmgr, t.enableDraft29) if err != nil { if !t.connManager.reuseportEnable { conn.Close() From 6d029410dc068f2e0116b9ed84c1ed3e88068a9d Mon Sep 17 00:00:00 2001 From: Marco Munizaga Date: Tue, 15 Nov 2022 12:55:12 -0800 Subject: [PATCH 03/19] Support only QUIC v1 in webtransport --- p2p/transport/webtransport/multiaddr.go | 4 +- p2p/transport/webtransport/multiaddr_test.go | 18 ++++---- p2p/transport/webtransport/transport.go | 6 ++- p2p/transport/webtransport/transport_test.go | 46 ++++++++++---------- 4 files changed, 38 insertions(+), 36 deletions(-) diff --git a/p2p/transport/webtransport/multiaddr.go b/p2p/transport/webtransport/multiaddr.go index 7789a3d1de..b6b79336c8 100644 --- a/p2p/transport/webtransport/multiaddr.go +++ b/p2p/transport/webtransport/multiaddr.go @@ -13,9 +13,9 @@ import ( "github.com/multiformats/go-multihash" ) -var webtransportMA = ma.StringCast("/quic/webtransport") +var webtransportMA = ma.StringCast("/quic-v1/webtransport") -var webtransportMatcher = mafmt.And(mafmt.IP, mafmt.Base(ma.P_UDP), mafmt.Base(ma.P_QUIC), mafmt.Base(ma.P_WEBTRANSPORT)) +var webtransportMatcher = mafmt.And(mafmt.IP, mafmt.Base(ma.P_UDP), mafmt.Base(ma.P_QUIC_V1), mafmt.Base(ma.P_WEBTRANSPORT)) func toWebtransportMultiaddr(na net.Addr) (ma.Multiaddr, error) { addr, err := manet.FromNetAddr(na) diff --git a/p2p/transport/webtransport/multiaddr_test.go b/p2p/transport/webtransport/multiaddr_test.go index 08be5fffde..ae3ebc4a3e 100644 --- a/p2p/transport/webtransport/multiaddr_test.go +++ b/p2p/transport/webtransport/multiaddr_test.go @@ -16,7 +16,7 @@ func TestWebtransportMultiaddr(t *testing.T) { t.Run("valid", func(t *testing.T) { addr, err := toWebtransportMultiaddr(&net.UDPAddr{IP: net.IPv4(127, 0, 0, 1), Port: 1337}) require.NoError(t, err) - require.Equal(t, "/ip4/127.0.0.1/udp/1337/quic/webtransport", addr.String()) + require.Equal(t, "/ip4/127.0.0.1/udp/1337/quic-v1/webtransport", addr.String()) }) t.Run("invalid", func(t *testing.T) { @@ -29,7 +29,7 @@ func TestWebtransportMultiaddrFromString(t *testing.T) { t.Run("valid", func(t *testing.T) { addr, err := stringToWebtransportMultiaddr("1.2.3.4:60042") require.NoError(t, err) - require.Equal(t, "/ip4/1.2.3.4/udp/60042/quic/webtransport", addr.String()) + require.Equal(t, "/ip4/1.2.3.4/udp/60042/quic-v1/webtransport", addr.String()) }) t.Run("invalid", func(t *testing.T) { @@ -63,9 +63,9 @@ func TestExtractCertHashes(t *testing.T) { addr string hashes []string }{ - {addr: "/ip4/127.0.0.1/udp/1234/quic/webtransport"}, - {addr: fmt.Sprintf("/ip4/127.0.0.1/udp/1234/quic/webtransport/certhash/%s", fooHash), hashes: []string{"foo"}}, - {addr: fmt.Sprintf("/ip4/127.0.0.1/udp/1234/quic/webtransport/certhash/%s/certhash/%s", fooHash, barHash), hashes: []string{"foo", "bar"}}, + {addr: "/ip4/127.0.0.1/udp/1234/quic-v1/webtransport"}, + {addr: fmt.Sprintf("/ip4/127.0.0.1/udp/1234/quic-v1/webtransport/certhash/%s", fooHash), hashes: []string{"foo"}}, + {addr: fmt.Sprintf("/ip4/127.0.0.1/udp/1234/quic-v1/webtransport/certhash/%s/certhash/%s", fooHash, barHash), hashes: []string{"foo", "bar"}}, } { ch, err := extractCertHashes(ma.StringCast(tc.addr)) require.NoError(t, err) @@ -78,9 +78,9 @@ func TestExtractCertHashes(t *testing.T) { func TestWebtransportResolve(t *testing.T) { testCases := []string{ - "/dns4/example.com/udp/1337/quic/webtransport", - "/dnsaddr/example.com/udp/1337/quic/webtransport", - "/ip4/127.0.0.1/udp/1337/quic/sni/example.com/webtransport", + "/dns4/example.com/udp/1337/quic-v1/webtransport", + "/dnsaddr/example.com/udp/1337/quic-v1/webtransport", + "/ip4/127.0.0.1/udp/1337/quic-v1/sni/example.com/webtransport", } tpt := &transport{} @@ -97,7 +97,7 @@ func TestWebtransportResolve(t *testing.T) { } t.Run("No sni", func(t *testing.T) { - outMa, err := tpt.Resolve(ctx, ma.StringCast("/ip4/127.0.0.1/udp/1337/quic/webtransport")) + outMa, err := tpt.Resolve(ctx, ma.StringCast("/ip4/127.0.0.1/udp/1337/quic-v1/webtransport")) require.NoError(t, err) _, err = outMa[0].ValueForProtocol(ma.P_SNI) require.Error(t, err) diff --git a/p2p/transport/webtransport/transport.go b/p2p/transport/webtransport/transport.go index 8e6ac77dbc..bfb8adb3bf 100644 --- a/p2p/transport/webtransport/transport.go +++ b/p2p/transport/webtransport/transport.go @@ -104,7 +104,9 @@ func New(key ic.PrivKey, gater connmgr.ConnectionGater, rcmgr network.ResourceMa clock: clock.New(), conns: map[uint64]*conn{}, } - t.quicConfig = &quic.Config{AllowConnectionWindowIncrease: t.allowWindowIncrease} + t.quicConfig = &quic.Config{ + AllowConnectionWindowIncrease: t.allowWindowIncrease, + Versions: []quic.VersionNumber{quic.Version1}} for _, opt := range opts { if err := opt(t); err != nil { return nil, err @@ -373,7 +375,7 @@ func (t *transport) Resolve(ctx context.Context, maddr ma.Multiaddr) ([]ma.Multi } beforeQuicMA, afterIncludingQuicMA := ma.SplitFunc(maddr, func(c ma.Component) bool { - return c.Protocol().Code == ma.P_QUIC + return c.Protocol().Code == ma.P_QUIC_V1 }) quicComponent, afterQuicMA := ma.SplitFirst(afterIncludingQuicMA) sniComponent, err := ma.NewComponent(ma.ProtocolWithCode(ma.P_SNI).Name, sni) diff --git a/p2p/transport/webtransport/transport_test.go b/p2p/transport/webtransport/transport_test.go index 3e856b2569..60780b5eca 100644 --- a/p2p/transport/webtransport/transport_test.go +++ b/p2p/transport/webtransport/transport_test.go @@ -103,7 +103,7 @@ func TestTransport(t *testing.T) { tr, err := libp2pwebtransport.New(serverKey, nil, &network.NullResourceManager{}) require.NoError(t, err) defer tr.(io.Closer).Close() - ln, err := tr.Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/quic/webtransport")) + ln, err := tr.Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/quic-v1/webtransport")) require.NoError(t, err) defer ln.Close() @@ -127,7 +127,7 @@ func TestTransport(t *testing.T) { require.NoError(t, err) _, port, err := net.SplitHostPort(addr) require.NoError(t, err) - require.Equal(t, ma.StringCast(fmt.Sprintf("/ip4/127.0.0.1/udp/%s/quic/webtransport", port)), conn.RemoteMultiaddr()) + require.Equal(t, ma.StringCast(fmt.Sprintf("/ip4/127.0.0.1/udp/%s/quic-v1/webtransport", port)), conn.RemoteMultiaddr()) addrChan <- conn.RemoteMultiaddr() }() @@ -149,7 +149,7 @@ func TestHashVerification(t *testing.T) { tr, err := libp2pwebtransport.New(serverKey, nil, &network.NullResourceManager{}) require.NoError(t, err) defer tr.(io.Closer).Close() - ln, err := tr.Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/quic/webtransport")) + ln, err := tr.Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/quic-v1/webtransport")) require.NoError(t, err) done := make(chan struct{}) go func() { @@ -184,10 +184,10 @@ func TestHashVerification(t *testing.T) { func TestCanDial(t *testing.T) { valid := []ma.Multiaddr{ - ma.StringCast("/ip4/127.0.0.1/udp/1234/quic/webtransport/certhash/" + randomMultihash(t)), - ma.StringCast("/ip6/b16b:8255:efc6:9cd5:1a54:ee86:2d7a:c2e6/udp/1234/quic/webtransport/certhash/" + randomMultihash(t)), - ma.StringCast(fmt.Sprintf("/ip4/127.0.0.1/udp/1234/quic/webtransport/certhash/%s/certhash/%s/certhash/%s", randomMultihash(t), randomMultihash(t), randomMultihash(t))), - ma.StringCast("/ip4/127.0.0.1/udp/1234/quic/webtransport"), // no certificate hash + ma.StringCast("/ip4/127.0.0.1/udp/1234/quic-v1/webtransport/certhash/" + randomMultihash(t)), + ma.StringCast("/ip6/b16b:8255:efc6:9cd5:1a54:ee86:2d7a:c2e6/udp/1234/quic-v1/webtransport/certhash/" + randomMultihash(t)), + ma.StringCast(fmt.Sprintf("/ip4/127.0.0.1/udp/1234/quic-v1/webtransport/certhash/%s/certhash/%s/certhash/%s", randomMultihash(t), randomMultihash(t), randomMultihash(t))), + ma.StringCast("/ip4/127.0.0.1/udp/1234/quic-v1/webtransport"), // no certificate hash } invalid := []ma.Multiaddr{ @@ -211,15 +211,15 @@ func TestCanDial(t *testing.T) { func TestListenAddrValidity(t *testing.T) { valid := []ma.Multiaddr{ - ma.StringCast("/ip6/::/udp/0/quic/webtransport/"), - ma.StringCast("/ip4/127.0.0.1/udp/1234/quic/webtransport/"), + ma.StringCast("/ip6/::/udp/0/quic-v1/webtransport/"), + ma.StringCast("/ip4/127.0.0.1/udp/1234/quic-v1/webtransport/"), } invalid := []ma.Multiaddr{ ma.StringCast("/ip4/127.0.0.1/udp/1234"), // missing webtransport ma.StringCast("/ip4/127.0.0.1/udp/1234/webtransport"), // missing quic ma.StringCast("/ip4/127.0.0.1/tcp/1234/webtransport"), // WebTransport over TCP? Is this a joke? - ma.StringCast("/ip4/127.0.0.1/udp/1234/quic/webtransport/certhash/" + randomMultihash(t)), + ma.StringCast("/ip4/127.0.0.1/udp/1234/quic-v1/webtransport/certhash/" + randomMultihash(t)), } _, key := newIdentity(t) @@ -244,9 +244,9 @@ func TestListenerAddrs(t *testing.T) { require.NoError(t, err) defer tr.(io.Closer).Close() - ln1, err := tr.Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/quic/webtransport")) + ln1, err := tr.Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/quic-v1/webtransport")) require.NoError(t, err) - ln2, err := tr.Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/quic/webtransport")) + ln2, err := tr.Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/quic-v1/webtransport")) require.NoError(t, err) hashes1 := extractCertHashes(ln1.Multiaddrs()[0]) require.Len(t, hashes1, 2) @@ -259,7 +259,7 @@ func TestResourceManagerDialing(t *testing.T) { defer ctrl.Finish() rcmgr := mocknetwork.NewMockResourceManager(ctrl) - addr := ma.StringCast("/ip4/9.8.7.6/udp/1234/quic/webtransport") + addr := ma.StringCast("/ip4/9.8.7.6/udp/1234/quic-v1/webtransport") p := peer.ID("foobar") _, key := newIdentity(t) @@ -289,7 +289,7 @@ func TestResourceManagerListening(t *testing.T) { rcmgr := mocknetwork.NewMockResourceManager(ctrl) tr, err := libp2pwebtransport.New(key, nil, rcmgr) require.NoError(t, err) - ln, err := tr.Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/quic/webtransport")) + ln, err := tr.Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/quic-v1/webtransport")) require.NoError(t, err) defer ln.Close() @@ -315,7 +315,7 @@ func TestResourceManagerListening(t *testing.T) { rcmgr := mocknetwork.NewMockResourceManager(ctrl) tr, err := libp2pwebtransport.New(key, nil, rcmgr) require.NoError(t, err) - ln, err := tr.Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/quic/webtransport")) + ln, err := tr.Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/quic-v1/webtransport")) require.NoError(t, err) defer ln.Close() @@ -360,7 +360,7 @@ func TestConnectionGaterDialing(t *testing.T) { tr, err := libp2pwebtransport.New(serverKey, nil, &network.NullResourceManager{}) require.NoError(t, err) defer tr.(io.Closer).Close() - ln, err := tr.Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/quic/webtransport")) + ln, err := tr.Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/quic-v1/webtransport")) require.NoError(t, err) defer ln.Close() @@ -384,7 +384,7 @@ func TestConnectionGaterInterceptAccept(t *testing.T) { tr, err := libp2pwebtransport.New(serverKey, connGater, &network.NullResourceManager{}) require.NoError(t, err) defer tr.(io.Closer).Close() - ln, err := tr.Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/quic/webtransport")) + ln, err := tr.Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/quic-v1/webtransport")) require.NoError(t, err) defer ln.Close() @@ -410,7 +410,7 @@ func TestConnectionGaterInterceptSecured(t *testing.T) { tr, err := libp2pwebtransport.New(serverKey, connGater, &network.NullResourceManager{}) require.NoError(t, err) defer tr.(io.Closer).Close() - ln, err := tr.Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/quic/webtransport")) + ln, err := tr.Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/quic-v1/webtransport")) require.NoError(t, err) defer ln.Close() @@ -476,7 +476,7 @@ func TestStaticTLSConf(t *testing.T) { tr, err := libp2pwebtransport.New(serverKey, nil, &network.NullResourceManager{}, libp2pwebtransport.WithTLSConfig(tlsConf)) require.NoError(t, err) defer tr.(io.Closer).Close() - ln, err := tr.Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/quic/webtransport")) + ln, err := tr.Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/quic-v1/webtransport")) require.NoError(t, err) defer ln.Close() require.Empty(t, extractCertHashes(ln.Multiaddrs()[0]), "listener address shouldn't contain any certhash") @@ -528,7 +528,7 @@ func TestAcceptQueueFilledUp(t *testing.T) { tr, err := libp2pwebtransport.New(serverKey, nil, &network.NullResourceManager{}) require.NoError(t, err) defer tr.(io.Closer).Close() - ln, err := tr.Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/quic/webtransport")) + ln, err := tr.Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/quic-v1/webtransport")) require.NoError(t, err) defer ln.Close() @@ -569,7 +569,7 @@ func TestSNIIsSent(t *testing.T) { require.NoError(t, err) defer tr.(io.Closer).Close() - ln1, err := tr.Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/quic/webtransport")) + ln1, err := tr.Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/quic-v1/webtransport")) require.NoError(t, err) _, key2 := newIdentity(t) @@ -578,7 +578,7 @@ func TestSNIIsSent(t *testing.T) { defer tr.(io.Closer).Close() beforeQuicMa, withQuicMa := ma.SplitFunc(ln1.Multiaddrs()[0], func(c ma.Component) bool { - return c.Protocol().Code == ma.P_QUIC + return c.Protocol().Code == ma.P_QUIC_V1 }) quicComponent, restMa := ma.SplitLast(withQuicMa) @@ -634,7 +634,7 @@ func TestFlowControlWindowIncrease(t *testing.T) { tr, err := libp2pwebtransport.New(serverKey, nil, serverRcmgr) require.NoError(t, err) defer tr.(io.Closer).Close() - ln, err := tr.Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/quic/webtransport")) + ln, err := tr.Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/quic-v1/webtransport")) require.NoError(t, err) defer ln.Close() From 65f3f870012c9597d38bd84f4de942720d91c77c Mon Sep 17 00:00:00 2001 From: Marco Munizaga Date: Tue, 15 Nov 2022 14:01:29 -0800 Subject: [PATCH 04/19] Update dialMatcher --- p2p/transport/quic/transport.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/p2p/transport/quic/transport.go b/p2p/transport/quic/transport.go index 08d313fb9f..da49d71c9a 100644 --- a/p2p/transport/quic/transport.go +++ b/p2p/transport/quic/transport.go @@ -413,7 +413,7 @@ loop: } // Don't use mafmt.QUIC as we don't want to dial DNS addresses. Just /ip{4,6}/udp/quic -var dialMatcher = mafmt.And(mafmt.IP, mafmt.Base(ma.P_UDP), mafmt.Base(ma.P_QUIC)) +var dialMatcher = mafmt.And(mafmt.IP, mafmt.Base(ma.P_UDP), mafmt.Or(mafmt.Base(ma.P_QUIC), mafmt.Base(ma.P_QUIC_V1))) // CanDial determines if we can dial to an address func (t *transport) CanDial(addr ma.Multiaddr) bool { @@ -474,7 +474,10 @@ func (t *transport) Proxy() bool { // Protocols returns the set of protocols handled by this transport. func (t *transport) Protocols() []int { - return []int{ma.P_QUIC} + if t.enableDraft29 { + return []int{ma.P_QUIC, ma.P_QUIC_V1} + } + return []int{ma.P_QUIC_V1} } func (t *transport) String() string { From 07ea612b1530db22f2d54a319fb4e9f297de179e Mon Sep 17 00:00:00 2001 From: Marco Munizaga Date: Tue, 15 Nov 2022 15:47:49 -0800 Subject: [PATCH 05/19] Update tests --- p2p/net/swarm/swarm_addr_test.go | 2 +- p2p/net/swarm/swarm_dial_test.go | 2 +- p2p/net/swarm/swarm_test.go | 2 +- p2p/transport/quic/transport.go | 8 +++++++- p2p/transport/quic/transport_test.go | 9 ++++++--- 5 files changed, 16 insertions(+), 7 deletions(-) diff --git a/p2p/net/swarm/swarm_addr_test.go b/p2p/net/swarm/swarm_addr_test.go index 56a2740b35..4809c4c01e 100644 --- a/p2p/net/swarm/swarm_addr_test.go +++ b/p2p/net/swarm/swarm_addr_test.go @@ -98,7 +98,7 @@ func TestDialAddressSelection(t *testing.T) { require.Equal(t, tcpTr, s.TransportForDialing(ma.StringCast("/ip4/127.0.0.1/tcp/1234"))) require.Equal(t, quicTr, s.TransportForDialing(ma.StringCast("/ip4/127.0.0.1/udp/1234/quic"))) require.Equal(t, circuitTr, s.TransportForDialing(ma.StringCast(fmt.Sprintf("/ip4/127.0.0.1/udp/1234/quic/p2p-circuit/p2p/%s", id)))) - require.Equal(t, webtransportTr, s.TransportForDialing(ma.StringCast(fmt.Sprintf("/ip4/127.0.0.1/udp/1234/quic/webtransport/certhash/%s", certHash)))) + require.Equal(t, webtransportTr, s.TransportForDialing(ma.StringCast(fmt.Sprintf("/ip4/127.0.0.1/udp/1234/quic-v1/webtransport/certhash/%s", certHash)))) require.Nil(t, s.TransportForDialing(ma.StringCast("/ip4/1.2.3.4"))) require.Nil(t, s.TransportForDialing(ma.StringCast("/ip4/1.2.3.4/tcp/443/ws"))) } diff --git a/p2p/net/swarm/swarm_dial_test.go b/p2p/net/swarm/swarm_dial_test.go index e39a78dd7b..763be49bba 100644 --- a/p2p/net/swarm/swarm_dial_test.go +++ b/p2p/net/swarm/swarm_dial_test.go @@ -196,7 +196,7 @@ func TestAddrResolutionRecursive(t *testing.T) { func TestRemoveWebTransportAddrs(t *testing.T) { tcpAddr := ma.StringCast("/ip4/9.5.6.4/tcp/1234") quicAddr := ma.StringCast("/ip4/1.2.3.4/udp/443/quic") - webtransportAddr := ma.StringCast("/ip4/1.2.3.4/udp/443/quic/webtransport") + webtransportAddr := ma.StringCast("/ip4/1.2.3.4/udp/443/quic-v1/webtransport") require.Equal(t, []ma.Multiaddr{tcpAddr, quicAddr}, maybeRemoveWebTransportAddrs([]ma.Multiaddr{tcpAddr, quicAddr})) require.Equal(t, []ma.Multiaddr{tcpAddr, webtransportAddr}, maybeRemoveWebTransportAddrs([]ma.Multiaddr{tcpAddr, webtransportAddr})) diff --git a/p2p/net/swarm/swarm_test.go b/p2p/net/swarm/swarm_test.go index 95aa71d756..209914adb6 100644 --- a/p2p/net/swarm/swarm_test.go +++ b/p2p/net/swarm/swarm_test.go @@ -552,7 +552,7 @@ func TestListenCloseCount(t *testing.T) { t.Fatal(err) } listenedAddrs := s.ListenAddresses() - require.Equal(t, 2, len(listenedAddrs)) + require.Equal(t, 3, len(listenedAddrs)) s.ListenClose(listenedAddrs...) diff --git a/p2p/transport/quic/transport.go b/p2p/transport/quic/transport.go index da49d71c9a..7128d4ea77 100644 --- a/p2p/transport/quic/transport.go +++ b/p2p/transport/quic/transport.go @@ -153,6 +153,8 @@ type transport struct { connMx sync.Mutex conns map[quic.Connection]*conn + + closeOnce sync.Once } var _ tpt.Transport = &transport{} @@ -485,5 +487,9 @@ func (t *transport) String() string { } func (t *transport) Close() error { - return t.connManager.Close() + // We may register multiple protocols, but they are all the same transport, so only support getting closed once. + t.closeOnce.Do(func() { + _ = t.connManager.Close() + }) + return nil } diff --git a/p2p/transport/quic/transport_test.go b/p2p/transport/quic/transport_test.go index 26cabbd7c4..0508614a94 100644 --- a/p2p/transport/quic/transport_test.go +++ b/p2p/transport/quic/transport_test.go @@ -35,11 +35,14 @@ func TestQUICProtocol(t *testing.T) { defer tr.(io.Closer).Close() protocols := tr.Protocols() - if len(protocols) != 1 { - t.Fatalf("expected to only support a single protocol, got %v", protocols) + if len(protocols) > 2 { + t.Fatalf("expected at most two protocols, got %v", protocols) } if protocols[0] != ma.P_QUIC { - t.Fatalf("expected the supported protocol to be QUIC, got %d", protocols[0]) + t.Fatalf("expected the supported protocol to be draft 29 QUIC, got %d", protocols[0]) + } + if protocols[1] != ma.P_QUIC_V1 { + t.Fatalf("expected the supported protocol to be QUIC v1, got %d", protocols[0]) } } From a2db174a3cd9982f64ce6183fc7dbef98c37b803 Mon Sep 17 00:00:00 2001 From: Marco Munizaga Date: Tue, 15 Nov 2022 17:26:09 -0800 Subject: [PATCH 06/19] Only use draft 29 when dialing if the server is a draft 29 server --- p2p/transport/quic/transport.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/p2p/transport/quic/transport.go b/p2p/transport/quic/transport.go index 7128d4ea77..718e795191 100644 --- a/p2p/transport/quic/transport.go +++ b/p2p/transport/quic/transport.go @@ -281,6 +281,11 @@ func (t *transport) Dial(ctx context.Context, raddr ma.Multiaddr, p peer.ID) (tp // The endpoint has explicit support for version 1, so we'll only use that version. clientConfig = t.clientConfig.Clone() clientConfig.Versions = []quic.VersionNumber{quic.Version1} + } else if v == quic.VersionDraft29 { + clientConfig = t.clientConfig.Clone() + clientConfig.Versions = []quic.VersionNumber{quic.VersionDraft29} + } else { + return nil, errors.New("unknown QUIC version") } qconn, err := quicDialContext(ctx, pconn, addr, host, tlsConf, clientConfig) From c9acf92a22d8b13feb004e20a1e4ba757feb0bcc Mon Sep 17 00:00:00 2001 From: Marco Munizaga Date: Wed, 16 Nov 2022 21:07:02 -0800 Subject: [PATCH 07/19] Removes QUIC draft 29 addrs if we have a QUIC v1 addr --- p2p/net/swarm/swarm_dial.go | 87 +++++++++++++++++++++++++++----- p2p/net/swarm/swarm_dial_test.go | 12 +++++ 2 files changed, 86 insertions(+), 13 deletions(-) diff --git a/p2p/net/swarm/swarm_dial.go b/p2p/net/swarm/swarm_dial.go index 1efde8c44a..da9b10c174 100644 --- a/p2p/net/swarm/swarm_dial.go +++ b/p2p/net/swarm/swarm_dial.go @@ -12,6 +12,7 @@ import ( "github.com/libp2p/go-libp2p/core/peer" "github.com/libp2p/go-libp2p/core/peerstore" "github.com/libp2p/go-libp2p/core/transport" + "github.com/lucas-clemente/quic-go" ma "github.com/multiformats/go-multiaddr" madns "github.com/multiformats/go-multiaddr-dns" @@ -439,15 +440,17 @@ func (s *Swarm) filterKnownUndialables(p peer.ID, addrs []ma.Multiaddr) []ma.Mul } } - return maybeRemoveWebTransportAddrs(ma.FilterAddrs(addrs, - func(addr ma.Multiaddr) bool { return !ma.Contains(ourAddrs, addr) }, - s.canDial, - // TODO: Consider allowing link-local addresses - func(addr ma.Multiaddr) bool { return !manet.IsIP6LinkLocal(addr) }, - func(addr ma.Multiaddr) bool { - return s.gater == nil || s.gater.InterceptAddrDial(p, addr) - }, - )) + return maybeRemoveWebTransportAddrs( + maybeRemoveQUICDraft29( + ma.FilterAddrs(addrs, + func(addr ma.Multiaddr) bool { return !ma.Contains(ourAddrs, addr) }, + s.canDial, + // TODO: Consider allowing link-local addresses + func(addr ma.Multiaddr) bool { return !manet.IsIP6LinkLocal(addr) }, + func(addr ma.Multiaddr) bool { + return s.gater == nil || s.gater.InterceptAddrDial(p, addr) + }, + ))) } // limitedDial will start a dial to the given peer when @@ -536,9 +539,32 @@ func isWebTransport(addr ma.Multiaddr) bool { return err == nil } -func isQUIC(addr ma.Multiaddr) bool { - _, err := addr.ValueForProtocol(ma.P_QUIC) - return err == nil && !isWebTransport(addr) +func quicVersion(addr ma.Multiaddr) (quic.VersionNumber, bool) { + found := false + foundWebTransport := false + var version quic.VersionNumber + ma.ForEach(addr, func(c ma.Component) bool { + switch c.Protocol().Code { + case ma.P_QUIC: + version = quic.VersionDraft29 + found = true + return true + case ma.P_QUIC_V1: + version = quic.Version1 + found = true + return true + case ma.P_WEBTRANSPORT: + version = quic.Version1 + foundWebTransport = true + return false + default: + return true + } + }) + if foundWebTransport { + return 0, false + } + return version, found } // If we have QUIC addresses, we don't want to dial WebTransport addresses. @@ -548,7 +574,7 @@ func isQUIC(addr ma.Multiaddr) bool { func maybeRemoveWebTransportAddrs(addrs []ma.Multiaddr) []ma.Multiaddr { var hasQuic, hasWebTransport bool for _, addr := range addrs { - if isQUIC(addr) { + if _, isQuic := quicVersion(addr); isQuic { hasQuic = true } if isWebTransport(addr) { @@ -568,3 +594,38 @@ func maybeRemoveWebTransportAddrs(addrs []ma.Multiaddr) []ma.Multiaddr { } return addrs[:c] } + +// If we have QUIC V1 addresses, we don't want to dial QUIC draft29 addresses. +// This is a similar hack to the above. If we add one more hack like this, let's +// define a `Filterer` interface like the `Resolver` interface that transports +// can optionally implement if they want to filter the multiaddrs. +// +// This mutates the input +func maybeRemoveQUICDraft29(addrs []ma.Multiaddr) []ma.Multiaddr { + var hasQuicV1, hasQuicDraft29 bool + for _, addr := range addrs { + v, isQuic := quicVersion(addr) + if !isQuic { + continue + } + + if v == quic.Version1 { + hasQuicV1 = true + } + if v == quic.VersionDraft29 { + hasQuicDraft29 = true + } + } + if !hasQuicDraft29 || !hasQuicV1 { + return addrs + } + var c int + for _, addr := range addrs { + if v, isQuic := quicVersion(addr); isQuic && v == quic.VersionDraft29 { + continue + } + addrs[c] = addr + c++ + } + return addrs[:c] +} diff --git a/p2p/net/swarm/swarm_dial_test.go b/p2p/net/swarm/swarm_dial_test.go index 763be49bba..49a4abe1cd 100644 --- a/p2p/net/swarm/swarm_dial_test.go +++ b/p2p/net/swarm/swarm_dial_test.go @@ -204,3 +204,15 @@ func TestRemoveWebTransportAddrs(t *testing.T) { require.Equal(t, []ma.Multiaddr{quicAddr}, maybeRemoveWebTransportAddrs([]ma.Multiaddr{quicAddr, webtransportAddr})) require.Equal(t, []ma.Multiaddr{webtransportAddr}, maybeRemoveWebTransportAddrs([]ma.Multiaddr{webtransportAddr})) } + +func TestRemoveQuicDraft29(t *testing.T) { + tcpAddr := ma.StringCast("/ip4/9.5.6.4/tcp/1234") + quicDraft29Addr := ma.StringCast("/ip4/1.2.3.4/udp/443/quic") + quicV1Addr := ma.StringCast("/ip4/1.2.3.4/udp/443/quic-v1") + + require.Equal(t, []ma.Multiaddr{tcpAddr, quicV1Addr}, maybeRemoveQUICDraft29([]ma.Multiaddr{tcpAddr, quicV1Addr})) + require.Equal(t, []ma.Multiaddr{tcpAddr, quicDraft29Addr}, maybeRemoveQUICDraft29([]ma.Multiaddr{tcpAddr, quicDraft29Addr})) + require.Equal(t, []ma.Multiaddr{tcpAddr, quicV1Addr}, maybeRemoveQUICDraft29([]ma.Multiaddr{tcpAddr, quicDraft29Addr, quicV1Addr})) + require.Equal(t, []ma.Multiaddr{quicV1Addr}, maybeRemoveQUICDraft29([]ma.Multiaddr{quicV1Addr, quicDraft29Addr})) + require.Equal(t, []ma.Multiaddr{quicDraft29Addr}, maybeRemoveQUICDraft29([]ma.Multiaddr{quicDraft29Addr})) +} From 8c13e0667f9c6b8238adc6970869350f4ddaca19 Mon Sep 17 00:00:00 2001 From: Marco Munizaga Date: Tue, 15 Nov 2022 18:12:23 -0800 Subject: [PATCH 08/19] Lint fix --- p2p/transport/quic/transport.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/p2p/transport/quic/transport.go b/p2p/transport/quic/transport.go index 718e795191..730d77824d 100644 --- a/p2p/transport/quic/transport.go +++ b/p2p/transport/quic/transport.go @@ -276,13 +276,11 @@ func (t *transport) Dial(ctx context.Context, raddr ma.Multiaddr, p peer.ID) (tp return nil, err } - clientConfig := t.clientConfig + clientConfig := t.clientConfig.Clone() if v == quic.Version1 { // The endpoint has explicit support for version 1, so we'll only use that version. - clientConfig = t.clientConfig.Clone() clientConfig.Versions = []quic.VersionNumber{quic.Version1} } else if v == quic.VersionDraft29 { - clientConfig = t.clientConfig.Clone() clientConfig.Versions = []quic.VersionNumber{quic.VersionDraft29} } else { return nil, errors.New("unknown QUIC version") From 12f9d1778261d87b9da51742e23d3c33ecf8195a Mon Sep 17 00:00:00 2001 From: Marco Munizaga Date: Tue, 15 Nov 2022 18:29:23 -0800 Subject: [PATCH 09/19] Add changes to deterministic certhashes after rebase --- p2p/test/webtransport/webtransport_test.go | 4 +-- p2p/transport/webtransport/transport_test.go | 31 +++++++++++++------- 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/p2p/test/webtransport/webtransport_test.go b/p2p/test/webtransport/webtransport_test.go index adc64b49a3..e9c612baac 100644 --- a/p2p/test/webtransport/webtransport_test.go +++ b/p2p/test/webtransport/webtransport_test.go @@ -33,7 +33,7 @@ func TestDeterministicCertsAfterReboot(t *testing.T) { cl.Add(time.Hour * 24 * 365) h, err := libp2p.New(libp2p.NoTransports, libp2p.Transport(libp2pwebtransport.New, libp2pwebtransport.WithClock(cl)), libp2p.Identity(priv)) require.NoError(t, err) - err = h.Network().Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/quic/webtransport")) + err = h.Network().Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/quic-v1/webtransport")) require.NoError(t, err) prevCerthashes := extractCertHashes(h.Addrs()[0]) @@ -42,7 +42,7 @@ func TestDeterministicCertsAfterReboot(t *testing.T) { h, err = libp2p.New(libp2p.NoTransports, libp2p.Transport(libp2pwebtransport.New, libp2pwebtransport.WithClock(cl)), libp2p.Identity(priv)) require.NoError(t, err) defer h.Close() - err = h.Network().Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/quic/webtransport")) + err = h.Network().Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/quic-v1/webtransport")) require.NoError(t, err) nextCertHashes := extractCertHashes(h.Addrs()[0]) diff --git a/p2p/transport/webtransport/transport_test.go b/p2p/transport/webtransport/transport_test.go index 60780b5eca..e6415d9fff 100644 --- a/p2p/transport/webtransport/transport_test.go +++ b/p2p/transport/webtransport/transport_test.go @@ -748,7 +748,7 @@ func serverSendsBackValidCert(timeSinceUnixEpoch time.Duration, keySeed int64, r if err != nil { return err } - l, err := tr.Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/quic/webtransport")) + l, err := tr.Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/quic-v1/webtransport")) if err != nil { return err } @@ -832,11 +832,11 @@ func TestServerRotatesCertCorrectly(t *testing.T) { return false } - l, err := tr.Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/quic/webtransport")) + l, err := tr.Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/quic-v1/webtransport")) if err != nil { return false } - certhashes := extractCertHashes(l.Multiaddr()) + certhashes := extractCertHashes(onlyWebtransportmultiaddr(t, l.Multiaddrs())) l.Close() // These two certificates together are valid for at most certValidity - (4*clockSkewAllowance) @@ -846,14 +846,15 @@ func TestServerRotatesCertCorrectly(t *testing.T) { return false } - l, err = tr.Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/quic/webtransport")) + l, err = tr.Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/quic-v1/webtransport")) if err != nil { return false } defer l.Close() var found bool - ma.ForEach(l.Multiaddr(), func(c ma.Component) bool { + addrs := onlyWebtransportmultiaddr(t, l.Multiaddrs()) + ma.ForEach(addrs, func(c ma.Component) bool { if c.Protocol().Code == ma.P_CERTHASH { for _, prevCerthash := range certhashes { if c.Value() == prevCerthash { @@ -870,6 +871,15 @@ func TestServerRotatesCertCorrectly(t *testing.T) { }, nil)) } +func onlyWebtransportmultiaddr(t testing.TB, addrs []ma.Multiaddr) ma.Multiaddr { + addrs = ma.FilterAddrs(addrs, func(m ma.Multiaddr) bool { + _, err := m.ValueForProtocol(ma.P_WEBTRANSPORT) + return err == nil + }) + require.NotEmpty(t, addrs) + return addrs[0] +} + func TestServerRotatesCertCorrectlyAfterSteps(t *testing.T) { cl := clock.NewMock() // Move one year ahead to avoid edge cases around epoch @@ -880,10 +890,10 @@ func TestServerRotatesCertCorrectlyAfterSteps(t *testing.T) { tr, err := libp2pwebtransport.New(priv, nil, &network.NullResourceManager{}, libp2pwebtransport.WithClock(cl)) require.NoError(t, err) - l, err := tr.Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/quic/webtransport")) + l, err := tr.Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/quic-v1/webtransport")) require.NoError(t, err) - certhashes := extractCertHashes(l.Multiaddr()) + certhashes := extractCertHashes(onlyWebtransportmultiaddr(t, l.Multiaddrs())) l.Close() // Traverse various time boundaries and make sure we always keep a common certhash. @@ -892,11 +902,12 @@ func TestServerRotatesCertCorrectlyAfterSteps(t *testing.T) { cl.Add(24 * time.Hour) tr, err := libp2pwebtransport.New(priv, nil, &network.NullResourceManager{}, libp2pwebtransport.WithClock(cl)) require.NoError(t, err) - l, err := tr.Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/quic/webtransport")) + l, err := tr.Listen(ma.StringCast("/ip4/127.0.0.1/udp/0/quic-v1/webtransport")) require.NoError(t, err) var found bool - ma.ForEach(l.Multiaddr(), func(c ma.Component) bool { + addrs := onlyWebtransportmultiaddr(t, l.Multiaddrs()) + ma.ForEach(addrs, func(c ma.Component) bool { if c.Protocol().Code == ma.P_CERTHASH { for _, prevCerthash := range certhashes { if prevCerthash == c.Value() { @@ -907,7 +918,7 @@ func TestServerRotatesCertCorrectlyAfterSteps(t *testing.T) { } return true }) - certhashes = extractCertHashes(l.Multiaddr()) + certhashes = extractCertHashes(onlyWebtransportmultiaddr(t, l.Multiaddrs())) l.Close() require.True(t, found, "Failed after hour: %v", i) From 9b6a728266ae994d847aaed53f70ff51998e5bb6 Mon Sep 17 00:00:00 2001 From: Marco Munizaga Date: Tue, 15 Nov 2022 18:36:08 -0800 Subject: [PATCH 10/19] Update p2p/transport/quic/options.go Co-authored-by: Marten Seemann --- p2p/transport/quic/options.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/p2p/transport/quic/options.go b/p2p/transport/quic/options.go index 857480778d..8a7c8ce399 100644 --- a/p2p/transport/quic/options.go +++ b/p2p/transport/quic/options.go @@ -25,6 +25,9 @@ func DisableReuseport() Option { } } +// DisableDraft29 disables support for QUIC draft-29. +// This option should be set, unless support for this legacy QUIC version is needed for backwards compatibility. +// Support for QUIC draft-29 is already deprecated and will be removed in the future, see https://github.com/libp2p/go-libp2p/issues/1841. func DisableDraft29() Option { return func(cfg *config) error { cfg.disableDraft29 = true From 6ac0c41b7718c12288e24d77a2f136984661976d Mon Sep 17 00:00:00 2001 From: Marco Munizaga Date: Tue, 15 Nov 2022 18:36:20 -0800 Subject: [PATCH 11/19] Update p2p/transport/quic/listener.go Co-authored-by: Marten Seemann --- p2p/transport/quic/listener.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/p2p/transport/quic/listener.go b/p2p/transport/quic/listener.go index d5d9e7712a..e3c5558730 100644 --- a/p2p/transport/quic/listener.go +++ b/p2p/transport/quic/listener.go @@ -142,7 +142,7 @@ func (l *listener) setupConn(qconn quic.Connection) (*conn, error) { localMultiaddr, found := l.localMultiaddrs[qconn.ConnectionState().Version] if !found { - return nil, errors.New("unknown quic version:" + qconn.ConnectionState().Version.String()) + return nil, errors.New("unknown QUIC version:" + qconn.ConnectionState().Version.String()) } l.conn.IncreaseCount() From aab420b87607dad5c6c5ad85b264c6e815695cb3 Mon Sep 17 00:00:00 2001 From: Marco Munizaga Date: Tue, 15 Nov 2022 18:36:37 -0800 Subject: [PATCH 12/19] Update p2p/transport/quic/quic_multiaddr.go Co-authored-by: Marten Seemann --- p2p/transport/quic/quic_multiaddr.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/p2p/transport/quic/quic_multiaddr.go b/p2p/transport/quic/quic_multiaddr.go index adf5a71b4f..7896e2b3b3 100644 --- a/p2p/transport/quic/quic_multiaddr.go +++ b/p2p/transport/quic/quic_multiaddr.go @@ -23,7 +23,7 @@ func toQuicMultiaddr(na net.Addr, version quic.VersionNumber) (ma.Multiaddr, err case quic.Version1: return udpMA.Encapsulate(quicV1MA), nil default: - return nil, errors.New("unknown quic version") + return nil, errors.New("unknown QUIC version") } } From cd3e8c0a7165b55683541041ff186d6c83ae64de Mon Sep 17 00:00:00 2001 From: Marco Munizaga Date: Tue, 15 Nov 2022 18:37:54 -0800 Subject: [PATCH 13/19] Stylize QUIC correctly --- p2p/transport/quic/quic_multiaddr.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/p2p/transport/quic/quic_multiaddr.go b/p2p/transport/quic/quic_multiaddr.go index 7896e2b3b3..94b6552a66 100644 --- a/p2p/transport/quic/quic_multiaddr.go +++ b/p2p/transport/quic/quic_multiaddr.go @@ -29,7 +29,7 @@ func toQuicMultiaddr(na net.Addr, version quic.VersionNumber) (ma.Multiaddr, err func fromQuicMultiaddr(addr ma.Multiaddr) (net.Addr, quic.VersionNumber, error) { var version quic.VersionNumber - var partsBeforeQuic ma.Multiaddr + var partsBeforeQUIC ma.Multiaddr ma.ForEach(addr, func(c ma.Component) bool { switch c.Protocol().Code { case ma.P_QUIC: @@ -39,22 +39,22 @@ func fromQuicMultiaddr(addr ma.Multiaddr) (net.Addr, quic.VersionNumber, error) version = quic.Version1 return false default: - if partsBeforeQuic == nil { - partsBeforeQuic = &c + if partsBeforeQUIC == nil { + partsBeforeQUIC = &c } else { - partsBeforeQuic = partsBeforeQuic.Encapsulate(&c) + partsBeforeQUIC = partsBeforeQUIC.Encapsulate(&c) } return true } }) - if partsBeforeQuic == nil { - return nil, version, errors.New("no addr before quic component") + if partsBeforeQUIC == nil { + return nil, version, errors.New("no addr before QUIC component") } if version == 0 { // Not found return nil, version, errors.New("unknown quic version") } - netAddr, err := manet.ToNetAddr(partsBeforeQuic) + netAddr, err := manet.ToNetAddr(partsBeforeQUIC) if err != nil { return nil, version, err } From 7107f7d3249cd2b3545cf42e6ace163848212836 Mon Sep 17 00:00:00 2001 From: Marco Munizaga Date: Tue, 15 Nov 2022 18:59:01 -0800 Subject: [PATCH 14/19] Update doc around ListenClose --- p2p/net/swarm/swarm_listen.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/p2p/net/swarm/swarm_listen.go b/p2p/net/swarm/swarm_listen.go index 59407162bb..6f50bbfbfc 100644 --- a/p2p/net/swarm/swarm_listen.go +++ b/p2p/net/swarm/swarm_listen.go @@ -37,7 +37,11 @@ func (s *Swarm) Listen(addrs ...ma.Multiaddr) error { return nil } -// ListenClose stop and delete listeners for all of the given addresses. +// ListenClose stop and delete listeners for all of the given addresses. If an +// any address belongs to one of the addreses a Listener provides, then the +// Listener will close for *all* addresses it provides. For example if you close +// and address with `/quic`, then the QUIC listener will close and also close +// any `/quic-v1` address. func (s *Swarm) ListenClose(addrs ...ma.Multiaddr) { var listenersToClose []transport.Listener From 30b21a7b08120a3349dc25c2cef61e2f5848bd4d Mon Sep 17 00:00:00 2001 From: Marco Munizaga Date: Tue, 15 Nov 2022 18:59:21 -0800 Subject: [PATCH 15/19] Preallocate a bit extra to avoid paying for an allocation later --- p2p/net/swarm/swarm_addr.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/p2p/net/swarm/swarm_addr.go b/p2p/net/swarm/swarm_addr.go index aa8b808084..f8bec53afa 100644 --- a/p2p/net/swarm/swarm_addr.go +++ b/p2p/net/swarm/swarm_addr.go @@ -16,7 +16,7 @@ func (s *Swarm) ListenAddresses() []ma.Multiaddr { } func (s *Swarm) listenAddressesNoLock() []ma.Multiaddr { - addrs := make([]ma.Multiaddr, 0, len(s.listeners.m)) + addrs := make([]ma.Multiaddr, 0, len(s.listeners.m)+10) // A bit extra so we may avoid an extra allocation in the for loop below. for l := range s.listeners.m { addrs = append(addrs, l.Multiaddrs()...) } From a6f08a837460400c4ae22ccd0e822b00ada741ed Mon Sep 17 00:00:00 2001 From: Marco Munizaga Date: Tue, 15 Nov 2022 18:59:38 -0800 Subject: [PATCH 16/19] Keep a list of multiaddrs, then join --- p2p/transport/quic/quic_multiaddr.go | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/p2p/transport/quic/quic_multiaddr.go b/p2p/transport/quic/quic_multiaddr.go index 94b6552a66..47dc7e905f 100644 --- a/p2p/transport/quic/quic_multiaddr.go +++ b/p2p/transport/quic/quic_multiaddr.go @@ -29,7 +29,7 @@ func toQuicMultiaddr(na net.Addr, version quic.VersionNumber) (ma.Multiaddr, err func fromQuicMultiaddr(addr ma.Multiaddr) (net.Addr, quic.VersionNumber, error) { var version quic.VersionNumber - var partsBeforeQUIC ma.Multiaddr + var partsBeforeQUIC []ma.Multiaddr ma.ForEach(addr, func(c ma.Component) bool { switch c.Protocol().Code { case ma.P_QUIC: @@ -39,22 +39,18 @@ func fromQuicMultiaddr(addr ma.Multiaddr) (net.Addr, quic.VersionNumber, error) version = quic.Version1 return false default: - if partsBeforeQUIC == nil { - partsBeforeQUIC = &c - } else { - partsBeforeQUIC = partsBeforeQUIC.Encapsulate(&c) - } + partsBeforeQUIC = append(partsBeforeQUIC, &c) return true } }) - if partsBeforeQUIC == nil { + if len(partsBeforeQUIC) == 0 { return nil, version, errors.New("no addr before QUIC component") } if version == 0 { // Not found - return nil, version, errors.New("unknown quic version") + return nil, version, errors.New("unknown QUIC version") } - netAddr, err := manet.ToNetAddr(partsBeforeQUIC) + netAddr, err := manet.ToNetAddr(ma.Join(partsBeforeQUIC...)) if err != nil { return nil, version, err } From 85b4573ad7ca77b5f4645bc654b3ab9276957dab Mon Sep 17 00:00:00 2001 From: Marco Munizaga Date: Tue, 15 Nov 2022 19:08:50 -0800 Subject: [PATCH 17/19] PR nits --- p2p/transport/quic/listener.go | 3 +-- p2p/transport/quic/listener_test.go | 2 ++ 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/p2p/transport/quic/listener.go b/p2p/transport/quic/listener.go index e3c5558730..3bdba01762 100644 --- a/p2p/transport/quic/listener.go +++ b/p2p/transport/quic/listener.go @@ -50,8 +50,7 @@ func newListener(pconn pConn, t *transport, localPeer peer.ID, key ic.PrivKey, i return nil, err } - localMultiaddrs := map[quic.VersionNumber]ma.Multiaddr{} - localMultiaddrs[quic.Version1] = localMultiaddr + localMultiaddrs := map[quic.VersionNumber]ma.Multiaddr{quic.Version1: localMultiaddr} if enableDraft29 { localMultiaddr, err := toQuicMultiaddr(ln.Addr(), quic.VersionDraft29) diff --git a/p2p/transport/quic/listener_test.go b/p2p/transport/quic/listener_test.go index ac7b2af638..a43ad2fed2 100644 --- a/p2p/transport/quic/listener_test.go +++ b/p2p/transport/quic/listener_test.go @@ -71,6 +71,7 @@ func TestListenAddr(t *testing.T) { multiaddrsStrings = append(multiaddrsStrings, a.String()) } require.Contains(t, multiaddrsStrings, fmt.Sprintf("/ip4/127.0.0.1/udp/%d/quic", port)) + require.Contains(t, multiaddrsStrings, fmt.Sprintf("/ip4/127.0.0.1/udp/%d/quic-v1", port)) }) t.Run("for IPv6", func(t *testing.T) { @@ -85,6 +86,7 @@ func TestListenAddr(t *testing.T) { multiaddrsStrings = append(multiaddrsStrings, a.String()) } require.Contains(t, multiaddrsStrings, fmt.Sprintf("/ip6/::/udp/%d/quic", port)) + require.Contains(t, multiaddrsStrings, fmt.Sprintf("/ip6/::/udp/%d/quic-v1", port)) }) } From 8e451022ff04fd3438c5b38432aa97a26fe5fc69 Mon Sep 17 00:00:00 2001 From: Marco Munizaga Date: Wed, 16 Nov 2022 20:18:45 -0800 Subject: [PATCH 18/19] Close transport or listener just once --- p2p/net/swarm/swarm.go | 8 +++++++- p2p/net/swarm/swarm_listen.go | 6 +++--- p2p/transport/quic/transport.go | 8 +------- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/p2p/net/swarm/swarm.go b/p2p/net/swarm/swarm.go index 213f0fc658..7606b80c82 100644 --- a/p2p/net/swarm/swarm.go +++ b/p2p/net/swarm/swarm.go @@ -237,8 +237,14 @@ func (s *Swarm) close() { s.transports.m = nil s.transports.Unlock() - var wg sync.WaitGroup + // Dedup transports that may be listening on multiple protocols + transportsToClose := make(map[transport.Transport]struct{}, len(transports)) for _, t := range transports { + transportsToClose[t] = struct{}{} + } + + var wg sync.WaitGroup + for t := range transportsToClose { if closer, ok := t.(io.Closer); ok { wg.Add(1) go func(c io.Closer) { diff --git a/p2p/net/swarm/swarm_listen.go b/p2p/net/swarm/swarm_listen.go index 6f50bbfbfc..2e4bb9c52b 100644 --- a/p2p/net/swarm/swarm_listen.go +++ b/p2p/net/swarm/swarm_listen.go @@ -43,7 +43,7 @@ func (s *Swarm) Listen(addrs ...ma.Multiaddr) error { // and address with `/quic`, then the QUIC listener will close and also close // any `/quic-v1` address. func (s *Swarm) ListenClose(addrs ...ma.Multiaddr) { - var listenersToClose []transport.Listener + listenersToClose := make(map[transport.Listener]struct{}, len(addrs)) s.listeners.Lock() for l := range s.listeners.m { @@ -52,12 +52,12 @@ func (s *Swarm) ListenClose(addrs ...ma.Multiaddr) { } delete(s.listeners.m, l) - listenersToClose = append(listenersToClose, l) + listenersToClose[l] = struct{}{} } s.listeners.cacheEOL = time.Time{} s.listeners.Unlock() - for _, l := range listenersToClose { + for l := range listenersToClose { l.Close() } } diff --git a/p2p/transport/quic/transport.go b/p2p/transport/quic/transport.go index 730d77824d..7f598bf3c9 100644 --- a/p2p/transport/quic/transport.go +++ b/p2p/transport/quic/transport.go @@ -153,8 +153,6 @@ type transport struct { connMx sync.Mutex conns map[quic.Connection]*conn - - closeOnce sync.Once } var _ tpt.Transport = &transport{} @@ -490,9 +488,5 @@ func (t *transport) String() string { } func (t *transport) Close() error { - // We may register multiple protocols, but they are all the same transport, so only support getting closed once. - t.closeOnce.Do(func() { - _ = t.connManager.Close() - }) - return nil + return t.connManager.Close() } From 38ad076e38dc948f30dadb354c180e90d466bd05 Mon Sep 17 00:00:00 2001 From: Marco Munizaga Date: Wed, 16 Nov 2022 21:21:14 -0800 Subject: [PATCH 19/19] Update go-multiaddr --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 365af5e1bf..016d915280 100644 --- a/go.mod +++ b/go.mod @@ -38,7 +38,7 @@ require ( github.com/minio/sha256-simd v1.0.0 github.com/mr-tron/base58 v1.2.0 github.com/multiformats/go-base32 v0.1.0 - github.com/multiformats/go-multiaddr v0.7.0 + github.com/multiformats/go-multiaddr v0.8.0 github.com/multiformats/go-multiaddr-dns v0.3.1 github.com/multiformats/go-multiaddr-fmt v0.1.0 github.com/multiformats/go-multibase v0.1.1 diff --git a/go.sum b/go.sum index e9473541c8..7e93ae7a51 100644 --- a/go.sum +++ b/go.sum @@ -373,8 +373,8 @@ github.com/multiformats/go-base36 v0.1.0 h1:JR6TyF7JjGd3m6FbLU2cOxhC0Li8z8dLNGQ8 github.com/multiformats/go-base36 v0.1.0/go.mod h1:kFGE83c6s80PklsHO9sRn2NCoffoRdUUOENyW/Vv6sM= github.com/multiformats/go-multiaddr v0.1.1/go.mod h1:aMKBKNEYmzmDmxfX88/vz+J5IU55txyt0p4aiWVohjo= github.com/multiformats/go-multiaddr v0.2.0/go.mod h1:0nO36NvPpyV4QzvTLi/lafl2y95ncPj0vFwVF6k6wJ4= -github.com/multiformats/go-multiaddr v0.7.0 h1:gskHcdaCyPtp9XskVwtvEeQOG465sCohbQIirSyqxrc= -github.com/multiformats/go-multiaddr v0.7.0/go.mod h1:Fs50eBDWvZu+l3/9S6xAE7ZYj6yhxlvaVZjakWN7xRs= +github.com/multiformats/go-multiaddr v0.8.0 h1:aqjksEcqK+iD/Foe1RRFsGZh8+XFiGo7FgUCZlpv3LU= +github.com/multiformats/go-multiaddr v0.8.0/go.mod h1:Fs50eBDWvZu+l3/9S6xAE7ZYj6yhxlvaVZjakWN7xRs= github.com/multiformats/go-multiaddr-dns v0.3.1 h1:QgQgR+LQVt3NPTjbrLLpsaT2ufAA2y0Mkk+QRVJbW3A= github.com/multiformats/go-multiaddr-dns v0.3.1/go.mod h1:G/245BRQ6FJGmryJCrOuTdB37AMA5AMOVuO6NY3JwTk= github.com/multiformats/go-multiaddr-fmt v0.1.0 h1:WLEFClPycPkp4fnIzoFoV9FVd49/eQsuaL3/CWe167E=