Skip to content

Commit 7d3166a

Browse files
wadeywadey
authored
cleanup ipv6 iputil helpers / skip reject for ICMP error packets and fragments (#1768)
* cleanup ipv6 iputil helpers With my refactoring in this PR I accidentally had some duplicate logic, this PR cleans it up: - #1766 * skip ICMP reject for ICMP error packets and fragments Per RFC 1122, ICMP error messages must not be generated in response to other ICMP error messages to prevent infinite error loops. This applies to both IPv4 (types 3, 4, 5, 11, 12) and IPv6 (types 1-4). Do not generate reject packets for IPv4 or IPv6 fragments. For IPv4, check MF flag and fragment offset. For IPv6, add isFragment return to ipv6FindUpperProtocol so a single traversal handles both protocol lookup and fragment detection. * do send rejects for the initial fragment RFC says "non-initial fragment"s * fix fragment checks
1 parent fe1c568 commit 7d3166a

2 files changed

Lines changed: 167 additions & 43 deletions

File tree

‎iputil/packet.go‎

Lines changed: 35 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,10 @@ func CreateRejectPacket(packet []byte, out []byte) []byte {
3535
if len(packet) < ipv4.HeaderLen {
3636
return nil
3737
}
38+
// Do not send reject packets for non-first fragments
39+
if packet[6]&0x1f != 0 || packet[7] != 0 {
40+
return nil
41+
}
3842
switch packet[9] {
3943
case 6: // tcp
4044
return ipv4CreateRejectTCPPacket(packet, out)
@@ -59,6 +63,14 @@ func ipv4CreateRejectICMPPacket(packet []byte, out []byte) []byte {
5963
return nil
6064
}
6165

66+
// Do not generate ICMP errors in response to ICMP error packets
67+
if packet[9] == 1 && len(packet) > ihl {
68+
icmpType := packet[ihl]
69+
if icmpType == 3 || icmpType == 4 || icmpType == 5 || icmpType == 11 || icmpType == 12 {
70+
return nil
71+
}
72+
}
73+
6274
// ICMP reply includes original header and first 8 bytes of the packet
6375
packetLen := min(len(packet), ihl+8)
6476

@@ -187,49 +199,27 @@ func ipv4CreateRejectTCPPacket(packet []byte, out []byte) []byte {
187199
}
188200

189201
func ipv6CreateRejectPacket(packet []byte, out []byte) []byte {
190-
proto := ipv6FindUpperProtocol(packet)
202+
proto, offset, isFragment := ipv6FindUpperProtocol(packet)
203+
if isFragment {
204+
return nil
205+
}
191206
switch proto {
192207
case 6: // tcp
193-
return ipv6CreateRejectTCPPacket(packet, out)
208+
return ipv6CreateRejectTCPPacket(packet, out, offset)
194209
default:
195-
return ipv6CreateRejectICMPPacket(packet, out)
210+
return ipv6CreateRejectICMPPacket(packet, out, proto, offset)
196211
}
197212
}
198213

199-
func ipv6FindUpperProtocol(packet []byte) uint8 {
200-
nextHeader := packet[6]
201-
offset := ipv6.HeaderLen
202-
203-
for {
204-
switch nextHeader {
205-
case 0, 43, 60: // Hop-by-Hop, Routing, Destination
206-
if len(packet) < offset+2 {
207-
return nextHeader
208-
}
209-
nextHeader = packet[offset]
210-
offset += int(packet[offset+1]+1) << 3
211-
212-
case 44: // Fragment
213-
if len(packet) < offset+8 {
214-
return nextHeader
215-
}
216-
nextHeader = packet[offset]
217-
offset += 8
218-
219-
case 51: // AH
220-
if len(packet) < offset+2 {
221-
return nextHeader
222-
}
223-
nextHeader = packet[offset]
224-
offset += int(packet[offset+1]+2) << 2
225-
226-
default:
227-
return nextHeader
214+
func ipv6CreateRejectICMPPacket(packet []byte, out []byte, proto uint8, offset int) []byte {
215+
// Do not generate ICMPv6 errors in response to ICMPv6 error packets
216+
if proto == 58 && len(packet) > offset {
217+
icmpType := packet[offset]
218+
if icmpType >= 1 && icmpType <= 4 {
219+
return nil
228220
}
229221
}
230-
}
231222

232-
func ipv6CreateRejectICMPPacket(packet []byte, out []byte) []byte {
233223
// Include as much of the original packet as possible, up to 1000 bytes,
234224
// so the response fits comfortably within any tunnel MTU.
235225
packetLen := min(len(packet), 1000)
@@ -277,10 +267,9 @@ func ipv6CreateRejectICMPPacket(packet []byte, out []byte) []byte {
277267
return out
278268
}
279269

280-
func ipv6CreateRejectTCPPacket(packet []byte, out []byte) []byte {
270+
func ipv6CreateRejectTCPPacket(packet []byte, out []byte, offset int) []byte {
281271
const tcpLen = 20
282272

283-
offset := ipv6FindUpperProtocolOffset(packet)
284273
if len(packet) < offset+tcpLen {
285274
return nil
286275
}
@@ -344,35 +333,38 @@ func ipv6CreateRejectTCPPacket(packet []byte, out []byte) []byte {
344333
return out
345334
}
346335

347-
func ipv6FindUpperProtocolOffset(packet []byte) int {
348-
nextHeader := packet[6]
349-
offset := ipv6.HeaderLen
336+
func ipv6FindUpperProtocol(packet []byte) (nextHeader uint8, offset int, isFragment bool) {
337+
nextHeader = packet[6]
338+
offset = ipv6.HeaderLen
350339

351340
for {
352341
switch nextHeader {
353342
case 0, 43, 60: // Hop-by-Hop, Routing, Destination
354343
if len(packet) < offset+2 {
355-
return offset
344+
return nextHeader, offset, isFragment
356345
}
357346
nextHeader = packet[offset]
358347
offset += int(packet[offset+1]+1) << 3
359348

360349
case 44: // Fragment
361350
if len(packet) < offset+8 {
362-
return offset
351+
return nextHeader, offset, isFragment
352+
}
353+
if packet[offset+2] != 0 || packet[offset+3]&0xf8 != 0 {
354+
isFragment = true
363355
}
364356
nextHeader = packet[offset]
365357
offset += 8
366358

367359
case 51: // AH
368360
if len(packet) < offset+2 {
369-
return offset
361+
return nextHeader, offset, isFragment
370362
}
371363
nextHeader = packet[offset]
372364
offset += int(packet[offset+1]+2) << 2
373365

374366
default:
375-
return offset
367+
return nextHeader, offset, isFragment
376368
}
377369
}
378370
}

‎iputil/packet_test.go‎

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,111 @@ func Test_CreateRejectPacket(t *testing.T) {
7474
assert.Len(t, rejectPacket, expectedLen)
7575
}
7676

77+
func Test_CreateRejectPacket_NoFragment(t *testing.T) {
78+
out := make([]byte, MaxRejectPacketSize)
79+
80+
// IPv4: non-zero fragment offset should not generate reject packet
81+
h := ipv4.Header{
82+
Len: 20,
83+
Src: net.IPv4(10, 0, 0, 1),
84+
Dst: net.IPv4(10, 0, 0, 2),
85+
Protocol: 17, // UDP
86+
}
87+
b, err := h.Marshal()
88+
if err != nil {
89+
t.Fatalf("h.Marshal: %v", err)
90+
}
91+
b = append(b, make([]byte, 8)...)
92+
// Set fragment offset to non-zero (byte 6-7, offset in 8-byte units)
93+
b[6] = 0x00
94+
b[7] = 0x01
95+
assert.Nil(t, CreateRejectPacket(b, out))
96+
97+
// MF flag with zero offset (first fragment) should still generate reject
98+
b[6] = 0x20 // MF flag set
99+
b[7] = 0x00
100+
assert.NotNil(t, CreateRejectPacket(b, out))
101+
102+
// Non-fragment should still generate reject packet
103+
b[6] = 0x00
104+
b[7] = 0x00
105+
assert.NotNil(t, CreateRejectPacket(b, out))
106+
107+
// DF flag only (not a fragment) should still generate reject packet
108+
b[6] = 0x40
109+
b[7] = 0x00
110+
assert.NotNil(t, CreateRejectPacket(b, out))
111+
}
112+
113+
func Test_CreateRejectPacketIPv6_NoFragment(t *testing.T) {
114+
src := net.ParseIP("fd00::1")
115+
dst := net.ParseIP("fd00::2")
116+
out := make([]byte, MaxRejectPacketSize)
117+
118+
// IPv6 with Fragment header and non-zero offset should not generate reject
119+
fragHeader := []byte{
120+
17, // next header: UDP
121+
0, // reserved
122+
0, 9, // fragment offset=1 (shifted left 3), M=1
123+
0, 0, 0, 1, // identification
124+
}
125+
udpPayload := make([]byte, 8)
126+
payload := append(fragHeader, udpPayload...)
127+
packet := makeIPv6Packet(src, dst, 44, payload) // next header 44 = Fragment
128+
assert.Nil(t, CreateRejectPacket(packet, out))
129+
130+
// Fragment header with zero offset (first fragment) should still generate reject
131+
fragHeader[2] = 0
132+
fragHeader[3] = 1 // offset=0, M=1
133+
payload = append(fragHeader, udpPayload...)
134+
packet = makeIPv6Packet(src, dst, 44, payload)
135+
assert.NotNil(t, CreateRejectPacket(packet, out))
136+
}
137+
138+
func Test_CreateRejectPacket_NoICMPError(t *testing.T) {
139+
out := make([]byte, MaxRejectPacketSize)
140+
141+
// ICMP error types should not generate reject packets
142+
icmpErrorTypes := []byte{3, 4, 5, 11, 12}
143+
for _, icmpType := range icmpErrorTypes {
144+
h := ipv4.Header{
145+
Len: 20,
146+
Src: net.IPv4(10, 0, 0, 1),
147+
Dst: net.IPv4(10, 0, 0, 2),
148+
Protocol: 1, // ICMP
149+
}
150+
151+
b, err := h.Marshal()
152+
if err != nil {
153+
t.Fatalf("h.Marshal: %v", err)
154+
}
155+
b = append(b, icmpType, 0, 0, 0, 0, 0, 0, 0)
156+
157+
rejectPacket := CreateRejectPacket(b, out)
158+
assert.Nil(t, rejectPacket, "ICMP type %d should not generate a reject packet", icmpType)
159+
}
160+
161+
// ICMP non-error types should still generate reject packets
162+
icmpNonErrorTypes := []byte{0, 8, 13, 14}
163+
for _, icmpType := range icmpNonErrorTypes {
164+
h := ipv4.Header{
165+
Len: 20,
166+
Src: net.IPv4(10, 0, 0, 1),
167+
Dst: net.IPv4(10, 0, 0, 2),
168+
Protocol: 1, // ICMP
169+
}
170+
171+
b, err := h.Marshal()
172+
if err != nil {
173+
t.Fatalf("h.Marshal: %v", err)
174+
}
175+
b = append(b, icmpType, 0, 0, 0, 0, 0, 0, 0)
176+
177+
rejectPacket := CreateRejectPacket(b, out)
178+
assert.NotNil(t, rejectPacket, "ICMP type %d should generate a reject packet", icmpType)
179+
}
180+
}
181+
77182
func makeIPv6Packet(src, dst net.IP, nextHeader uint8, payload []byte) []byte {
78183
b := make([]byte, ipv6.HeaderLen+len(payload))
79184
b[0] = ipv6.Version << 4
@@ -197,6 +302,33 @@ func Test_CreateRejectPacketIPv6_TCPWithACK(t *testing.T) {
197302
assert.Equal(t, uint32(2000), binary.BigEndian.Uint32(tcpOut[4:]))
198303
}
199304

305+
func Test_CreateRejectPacketIPv6_NoICMPError(t *testing.T) {
306+
src := net.ParseIP("fd00::1")
307+
dst := net.ParseIP("fd00::2")
308+
out := make([]byte, MaxRejectPacketSize)
309+
310+
// ICMPv6 error types (1-4) should not generate reject packets
311+
for icmpType := byte(1); icmpType <= 4; icmpType++ {
312+
payload := make([]byte, 8)
313+
payload[0] = icmpType
314+
packet := makeIPv6Packet(src, dst, 58, payload)
315+
316+
rejectPacket := CreateRejectPacket(packet, out)
317+
assert.Nil(t, rejectPacket, "ICMPv6 type %d should not generate a reject packet", icmpType)
318+
}
319+
320+
// ICMPv6 non-error types should still generate reject packets
321+
nonErrorTypes := []byte{128, 129, 133, 134}
322+
for _, icmpType := range nonErrorTypes {
323+
payload := make([]byte, 8)
324+
payload[0] = icmpType
325+
packet := makeIPv6Packet(src, dst, 58, payload)
326+
327+
rejectPacket := CreateRejectPacket(packet, out)
328+
assert.NotNil(t, rejectPacket, "ICMPv6 type %d should generate a reject packet", icmpType)
329+
}
330+
}
331+
200332
func Test_CreateRejectPacketIPv6_TooShort(t *testing.T) {
201333
// Packet too short to be valid IPv6
202334
out := make([]byte, MaxRejectPacketSize)

0 commit comments

Comments
 (0)