From 2a2cb29e1cc5c94299b79a3e561d7a6915158ae6 Mon Sep 17 00:00:00 2001 From: Arthur Sfez Date: Thu, 18 Feb 2021 12:27:53 -0800 Subject: Validate IGMP packets This change also adds support for Router Alert option processing on incoming packets, a new stat for Router Alert option, and exports all the IP-option related stats. Fixes #5491 PiperOrigin-RevId: 358238123 --- pkg/sentry/socket/netstack/netstack.go | 4 + pkg/tcpip/header/ipv4.go | 46 ++++++- pkg/tcpip/network/internal/ip/stats.go | 21 ++-- pkg/tcpip/network/ipv4/icmp.go | 2 +- pkg/tcpip/network/ipv4/igmp.go | 63 +++++++++- pkg/tcpip/network/ipv4/igmp_test.go | 193 ++++++++++++++++++++++++++---- pkg/tcpip/network/ipv4/ipv4.go | 138 +++++++++++++-------- pkg/tcpip/network/multicast_group_test.go | 30 +++-- pkg/tcpip/tcpip.go | 13 +- 9 files changed, 404 insertions(+), 106 deletions(-) diff --git a/pkg/sentry/socket/netstack/netstack.go b/pkg/sentry/socket/netstack/netstack.go index a632b8bcd..f77a867f1 100644 --- a/pkg/sentry/socket/netstack/netstack.go +++ b/pkg/sentry/socket/netstack/netstack.go @@ -184,6 +184,10 @@ var Metrics = tcpip.Stats{ IPTablesPreroutingDropped: mustCreateMetric("/netstack/ip/iptables/prerouting_dropped", "Total number of IP packets dropped in the Prerouting chain."), IPTablesInputDropped: mustCreateMetric("/netstack/ip/iptables/input_dropped", "Total number of IP packets dropped in the Input chain."), IPTablesOutputDropped: mustCreateMetric("/netstack/ip/iptables/output_dropped", "Total number of IP packets dropped in the Output chain."), + OptionTimestampReceived: mustCreateMetric("/netstack/ip/options/timestamp_received", "Total number of timestamp options found in received IP packets."), + OptionRecordRouteReceived: mustCreateMetric("/netstack/ip/options/record_route_received", "Total number of record route options found in received IP packets."), + OptionRouterAlertReceived: mustCreateMetric("/netstack/ip/options/router_alert_received", "Total number of router alert options found in received IP packets."), + OptionUnknownReceived: mustCreateMetric("/netstack/ip/options/unknown_received", "Total number of unknown options found in received IP packets."), }, ARP: tcpip.ARPStats{ PacketsReceived: mustCreateMetric("/netstack/arp/packets_received", "Number of ARP packets received from the link layer."), diff --git a/pkg/tcpip/header/ipv4.go b/pkg/tcpip/header/ipv4.go index 48ca60319..f588311e0 100644 --- a/pkg/tcpip/header/ipv4.go +++ b/pkg/tcpip/header/ipv4.go @@ -519,6 +519,7 @@ func (o *IPv4OptionGeneric) Contents() []byte { return []byte(*o) } // IPv4OptionIterator is an iterator pointing to a specific IP option // at any point of time. It also holds information as to a new options buffer // that we are building up to hand back to the caller. +// TODO(https://gvisor.dev/issues/5513): Add unit tests for IPv4OptionIterator. type IPv4OptionIterator struct { options IPv4Options // ErrCursor is where we are while parsing options. It is exported as any @@ -539,6 +540,15 @@ func (o IPv4Options) MakeIterator() IPv4OptionIterator { } } +// InitReplacement copies the option into the new option buffer. +func (i *IPv4OptionIterator) InitReplacement(option IPv4Option) IPv4Options { + replacementOption := i.RemainingBuffer()[:option.Size()] + if copied := copy(replacementOption, option.Contents()); copied != len(replacementOption) { + panic(fmt.Sprintf("copied %d bytes in the replacement option buffer, expected %d bytes", copied, len(replacementOption))) + } + return replacementOption +} + // RemainingBuffer returns the remaining (unused) part of the new option buffer, // into which a new option may be written. func (i *IPv4OptionIterator) RemainingBuffer() IPv4Options { @@ -649,6 +659,17 @@ func (i *IPv4OptionIterator) Next() (IPv4Option, bool, *IPv4OptParameterProblem) } retval := IPv4OptionRecordRoute(optionBody) return &retval, false, nil + + case IPv4OptionRouterAlertType: + if optLen != IPv4OptionRouterAlertLength { + i.ErrCursor++ + return nil, false, &IPv4OptParameterProblem{ + Pointer: i.ErrCursor, + NeedICMP: true, + } + } + retval := IPv4OptionRouterAlert(optionBody) + return &retval, false, nil } retval := IPv4OptionGeneric(optionBody) return &retval, false, nil @@ -896,11 +917,30 @@ const ( // payload of the router alert option. IPv4OptionRouterAlertValue = 0 - // iPv4OptionRouterAlertValueOffset is the offset for the value of a + // IPv4OptionRouterAlertValueOffset is the offset for the value of a // RouterAlert option. - iPv4OptionRouterAlertValueOffset = 2 + IPv4OptionRouterAlertValueOffset = 2 ) +var _ IPv4Option = (*IPv4OptionRouterAlert)(nil) + +// IPv4OptionRouterAlert is an IPv4 RouterAlert option defined by RFC 2113. +type IPv4OptionRouterAlert []byte + +// Type implements IPv4Option. +func (*IPv4OptionRouterAlert) Type() IPv4OptionType { return IPv4OptionRouterAlertType } + +// Size implements IPv4Option. +func (ra *IPv4OptionRouterAlert) Size() uint8 { return uint8(len(*ra)) } + +// Contents implements IPv4Option. +func (ra *IPv4OptionRouterAlert) Contents() []byte { return []byte(*ra) } + +// Value returns the value of the IPv4OptionRouterAlert. +func (ra *IPv4OptionRouterAlert) Value() uint16 { + return binary.BigEndian.Uint16(ra.Contents()[IPv4OptionRouterAlertValueOffset:]) +} + // IPv4SerializableOption is an interface to represent serializable IPv4 option // types. type IPv4SerializableOption interface { @@ -999,7 +1039,7 @@ func (*IPv4SerializableRouterAlertOption) optionType() IPv4OptionType { // Length implements IPv4SerializableOption. func (*IPv4SerializableRouterAlertOption) length() uint8 { - return IPv4OptionRouterAlertLength - iPv4OptionRouterAlertValueOffset + return IPv4OptionRouterAlertLength - IPv4OptionRouterAlertValueOffset } // SerializeInto implements IPv4SerializableOption. diff --git a/pkg/tcpip/network/internal/ip/stats.go b/pkg/tcpip/network/internal/ip/stats.go index 898f8b356..5f7e60c5c 100644 --- a/pkg/tcpip/network/internal/ip/stats.go +++ b/pkg/tcpip/network/internal/ip/stats.go @@ -68,11 +68,17 @@ type MultiCounterIPStats struct { // Output chain. IPTablesOutputDropped tcpip.MultiCounterStat - // OptionTSReceived is the number of Timestamp options seen. - OptionTSReceived tcpip.MultiCounterStat + // TODO(https://gvisor.dev/issues/5529): Move the IPv4-only option stats out - // OptionRRReceived is the number of Record Route options seen. - OptionRRReceived tcpip.MultiCounterStat + // of IPStats. + // OptionTimestampReceived is the number of Timestamp options seen. + OptionTimestampReceived tcpip.MultiCounterStat + + // OptionRecordRouteReceived is the number of Record Route options seen. + OptionRecordRouteReceived tcpip.MultiCounterStat + + // OptionRouterAlertReceived is the number of Router Alert options seen. + OptionRouterAlertReceived tcpip.MultiCounterStat // OptionUnknownReceived is the number of unknown IP options seen. OptionUnknownReceived tcpip.MultiCounterStat @@ -92,9 +98,10 @@ func (m *MultiCounterIPStats) Init(a, b *tcpip.IPStats) { m.IPTablesPreroutingDropped.Init(a.IPTablesPreroutingDropped, b.IPTablesPreroutingDropped) m.IPTablesInputDropped.Init(a.IPTablesInputDropped, b.IPTablesInputDropped) m.IPTablesOutputDropped.Init(a.IPTablesOutputDropped, b.IPTablesOutputDropped) - m.OptionTSReceived.Init(a.OptionTSReceived, b.OptionTSReceived) - m.OptionRRReceived.Init(a.OptionRRReceived, b.OptionRRReceived) + m.OptionTimestampReceived.Init(a.OptionTimestampReceived, b.OptionTimestampReceived) + m.OptionRecordRouteReceived.Init(a.OptionRecordRouteReceived, b.OptionRecordRouteReceived) + m.OptionRouterAlertReceived.Init(a.OptionRouterAlertReceived, b.OptionRouterAlertReceived) m.OptionUnknownReceived.Init(a.OptionUnknownReceived, b.OptionUnknownReceived) } -// LINT.ThenChange(:MultiCounterIPStats, ../../tcpip.go:IPStats) +// LINT.ThenChange(:MultiCounterIPStats, ../../../tcpip.go:IPStats) diff --git a/pkg/tcpip/network/ipv4/icmp.go b/pkg/tcpip/network/ipv4/icmp.go index b44304cee..bd0eabad1 100644 --- a/pkg/tcpip/network/ipv4/icmp.go +++ b/pkg/tcpip/network/ipv4/icmp.go @@ -214,7 +214,7 @@ func (e *endpoint) handleICMP(pkt *stack.PacketBuffer) { op = &optionUsageReceive{} } var optProblem *header.IPv4OptParameterProblem - newOptions, optProblem = e.processIPOptions(pkt, opts, op) + newOptions, _, optProblem = e.processIPOptions(pkt, opts, op) if optProblem != nil { if optProblem.NeedICMP { _ = e.protocol.returnError(&icmpReasonParamProblem{ diff --git a/pkg/tcpip/network/ipv4/igmp.go b/pkg/tcpip/network/ipv4/igmp.go index 12632aceb..0a15ae897 100644 --- a/pkg/tcpip/network/ipv4/igmp.go +++ b/pkg/tcpip/network/ipv4/igmp.go @@ -145,10 +145,57 @@ func (igmp *igmpState) init(ep *endpoint) { }) } +// Precondition: igmp.ep.mu must be locked. +func (igmp *igmpState) isSourceIPValidLocked(src tcpip.Address, messageType header.IGMPType) bool { + if messageType == header.IGMPMembershipQuery { + // RFC 2236 does not require the IGMP implementation to check the source IP + // for Membership Query messages. + return true + } + + // As per RFC 2236 section 10, + // + // Ignore the Report if you cannot identify the source address of the + // packet as belonging to a subnet assigned to the interface on which the + // packet was received. + // + // Ignore the Leave message if you cannot identify the source address of + // the packet as belonging to a subnet assigned to the interface on which + // the packet was received. + // + // Note: this rule applies to both V1 and V2 Membership Reports. + var isSourceIPValid bool + igmp.ep.mu.addressableEndpointState.ForEachPrimaryEndpoint(func(addressEndpoint stack.AddressEndpoint) bool { + if subnet := addressEndpoint.Subnet(); subnet.Contains(src) { + isSourceIPValid = true + return false + } + return true + }) + + return isSourceIPValid +} + +// Precondition: igmp.ep.mu must be locked. +func (igmp *igmpState) isPacketValidLocked(pkt *stack.PacketBuffer, messageType header.IGMPType, hasRouterAlertOption bool) bool { + // We can safely assume that the IP header is valid if we got this far. + iph := header.IPv4(pkt.NetworkHeader().View()) + + // As per RFC 2236 section 2, + // + // All IGMP messages described in this document are sent with IP TTL 1, and + // contain the IP Router Alert option [RFC 2113] in their IP header. + if !hasRouterAlertOption || iph.TTL() != header.IGMPTTL { + return false + } + + return igmp.isSourceIPValidLocked(iph.SourceAddress(), messageType) +} + // handleIGMP handles an IGMP packet. // // Precondition: igmp.ep.mu must be locked. -func (igmp *igmpState) handleIGMP(pkt *stack.PacketBuffer) { +func (igmp *igmpState) handleIGMP(pkt *stack.PacketBuffer, hasRouterAlertOption bool) { received := igmp.ep.stats.igmp.packetsReceived headerView, ok := pkt.Data.PullUp(header.IGMPMinimumSize) if !ok { @@ -168,30 +215,38 @@ func (igmp *igmpState) handleIGMP(pkt *stack.PacketBuffer) { return } + isValid := func(minimumSize int) bool { + return len(headerView) >= minimumSize && igmp.isPacketValidLocked(pkt, h.Type(), hasRouterAlertOption) + } + switch h.Type() { case header.IGMPMembershipQuery: received.membershipQuery.Increment() - if len(headerView) < header.IGMPQueryMinimumSize { + if !isValid(header.IGMPQueryMinimumSize) { received.invalid.Increment() return } igmp.handleMembershipQuery(h.GroupAddress(), h.MaxRespTime()) case header.IGMPv1MembershipReport: received.v1MembershipReport.Increment() - if len(headerView) < header.IGMPReportMinimumSize { + if !isValid(header.IGMPReportMinimumSize) { received.invalid.Increment() return } igmp.handleMembershipReport(h.GroupAddress()) case header.IGMPv2MembershipReport: received.v2MembershipReport.Increment() - if len(headerView) < header.IGMPReportMinimumSize { + if !isValid(header.IGMPReportMinimumSize) { received.invalid.Increment() return } igmp.handleMembershipReport(h.GroupAddress()) case header.IGMPLeaveGroup: received.leaveGroup.Increment() + if !isValid(header.IGMPLeaveMessageMinimumSize) { + received.invalid.Increment() + return + } // As per RFC 2236 Section 6, Page 7: "IGMP messages other than Query or // Report, are ignored in all states" diff --git a/pkg/tcpip/network/ipv4/igmp_test.go b/pkg/tcpip/network/ipv4/igmp_test.go index 95fd75ab7..c5f68e411 100644 --- a/pkg/tcpip/network/ipv4/igmp_test.go +++ b/pkg/tcpip/network/ipv4/igmp_test.go @@ -29,22 +29,25 @@ import ( ) const ( - linkAddr = tcpip.LinkAddress("\x02\x02\x03\x04\x05\x06") - addr = tcpip.Address("\x0a\x00\x00\x01") - multicastAddr = tcpip.Address("\xe0\x00\x00\x03") - nicID = 1 + linkAddr = tcpip.LinkAddress("\x02\x02\x03\x04\x05\x06") + stackAddr = tcpip.Address("\x0a\x00\x00\x01") + remoteAddr = tcpip.Address("\x0a\x00\x00\x02") + multicastAddr = tcpip.Address("\xe0\x00\x00\x03") + nicID = 1 + defaultTTL = 1 + defaultPrefixLength = 24 ) // validateIgmpPacket checks that a passed PacketInfo is an IPv4 IGMP packet // sent to the provided address with the passed fields set. Raises a t.Error if // any field does not match. -func validateIgmpPacket(t *testing.T, p channel.PacketInfo, remoteAddress tcpip.Address, igmpType header.IGMPType, maxRespTime byte, groupAddress tcpip.Address) { +func validateIgmpPacket(t *testing.T, p channel.PacketInfo, igmpType header.IGMPType, maxRespTime byte, srcAddr, dstAddr, groupAddress tcpip.Address) { t.Helper() payload := header.IPv4(stack.PayloadSince(p.Pkt.NetworkHeader())) checker.IPv4(t, payload, - checker.SrcAddr(addr), - checker.DstAddr(remoteAddress), + checker.SrcAddr(srcAddr), + checker.DstAddr(dstAddr), // TTL for an IGMP message must be 1 as per RFC 2236 section 2. checker.TTL(1), checker.IPv4RouterAlert(), @@ -77,20 +80,27 @@ func createStack(t *testing.T, igmpEnabled bool) (*channel.Endpoint, *stack.Stac return e, s, clock } -func createAndInjectIGMPPacket(e *channel.Endpoint, igmpType header.IGMPType, maxRespTime byte, groupAddress tcpip.Address) { - buf := buffer.NewView(header.IPv4MinimumSize + header.IGMPQueryMinimumSize) +func createAndInjectIGMPPacket(e *channel.Endpoint, igmpType header.IGMPType, maxRespTime byte, ttl uint8, srcAddr, dstAddr, groupAddress tcpip.Address, hasRouterAlertOption bool) { + var options header.IPv4OptionsSerializer + if hasRouterAlertOption { + options = header.IPv4OptionsSerializer{ + &header.IPv4SerializableRouterAlertOption{}, + } + } + buf := buffer.NewView(header.IPv4MinimumSize + int(options.Length()) + header.IGMPQueryMinimumSize) ip := header.IPv4(buf) ip.Encode(&header.IPv4Fields{ TotalLength: uint16(len(buf)), - TTL: 1, + TTL: ttl, Protocol: uint8(header.IGMPProtocolNumber), - SrcAddr: header.IPv4Any, - DstAddr: header.IPv4AllSystems, + SrcAddr: srcAddr, + DstAddr: dstAddr, + Options: options, }) ip.SetChecksum(^ip.CalculateChecksum()) - igmp := header.IGMP(buf[header.IPv4MinimumSize:]) + igmp := header.IGMP(ip.Payload()) igmp.SetType(igmpType) igmp.SetMaxRespTime(maxRespTime) igmp.SetGroupAddress(groupAddress) @@ -106,8 +116,9 @@ func createAndInjectIGMPPacket(e *channel.Endpoint, igmpType header.IGMPType, ma // cycles. func TestIGMPV1Present(t *testing.T) { e, s, clock := createStack(t, true) - if err := s.AddAddress(nicID, ipv4.ProtocolNumber, addr); err != nil { - t.Fatalf("AddAddress(%d, %d, %s): %s", nicID, ipv4.ProtocolNumber, addr, err) + addr := tcpip.AddressWithPrefix{Address: stackAddr, PrefixLen: defaultPrefixLength} + if err := s.AddAddressWithPrefix(nicID, ipv4.ProtocolNumber, addr); err != nil { + t.Fatalf("AddAddressWithPrefix(%d, %d, %s): %s", nicID, ipv4.ProtocolNumber, addr, err) } if err := s.JoinGroup(ipv4.ProtocolNumber, nicID, multicastAddr); err != nil { @@ -124,7 +135,7 @@ func TestIGMPV1Present(t *testing.T) { if got := s.Stats().IGMP.PacketsSent.V2MembershipReport.Value(); got != 1 { t.Fatalf("got V2MembershipReport messages sent = %d, want = 1", got) } - validateIgmpPacket(t, p, multicastAddr, header.IGMPv2MembershipReport, 0, multicastAddr) + validateIgmpPacket(t, p, header.IGMPv2MembershipReport, 0, stackAddr, multicastAddr, multicastAddr) } if t.Failed() { t.FailNow() @@ -134,7 +145,7 @@ func TestIGMPV1Present(t *testing.T) { // membership query except the Max Response Time is set to 0, which will tell // the stack that this is a router using IGMPv1. Send it to the all systems // group which is the only group this host belongs to. - createAndInjectIGMPPacket(e, header.IGMPMembershipQuery, 0, header.IPv4AllSystems) + createAndInjectIGMPPacket(e, header.IGMPMembershipQuery, 0, defaultTTL, remoteAddr, stackAddr, header.IPv4AllSystems, true /* hasRouterAlertOption */) if got := s.Stats().IGMP.PacketsReceived.MembershipQuery.Value(); got != 1 { t.Fatalf("got Membership Queries received = %d, want = 1", got) } @@ -159,7 +170,7 @@ func TestIGMPV1Present(t *testing.T) { if got := s.Stats().IGMP.PacketsSent.V1MembershipReport.Value(); got != 1 { t.Fatalf("got V1MembershipReport messages sent = %d, want = 1", got) } - validateIgmpPacket(t, p, multicastAddr, header.IGMPv1MembershipReport, 0, multicastAddr) + validateIgmpPacket(t, p, header.IGMPv1MembershipReport, 0, stackAddr, multicastAddr, multicastAddr) } // Cycling the interface should reset the V1 present flag. @@ -177,7 +188,7 @@ func TestIGMPV1Present(t *testing.T) { if got := s.Stats().IGMP.PacketsSent.V2MembershipReport.Value(); got != 2 { t.Fatalf("got V2MembershipReport messages sent = %d, want = 2", got) } - validateIgmpPacket(t, p, multicastAddr, header.IGMPv2MembershipReport, 0, multicastAddr) + validateIgmpPacket(t, p, header.IGMPv2MembershipReport, 0, stackAddr, multicastAddr, multicastAddr) } } @@ -200,8 +211,8 @@ func TestSendQueuedIGMPReports(t *testing.T) { // The initial set of IGMP reports that were queued should be sent once an // address is assigned. - if err := s.AddAddress(nicID, ipv4.ProtocolNumber, addr); err != nil { - t.Fatalf("AddAddress(%d, %d, %s): %s", nicID, ipv4.ProtocolNumber, addr, err) + if err := s.AddAddress(nicID, ipv4.ProtocolNumber, stackAddr); err != nil { + t.Fatalf("AddAddress(%d, %d, %s): %s", nicID, ipv4.ProtocolNumber, stackAddr, err) } if got := reportStat.Value(); got != 1 { t.Errorf("got reportStat.Value() = %d, want = 1", got) @@ -209,7 +220,7 @@ func TestSendQueuedIGMPReports(t *testing.T) { if p, ok := e.Read(); !ok { t.Error("expected to send an IGMP membership report") } else { - validateIgmpPacket(t, p, multicastAddr, header.IGMPv2MembershipReport, 0, multicastAddr) + validateIgmpPacket(t, p, header.IGMPv2MembershipReport, 0, stackAddr, multicastAddr, multicastAddr) } if t.Failed() { t.FailNow() @@ -221,7 +232,7 @@ func TestSendQueuedIGMPReports(t *testing.T) { if p, ok := e.Read(); !ok { t.Error("expected to send an IGMP membership report") } else { - validateIgmpPacket(t, p, multicastAddr, header.IGMPv2MembershipReport, 0, multicastAddr) + validateIgmpPacket(t, p, header.IGMPv2MembershipReport, 0, stackAddr, multicastAddr, multicastAddr) } if t.Failed() { t.FailNow() @@ -234,3 +245,139 @@ func TestSendQueuedIGMPReports(t *testing.T) { t.Fatalf("got unexpected packet = %#v", p) } } + +func TestIGMPPacketValidation(t *testing.T) { + tests := []struct { + name string + messageType header.IGMPType + stackAddresses []tcpip.AddressWithPrefix + srcAddr tcpip.Address + includeRouterAlertOption bool + ttl uint8 + expectValidIGMP bool + getMessageTypeStatValue func(tcpip.Stats) uint64 + }{ + { + name: "valid", + messageType: header.IGMPLeaveGroup, + includeRouterAlertOption: true, + stackAddresses: []tcpip.AddressWithPrefix{{Address: stackAddr, PrefixLen: 24}}, + srcAddr: remoteAddr, + ttl: 1, + expectValidIGMP: true, + getMessageTypeStatValue: func(stats tcpip.Stats) uint64 { return stats.IGMP.PacketsReceived.LeaveGroup.Value() }, + }, + { + name: "bad ttl", + messageType: header.IGMPv1MembershipReport, + includeRouterAlertOption: true, + stackAddresses: []tcpip.AddressWithPrefix{{Address: stackAddr, PrefixLen: 24}}, + srcAddr: remoteAddr, + ttl: 2, + expectValidIGMP: false, + getMessageTypeStatValue: func(stats tcpip.Stats) uint64 { return stats.IGMP.PacketsReceived.V1MembershipReport.Value() }, + }, + { + name: "missing router alert ip option", + messageType: header.IGMPv2MembershipReport, + includeRouterAlertOption: false, + stackAddresses: []tcpip.AddressWithPrefix{{Address: stackAddr, PrefixLen: 24}}, + srcAddr: remoteAddr, + ttl: 1, + expectValidIGMP: false, + getMessageTypeStatValue: func(stats tcpip.Stats) uint64 { return stats.IGMP.PacketsReceived.V2MembershipReport.Value() }, + }, + { + name: "igmp leave group and src ip does not belong to nic subnet", + messageType: header.IGMPLeaveGroup, + includeRouterAlertOption: true, + stackAddresses: []tcpip.AddressWithPrefix{{Address: stackAddr, PrefixLen: 24}}, + srcAddr: tcpip.Address("\x0a\x00\x01\x02"), + ttl: 1, + expectValidIGMP: false, + getMessageTypeStatValue: func(stats tcpip.Stats) uint64 { return stats.IGMP.PacketsReceived.LeaveGroup.Value() }, + }, + { + name: "igmp query and src ip does not belong to nic subnet", + messageType: header.IGMPMembershipQuery, + includeRouterAlertOption: true, + stackAddresses: []tcpip.AddressWithPrefix{{Address: stackAddr, PrefixLen: 24}}, + srcAddr: tcpip.Address("\x0a\x00\x01\x02"), + ttl: 1, + expectValidIGMP: true, + getMessageTypeStatValue: func(stats tcpip.Stats) uint64 { return stats.IGMP.PacketsReceived.MembershipQuery.Value() }, + }, + { + name: "igmp report v1 and src ip does not belong to nic subnet", + messageType: header.IGMPv1MembershipReport, + includeRouterAlertOption: true, + stackAddresses: []tcpip.AddressWithPrefix{{Address: stackAddr, PrefixLen: 24}}, + srcAddr: tcpip.Address("\x0a\x00\x01\x02"), + ttl: 1, + expectValidIGMP: false, + getMessageTypeStatValue: func(stats tcpip.Stats) uint64 { return stats.IGMP.PacketsReceived.V1MembershipReport.Value() }, + }, + { + name: "igmp report v2 and src ip does not belong to nic subnet", + messageType: header.IGMPv2MembershipReport, + includeRouterAlertOption: true, + stackAddresses: []tcpip.AddressWithPrefix{{Address: stackAddr, PrefixLen: 24}}, + srcAddr: tcpip.Address("\x0a\x00\x01\x02"), + ttl: 1, + expectValidIGMP: false, + getMessageTypeStatValue: func(stats tcpip.Stats) uint64 { return stats.IGMP.PacketsReceived.V2MembershipReport.Value() }, + }, + { + name: "src ip belongs to the subnet of the nic's second address", + messageType: header.IGMPv2MembershipReport, + includeRouterAlertOption: true, + stackAddresses: []tcpip.AddressWithPrefix{ + {Address: tcpip.Address("\x0a\x00\x0f\x01"), PrefixLen: 24}, + {Address: stackAddr, PrefixLen: 24}, + }, + srcAddr: remoteAddr, + ttl: 1, + expectValidIGMP: true, + getMessageTypeStatValue: func(stats tcpip.Stats) uint64 { return stats.IGMP.PacketsReceived.V2MembershipReport.Value() }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + e, s, _ := createStack(t, true) + for _, address := range test.stackAddresses { + if err := s.AddAddressWithPrefix(nicID, ipv4.ProtocolNumber, address); err != nil { + t.Fatalf("AddAddressWithPrefix(%d, %d, %s): %s", nicID, ipv4.ProtocolNumber, address, err) + } + } + stats := s.Stats() + // Verify that every relevant stats is zero'd before we send a packet. + if got := test.getMessageTypeStatValue(s.Stats()); got != 0 { + t.Errorf("got test.getMessageTypeStatValue(s.Stats()) = %d, want = 0", got) + } + if got := stats.IGMP.PacketsReceived.Invalid.Value(); got != 0 { + t.Errorf("got stats.IGMP.PacketsReceived.Invalid.Value() = %d, want = 0", got) + } + if got := stats.IP.PacketsDelivered.Value(); got != 0 { + t.Fatalf("got stats.IP.PacketsDelivered.Value() = %d, want = 0", got) + } + createAndInjectIGMPPacket(e, test.messageType, 0, test.ttl, test.srcAddr, header.IPv4AllSystems, header.IPv4AllSystems, test.includeRouterAlertOption) + // We always expect the packet to pass IP validation. + if got := stats.IP.PacketsDelivered.Value(); got != 1 { + t.Fatalf("got stats.IP.PacketsDelivered.Value() = %d, want = 1", got) + } + // Even when the IGMP-specific validation checks fail, we expect the + // corresponding IGMP counter to be incremented. + if got := test.getMessageTypeStatValue(s.Stats()); got != 1 { + t.Errorf("got test.getMessageTypeStatValue(s.Stats()) = %d, want = 1", got) + } + var expectedInvalidCount uint64 + if !test.expectValidIGMP { + expectedInvalidCount = 1 + } + if got := stats.IGMP.PacketsReceived.Invalid.Value(); got != expectedInvalidCount { + t.Errorf("got stats.IGMP.PacketsReceived.Invalid.Value() = %d, want = %d", got, expectedInvalidCount) + } + }) + } +} diff --git a/pkg/tcpip/network/ipv4/ipv4.go b/pkg/tcpip/network/ipv4/ipv4.go index 250e4846a..4a429ea6c 100644 --- a/pkg/tcpip/network/ipv4/ipv4.go +++ b/pkg/tcpip/network/ipv4/ipv4.go @@ -562,7 +562,7 @@ func (e *endpoint) forwardPacket(pkt *stack.PacketBuffer) tcpip.Error { } if opts := h.Options(); len(opts) != 0 { - newOpts, optProblem := e.processIPOptions(pkt, opts, &optionUsageForward{}) + newOpts, _, optProblem := e.processIPOptions(pkt, opts, &optionUsageForward{}) if optProblem != nil { if optProblem.NeedICMP { _ = e.protocol.returnError(&icmpReasonParamProblem{ @@ -776,7 +776,7 @@ func (e *endpoint) handlePacket(pkt *stack.PacketBuffer) { // If there are options we need to check them before we do assembly // or we could be assembling errant packets. However we do not change the // options as that could lead to double processing later. - if _, optProblem := e.processIPOptions(pkt, opts, &optionUsageVerify{}); optProblem != nil { + if _, _, optProblem := e.processIPOptions(pkt, opts, &optionUsageVerify{}); optProblem != nil { if optProblem.NeedICMP { _ = e.protocol.returnError(&icmpReasonParamProblem{ pointer: optProblem.Pointer, @@ -846,8 +846,9 @@ func (e *endpoint) handlePacket(pkt *stack.PacketBuffer) { return } // ICMP handles options itself but do it here for all remaining destinations. + var hasRouterAlertOption bool if opts := h.Options(); len(opts) != 0 { - newOpts, optProblem := e.processIPOptions(pkt, opts, &optionUsageReceive{}) + newOpts, processedOpts, optProblem := e.processIPOptions(pkt, opts, &optionUsageReceive{}) if optProblem != nil { if optProblem.NeedICMP { _ = e.protocol.returnError(&icmpReasonParamProblem{ @@ -858,6 +859,7 @@ func (e *endpoint) handlePacket(pkt *stack.PacketBuffer) { } return } + hasRouterAlertOption = processedOpts.routerAlert copied := copy(opts, newOpts) if copied != len(newOpts) { panic(fmt.Sprintf("copied %d bytes of new options, expected %d bytes", copied, len(newOpts))) @@ -869,7 +871,7 @@ func (e *endpoint) handlePacket(pkt *stack.PacketBuffer) { } if p == header.IGMPProtocolNumber { e.mu.Lock() - e.mu.igmp.handleIGMP(pkt) + e.mu.igmp.handleIGMP(pkt, hasRouterAlertOption) e.mu.Unlock() return } @@ -1291,9 +1293,12 @@ type optionActions struct { // timestamp controls what to do with a Timestamp option. timestamp optionAction - // recordroute controls what to do with a Record Route option. + // recordRoute controls what to do with a Record Route option. recordRoute optionAction + // routerAlert controls what to do with a Router Alert option. + routerAlert optionAction + // unknown controls what to do with an unknown option. unknown optionAction } @@ -1314,6 +1319,7 @@ func (*optionUsageVerify) actions() optionActions { return optionActions{ timestamp: optionVerify, recordRoute: optionVerify, + routerAlert: optionVerify, unknown: optionRemove, } } @@ -1327,6 +1333,7 @@ func (*optionUsageReceive) actions() optionActions { return optionActions{ timestamp: optionProcess, recordRoute: optionProcess, + routerAlert: optionVerify, unknown: optionPass, } } @@ -1341,6 +1348,7 @@ func (*optionUsageForward) actions() optionActions { return optionActions{ timestamp: optionProcess, recordRoute: optionProcess, + routerAlert: optionVerify, unknown: optionPass, } } @@ -1354,6 +1362,7 @@ func (*optionUsageEcho) actions() optionActions { return optionActions{ timestamp: optionProcess, recordRoute: optionProcess, + routerAlert: optionVerify, unknown: optionRemove, } } @@ -1551,45 +1560,64 @@ func handleRecordRoute(rrOpt header.IPv4OptionRecordRoute, localAddress tcpip.Ad return nil } +// handleRouterAlert performs sanity checks on a Router Alert option. +func handleRouterAlert(raOpt header.IPv4OptionRouterAlert) *header.IPv4OptParameterProblem { + // Only the zero value is acceptable, as per RFC 2113, section 2.1: + // Value: A two octet code with the following values: + // 0 - Router shall examine packet + // 1-65535 - Reserved + if raOpt.Value() != header.IPv4OptionRouterAlertValue { + return &header.IPv4OptParameterProblem{ + Pointer: header.IPv4OptionRouterAlertValueOffset, + NeedICMP: true, + } + } + return nil +} + +type optionTracker struct { + timestamp bool + recordRoute bool + routerAlert bool +} + // processIPOptions parses the IPv4 options and produces a new set of options // suitable for use in the next step of packet processing as informed by usage. // The original will not be touched. // -// Returns -// - The location of an error if there was one (or 0 if no error) -// - If there is an error, information as to what it was was. -// - The replacement option set. -func (e *endpoint) processIPOptions(pkt *stack.PacketBuffer, orig header.IPv4Options, usage optionsUsage) (header.IPv4Options, *header.IPv4OptParameterProblem) { +// If there were no errors during parsing, the new set of options is returned as +// a new buffer. +func (e *endpoint) processIPOptions(pkt *stack.PacketBuffer, orig header.IPv4Options, usage optionsUsage) (header.IPv4Options, optionTracker, *header.IPv4OptParameterProblem) { stats := e.stats.ip opts := header.IPv4Options(orig) optIter := opts.MakeIterator() - // Each option other than NOP must only appear (RFC 791 section 3.1, at the - // definition of every type). Keep track of each of the possible types in - // the 8 bit 'type' field. + // Except NOP, each option must only appear at most once (RFC 791 section 3.1, + // at the definition of every type). + // Keep track of each option we find to enable duplicate option detection. var seenOptions [math.MaxUint8 + 1]bool - // TODO(gvisor.dev/issue/4586): - // This will need tweaking when we start really forwarding packets - // as we may need to get two addresses, for rx and tx interfaces. - // We will also have to take usage into account. + // TODO(https://gvisor.dev/issue/4586): This will need tweaking when we start + // really forwarding packets as we may need to get two addresses, for rx and + // tx interfaces. We will also have to take usage into account. prefixedAddress, ok := e.protocol.stack.GetMainNICAddress(e.nic.ID(), ProtocolNumber) localAddress := prefixedAddress.Address if !ok { h := header.IPv4(pkt.NetworkHeader().View()) dstAddr := h.DestinationAddress() if pkt.NetworkPacketInfo.LocalAddressBroadcast || header.IsV4MulticastAddress(dstAddr) { - return nil, &header.IPv4OptParameterProblem{ + return nil, optionTracker{}, &header.IPv4OptParameterProblem{ NeedICMP: false, } } localAddress = dstAddr } + var optionsProcessed optionTracker for { option, done, optProblem := optIter.Next() if done || optProblem != nil { - return optIter.Finalize(), optProblem + return optIter.Finalize(), optionsProcessed, optProblem } optType := option.Type() if optType == header.IPv4OptionNOPType { @@ -1598,53 +1626,61 @@ func (e *endpoint) processIPOptions(pkt *stack.PacketBuffer, orig header.IPv4Opt } if optType == header.IPv4OptionListEndType { optIter.PushNOPOrEnd(optType) - return optIter.Finalize(), nil + return optIter.Finalize(), optionsProcessed, nil } // check for repeating options (multiple NOPs are OK) if seenOptions[optType] { - return nil, &header.IPv4OptParameterProblem{ + return nil, optionTracker{}, &header.IPv4OptParameterProblem{ Pointer: optIter.ErrCursor, NeedICMP: true, } } seenOptions[optType] = true - optLen := int(option.Size()) - switch option := option.(type) { - case *header.IPv4OptionTimestamp: - stats.OptionTSReceived.Increment() - if usage.actions().timestamp != optionRemove { - clock := e.protocol.stack.Clock() - newBuffer := optIter.RemainingBuffer()[:len(*option)] - _ = copy(newBuffer, option.Contents()) - if optProblem := handleTimestamp(header.IPv4OptionTimestamp(newBuffer), localAddress, clock, usage); optProblem != nil { - optProblem.Pointer += optIter.ErrCursor - return nil, optProblem + optLen, optProblem := func() (int, *header.IPv4OptParameterProblem) { + switch option := option.(type) { + case *header.IPv4OptionTimestamp: + stats.OptionTimestampReceived.Increment() + optionsProcessed.timestamp = true + if usage.actions().timestamp != optionRemove { + clock := e.protocol.stack.Clock() + newBuffer := optIter.InitReplacement(option) + optProblem := handleTimestamp(header.IPv4OptionTimestamp(newBuffer), localAddress, clock, usage) + return len(newBuffer), optProblem } - optIter.ConsumeBuffer(optLen) - } - case *header.IPv4OptionRecordRoute: - stats.OptionRRReceived.Increment() - if usage.actions().recordRoute != optionRemove { - newBuffer := optIter.RemainingBuffer()[:len(*option)] - _ = copy(newBuffer, option.Contents()) - if optProblem := handleRecordRoute(header.IPv4OptionRecordRoute(newBuffer), localAddress, usage); optProblem != nil { - optProblem.Pointer += optIter.ErrCursor - return nil, optProblem + case *header.IPv4OptionRecordRoute: + stats.OptionRecordRouteReceived.Increment() + optionsProcessed.recordRoute = true + if usage.actions().recordRoute != optionRemove { + newBuffer := optIter.InitReplacement(option) + optProblem := handleRecordRoute(header.IPv4OptionRecordRoute(newBuffer), localAddress, usage) + return len(newBuffer), optProblem } - optIter.ConsumeBuffer(optLen) - } - default: - stats.OptionUnknownReceived.Increment() - if usage.actions().unknown == optionPass { - newBuffer := optIter.RemainingBuffer()[:optLen] - // Arguments already heavily checked.. ignore result. - _ = copy(newBuffer, option.Contents()) - optIter.ConsumeBuffer(optLen) + case *header.IPv4OptionRouterAlert: + stats.OptionRouterAlertReceived.Increment() + optionsProcessed.routerAlert = true + if usage.actions().routerAlert != optionRemove { + newBuffer := optIter.InitReplacement(option) + optProblem := handleRouterAlert(header.IPv4OptionRouterAlert(newBuffer)) + return len(newBuffer), optProblem + } + + default: + stats.OptionUnknownReceived.Increment() + if usage.actions().unknown == optionPass { + return len(optIter.InitReplacement(option)), nil + } } + return 0, nil + }() + + if optProblem != nil { + optProblem.Pointer += optIter.ErrCursor + return nil, optionTracker{}, optProblem } + optIter.ConsumeBuffer(optLen) } } diff --git a/pkg/tcpip/network/multicast_group_test.go b/pkg/tcpip/network/multicast_group_test.go index 0f4f0e1e1..03b7ecffb 100644 --- a/pkg/tcpip/network/multicast_group_test.go +++ b/pkg/tcpip/network/multicast_group_test.go @@ -35,8 +35,9 @@ import ( const ( linkAddr = tcpip.LinkAddress("\x02\x02\x03\x04\x05\x06") - ipv4Addr = tcpip.Address("\x0a\x00\x00\x01") - ipv6Addr = tcpip.Address("\xfe\x80\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01") + stackIPv4Addr = tcpip.Address("\x0a\x00\x00\x01") + defaultIPv4PrefixLength = 24 + ipv6Addr = tcpip.Address("\xfe\x80\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01") ipv4MulticastAddr1 = tcpip.Address("\xe0\x00\x00\x03") ipv4MulticastAddr2 = tcpip.Address("\xe0\x00\x00\x04") @@ -99,7 +100,7 @@ func validateIGMPPacket(t *testing.T, p channel.PacketInfo, remoteAddress tcpip. payload := header.IPv4(stack.PayloadSince(p.Pkt.NetworkHeader())) checker.IPv4(t, payload, - checker.SrcAddr(ipv4Addr), + checker.SrcAddr(stackIPv4Addr), checker.DstAddr(remoteAddress), // TTL for an IGMP message must be 1 as per RFC 2236 section 2. checker.TTL(1), @@ -145,8 +146,12 @@ func createStackWithLinkEndpoint(t *testing.T, v4, mgpEnabled bool, e stack.Link if err := s.CreateNIC(nicID, e); err != nil { t.Fatalf("CreateNIC(%d, _) = %s", nicID, err) } - if err := s.AddAddress(nicID, ipv4.ProtocolNumber, ipv4Addr); err != nil { - t.Fatalf("AddAddress(%d, %d, %s): %s", nicID, ipv4.ProtocolNumber, ipv4Addr, err) + addr := tcpip.AddressWithPrefix{ + Address: stackIPv4Addr, + PrefixLen: defaultIPv4PrefixLength, + } + if err := s.AddAddressWithPrefix(nicID, ipv4.ProtocolNumber, addr); err != nil { + t.Fatalf("AddAddressWithPrefix(%d, %d, %s): %s", nicID, ipv4.ProtocolNumber, addr, err) } if err := s.AddAddress(nicID, ipv6.ProtocolNumber, ipv6Addr); err != nil { t.Fatalf("AddAddress(%d, %d, %s): %s", nicID, ipv6.ProtocolNumber, ipv6Addr, err) @@ -202,24 +207,23 @@ func checkInitialIPv6Groups(t *testing.T, e *channel.Endpoint, s *stack.Stack, c // createAndInjectIGMPPacket creates and injects an IGMP packet with the // specified fields. -// -// Note, the router alert option is not included in this packet. -// -// TODO(b/162198658): set the router alert option. func createAndInjectIGMPPacket(e *channel.Endpoint, igmpType byte, maxRespTime byte, groupAddress tcpip.Address) { - buf := buffer.NewView(header.IPv4MinimumSize + header.IGMPQueryMinimumSize) - + options := header.IPv4OptionsSerializer{ + &header.IPv4SerializableRouterAlertOption{}, + } + buf := buffer.NewView(header.IPv4MinimumSize + int(options.Length()) + header.IGMPQueryMinimumSize) ip := header.IPv4(buf) ip.Encode(&header.IPv4Fields{ TotalLength: uint16(len(buf)), TTL: header.IGMPTTL, Protocol: uint8(header.IGMPProtocolNumber), - SrcAddr: header.IPv4Any, + SrcAddr: remoteIPv4Addr, DstAddr: header.IPv4AllSystems, + Options: options, }) ip.SetChecksum(^ip.CalculateChecksum()) - igmp := header.IGMP(buf[header.IPv4MinimumSize:]) + igmp := header.IGMP(ip.Payload()) igmp.SetType(header.IGMPType(igmpType)) igmp.SetMaxRespTime(maxRespTime) igmp.SetGroupAddress(groupAddress) diff --git a/pkg/tcpip/tcpip.go b/pkg/tcpip/tcpip.go index ba063dc26..01a4389e3 100644 --- a/pkg/tcpip/tcpip.go +++ b/pkg/tcpip/tcpip.go @@ -1581,11 +1581,16 @@ type IPStats struct { // the Output chain. IPTablesOutputDropped *StatCounter - // OptionTSReceived is the number of Timestamp options seen. - OptionTSReceived *StatCounter + // TODO(https://gvisor.dev/issues/5529): Move the IPv4-only option stats out + // of IPStats. + // OptionTimestampReceived is the number of Timestamp options seen. + OptionTimestampReceived *StatCounter - // OptionRRReceived is the number of Record Route options seen. - OptionRRReceived *StatCounter + // OptionRecordRouteReceived is the number of Record Route options seen. + OptionRecordRouteReceived *StatCounter + + // OptionRouterAlertReceived is the number of Router Alert options seen. + OptionRouterAlertReceived *StatCounter // OptionUnknownReceived is the number of unknown IP options seen. OptionUnknownReceived *StatCounter -- cgit v1.2.3