improve the packet API (#30)

Author: marten-seemann

conn.go

@@ -243,7 +243,7 @@ func (c *Conn) writeToStream() error {
 	}
 }
 
-func (c *Conn) Read(b []byte) (n int, err error) {
+func (c *Conn) ReadPacket(b []byte) (n int, err error) {
 start:
 	data, err := c.str.ReceiveDatagram(context.Background())
 	if err != nil {
@@ -339,34 +339,31 @@ func (c *Conn) handleIncomingProxiedPacket(data []byte) error {
 	return nil
 }
 
-type PacketTooBigError struct {
-	ICMPPacket []byte
-}
-
-func (e *PacketTooBigError) Error() string { return "connect-ip: packet too big" }
-
-func (c *Conn) Write(b []byte) (n int, err error) {
+// WritePacket writes an IP packet to the stream.
+// If sending the packet fails, it might return an ICMP packet.
+// It is the caller's responsibility to send the ICMP packet to the sender.
+func (c *Conn) WritePacket(b []byte) (icmp []byte, err error) {
 	data, err := c.composeDatagram(b)
 	if err != nil {
 		log.Printf("dropping proxied packet (%d bytes) that can't be proxied: %s", len(b), err)
-		return 0, nil
+		return nil, nil
 	}
 	if err := c.str.SendDatagram(data); err != nil {
 		if errors.Is(err, &quic.DatagramTooLargeError{}) {
 			icmpPacket, err := composeICMPTooLargePacket(b, minMTU)
 			if err != nil {
 				log.Printf("failed to compose ICMP too large packet: %s", err)
 			}
-			return 0, &PacketTooBigError{ICMPPacket: icmpPacket}
+			return icmpPacket, nil
 		}
 		select {
 		case <-c.closeChan:
-			return 0, c.closeErr
+			return nil, c.closeErr
 		default:
-			return 0, err
+			return nil, err
 		}
 	}
-	return len(b), nil
+	return nil, nil
 }
 
 func (c *Conn) composeDatagram(b []byte) ([]byte, error) {

conn_test.go

@@ -275,8 +275,7 @@ func TestSendLargeDatagrams(t *testing.T) {
 		Protocol: 17,
 	}).Marshal()
 	require.NoError(t, err)
-	_, err = conn.Write(data)
-	var pktTooBigErr *PacketTooBigError
-	require.ErrorAs(t, err, &pktTooBigErr)
-	require.NotNil(t, pktTooBigErr.ICMPPacket)
+	icmp, err := conn.WritePacket(data)
+	require.NoError(t, err)
+	require.NotNil(t, icmp)
 }

integration/client/client.go

@@ -204,7 +204,7 @@ func proxy(ipconn *connectip.Conn, dev *water.Interface) error {
 	go func() {
 		for {
 			b := make([]byte, 1500)
-			n, err := ipconn.Read(b)
+			n, err := ipconn.ReadPacket(b)
 			if err != nil {
 				errChan <- fmt.Errorf("failed to read from connection: %w", err)
 				return
@@ -226,19 +226,17 @@ func proxy(ipconn *connectip.Conn, dev *water.Interface) error {
 				return
 			}
 			log.Printf("read %d bytes from TUN", n)
-			if _, err := ipconn.Write(b[:n]); err != nil {
-				var tooBigErr *connectip.PacketTooBigError
-				if errors.As(err, &tooBigErr) {
-					if len(tooBigErr.ICMPPacket) > 0 {
-						if _, err := dev.Write(tooBigErr.ICMPPacket); err != nil {
-							log.Printf("faield to write ICMP packet to %s: %v", dev.Name(), err)
-						}
-					}
-					continue
-				}
+			icmp, err := ipconn.WritePacket(b[:n])
+			if err != nil {
 				errChan <- fmt.Errorf("failed to write to connection: %w", err)
 				return
 			}
+			if len(icmp) > 0 {
+				log.Printf("sending ICMP packet on %s", dev.Name())
+				if _, err := dev.Write(icmp); err != nil {
+					log.Printf("failed to write ICMP packet: %v", err)
+				}
+			}
 		}
 	}()
 

integration/proxy/proxy.go

@@ -5,7 +5,6 @@ package main
 import (
 	"context"
 	"crypto/tls"
-	"errors"
 	"fmt"
 	"log"
 	"net"
@@ -217,7 +216,7 @@ func handleConn(conn *connectip.Conn, addr netip.Addr, route netip.Prefix, ipPro
 	go func() {
 		for {
 			b := make([]byte, 1500)
-			n, err := conn.Read(b)
+			n, err := conn.ReadPacket(b)
 			if err != nil {
 				errChan <- fmt.Errorf("failed to read from connection: %w", err)
 				return
@@ -239,21 +238,16 @@ func handleConn(conn *connectip.Conn, addr netip.Addr, route netip.Prefix, ipPro
 				return
 			}
 			log.Printf("read %d bytes from %s", n, ifaceName)
-			if _, err := conn.Write(b[:n]); err != nil {
-				var tooBigErr *connectip.PacketTooBigError
-				if errors.As(err, &tooBigErr) {
-					if len(tooBigErr.ICMPPacket) > 0 {
-						if err := sendOnSocket(serverSocketSend, tooBigErr.ICMPPacket); err != nil {
-							errChan <- fmt.Errorf("writing to server socket: %w", err)
-							return
-						}
-					}
-					continue
-				}
-
+			icmp, err := conn.WritePacket(b[:n])
+			if err != nil {
 				errChan <- fmt.Errorf("failed to write to connection: %w", err)
 				return
 			}
+			if len(icmp) > 0 {
+				if err := sendOnSocket(serverSocketSend, icmp); err != nil {
+					log.Printf("failed to send ICMP packet: %v", err)
+				}
+			}
 		}
 	}()
 

proxy_test.go

@@ -159,8 +159,9 @@ func TestTTLs(t *testing.T) {
 		}
 		packetTTL1, err := hdrTTL1.Marshal()
 		require.NoError(t, err)
-		_, err = client.Write(packetTTL1)
+		icmp, err := client.WritePacket(packetTTL1)
 		require.NoError(t, err)
+		require.Empty(t, icmp)
 
 		// now send a packet with TTL 42
 		hdr := &ipv4.Header{
@@ -171,11 +172,12 @@ func TestTTLs(t *testing.T) {
 		}
 		packet, err := hdr.Marshal()
 		require.NoError(t, err)
-		_, err = client.Write(packet)
+		icmp, err = client.WritePacket(packet)
 		require.NoError(t, err)
+		require.Empty(t, icmp)
 
 		receivedPacket := make([]byte, 1500)
-		n, err := server.Read(receivedPacket)
+		n, err := server.ReadPacket(receivedPacket)
 		require.NoError(t, err)
 		receivedPacket = receivedPacket[:n]
 
@@ -204,8 +206,9 @@ func TestTTLs(t *testing.T) {
 			0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, // Source IP
 			0x20, 0x01, 0x48, 0x60, 0x48, 0x60, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x88, 0x88, // Destination IP
 		}
-		_, err := client.Write(packetHopLimit1)
+		icmp, err := client.WritePacket(packetHopLimit1)
 		require.NoError(t, err)
+		require.Empty(t, icmp)
 
 		// now send a packet with Hop Limit 42
 		packet := []byte{
@@ -215,11 +218,12 @@ func TestTTLs(t *testing.T) {
 			0x20, 0x01, 0x0d, 0xb8, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, // Source IP
 			0x20, 0x01, 0x48, 0x60, 0x48, 0x60, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x88, 0x88, // Destination IP
 		}
-		_, err = client.Write(packet)
+		icmp, err = client.WritePacket(packet)
 		require.NoError(t, err)
+		require.Empty(t, icmp)
 
 		receivedPacket := make([]byte, 1500)
-		n, err := server.Read(receivedPacket)
+		n, err := server.ReadPacket(receivedPacket)
 		require.NoError(t, err)
 		receivedPacket = receivedPacket[:n]
 
@@ -269,9 +273,9 @@ func TestClosing(t *testing.T) {
 		}),
 		net.ErrClosed,
 	)
-	_, err = client.Read([]byte{0})
+	_, err = client.ReadPacket([]byte{0})
 	require.ErrorIs(t, err, net.ErrClosed)
-	_, err = client.Write(ipv6Packet)
+	_, err = client.WritePacket(ipv6Packet)
 	require.ErrorIs(t, err, net.ErrClosed)
 
 	select {
@@ -288,8 +292,8 @@ func TestClosing(t *testing.T) {
 		t.Fatal("timeout")
 	}
 
-	_, err = server.Read([]byte{0})
+	_, err = server.ReadPacket([]byte{0})
 	require.ErrorIs(t, err, net.ErrClosed)
-	_, err = server.Write(ipv6Packet)
+	_, err = server.WritePacket(ipv6Packet)
 	require.ErrorIs(t, err, net.ErrClosed)
 }