From 190634e0fcf4cf25a449e1bd39533ca2ddad66e6 Mon Sep 17 00:00:00 2001 From: Julian Elischer Date: Fri, 14 Aug 2020 02:05:23 -0700 Subject: Give the ICMP Code its own type This is a preparatory commit for a larger commit working on ICMP generation in error cases. This is removal of technical debt and cleanup in the gvisor code as part of gvisor issue 2211. Updates #2211. PiperOrigin-RevId: 326615389 --- pkg/tcpip/checker/checker.go | 4 +- pkg/tcpip/header/icmpv4.go | 18 +++++---- pkg/tcpip/header/icmpv6.go | 43 ++++++++++++++++------ pkg/tcpip/network/ip_test.go | 4 +- pkg/tcpip/network/ipv6/ndp_test.go | 4 +- pkg/tcpip/transport/tcp/testing/context/context.go | 2 +- test/packetimpact/testbench/layers.go | 22 ++++++++--- test/packetimpact/testbench/layers_test.go | 2 +- .../tests/ipv6_fragment_reassembly_test.go | 6 +-- .../tests/ipv6_unknown_options_action_test.go | 2 +- .../tests/tcp_network_unreachable_test.go | 6 ++- .../tests/udp_icmp_error_propagation_test.go | 8 +++- 12 files changed, 81 insertions(+), 40 deletions(-) diff --git a/pkg/tcpip/checker/checker.go b/pkg/tcpip/checker/checker.go index 1e5f5abf2..b769094dc 100644 --- a/pkg/tcpip/checker/checker.go +++ b/pkg/tcpip/checker/checker.go @@ -699,7 +699,7 @@ func ICMPv4Type(want header.ICMPv4Type) TransportChecker { } // ICMPv4Code creates a checker that checks the ICMPv4 Code field. -func ICMPv4Code(want byte) TransportChecker { +func ICMPv4Code(want header.ICMPv4Code) TransportChecker { return func(t *testing.T, h header.Transport) { t.Helper() @@ -757,7 +757,7 @@ func ICMPv6Type(want header.ICMPv6Type) TransportChecker { } // ICMPv6Code creates a checker that checks the ICMPv6 Code field. -func ICMPv6Code(want byte) TransportChecker { +func ICMPv6Code(want header.ICMPv6Code) TransportChecker { return func(t *testing.T, h header.Transport) { t.Helper() diff --git a/pkg/tcpip/header/icmpv4.go b/pkg/tcpip/header/icmpv4.go index 1a631b31a..be03fb086 100644 --- a/pkg/tcpip/header/icmpv4.go +++ b/pkg/tcpip/header/icmpv4.go @@ -54,6 +54,9 @@ const ( // ICMPv4Type is the ICMP type field described in RFC 792. type ICMPv4Type byte +// ICMPv4Code is the ICMP code field described in RFC 792. +type ICMPv4Code byte + // Typical values of ICMPv4Type defined in RFC 792. const ( ICMPv4EchoReply ICMPv4Type = 0 @@ -69,12 +72,13 @@ const ( ICMPv4InfoReply ICMPv4Type = 16 ) -// Values for ICMP code as defined in RFC 792. +// ICMP codes for ICMPv4 Destination Unreachable messages as defined in RFC 792. const ( - ICMPv4TTLExceeded = 0 - ICMPv4HostUnreachable = 1 - ICMPv4PortUnreachable = 3 - ICMPv4FragmentationNeeded = 4 + ICMPv4TTLExceeded ICMPv4Code = 0 + ICMPv4HostUnreachable ICMPv4Code = 1 + ICMPv4ProtoUnreachable ICMPv4Code = 2 + ICMPv4PortUnreachable ICMPv4Code = 3 + ICMPv4FragmentationNeeded ICMPv4Code = 4 ) // Type is the ICMP type field. @@ -84,10 +88,10 @@ func (b ICMPv4) Type() ICMPv4Type { return ICMPv4Type(b[0]) } func (b ICMPv4) SetType(t ICMPv4Type) { b[0] = byte(t) } // Code is the ICMP code field. Its meaning depends on the value of Type. -func (b ICMPv4) Code() byte { return b[1] } +func (b ICMPv4) Code() ICMPv4Code { return ICMPv4Code(b[1]) } // SetCode sets the ICMP code field. -func (b ICMPv4) SetCode(c byte) { b[1] = c } +func (b ICMPv4) SetCode(c ICMPv4Code) { b[1] = byte(c) } // Checksum is the ICMP checksum field. func (b ICMPv4) Checksum() uint16 { diff --git a/pkg/tcpip/header/icmpv6.go b/pkg/tcpip/header/icmpv6.go index a13b4b809..20b01d8f4 100644 --- a/pkg/tcpip/header/icmpv6.go +++ b/pkg/tcpip/header/icmpv6.go @@ -92,7 +92,6 @@ const ( // ICMPv6Type is the ICMP type field described in RFC 4443 and friends. type ICMPv6Type byte -// Typical values of ICMPv6Type defined in RFC 4443. const ( ICMPv6DstUnreachable ICMPv6Type = 1 ICMPv6PacketTooBig ICMPv6Type = 2 @@ -110,18 +109,38 @@ const ( ICMPv6RedirectMsg ICMPv6Type = 137 ) -// Values for ICMP destination unreachable code as defined in RFC 4443 section -// 3.1. +// ICMPv6Code is the ICMP code field described in RFC 4443. +type ICMPv6Code byte + +// ICMP codes used with Destination Unreachable (Type 1). As per RFC 4443 +// section 3.1. +const ( + ICMPv6NetworkUnreachable ICMPv6Code = 0 + ICMPv6Prohibited ICMPv6Code = 1 + ICMPv6BeyondScope ICMPv6Code = 2 + ICMPv6AddressUnreachable ICMPv6Code = 3 + ICMPv6PortUnreachable ICMPv6Code = 4 + ICMPv6Policy ICMPv6Code = 5 + ICMPv6RejectRoute ICMPv6Code = 6 +) + +// ICMP codes used with Time Exceeded (Type 3). As per RFC 4443 section 3.3. const ( - ICMPv6NetworkUnreachable = 0 - ICMPv6Prohibited = 1 - ICMPv6BeyondScope = 2 - ICMPv6AddressUnreachable = 3 - ICMPv6PortUnreachable = 4 - ICMPv6Policy = 5 - ICMPv6RejectRoute = 6 + ICMPv6HopLimitExceeded ICMPv6Code = 0 + ICMPv6ReassemblyTimeout ICMPv6Code = 1 ) +// ICMP codes used with Parameter Problem (Type 4). As per RFC 4443 section 3.4. +const ( + ICMPv6ErroneousHeader ICMPv6Code = 0 + ICMPv6UnknownHeader ICMPv6Code = 1 + ICMPv6UnknownOption ICMPv6Code = 2 +) + +// ICMPv6UnusedCode is the code value used with ICMPv6 messages which don't use +// the code field. (Types not mentioned above.) +const ICMPv6UnusedCode ICMPv6Code = 0 + // Type is the ICMP type field. func (b ICMPv6) Type() ICMPv6Type { return ICMPv6Type(b[0]) } @@ -129,10 +148,10 @@ func (b ICMPv6) Type() ICMPv6Type { return ICMPv6Type(b[0]) } func (b ICMPv6) SetType(t ICMPv6Type) { b[0] = byte(t) } // Code is the ICMP code field. Its meaning depends on the value of Type. -func (b ICMPv6) Code() byte { return b[1] } +func (b ICMPv6) Code() ICMPv6Code { return ICMPv6Code(b[1]) } // SetCode sets the ICMP code field. -func (b ICMPv6) SetCode(c byte) { b[1] = c } +func (b ICMPv6) SetCode(c ICMPv6Code) { b[1] = byte(c) } // Checksum is the ICMP checksum field. func (b ICMPv6) Checksum() uint16 { diff --git a/pkg/tcpip/network/ip_test.go b/pkg/tcpip/network/ip_test.go index e6768258a..491d936a1 100644 --- a/pkg/tcpip/network/ip_test.go +++ b/pkg/tcpip/network/ip_test.go @@ -321,7 +321,7 @@ func TestIPv4ReceiveControl(t *testing.T) { name string expectedCount int fragmentOffset uint16 - code uint8 + code header.ICMPv4Code expectedTyp stack.ControlType expectedExtra uint32 trunc int @@ -579,7 +579,7 @@ func TestIPv6ReceiveControl(t *testing.T) { expectedCount int fragmentOffset *uint16 typ header.ICMPv6Type - code uint8 + code header.ICMPv6Code expectedTyp stack.ControlType expectedExtra uint32 trunc int diff --git a/pkg/tcpip/network/ipv6/ndp_test.go b/pkg/tcpip/network/ipv6/ndp_test.go index fe159b24f..2efa82e60 100644 --- a/pkg/tcpip/network/ipv6/ndp_test.go +++ b/pkg/tcpip/network/ipv6/ndp_test.go @@ -637,7 +637,7 @@ func TestNDPValidation(t *testing.T) { name string atomicFragment bool hopLimit uint8 - code uint8 + code header.ICMPv6Code valid bool }{ { @@ -730,7 +730,7 @@ func TestRouterAdvertValidation(t *testing.T) { name string src tcpip.Address hopLimit uint8 - code uint8 + code header.ICMPv6Code ndpPayload []byte expectedSuccess bool }{ diff --git a/pkg/tcpip/transport/tcp/testing/context/context.go b/pkg/tcpip/transport/tcp/testing/context/context.go index 927bc71e0..b6031354e 100644 --- a/pkg/tcpip/transport/tcp/testing/context/context.go +++ b/pkg/tcpip/transport/tcp/testing/context/context.go @@ -292,7 +292,7 @@ func (c *Context) GetPacketNonBlocking() []byte { } // SendICMPPacket builds and sends an ICMPv4 packet via the link layer endpoint. -func (c *Context) SendICMPPacket(typ header.ICMPv4Type, code uint8, p1, p2 []byte, maxTotalSize int) { +func (c *Context) SendICMPPacket(typ header.ICMPv4Type, code header.ICMPv4Code, p1, p2 []byte, maxTotalSize int) { // Allocate a buffer data and headers. buf := buffer.NewView(header.IPv4MinimumSize + header.ICMPv4PayloadOffset + len(p2)) if len(buf) > maxTotalSize { diff --git a/test/packetimpact/testbench/layers.go b/test/packetimpact/testbench/layers.go index 24aa46cce..a35562ca8 100644 --- a/test/packetimpact/testbench/layers.go +++ b/test/packetimpact/testbench/layers.go @@ -775,7 +775,7 @@ func (l *IPv6FragmentExtHdr) String() string { type ICMPv6 struct { LayerBase Type *header.ICMPv6Type - Code *byte + Code *header.ICMPv6Code Checksum *uint16 Payload []byte } @@ -823,6 +823,12 @@ func ICMPv6Type(v header.ICMPv6Type) *header.ICMPv6Type { return &v } +// ICMPv6Code is a helper routine that allocates a new ICMPv6Type value to store +// v and returns a pointer to it. +func ICMPv6Code(v header.ICMPv6Code) *header.ICMPv6Code { + return &v +} + // Byte is a helper routine that allocates a new byte value to store // v and returns a pointer to it. func Byte(v byte) *byte { @@ -834,7 +840,7 @@ func parseICMPv6(b []byte) (Layer, layerParser) { h := header.ICMPv6(b) icmpv6 := ICMPv6{ Type: ICMPv6Type(h.Type()), - Code: Byte(h.Code()), + Code: ICMPv6Code(h.Code()), Checksum: Uint16(h.Checksum()), Payload: h.NDPPayload(), } @@ -861,11 +867,17 @@ func ICMPv4Type(t header.ICMPv4Type) *header.ICMPv4Type { return &t } +// ICMPv4Code is a helper routine that allocates a new header.ICMPv4Code value +// to store t and returns a pointer to it. +func ICMPv4Code(t header.ICMPv4Code) *header.ICMPv4Code { + return &t +} + // ICMPv4 can construct and match an ICMPv4 encapsulation. type ICMPv4 struct { LayerBase Type *header.ICMPv4Type - Code *uint8 + Code *header.ICMPv4Code Checksum *uint16 } @@ -881,7 +893,7 @@ func (l *ICMPv4) ToBytes() ([]byte, error) { h.SetType(*l.Type) } if l.Code != nil { - h.SetCode(byte(*l.Code)) + h.SetCode(*l.Code) } if l.Checksum != nil { h.SetChecksum(*l.Checksum) @@ -901,7 +913,7 @@ func parseICMPv4(b []byte) (Layer, layerParser) { h := header.ICMPv4(b) icmpv4 := ICMPv4{ Type: ICMPv4Type(h.Type()), - Code: Uint8(h.Code()), + Code: ICMPv4Code(h.Code()), Checksum: Uint16(h.Checksum()), } return &icmpv4, parsePayload diff --git a/test/packetimpact/testbench/layers_test.go b/test/packetimpact/testbench/layers_test.go index a2a763034..eca0780b5 100644 --- a/test/packetimpact/testbench/layers_test.go +++ b/test/packetimpact/testbench/layers_test.go @@ -594,7 +594,7 @@ func TestIPv6ExtHdrOptions(t *testing.T) { }, &ICMPv6{ Type: ICMPv6Type(header.ICMPv6ParamProblem), - Code: Byte(0), + Code: ICMPv6Code(header.ICMPv6ErroneousHeader), Checksum: Uint16(0x5f98), Payload: []byte{0x00, 0x00, 0x00, 0x06}, }, diff --git a/test/packetimpact/tests/ipv6_fragment_reassembly_test.go b/test/packetimpact/tests/ipv6_fragment_reassembly_test.go index b5f94ad4b..a24c85566 100644 --- a/test/packetimpact/tests/ipv6_fragment_reassembly_test.go +++ b/test/packetimpact/tests/ipv6_fragment_reassembly_test.go @@ -67,7 +67,7 @@ func TestIPv6FragmentReassembly(t *testing.T) { rIP := tcpip.Address(net.ParseIP(testbench.RemoteIPv6).To16()) icmpv6 := testbench.ICMPv6{ Type: testbench.ICMPv6Type(header.ICMPv6EchoRequest), - Code: testbench.Byte(0), + Code: testbench.ICMPv6Code(header.ICMPv6UnusedCode), Payload: icmpv6EchoPayload, } icmpv6Bytes, err := icmpv6.ToBytes() @@ -89,7 +89,7 @@ func TestIPv6FragmentReassembly(t *testing.T) { }, &testbench.ICMPv6{ Type: testbench.ICMPv6Type(header.ICMPv6EchoRequest), - Code: testbench.Byte(0), + Code: testbench.ICMPv6Code(header.ICMPv6UnusedCode), Payload: icmpv6EchoPayload, Checksum: &cksum, }) @@ -116,7 +116,7 @@ func TestIPv6FragmentReassembly(t *testing.T) { }, &testbench.ICMPv6{ Type: testbench.ICMPv6Type(header.ICMPv6EchoReply), - Code: testbench.Byte(0), + Code: testbench.ICMPv6Code(header.ICMPv6UnusedCode), }, }, time.Second) if err != nil { diff --git a/test/packetimpact/tests/ipv6_unknown_options_action_test.go b/test/packetimpact/tests/ipv6_unknown_options_action_test.go index d7d63cbd2..e79d74476 100644 --- a/test/packetimpact/tests/ipv6_unknown_options_action_test.go +++ b/test/packetimpact/tests/ipv6_unknown_options_action_test.go @@ -172,7 +172,7 @@ func TestIPv6UnknownOptionAction(t *testing.T) { &testbench.IPv6{}, &testbench.ICMPv6{ Type: testbench.ICMPv6Type(header.ICMPv6ParamProblem), - Code: testbench.Byte(2), + Code: testbench.ICMPv6Code(header.ICMPv6UnknownOption), Payload: icmpv6Payload, }, }, time.Second) diff --git a/test/packetimpact/tests/tcp_network_unreachable_test.go b/test/packetimpact/tests/tcp_network_unreachable_test.go index 900352fa1..2f57dff19 100644 --- a/test/packetimpact/tests/tcp_network_unreachable_test.go +++ b/test/packetimpact/tests/tcp_network_unreachable_test.go @@ -72,7 +72,9 @@ func TestTCPSynSentUnreachable(t *testing.T) { if !ok { t.Fatalf("expected %s to be TCP", tcpLayers[tcpLayer]) } - var icmpv4 testbench.ICMPv4 = testbench.ICMPv4{Type: testbench.ICMPv4Type(header.ICMPv4DstUnreachable), Code: testbench.Uint8(header.ICMPv4HostUnreachable)} + var icmpv4 testbench.ICMPv4 = testbench.ICMPv4{ + Type: testbench.ICMPv4Type(header.ICMPv4DstUnreachable), + Code: testbench.ICMPv4Code(header.ICMPv4HostUnreachable)} layers = append(layers, &icmpv4, ip, tcp) rawConn.SendFrameStateless(t, layers) @@ -126,7 +128,7 @@ func TestTCPSynSentUnreachable6(t *testing.T) { } var icmpv6 testbench.ICMPv6 = testbench.ICMPv6{ Type: testbench.ICMPv6Type(header.ICMPv6DstUnreachable), - Code: testbench.Uint8(header.ICMPv6NetworkUnreachable), + Code: testbench.ICMPv6Code(header.ICMPv6NetworkUnreachable), // Per RFC 4443 3.1, the payload contains 4 zeroed bytes. Payload: []byte{0, 0, 0, 0}, } diff --git a/test/packetimpact/tests/udp_icmp_error_propagation_test.go b/test/packetimpact/tests/udp_icmp_error_propagation_test.go index b47ddb6c3..df35d16c8 100644 --- a/test/packetimpact/tests/udp_icmp_error_propagation_test.go +++ b/test/packetimpact/tests/udp_icmp_error_propagation_test.go @@ -62,9 +62,13 @@ func (e icmpError) String() string { func (e icmpError) ToICMPv4() *testbench.ICMPv4 { switch e { case portUnreachable: - return &testbench.ICMPv4{Type: testbench.ICMPv4Type(header.ICMPv4DstUnreachable), Code: testbench.Uint8(header.ICMPv4PortUnreachable)} + return &testbench.ICMPv4{ + Type: testbench.ICMPv4Type(header.ICMPv4DstUnreachable), + Code: testbench.ICMPv4Code(header.ICMPv4PortUnreachable)} case timeToLiveExceeded: - return &testbench.ICMPv4{Type: testbench.ICMPv4Type(header.ICMPv4TimeExceeded), Code: testbench.Uint8(header.ICMPv4TTLExceeded)} + return &testbench.ICMPv4{ + Type: testbench.ICMPv4Type(header.ICMPv4TimeExceeded), + Code: testbench.ICMPv4Code(header.ICMPv4TTLExceeded)} } return nil } -- cgit v1.2.3