Skip to content

Commit 588ab7e

Browse files
DevinCarrGoncaloGarcia
authored andcommitted
TUN-8640: Add ICMP support for datagram V3
Closes TUN-8640
1 parent dfbccd9 commit 588ab7e

File tree

10 files changed

+491
-27
lines changed

10 files changed

+491
-27
lines changed

connection/quic_connection_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -752,7 +752,8 @@ func testTunnelConnection(t *testing.T, serverAddr netip.AddrPort, index uint8)
752752
sessionDemuxChan := make(chan *packet.Session, 4)
753753
datagramMuxer := cfdquic.NewDatagramMuxerV2(conn, &log, sessionDemuxChan)
754754
sessionManager := datagramsession.NewManager(&log, datagramMuxer.SendToSession, sessionDemuxChan)
755-
packetRouter := ingress.NewPacketRouter(nil, datagramMuxer, 0, &log)
755+
var connIndex uint8 = 0
756+
packetRouter := ingress.NewPacketRouter(nil, datagramMuxer, connIndex, &log)
756757

757758
datagramConn := &datagramV2Connection{
758759
conn,

connection/quic_datagram_v3.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"github.com/quic-go/quic-go"
1111
"github.com/rs/zerolog"
1212

13+
"github.com/cloudflare/cloudflared/ingress"
1314
"github.com/cloudflare/cloudflared/management"
1415
cfdquic "github.com/cloudflare/cloudflared/quic/v3"
1516
"github.com/cloudflare/cloudflared/tunnelrpc/pogs"
@@ -25,6 +26,7 @@ type datagramV3Connection struct {
2526
func NewDatagramV3Connection(ctx context.Context,
2627
conn quic.Connection,
2728
sessionManager cfdquic.SessionManager,
29+
icmpRouter ingress.ICMPRouter,
2830
index uint8,
2931
metrics cfdquic.Metrics,
3032
logger *zerolog.Logger,
@@ -34,7 +36,7 @@ func NewDatagramV3Connection(ctx context.Context,
3436
Int(management.EventTypeKey, int(management.UDP)).
3537
Uint8(LogFieldConnIndex, index).
3638
Logger()
37-
datagramMuxer := cfdquic.NewDatagramConn(conn, sessionManager, index, metrics, &log)
39+
datagramMuxer := cfdquic.NewDatagramConn(conn, sessionManager, icmpRouter, index, metrics, &log)
3840

3941
return &datagramV3Connection{
4042
conn,

quic/v3/datagram.go

Lines changed: 63 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -222,11 +222,11 @@ const (
222222
//
223223
// This method should be used in-place of MarshalBinary which will allocate in-place the required byte array to return.
224224
func MarshalPayloadHeaderTo(requestID RequestID, payload []byte) error {
225-
if len(payload) < 17 {
225+
if len(payload) < DatagramPayloadHeaderLen {
226226
return wrapMarshalErr(ErrDatagramPayloadHeaderTooSmall)
227227
}
228228
payload[0] = byte(UDPSessionPayloadType)
229-
return requestID.MarshalBinaryTo(payload[1:17])
229+
return requestID.MarshalBinaryTo(payload[1:DatagramPayloadHeaderLen])
230230
}
231231

232232
func (s *UDPSessionPayloadDatagram) UnmarshalBinary(data []byte) error {
@@ -239,18 +239,18 @@ func (s *UDPSessionPayloadDatagram) UnmarshalBinary(data []byte) error {
239239
}
240240

241241
// Make sure that the slice provided is the right size to be parsed.
242-
if len(data) < 17 || len(data) > maxPayloadPlusHeaderLen {
242+
if len(data) < DatagramPayloadHeaderLen || len(data) > maxPayloadPlusHeaderLen {
243243
return wrapUnmarshalErr(ErrDatagramPayloadInvalidSize)
244244
}
245245

246-
requestID, err := RequestIDFromSlice(data[1:17])
246+
requestID, err := RequestIDFromSlice(data[1:DatagramPayloadHeaderLen])
247247
if err != nil {
248248
return wrapUnmarshalErr(err)
249249
}
250250

251251
*s = UDPSessionPayloadDatagram{
252252
RequestID: requestID,
253-
Payload: data[17:],
253+
Payload: data[DatagramPayloadHeaderLen:],
254254
}
255255
return nil
256256
}
@@ -370,3 +370,61 @@ func (s *UDPSessionRegistrationResponseDatagram) UnmarshalBinary(data []byte) er
370370
}
371371
return nil
372372
}
373+
374+
// ICMPDatagram is used to propagate ICMPv4 and ICMPv6 payloads.
375+
type ICMPDatagram struct {
376+
Payload []byte
377+
}
378+
379+
// The maximum size that an ICMP packet can be.
380+
const maxICMPPayloadLen = maxDatagramPayloadLen
381+
382+
// The datagram structure for ICMPDatagram is:
383+
//
384+
// 0 1 2 3 4 5 6 7 0 1 2 3 4 5 6 7 0 1 2 3 4 5 6 7 0 1 2 3 4 5 6 7
385+
// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
386+
// 0| Type | |
387+
// +-+-+-+-+-+-+-+-+ +
388+
// . Payload .
389+
// . .
390+
// . .
391+
// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
392+
393+
func (d *ICMPDatagram) MarshalBinary() (data []byte, err error) {
394+
if len(d.Payload) > maxICMPPayloadLen {
395+
return nil, wrapMarshalErr(ErrDatagramICMPPayloadTooLarge)
396+
}
397+
// We shouldn't attempt to marshal an ICMP datagram with no ICMP payload provided
398+
if len(d.Payload) == 0 {
399+
return nil, wrapMarshalErr(ErrDatagramICMPPayloadMissing)
400+
}
401+
// Make room for the 1 byte ICMPType header
402+
datagram := make([]byte, len(d.Payload)+datagramTypeLen)
403+
datagram[0] = byte(ICMPType)
404+
copy(datagram[1:], d.Payload)
405+
return datagram, nil
406+
}
407+
408+
func (d *ICMPDatagram) UnmarshalBinary(data []byte) error {
409+
datagramType, err := ParseDatagramType(data)
410+
if err != nil {
411+
return wrapUnmarshalErr(err)
412+
}
413+
if datagramType != ICMPType {
414+
return wrapUnmarshalErr(ErrInvalidDatagramType)
415+
}
416+
417+
if len(data[1:]) > maxDatagramPayloadLen {
418+
return wrapUnmarshalErr(ErrDatagramICMPPayloadTooLarge)
419+
}
420+
421+
// We shouldn't attempt to unmarshal an ICMP datagram with no ICMP payload provided
422+
if len(data[1:]) == 0 {
423+
return wrapUnmarshalErr(ErrDatagramICMPPayloadMissing)
424+
}
425+
426+
payload := make([]byte, len(data[1:]))
427+
copy(payload, data[1:])
428+
d.Payload = payload
429+
return nil
430+
}

quic/v3/datagram_errors.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ var (
1515
ErrDatagramResponseInvalidSize error = errors.New("datagram response is an invalid size")
1616
ErrDatagramResponseMsgTooLargeMaximum error = fmt.Errorf("datagram response error message length exceeds the length of the datagram maximum: %d", maxResponseErrorMessageLen)
1717
ErrDatagramResponseMsgTooLargeDatagram error = fmt.Errorf("datagram response error message length exceeds the length of the provided datagram")
18+
ErrDatagramICMPPayloadTooLarge error = fmt.Errorf("datagram icmp payload exceeds %d bytes", maxICMPPayloadLen)
19+
ErrDatagramICMPPayloadMissing error = errors.New("datagram icmp payload is missing")
1820
)
1921

2022
func wrapMarshalErr(err error) error {

quic/v3/datagram_test.go

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,12 @@ func TestTypeUnmarshalErrors(t *testing.T) {
160160
if !errors.Is(err, v3.ErrDatagramHeaderTooSmall) {
161161
t.Errorf("expected invalid length to throw error")
162162
}
163+
164+
d4 := v3.ICMPDatagram{}
165+
err = d4.UnmarshalBinary([]byte{})
166+
if !errors.Is(err, v3.ErrDatagramHeaderTooSmall) {
167+
t.Errorf("expected invalid length to throw error")
168+
}
163169
})
164170

165171
t.Run("invalid types", func(t *testing.T) {
@@ -180,6 +186,12 @@ func TestTypeUnmarshalErrors(t *testing.T) {
180186
if !errors.Is(err, v3.ErrInvalidDatagramType) {
181187
t.Errorf("expected invalid type to throw error")
182188
}
189+
190+
d4 := v3.ICMPDatagram{}
191+
err = d4.UnmarshalBinary([]byte{byte(v3.UDPSessionPayloadType)})
192+
if !errors.Is(err, v3.ErrInvalidDatagramType) {
193+
t.Errorf("expected invalid type to throw error")
194+
}
183195
})
184196
}
185197

@@ -343,6 +355,54 @@ func TestSessionRegistrationResponse(t *testing.T) {
343355
})
344356
}
345357

358+
func TestICMPDatagram(t *testing.T) {
359+
t.Run("basic", func(t *testing.T) {
360+
payload := makePayload(128)
361+
datagram := v3.ICMPDatagram{Payload: payload}
362+
marshaled, err := datagram.MarshalBinary()
363+
if err != nil {
364+
t.Error(err)
365+
}
366+
unmarshaled := &v3.ICMPDatagram{}
367+
err = unmarshaled.UnmarshalBinary(marshaled)
368+
if err != nil {
369+
t.Error(err)
370+
}
371+
require.Equal(t, payload, unmarshaled.Payload)
372+
})
373+
374+
t.Run("payload size empty", func(t *testing.T) {
375+
payload := []byte{}
376+
datagram := v3.ICMPDatagram{Payload: payload}
377+
_, err := datagram.MarshalBinary()
378+
if !errors.Is(err, v3.ErrDatagramICMPPayloadMissing) {
379+
t.Errorf("expected an error: %s", err)
380+
}
381+
payload = []byte{byte(v3.ICMPType)}
382+
unmarshaled := &v3.ICMPDatagram{}
383+
err = unmarshaled.UnmarshalBinary(payload)
384+
if !errors.Is(err, v3.ErrDatagramICMPPayloadMissing) {
385+
t.Errorf("expected an error: %s", err)
386+
}
387+
})
388+
389+
t.Run("payload size too large", func(t *testing.T) {
390+
payload := makePayload(1280 + 1) // larger than the datagram size could be
391+
datagram := v3.ICMPDatagram{Payload: payload}
392+
_, err := datagram.MarshalBinary()
393+
if !errors.Is(err, v3.ErrDatagramICMPPayloadTooLarge) {
394+
t.Errorf("expected an error: %s", err)
395+
}
396+
payload = makePayload(1280 + 2) // larger than the datagram size could be + header
397+
payload[0] = byte(v3.ICMPType)
398+
unmarshaled := &v3.ICMPDatagram{}
399+
err = unmarshaled.UnmarshalBinary(payload)
400+
if !errors.Is(err, v3.ErrDatagramICMPPayloadTooLarge) {
401+
t.Errorf("expected an error: %s", err)
402+
}
403+
})
404+
}
405+
346406
func compareRegistrationDatagrams(t *testing.T, l *v3.UDPSessionRegistrationDatagram, r *v3.UDPSessionRegistrationDatagram) bool {
347407
require.Equal(t, l.Payload, r.Payload)
348408
return l.RequestID == r.RequestID &&
@@ -377,3 +437,13 @@ func FuzzRegistrationResponseDatagram(f *testing.F) {
377437
}
378438
})
379439
}
440+
441+
func FuzzICMPDatagram(f *testing.F) {
442+
f.Fuzz(func(t *testing.T, data []byte) {
443+
unmarshaled := v3.ICMPDatagram{}
444+
err := unmarshaled.UnmarshalBinary(data)
445+
if err == nil {
446+
_, _ = unmarshaled.MarshalBinary()
447+
}
448+
})
449+
}

quic/v3/icmp.go

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
package v3
2+
3+
import (
4+
"context"
5+
6+
"github.com/rs/zerolog"
7+
"go.opentelemetry.io/otel/trace"
8+
9+
"github.com/cloudflare/cloudflared/ingress"
10+
"github.com/cloudflare/cloudflared/packet"
11+
"github.com/cloudflare/cloudflared/tracing"
12+
)
13+
14+
// packetResponder is an implementation of the [ingress.ICMPResponder] which provides the ICMP Flow manager the
15+
// return path to return and ICMP Echo response back to the QUIC muxer.
16+
type packetResponder struct {
17+
datagramMuxer DatagramICMPWriter
18+
connID uint8
19+
}
20+
21+
func newPacketResponder(datagramMuxer DatagramICMPWriter, connID uint8) ingress.ICMPResponder {
22+
return &packetResponder{
23+
datagramMuxer,
24+
connID,
25+
}
26+
}
27+
28+
func (pr *packetResponder) ConnectionIndex() uint8 {
29+
return pr.connID
30+
}
31+
32+
func (pr *packetResponder) ReturnPacket(pk *packet.ICMP) error {
33+
return pr.datagramMuxer.SendICMPPacket(pk)
34+
}
35+
36+
func (pr *packetResponder) AddTraceContext(tracedCtx *tracing.TracedContext, serializedIdentity []byte) {
37+
// datagram v3 does not support tracing ICMP packets
38+
}
39+
40+
func (pr *packetResponder) RequestSpan(ctx context.Context, pk *packet.ICMP) (context.Context, trace.Span) {
41+
// datagram v3 does not support tracing ICMP packets
42+
return ctx, tracing.NewNoopSpan()
43+
}
44+
45+
func (pr *packetResponder) ReplySpan(ctx context.Context, logger *zerolog.Logger) (context.Context, trace.Span) {
46+
// datagram v3 does not support tracing ICMP packets
47+
return ctx, tracing.NewNoopSpan()
48+
}
49+
50+
func (pr *packetResponder) ExportSpan() {
51+
// datagram v3 does not support tracing ICMP packets
52+
}

quic/v3/icmp_test.go

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
package v3_test
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
"github.com/cloudflare/cloudflared/ingress"
8+
"github.com/cloudflare/cloudflared/packet"
9+
)
10+
11+
type noopICMPRouter struct{}
12+
13+
func (noopICMPRouter) Request(ctx context.Context, pk *packet.ICMP, responder ingress.ICMPResponder) error {
14+
return nil
15+
}
16+
func (noopICMPRouter) ConvertToTTLExceeded(pk *packet.ICMP, rawPacket packet.RawPacket) *packet.ICMP {
17+
return nil
18+
}
19+
20+
type mockICMPRouter struct {
21+
recv chan *packet.ICMP
22+
}
23+
24+
func newMockICMPRouter() *mockICMPRouter {
25+
return &mockICMPRouter{
26+
recv: make(chan *packet.ICMP, 1),
27+
}
28+
}
29+
30+
func (m *mockICMPRouter) Request(ctx context.Context, pk *packet.ICMP, responder ingress.ICMPResponder) error {
31+
m.recv <- pk
32+
return nil
33+
}
34+
func (mockICMPRouter) ConvertToTTLExceeded(pk *packet.ICMP, rawPacket packet.RawPacket) *packet.ICMP {
35+
return packet.NewICMPTTLExceedPacket(pk.IP, rawPacket, testLocalAddr.AddrPort().Addr())
36+
}
37+
38+
func assertICMPEqual(t *testing.T, expected *packet.ICMP, actual *packet.ICMP) {
39+
if expected.Src != actual.Src {
40+
t.Fatalf("Src address not equal: %+v\t%+v", expected, actual)
41+
}
42+
if expected.Dst != actual.Dst {
43+
t.Fatalf("Dst address not equal: %+v\t%+v", expected, actual)
44+
}
45+
}

0 commit comments

Comments
 (0)