Repository: quic-go/quic-go
Pull request #4740 (2024-12-01 05:21:08)

remove the Token field from the StatelessResetError
There's nothing the application could possibly do with this value.

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/quic-go/quic-go/pull/4740?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 83.75%. Comparing base [(`aed4d8d`)](https://app.codecov.io/gh/quic-go/quic-go/commit/aed4d8df0c9771b9e307ff231dc988efc60160b5?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go) to head [(`05acb79`)](https://app.codecov.io/gh/quic-go/quic-go/commit/05acb795cb2ff6d504e43026b75d28671b91f485?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go).

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #4740      +/-   ##
==========================================
- Coverage   83.83%   83.75%   -0.09%     
==========================================
  Files         149      146       -3     
  Lines       16047    15701     -346     
==========================================
- Hits        13453    13149     -304     
+ Misses       2061     2037      -24     
+ Partials      533      515      -18     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/quic-go/quic-go/pull/4740?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go).


Patch:
diff --git a/internal/qerr/errors.go b/internal/qerr/errors.go
index 8f5936df..0879bee4 100644
--- a/internal/qerr/errors.go
+++ b/internal/qerr/errors.go
@@ -114,14 +114,12 @@ func (e *VersionNegotiationError) Is(target error) bool {
 }
 
 // A StatelessResetError occurs when we receive a stateless reset.
-type StatelessResetError struct {
-	Token protocol.StatelessResetToken
-}
+type StatelessResetError struct{}
 
 var _ net.Error = &StatelessResetError{}
 
 func (e *StatelessResetError) Error() string {
-	return fmt.Sprintf("received a stateless reset with token %x", e.Token)
+	return "received a stateless reset"
 }
 
 func (e *StatelessResetError) Is(target error) bool {
diff --git a/qlog/event.go b/qlog/event.go
index 54134699..90a642af 100644
--- a/qlog/event.go
+++ b/qlog/event.go
@@ -124,7 +124,6 @@ func (e eventConnectionClosed) MarshalJSONObject(enc *gojay.Encoder) {
 	case errors.As(e.e, &statelessResetErr):
 		enc.StringKey("owner", ownerRemote.String())
 		enc.StringKey("trigger", "stateless_reset")
-		enc.StringKey("stateless_reset_token", fmt.Sprintf("%x", statelessResetErr.Token))
 	case errors.As(e.e, &handshakeTimeoutErr):
 		enc.StringKey("owner", ownerLocal.String())
 		enc.StringKey("trigger", "handshake_timeout")
diff --git a/sys_conn.go b/sys_conn.go
index 71cc4607..811131d9 100644
--- a/sys_conn.go
+++ b/sys_conn.go
@@ -58,8 +58,8 @@ func wrapConn(pc net.PacketConn) (rawConn, error) {
 			return nil, err
 		}
 
+		// only set DF on UDP sockets
 		if _, ok := pc.LocalAddr().(*net.UDPAddr); ok {
-			// Only set DF on sockets that we expect to be able to handle that configuration.
 			var err error
 			supportsDF, err = setDF(rawConn)
 			if err != nil {
diff --git a/sys_conn_df_darwin.go b/sys_conn_df_darwin.go
index b51cd8f1..8ed273ee 100644
--- a/sys_conn_df_darwin.go
+++ b/sys_conn_df_darwin.go
@@ -4,47 +4,67 @@ package quic
 
 import (
 	"errors"
+	"fmt"
 	"strconv"
 	"strings"
 	"syscall"
 
 	"golang.org/x/sys/unix"
+)
 
-	"github.com/quic-go/quic-go/internal/utils"
+// for macOS versions, see https://en.wikipedia.org/wiki/Darwin_(operating_system)#Darwin_20_onwards
+const (
+	macOSVersion11 = 20
+	macOSVersion15 = 24
 )
 
 func setDF(rawConn syscall.RawConn) (bool, error) {
-	// Setting DF bit is only supported from macOS11
+	// Setting DF bit is only supported from macOS 11.
 	// https://github.com/chromium/chromium/blob/117.0.5881.2/net/socket/udp_socket_posix.cc#L555
-	if supportsDF, err := isAtLeastMacOS11(); !supportsDF || err != nil {
+	version, err := getMacOSVersion()
+	if err != nil || version < macOSVersion11 {
 		return false, err
 	}
 
-	// Enabling IP_DONTFRAG will force the kernel to return "sendto: message too long"
-	// and the datagram will not be fragmented
-	var errDFIPv4, errDFIPv6 error
+	var controlErr error
+	var disableDF bool
 	if err := rawConn.Control(func(fd uintptr) {
-		errDFIPv4 = unix.SetsockoptInt(int(fd), unix.IPPROTO_IP, unix.IP_DONTFRAG, 1)
-		errDFIPv6 = unix.SetsockoptInt(int(fd), unix.IPPROTO_IPV6, unix.IPV6_DONTFRAG, 1)
+		addr, err := unix.Getsockname(int(fd))
+		if err != nil {
+			controlErr = fmt.Errorf("getsockname: %w", err)
+			return
+		}
+
+		// Dual-stack sockets are effectively IPv6 sockets (with IPV6_ONLY set to 0).
+		// On macOS, the DF bit on dual-stack sockets is controlled by the IPV6_DONTFRAG option.
+		// See https://datatracker.ietf.org/doc/draft-seemann-tsvwg-udp-fragmentation/ for details.
+		switch addr.(type) {
+		case *unix.SockaddrInet4:
+			controlErr = unix.SetsockoptInt(int(fd), unix.IPPROTO_IP, unix.IP_DONTFRAG, 1)
+		case *unix.SockaddrInet6:
+			controlErr = unix.SetsockoptInt(int(fd), unix.IPPROTO_IPV6, unix.IPV6_DONTFRAG, 1)
+
+			// Setting the DF bit on dual-stack sockets works since macOS Sequoia.
+			// Disable DF on dual-stack sockets before Sequoia.
+			if version < macOSVersion15 {
+				// check if this is a dual-stack socket by reading the IPV6_V6ONLY flag
+				v6only, err := unix.GetsockoptInt(int(fd), unix.IPPROTO_IPV6, unix.IPV6_V6ONLY)
+				if err != nil {
+					controlErr = fmt.Errorf("getting IPV6_V6ONLY: %w", err)
+					return
+				}
+				disableDF = v6only == 0
+			}
+		default:
+			controlErr = fmt.Errorf("unknown address type: %T", addr)
+		}
 	}); err != nil {
 		return false, err
 	}
-	switch {
-	case errDFIPv4 == nil && errDFIPv6 == nil:
-		utils.DefaultLogger.Debugf("Setting DF for IPv4 and IPv6.")
-	case errDFIPv4 == nil && errDFIPv6 != nil:
-		utils.DefaultLogger.Debugf("Setting DF for IPv4.")
-	case errDFIPv4 != nil && errDFIPv6 == nil:
-		utils.DefaultLogger.Debugf("Setting DF for IPv6.")
-		// On macOS, the syscall for setting DF bit for IPv4 fails on dual-stack listeners.
-		// Treat the connection as not having DF enabled, even though the DF bit will be set
-		// when used for IPv6.
-		// See https://github.com/quic-go/quic-go/issues/3793 for details.
-		return false, nil
-	case errDFIPv4 != nil && errDFIPv6 != nil:
-		return false, errors.New("setting DF failed for both IPv4 and IPv6")
+	if controlErr != nil {
+		return false, controlErr
 	}
-	return true, nil
+	return !disableDF, nil
 }
 
 func isSendMsgSizeErr(err error) bool {
@@ -53,22 +73,20 @@ func isSendMsgSizeErr(err error) bool {
 
 func isRecvMsgSizeErr(error) bool { return false }
 
-func isAtLeastMacOS11() (bool, error) {
+func getMacOSVersion() (int, error) {
 	uname := &unix.Utsname{}
-	err := unix.Uname(uname)
-	if err != nil {
-		return false, err
+	if err := unix.Uname(uname); err != nil {
+		return 0, err
 	}
 
 	release := string(uname.Release[:])
-	if idx := strings.Index(release, "."); idx != -1 {
-		version, err := strconv.Atoi(release[:idx])
-		if err != nil {
-			return false, err
-		}
-		// Darwin version 20 is macOS version 11
-		// https://en.wikipedia.org/wiki/Darwin_(operating_system)#Darwin_20_onwards
-		return version >= 20, nil
+	idx := strings.Index(release, ".")
+	if idx == -1 {
+		return 0, nil
+	}
+	version, err := strconv.Atoi(release[:idx])
+	if err != nil {
+		return 0, err
 	}
-	return false, nil
+	return version, nil
 }
diff --git a/transport.go b/transport.go
index 059f30f5..100f3037 100644
--- a/transport.go
+++ b/transport.go
@@ -489,7 +489,7 @@ func (t *Transport) maybeHandleStatelessReset(data []byte) bool {
 	token := *(*protocol.StatelessResetToken)(data[len(data)-16:])
 	if conn, ok := t.handlerMap.GetByResetToken(token); ok {
 		t.logger.Debugf("Received a stateless reset with token %#x. Closing connection.", token)
-		go conn.destroy(&StatelessResetError{Token: token})
+		go conn.destroy(&StatelessResetError{})
 		return true
 	}
 	return false


Test patch:
diff --git a/connection_test.go b/connection_test.go
index bbe0fe6b..6b2b233f 100644
--- a/connection_test.go
+++ b/connection_test.go
@@ -622,20 +622,17 @@ var _ = Describe("Connection", func() {
 		})
 
 		It("closes due to a stateless reset", func() {
-			token := protocol.StatelessResetToken{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}
 			runConn()
 			gomock.InOrder(
 				tracer.EXPECT().ClosedConnection(gomock.Any()).Do(func(e error) {
-					var srErr *StatelessResetError
-					Expect(errors.As(e, &srErr)).To(BeTrue())
-					Expect(srErr.Token).To(Equal(token))
+					Expect(errors.Is(e, &qerr.StatelessResetError{})).To(BeTrue())
 				}),
 				tracer.EXPECT().Close(),
 			)
 			streamManager.EXPECT().CloseWithError(gomock.Any())
 			connRunner.EXPECT().Remove(gomock.Any()).AnyTimes()
 			cryptoSetup.EXPECT().Close()
-			conn.destroy(&StatelessResetError{Token: token})
+			conn.destroy(&StatelessResetError{})
 		})
 	})
 
diff --git a/integrationtests/self/mtu_test.go b/integrationtests/self/mtu_test.go
index 744d9fa0..cd96ad28 100644
--- a/integrationtests/self/mtu_test.go
+++ b/integrationtests/self/mtu_test.go
@@ -72,7 +72,7 @@ var _ = Describe("DPLPMTUD", func() {
 		defer proxy.Close()
 
 		// Make sure to use v4-only socket here.
-		// We can't reliably set the DF bit on dual-stack sockets on macOS.
+		// We can't reliably set the DF bit on dual-stack sockets on macOS before Sequoia (macOS 15).
 		udpConn, err := net.ListenUDP("udp4", &net.UDPAddr{IP: net.IPv4(127, 0, 0, 1), Port: 0})
 		Expect(err).ToNot(HaveOccurred())
 		defer udpConn.Close()
diff --git a/internal/qerr/errors_test.go b/internal/qerr/errors_test.go
index fabac98d..5e39c170 100644
--- a/internal/qerr/errors_test.go
+++ b/internal/qerr/errors_test.go
@@ -117,9 +117,7 @@ func TestVersionNegotiationErrorString(t *testing.T) {
 }
 
 func TestStatelessResetErrorString(t *testing.T) {
-	token := protocol.StatelessResetToken{0x0, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8, 0x9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf}
-	err := &StatelessResetError{Token: token}
-	require.Equal(t, "received a stateless reset with token 000102030405060708090a0b0c0d0e0f", err.Error())
+	require.Equal(t, "received a stateless reset", (&StatelessResetError{}).Error())
 }
 
 func TestStatelessResetErrorIsNetError(t *testing.T) {
diff --git a/qlog/connection_tracer_test.go b/qlog/connection_tracer_test.go
index 50c19300..9869430f 100644
--- a/qlog/connection_tracer_test.go
+++ b/qlog/connection_tracer_test.go
@@ -134,18 +134,15 @@ func TestHandshakeTimeouts(t *testing.T) {
 
 func TestReceivedStatelessResetPacket(t *testing.T) {
 	tracer, buf := newConnectionTracer()
-	tracer.ClosedConnection(&quic.StatelessResetError{
-		Token: protocol.StatelessResetToken{0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99, 0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff},
-	})
+	tracer.ClosedConnection(&quic.StatelessResetError{})
 	tracer.Close()
 	entry := exportAndParseSingle(t, buf)
 	require.WithinDuration(t, time.Now(), entry.Time, scaleDuration(10*time.Millisecond))
 	require.Equal(t, "transport:connection_closed", entry.Name)
 	ev := entry.Event
-	require.Len(t, ev, 3)
+	require.Len(t, ev, 2)
 	require.Equal(t, "remote", ev["owner"])
 	require.Equal(t, "stateless_reset", ev["trigger"])
-	require.Equal(t, "00112233445566778899aabbccddeeff", ev["stateless_reset_token"])
 }
 
 func TestVersionNegotiationFailure(t *testing.T) {
diff --git a/sys_conn_df_darwin_test.go b/sys_conn_df_darwin_test.go
new file mode 100644
index 00000000..335003ef
--- /dev/null
+++ b/sys_conn_df_darwin_test.go
@@ -0,0 +1,99 @@
+package quic
+
+import (
+	"net"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestIPFragmentation(t *testing.T) {
+	sink, err := net.ListenUDP("udp", &net.UDPAddr{Port: 0})
+	require.NoError(t, err)
+	t.Cleanup(func() { sink.Close() })
+	sinkPort := sink.LocalAddr().(*net.UDPAddr).Port
+
+	canSendIPv4 := func(conn *net.UDPConn) bool {
+		_, err := conn.WriteTo([]byte("hello"), &net.UDPAddr{IP: net.IPv4(127, 0, 0, 1), Port: sinkPort})
+		return err == nil
+	}
+
+	canSendIPv6 := func(conn *net.UDPConn) bool {
+		_, err := conn.WriteTo([]byte("hello"), &net.UDPAddr{IP: net.IPv6loopback, Port: sinkPort})
+		return err == nil
+	}
+
+	t.Run("udp4", func(t *testing.T) {
+		conn, err := net.ListenUDP("udp4", &net.UDPAddr{IP: net.IPv4(127, 0, 0, 1), Port: 0})
+		require.NoError(t, err)
+		defer conn.Close()
+
+		require.True(t, canSendIPv4(conn))
+		require.False(t, canSendIPv6(conn))
+
+		raw, err := conn.SyscallConn()
+		require.NoError(t, err)
+		canDF, _ := setDF(raw)
+		require.True(t, canDF)
+	})
+
+	t.Run("udp6", func(t *testing.T) {
+		conn, err := net.ListenUDP("udp6", &net.UDPAddr{IP: net.IPv6loopback, Port: 0})
+		require.NoError(t, err)
+		defer conn.Close()
+
+		require.False(t, canSendIPv4(conn))
+		require.True(t, canSendIPv6(conn))
+
+		raw, err := conn.SyscallConn()
+		require.NoError(t, err)
+		canDF, _ := setDF(raw)
+		require.True(t, canDF)
+	})
+
+	t.Run("udp, dual-stack", func(t *testing.T) {
+		if version, err := getMacOSVersion(); err != nil || version < macOSVersion15 {
+			t.Skipf("skipping on darwin %d", version-9)
+		}
+
+		conn, err := net.ListenUDP("udp", &net.UDPAddr{Port: 0})
+		require.NoError(t, err)
+		defer conn.Close()
+
+		require.True(t, canSendIPv4(conn))
+		require.True(t, canSendIPv6(conn))
+
+		raw, err := conn.SyscallConn()
+		require.NoError(t, err)
+		canDF, _ := setDF(raw)
+		require.True(t, canDF)
+	})
+
+	t.Run("udp, listening on IPv4", func(t *testing.T) {
+		conn, err := net.ListenUDP("udp", &net.UDPAddr{IP: net.IPv4(127, 0, 0, 1), Port: 0})
+		require.NoError(t, err)
+		defer conn.Close()
+
+		require.True(t, canSendIPv4(conn))
+		require.False(t, canSendIPv6(conn))
+
+		raw, err := conn.SyscallConn()
+		require.NoError(t, err)
+		canDF, _ := setDF(raw)
+		require.True(t, canDF)
+	})
+
+	t.Run("udp, listening on IPv6", func(t *testing.T) {
+		conn, err := net.ListenUDP("udp6", &net.UDPAddr{IP: net.IPv6loopback, Port: 0})
+		require.NoError(t, err)
+		defer conn.Close()
+
+		require.False(t, canSendIPv4(conn))
+		require.True(t, canSendIPv6(conn))
+
+		raw, err := conn.SyscallConn()
+		require.NoError(t, err)
+		canDF, _ := setDF(raw)
+		require.True(t, canDF)
+	})
+}
diff --git a/transport_test.go b/transport_test.go
index aeeb1153..9a7718c8 100644
--- a/transport_test.go
+++ b/transport_test.go
@@ -265,7 +265,7 @@ var _ = Describe("Transport", func() {
 			phm.EXPECT().Get(connID),
 			phm.EXPECT().GetByResetToken(token).Return(conn, true),
 			conn.EXPECT().destroy(gomock.Any()).Do(func(err error) {
-				Expect(err).To(MatchError(&StatelessResetError{Token: token}))
+				Expect(err).To(MatchError(&StatelessResetError{}))
 				close(destroyed)
 			}),
 		)