diff --git a/pkg/tcpip/network/ipv4/icmp.go b/pkg/tcpip/network/ipv4/icmp.go index 33a1b837e..2a024fe38 100644 --- a/pkg/tcpip/network/ipv4/icmp.go +++ b/pkg/tcpip/network/ipv4/icmp.go @@ -219,7 +219,7 @@ func (e *endpoint) handleICMP(pkt *stack.PacketBuffer) { if optProblem.NeedICMP { _ = e.protocol.returnError(&icmpReasonParamProblem{ pointer: optProblem.Pointer, - }, pkt) + }, pkt, true /* deliveredLocally */) e.stats.ip.MalformedPacketsReceived.Increment() } return @@ -387,9 +387,6 @@ func (e *endpoint) handleICMP(pkt *stack.PacketBuffer) { // icmpReason is a marker interface for IPv4 specific ICMP errors. type icmpReason interface { isICMPReason() - // isForwarding indicates whether or not the error arose while attempting to - // forward a packet. - isForwarding() bool } // icmpReasonPortUnreachable is an error where the transport protocol has no @@ -397,18 +394,12 @@ type icmpReason interface { type icmpReasonPortUnreachable struct{} func (*icmpReasonPortUnreachable) isICMPReason() {} -func (*icmpReasonPortUnreachable) isForwarding() bool { - return false -} // icmpReasonProtoUnreachable is an error where the transport protocol is // not supported. type icmpReasonProtoUnreachable struct{} func (*icmpReasonProtoUnreachable) isICMPReason() {} -func (*icmpReasonProtoUnreachable) isForwarding() bool { - return false -} // icmpReasonTTLExceeded is an error where a packet's time to live exceeded in // transit to its final destination, as per RFC 792 page 6, Time Exceeded @@ -416,15 +407,6 @@ func (*icmpReasonProtoUnreachable) isForwarding() bool { type icmpReasonTTLExceeded struct{} func (*icmpReasonTTLExceeded) isICMPReason() {} -func (*icmpReasonTTLExceeded) isForwarding() bool { - // If we hit a TTL Exceeded error, then we know we are operating as a router. - // As per RFC 792 page 6, Time Exceeded Message, - // - // If the gateway processing a datagram finds the time to live field - // is zero it must discard the datagram. The gateway may also notify - // the source host via the time exceeded message. - return true -} // icmpReasonReassemblyTimeout is an error where insufficient fragments are // received to complete reassembly of a packet within a configured time after @@ -432,38 +414,20 @@ func (*icmpReasonTTLExceeded) isForwarding() bool { type icmpReasonReassemblyTimeout struct{} func (*icmpReasonReassemblyTimeout) isICMPReason() {} -func (*icmpReasonReassemblyTimeout) isForwarding() bool { - return false -} // icmpReasonParamProblem is an error to use to request a Parameter Problem // message to be sent. type icmpReasonParamProblem struct { - pointer byte - forwarding bool + pointer byte } func (*icmpReasonParamProblem) isICMPReason() {} -func (r *icmpReasonParamProblem) isForwarding() bool { - return r.forwarding -} // icmpReasonNetworkUnreachable is an error in which the network specified in // the internet destination field of the datagram is unreachable. type icmpReasonNetworkUnreachable struct{} func (*icmpReasonNetworkUnreachable) isICMPReason() {} -func (*icmpReasonNetworkUnreachable) isForwarding() bool { - // If we hit a Net Unreachable error, then we know we are operating as - // a router. As per RFC 792 page 5, Destination Unreachable Message, - // - // If, according to the information in the gateway's routing tables, - // the network specified in the internet destination field of a - // datagram is unreachable, e.g., the distance to the network is - // infinity, the gateway may send a destination unreachable message to - // the internet source host of the datagram. - return true -} // icmpReasonFragmentationNeeded is an error where a packet requires // fragmentation while also having the Don't Fragment flag set, as per RFC 792 @@ -471,38 +435,19 @@ func (*icmpReasonNetworkUnreachable) isForwarding() bool { type icmpReasonFragmentationNeeded struct{} func (*icmpReasonFragmentationNeeded) isICMPReason() {} -func (*icmpReasonFragmentationNeeded) isForwarding() bool { - // If we hit a Don't Fragment error, then we know we are operating as a router. - // As per RFC 792 page 4, Destination Unreachable Message, - // - // Another case is when a datagram must be fragmented to be forwarded by a - // gateway yet the Don't Fragment flag is on. In this case the gateway must - // discard the datagram and may return a destination unreachable message. - return true -} // icmpReasonHostUnreachable is an error in which the host specified in the // internet destination field of the datagram is unreachable. type icmpReasonHostUnreachable struct{} func (*icmpReasonHostUnreachable) isICMPReason() {} -func (*icmpReasonHostUnreachable) isForwarding() bool { - // If we hit a Host Unreachable error, then we know we are operating as a - // router. As per RFC 792 page 5, Destination Unreachable Message, - // - // In addition, in some networks, the gateway may be able to determine - // if the internet destination host is unreachable. Gateways in these - // networks may send destination unreachable messages to the source host - // when the destination host is unreachable. - return true -} // returnError takes an error descriptor and generates the appropriate ICMP // error packet for IPv4 and sends it back to the remote device that sent // the problematic packet. It incorporates as much of that packet as // possible as well as any error metadata as is available. returnError // expects pkt to hold a valid IPv4 packet as per the wire format. -func (p *protocol) returnError(reason icmpReason, pkt *stack.PacketBuffer) tcpip.Error { +func (p *protocol) returnError(reason icmpReason, pkt *stack.PacketBuffer, deliveredLocally bool) tcpip.Error { origIPHdr := header.IPv4(pkt.NetworkHeader().View()) origIPHdrSrc := origIPHdr.SourceAddress() origIPHdrDst := origIPHdr.DestinationAddress() @@ -534,11 +479,11 @@ func (p *protocol) returnError(reason icmpReason, pkt *stack.PacketBuffer) tcpip return nil } - // If we are operating as a router/gateway, don't use the packet's destination + // If the packet wasn't delivered locally, do not use the packet's destination // address as the response's source address as we should not not own the // destination address of a packet we are forwarding. localAddr := origIPHdrDst - if reason.isForwarding() { + if !deliveredLocally { localAddr = "" } @@ -704,6 +649,6 @@ func (p *protocol) OnReassemblyTimeout(pkt *stack.PacketBuffer) { // If fragment zero is not available then no time exceeded need be sent at // all. if pkt != nil { - p.returnError(&icmpReasonReassemblyTimeout{}, pkt) + p.returnError(&icmpReasonReassemblyTimeout{}, pkt, true /* deliveredLocally */) } } diff --git a/pkg/tcpip/network/ipv4/ipv4.go b/pkg/tcpip/network/ipv4/ipv4.go index 97b58ceab..8afb52a99 100644 --- a/pkg/tcpip/network/ipv4/ipv4.go +++ b/pkg/tcpip/network/ipv4/ipv4.go @@ -109,7 +109,7 @@ func (e *endpoint) HandleLinkResolutionFailure(pkt *stack.PacketBuffer) { if pkt.NetworkPacketInfo.IsForwardedPacket { // TODO(gvisor.dev/issue/6005): Propagate asynchronously generated ICMP // errors to local endpoints. - e.protocol.returnError(&icmpReasonHostUnreachable{}, pkt) + e.protocol.returnError(&icmpReasonHostUnreachable{}, pkt, false /* deliveredLocally */) e.stats.ip.Forwarding.Errors.Increment() e.stats.ip.Forwarding.HostUnreachable.Increment() return @@ -597,7 +597,7 @@ func (e *endpoint) forwardPacket(pkt *stack.PacketBuffer) ip.ForwardingError { // We return the original error rather than the result of returning // the ICMP packet because the original error is more relevant to // the caller. - _ = e.protocol.returnError(&icmpReasonTTLExceeded{}, pkt) + _ = e.protocol.returnError(&icmpReasonTTLExceeded{}, pkt, false /* deliveredLocally */) return &ip.ErrTTLExceeded{} } @@ -606,9 +606,8 @@ func (e *endpoint) forwardPacket(pkt *stack.PacketBuffer) ip.ForwardingError { if optProblem != nil { if optProblem.NeedICMP { _ = e.protocol.returnError(&icmpReasonParamProblem{ - pointer: optProblem.Pointer, - forwarding: true, - }, pkt) + pointer: optProblem.Pointer, + }, pkt, false /* deliveredLocally */) } return &ip.ErrParameterProblem{} } @@ -651,7 +650,7 @@ func (e *endpoint) forwardPacket(pkt *stack.PacketBuffer) ip.ForwardingError { // We return the original error rather than the result of returning // the ICMP packet because the original error is more relevant to // the caller. - _ = e.protocol.returnError(&icmpReasonNetworkUnreachable{}, pkt) + _ = e.protocol.returnError(&icmpReasonNetworkUnreachable{}, pkt, false /* deliveredLocally */) return &ip.ErrNoRoute{} default: return &ip.ErrOther{Err: err} @@ -705,7 +704,7 @@ func (e *endpoint) forwardPacket(pkt *stack.PacketBuffer) ip.ForwardingError { // WriteHeaderIncludedPacket checks for the presence of the Don't Fragment bit // while sending the packet and returns this error iff fragmentation is // necessary and the bit is also set. - _ = e.protocol.returnError(&icmpReasonFragmentationNeeded{}, pkt) + _ = e.protocol.returnError(&icmpReasonFragmentationNeeded{}, pkt, false /* deliveredLocally */) return &ip.ErrMessageTooLong{} default: return &ip.ErrOther{Err: err} @@ -880,7 +879,7 @@ func (e *endpoint) handleValidatedPacket(h header.IPv4, pkt *stack.PacketBuffer, if optProblem.NeedICMP { _ = e.protocol.returnError(&icmpReasonParamProblem{ pointer: optProblem.Pointer, - }, pkt) + }, pkt, true /* deliveredLocally */) e.stats.ip.MalformedPacketsReceived.Increment() } return @@ -957,7 +956,7 @@ func (e *endpoint) handleValidatedPacket(h header.IPv4, pkt *stack.PacketBuffer, if optProblem.NeedICMP { _ = e.protocol.returnError(&icmpReasonParamProblem{ pointer: optProblem.Pointer, - }, pkt) + }, pkt, true /* deliveredLocally */) stats.ip.MalformedPacketsReceived.Increment() } return @@ -987,13 +986,13 @@ func (e *endpoint) handleValidatedPacket(h header.IPv4, pkt *stack.PacketBuffer, // 3 (Port Unreachable), when the designated transport protocol // (e.g., UDP) is unable to demultiplex the datagram but has no // protocol mechanism to inform the sender. - _ = e.protocol.returnError(&icmpReasonPortUnreachable{}, pkt) + _ = e.protocol.returnError(&icmpReasonPortUnreachable{}, pkt, true /* deliveredLocally */) case stack.TransportPacketProtocolUnreachable: // As per RFC: 1122 Section 3.2.2.1 // A host SHOULD generate Destination Unreachable messages with code: // 2 (Protocol Unreachable), when the designated transport protocol // is not supported - _ = e.protocol.returnError(&icmpReasonProtoUnreachable{}, pkt) + _ = e.protocol.returnError(&icmpReasonProtoUnreachable{}, pkt, true /* deliveredLocally */) default: panic(fmt.Sprintf("unrecognized result from DeliverTransportPacket = %d", res)) } diff --git a/pkg/tcpip/network/ipv6/icmp.go b/pkg/tcpip/network/ipv6/icmp.go index 402c4c8a8..0ae6bb3e8 100644 --- a/pkg/tcpip/network/ipv6/icmp.go +++ b/pkg/tcpip/network/ipv6/icmp.go @@ -928,9 +928,6 @@ func (*endpoint) ResolveStaticAddress(addr tcpip.Address) (tcpip.LinkAddress, bo // icmpReason is a marker interface for IPv6 specific ICMP errors. type icmpReason interface { isICMPReason() - // isForwarding indicates whether or not the error arose while attempting to - // forward a packet. - isForwarding() bool // respondToMulticast indicates whether this error falls under the exception // outlined by RFC 4443 section 2.4 point e.3 exception 2: // @@ -958,15 +955,10 @@ type icmpReasonParameterProblem struct { // in the maximum size of an ICMPv6 error message. pointer uint32 - forwarding bool - respondToMulticast bool } func (*icmpReasonParameterProblem) isICMPReason() {} -func (p *icmpReasonParameterProblem) isForwarding() bool { - return p.forwarding -} func (p *icmpReasonParameterProblem) respondsToMulticast() bool { return p.respondToMulticast @@ -978,10 +970,6 @@ type icmpReasonPortUnreachable struct{} func (*icmpReasonPortUnreachable) isICMPReason() {} -func (*icmpReasonPortUnreachable) isForwarding() bool { - return false -} - func (*icmpReasonPortUnreachable) respondsToMulticast() bool { return false } @@ -992,16 +980,6 @@ type icmpReasonNetUnreachable struct{} func (*icmpReasonNetUnreachable) isICMPReason() {} -func (*icmpReasonNetUnreachable) isForwarding() bool { - // If we hit a Network Unreachable error, then we also know we are - // operating as a router. As per RFC 4443 section 3.1: - // - // If the reason for the failure to deliver is lack of a matching - // entry in the forwarding node's routing table, the Code field is - // set to 0 (Network Unreachable). - return true -} - func (*icmpReasonNetUnreachable) respondsToMulticast() bool { return false } @@ -1011,16 +989,6 @@ func (*icmpReasonNetUnreachable) respondsToMulticast() bool { type icmpReasonHostUnreachable struct{} func (*icmpReasonHostUnreachable) isICMPReason() {} -func (*icmpReasonHostUnreachable) isForwarding() bool { - // If we hit a Host Unreachable error, then we know we are operating as a - // router. As per RFC 4443 page 8, Destination Unreachable Message, - // - // If the reason for the failure to deliver cannot be mapped to any of - // other codes, the Code field is set to 3. Example of such cases are - // an inability to resolve the IPv6 destination address into a - // corresponding link address, or a link-specific problem of some sort. - return true -} func (*icmpReasonHostUnreachable) respondsToMulticast() bool { return false @@ -1032,16 +1000,6 @@ type icmpReasonPacketTooBig struct{} func (*icmpReasonPacketTooBig) isICMPReason() {} -func (*icmpReasonPacketTooBig) isForwarding() bool { - // If we hit a Packet Too Big error, then we know we are operating as a router. - // As per RFC 4443 section 3.2: - // - // A Packet Too Big MUST be sent by a router in response to a packet that it - // cannot forward because the packet is larger than the MTU of the outgoing - // link. - return true -} - func (*icmpReasonPacketTooBig) respondsToMulticast() bool { return true } @@ -1052,18 +1010,6 @@ type icmpReasonHopLimitExceeded struct{} func (*icmpReasonHopLimitExceeded) isICMPReason() {} -func (*icmpReasonHopLimitExceeded) isForwarding() bool { - // If we hit a Hop Limit Exceeded error, then we know we are operating - // as a router. As per RFC 4443 section 3.3: - // - // If a router receives a packet with a Hop Limit of zero, or if a - // router decrements a packet's Hop Limit to zero, it MUST discard - // the packet and originate an ICMPv6 Time Exceeded message with Code - // 0 to the source of the packet. This indicates either a routing - // loop or too small an initial Hop Limit value. - return true -} - func (*icmpReasonHopLimitExceeded) respondsToMulticast() bool { return false } @@ -1075,17 +1021,13 @@ type icmpReasonReassemblyTimeout struct{} func (*icmpReasonReassemblyTimeout) isICMPReason() {} -func (*icmpReasonReassemblyTimeout) isForwarding() bool { - return false -} - func (*icmpReasonReassemblyTimeout) respondsToMulticast() bool { return false } // returnError takes an error descriptor and generates the appropriate ICMP // error packet for IPv6 and sends it. -func (p *protocol) returnError(reason icmpReason, pkt *stack.PacketBuffer) tcpip.Error { +func (p *protocol) returnError(reason icmpReason, pkt *stack.PacketBuffer, deliveredLocally bool) tcpip.Error { origIPHdr := header.IPv6(pkt.NetworkHeader().View()) origIPHdrSrc := origIPHdr.SourceAddress() origIPHdrDst := origIPHdr.DestinationAddress() @@ -1117,7 +1059,7 @@ func (p *protocol) returnError(reason icmpReason, pkt *stack.PacketBuffer) tcpip return nil } - // If we are operating as a router, do not use the packet's destination + // If the packet wasn't delivered locally, do not use the packet's destination // address as the response's source address as we should not own the // destination address of a packet we are forwarding. // @@ -1126,7 +1068,7 @@ func (p *protocol) returnError(reason icmpReason, pkt *stack.PacketBuffer) tcpip // packet as "multicast addresses must not be used as source addresses in IPv6 // packets", as per RFC 4291 section 2.7. localAddr := origIPHdrDst - if reason.isForwarding() || isOrigDstMulticast { + if !deliveredLocally || isOrigDstMulticast { localAddr = "" } // Even if we were able to receive a packet from some remote, we may not have @@ -1255,6 +1197,6 @@ func (p *protocol) OnReassemblyTimeout(pkt *stack.PacketBuffer) { // been received, an ICMP Time Exceeded -- Fragment Reassembly Time Exceeded // message should be sent to the source of that fragment. if pkt != nil { - p.returnError(&icmpReasonReassemblyTimeout{}, pkt) + p.returnError(&icmpReasonReassemblyTimeout{}, pkt, true /* deliveredLocally */) } } diff --git a/pkg/tcpip/network/ipv6/ipv6.go b/pkg/tcpip/network/ipv6/ipv6.go index 94fa59f82..8b2d09bc3 100644 --- a/pkg/tcpip/network/ipv6/ipv6.go +++ b/pkg/tcpip/network/ipv6/ipv6.go @@ -286,7 +286,7 @@ func (e *endpoint) HandleLinkResolutionFailure(pkt *stack.PacketBuffer) { if pkt.NetworkPacketInfo.IsForwardedPacket { // TODO(gvisor.dev/issue/6005): Propagate asynchronously generated ICMP // errors to local endpoints. - e.protocol.returnError(&icmpReasonHostUnreachable{}, pkt) + e.protocol.returnError(&icmpReasonHostUnreachable{}, pkt, false /* deliveredLocally */) e.stats.ip.Forwarding.Errors.Increment() e.stats.ip.Forwarding.HostUnreachable.Increment() return @@ -891,7 +891,7 @@ func (e *endpoint) forwardPacket(pkt *stack.PacketBuffer) ip.ForwardingError { // We return the original error rather than the result of returning // the ICMP packet because the original error is more relevant to // the caller. - _ = e.protocol.returnError(&icmpReasonHopLimitExceeded{}, pkt) + _ = e.protocol.returnError(&icmpReasonHopLimitExceeded{}, pkt, false /* deliveredLocally */) return &ip.ErrTTLExceeded{} } @@ -923,7 +923,7 @@ func (e *endpoint) forwardPacket(pkt *stack.PacketBuffer) ip.ForwardingError { case *tcpip.ErrNoRoute, *tcpip.ErrNetworkUnreachable: // We return the original error rather than the result of returning the // ICMP packet because the original error is more relevant to the caller. - _ = e.protocol.returnError(&icmpReasonNetUnreachable{}, pkt) + _ = e.protocol.returnError(&icmpReasonNetUnreachable{}, pkt, false /* deliveredLocally */) return &ip.ErrNoRoute{} default: return &ip.ErrOther{Err: err} @@ -965,7 +965,7 @@ func (e *endpoint) forwardPacket(pkt *stack.PacketBuffer) ip.ForwardingError { // A Packet Too Big MUST be sent by a router in response to a packet that // it cannot forward because the packet is larger than the MTU of the // outgoing link. - _ = e.protocol.returnError(&icmpReasonPacketTooBig{}, pkt) + _ = e.protocol.returnError(&icmpReasonPacketTooBig{}, pkt, false /* deliveredLocally */) return &ip.ErrMessageTooLong{} default: return &ip.ErrOther{Err: err} @@ -1173,10 +1173,9 @@ func (e *endpoint) processExtensionHeaders(h header.IPv6, pkt *stack.PacketBuffe // restricted to appear immediately after an IPv6 fixed header. if previousHeaderStart != 0 { _ = e.protocol.returnError(&icmpReasonParameterProblem{ - code: header.ICMPv6UnknownHeader, - pointer: previousHeaderStart, - forwarding: forwarding, - }, pkt) + code: header.ICMPv6UnknownHeader, + pointer: previousHeaderStart, + }, pkt, !forwarding /* deliveredLocally */) return fmt.Errorf("found Hop-by-Hop header = %#v with non-zero previous header offset = %d", extHdr, previousHeaderStart) } @@ -1227,8 +1226,7 @@ func (e *endpoint) processExtensionHeaders(h header.IPv6, pkt *stack.PacketBuffe code: header.ICMPv6UnknownOption, pointer: it.ParseOffset() + optsIt.OptionOffset(), respondToMulticast: true, - forwarding: forwarding, - }, pkt) + }, pkt, !forwarding /* deliveredLocally */) return fmt.Errorf("found unknown hop-by-hop header option = %#v with discard action", opt) default: panic(fmt.Sprintf("unrecognized action for an unrecognized Hop By Hop extension header option = %#v", opt)) @@ -1253,12 +1251,7 @@ func (e *endpoint) processExtensionHeaders(h header.IPv6, pkt *stack.PacketBuffe _ = e.protocol.returnError(&icmpReasonParameterProblem{ code: header.ICMPv6ErroneousHeader, pointer: it.ParseOffset(), - // For the sake of consistency, we're using the value of `forwarding` - // here, even though it should always be false if we've reached this - // point. If `forwarding` is true here, we're executing undefined - // behavior no matter what. - forwarding: forwarding, - }, pkt) + }, pkt, true /* deliveredLocally */) return fmt.Errorf("found unrecognized routing type with non-zero segments left in header = %#v", extHdr) } @@ -1348,7 +1341,7 @@ func (e *endpoint) processExtensionHeaders(h header.IPv6, pkt *stack.PacketBuffe _ = e.protocol.returnError(&icmpReasonParameterProblem{ code: header.ICMPv6ErroneousHeader, pointer: header.IPv6PayloadLenOffset, - }, pkt) + }, pkt, true /* deliveredLocally */) return fmt.Errorf("found fragment length = %d that is not a multiple of 8 octets", fragmentPayloadLen) } @@ -1370,7 +1363,7 @@ func (e *endpoint) processExtensionHeaders(h header.IPv6, pkt *stack.PacketBuffe _ = e.protocol.returnError(&icmpReasonParameterProblem{ code: header.ICMPv6ErroneousHeader, pointer: fragmentFieldOffset, - }, pkt) + }, pkt, true /* deliveredLocally */) return fmt.Errorf("determined that reassembled packet length = %d would exceed allowed length = %d", lengthAfterReassembly, header.IPv6MaximumPayloadSize) } @@ -1445,7 +1438,7 @@ func (e *endpoint) processExtensionHeaders(h header.IPv6, pkt *stack.PacketBuffe code: header.ICMPv6UnknownOption, pointer: it.ParseOffset() + optsIt.OptionOffset(), respondToMulticast: true, - }, pkt) + }, pkt, true /* deliveredLocally */) return fmt.Errorf("found unknown destination header option %#v with discard action", opt) default: panic(fmt.Sprintf("unrecognized action for an unrecognized Destination extension header option = %#v", opt)) @@ -1493,7 +1486,7 @@ func (e *endpoint) processExtensionHeaders(h header.IPv6, pkt *stack.PacketBuffe // message with Code 4 in response to a packet for which the // transport protocol (e.g., UDP) has no listener, if that transport // protocol has no alternative means to inform the sender. - _ = e.protocol.returnError(&icmpReasonPortUnreachable{}, pkt) + _ = e.protocol.returnError(&icmpReasonPortUnreachable{}, pkt, true /* deliveredLocally */) return fmt.Errorf("destination port unreachable") case stack.TransportPacketProtocolUnreachable: // As per RFC 8200 section 4. (page 7): @@ -1525,7 +1518,7 @@ func (e *endpoint) processExtensionHeaders(h header.IPv6, pkt *stack.PacketBuffe _ = e.protocol.returnError(&icmpReasonParameterProblem{ code: header.ICMPv6UnknownHeader, pointer: prevHdrIDOffset, - }, pkt) + }, pkt, true /* deliveredLocally */) return fmt.Errorf("transport protocol unreachable") default: panic(fmt.Sprintf("unrecognized result from DeliverTransportPacket = %d", res))