Skip to content

Commit

Permalink
TUN-8688: Correct UDP bind for IPv6 edge connectivity on macOS
Browse files Browse the repository at this point in the history
For macOS, we want to set the DF bit for the UDP packets used by the QUIC
connection; to achieve this, you need to explicitly set the network
to either "udp4" or "udp6". When determining which network type to pick
we need to use the edge IP address chosen to align with what the local
IP family interface we will use. This means we want cloudflared to bind
to local interfaces for a random port, so we provide a zero IP and 0 port
number (ex. 0.0.0.0:0). However, instead of providing the zero IP, we
can leave the value as nil and let the kernel decide which interface and
pick a random port as defined by the target edge IP family.

This was previously broken for IPv6-only edge connectivity on macOS and
all other operating systems should be unaffected because the network type
was left as default "udp" which will rely on the provided local or remote
IP for selection.

Closes TUN-8688
  • Loading branch information
DevinCarr committed Oct 18, 2024
1 parent d608a64 commit 92e0f5f
Show file tree
Hide file tree
Showing 9 changed files with 904 additions and 21 deletions.
17 changes: 7 additions & 10 deletions connection/quic.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ type QUICConnection struct {
func NewQUICConnection(
ctx context.Context,
quicConfig *quic.Config,
edgeAddr net.Addr,
edgeAddr netip.AddrPort,
localAddr net.IP,
connIndex uint8,
tlsConfig *tls.Config,
Expand All @@ -103,12 +103,12 @@ func NewQUICConnection(
streamWriteTimeout time.Duration,
gracePeriod time.Duration,
) (*QUICConnection, error) {
udpConn, err := createUDPConnForConnIndex(connIndex, localAddr, logger)
udpConn, err := createUDPConnForConnIndex(connIndex, localAddr, edgeAddr, logger)
if err != nil {
return nil, err
}

session, err := quic.Dial(ctx, udpConn, edgeAddr, tlsConfig, quicConfig)
session, err := quic.Dial(ctx, udpConn, net.UDPAddrFromAddrPort(edgeAddr), tlsConfig, quicConfig)
if err != nil {
// close the udp server socket in case of error connecting to the edge
udpConn.Close()
Expand Down Expand Up @@ -641,18 +641,15 @@ func (rp *muxerWrapper) Close() error {
return nil
}

func createUDPConnForConnIndex(connIndex uint8, localIP net.IP, logger *zerolog.Logger) (*net.UDPConn, error) {
func createUDPConnForConnIndex(connIndex uint8, localIP net.IP, edgeIP netip.AddrPort, logger *zerolog.Logger) (*net.UDPConn, error) {
portMapMutex.Lock()
defer portMapMutex.Unlock()

if localIP == nil {
localIP = net.IPv4zero
}

listenNetwork := "udp"
// https://github.com/quic-go/quic-go/issues/3793 DF bit cannot be set for dual stack listener on OSX
// https://github.com/quic-go/quic-go/issues/3793 DF bit cannot be set for dual stack listener ("udp") on macOS,
// to set the DF bit properly, the network string needs to be specific to the IP family.
if runtime.GOOS == "darwin" {
if localIP.To4() != nil {
if edgeIP.Addr().Is4() {
listenNetwork = "udp4"
} else {
listenNetwork = "udp6"
Expand Down
29 changes: 21 additions & 8 deletions connection/quic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"math/big"
"net"
"net/http"
"net/netip"
"net/url"
"os"
"strings"
Expand All @@ -26,6 +27,7 @@ import (
"github.com/rs/zerolog"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"golang.org/x/net/nettest"

"github.com/cloudflare/cloudflared/datagramsession"
cfdquic "github.com/cloudflare/cloudflared/quic"
Expand Down Expand Up @@ -162,7 +164,7 @@ func TestQUICServer(t *testing.T) {
close(serverDone)
}()

qc := testQUICConnection(udpListener.LocalAddr(), t, uint8(i))
qc := testQUICConnection(netip.MustParseAddrPort(udpListener.LocalAddr().String()), t, uint8(i))

connDone := make(chan struct{})
go func() {
Expand Down Expand Up @@ -632,7 +634,6 @@ func TestServeUDPSession(t *testing.T) {
defer udpListener.Close()

ctx, cancel := context.WithCancel(context.Background())
val := udpListener.LocalAddr()

// Establish QUIC connection with edge
edgeQUICSessionChan := make(chan quic.Connection)
Expand All @@ -646,7 +647,7 @@ func TestServeUDPSession(t *testing.T) {
}()

// Random index to avoid reusing port
qc := testQUICConnection(val, t, 28)
qc := testQUICConnection(netip.MustParseAddrPort(udpListener.LocalAddr().String()), t, 28)
go qc.Serve(ctx)

edgeQUICSession := <-edgeQUICSessionChan
Expand Down Expand Up @@ -695,8 +696,20 @@ func TestNopCloserReadWriterCloseAfterEOF(t *testing.T) {
}

func TestCreateUDPConnReuseSourcePort(t *testing.T) {
edgeIPv4 := netip.MustParseAddrPort("0.0.0.0:0")
edgeIPv6 := netip.MustParseAddrPort("[::]:0")

// We assume the test environment has access to an IPv4 interface
testCreateUDPConnReuseSourcePortForEdgeIP(t, edgeIPv4)

if nettest.SupportsIPv6() {
testCreateUDPConnReuseSourcePortForEdgeIP(t, edgeIPv6)
}
}

func testCreateUDPConnReuseSourcePortForEdgeIP(t *testing.T, edgeIP netip.AddrPort) {
logger := zerolog.Nop()
conn, err := createUDPConnForConnIndex(0, nil, &logger)
conn, err := createUDPConnForConnIndex(0, nil, edgeIP, &logger)
require.NoError(t, err)

getPortFunc := func(conn *net.UDPConn) int {
Expand All @@ -710,17 +723,17 @@ func TestCreateUDPConnReuseSourcePort(t *testing.T) {
conn.Close()

// should get the same port as before.
conn, err = createUDPConnForConnIndex(0, nil, &logger)
conn, err = createUDPConnForConnIndex(0, nil, edgeIP, &logger)
require.NoError(t, err)
require.Equal(t, initialPort, getPortFunc(conn))

// new index, should get a different port
conn1, err := createUDPConnForConnIndex(1, nil, &logger)
conn1, err := createUDPConnForConnIndex(1, nil, edgeIP, &logger)
require.NoError(t, err)
require.NotEqual(t, initialPort, getPortFunc(conn1))

// not closing the conn and trying to obtain a new conn for same index should give a different random port
conn, err = createUDPConnForConnIndex(0, nil, &logger)
conn, err = createUDPConnForConnIndex(0, nil, edgeIP, &logger)
require.NoError(t, err)
require.NotEqual(t, initialPort, getPortFunc(conn))
}
Expand Down Expand Up @@ -832,7 +845,7 @@ func (s mockSessionRPCServer) UnregisterUdpSession(ctx context.Context, sessionI
return nil
}

func testQUICConnection(udpListenerAddr net.Addr, t *testing.T, index uint8) *QUICConnection {
func testQUICConnection(udpListenerAddr netip.AddrPort, t *testing.T, index uint8) *QUICConnection {
tlsClientConfig := &tls.Config{
InsecureSkipVerify: true,
NextProtos: []string{"argotunnel"},
Expand Down
7 changes: 4 additions & 3 deletions supervisor/tunnel.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"crypto/tls"
"fmt"
"net"
"net/netip"
"runtime/debug"
"strings"
"sync"
Expand Down Expand Up @@ -465,7 +466,7 @@ func (e *EdgeTunnelServer) serveConnection(
case connection.QUIC:
connOptions := e.config.connectionOptions(addr.UDP.String(), uint8(backoff.Retries()))
return e.serveQUIC(ctx,
addr.UDP,
addr.UDP.AddrPort(),
connLog,
connOptions,
controlStream,
Expand Down Expand Up @@ -548,7 +549,7 @@ func (e *EdgeTunnelServer) serveHTTP2(

func (e *EdgeTunnelServer) serveQUIC(
ctx context.Context,
edgeAddr *net.UDPAddr,
edgeAddr netip.AddrPort,
connLogger *ConnAwareLogger,
connOptions *pogs.ConnectionOptions,
controlStreamHandler connection.ControlStreamHandler,
Expand All @@ -571,7 +572,7 @@ func (e *EdgeTunnelServer) serveQUIC(
// quic-go 0.44 increases the initial packet size to 1280 by default. That breaks anyone running tunnel through WARP
// because WARP MTU is 1280.
var initialPacketSize uint16 = 1252
if edgeAddr.IP.To4() == nil {
if edgeAddr.Addr().Is4() {
initialPacketSize = 1232
}

Expand Down
Loading

0 comments on commit 92e0f5f

Please sign in to comment.