Don't conclude broadcast from route destination

The routing table (in its current) form should not be used to make
decisions about whether a remote address is a broadcast address or
not (for IPv4).

Note, a destination subnet does not always map to a network.
E.g. RouterA may have a route to 192.168.0.0/22 through RouterB,
but RouterB may be configured with 4x /24 subnets on 4 different
interfaces.

See https://github.com/google/gvisor/issues/3938.

PiperOrigin-RevId: 331819868
This commit is contained in:
Ghanan Gowripalan 2020-09-15 11:51:17 -07:00 committed by gVisor bot
parent 52ffeb2d64
commit d3880b76cb
3 changed files with 16 additions and 17 deletions

View File

@ -48,10 +48,6 @@ type Route struct {
// Loop controls where WritePacket should send packets.
Loop PacketLooping
// directedBroadcast indicates whether this route is sending a directed
// broadcast packet.
directedBroadcast bool
}
// makeRoute initializes a new route. It takes ownership of the provided
@ -303,24 +299,27 @@ func (r *Route) Stack() *Stack {
return r.ref.stack()
}
func (r *Route) isV4Broadcast(addr tcpip.Address) bool {
if addr == header.IPv4Broadcast {
return true
}
subnet := r.ref.addrWithPrefix().Subnet()
return subnet.IsBroadcast(addr)
}
// IsOutboundBroadcast returns true if the route is for an outbound broadcast
// packet.
func (r *Route) IsOutboundBroadcast() bool {
// Only IPv4 has a notion of broadcast.
return r.directedBroadcast || r.RemoteAddress == header.IPv4Broadcast
return r.isV4Broadcast(r.RemoteAddress)
}
// IsInboundBroadcast returns true if the route is for an inbound broadcast
// packet.
func (r *Route) IsInboundBroadcast() bool {
// Only IPv4 has a notion of broadcast.
if r.LocalAddress == header.IPv4Broadcast {
return true
}
addr := r.ref.addrWithPrefix()
subnet := addr.Subnet()
return subnet.IsBroadcast(r.LocalAddress)
return r.isV4Broadcast(r.LocalAddress)
}
// ReverseRoute returns new route with given source and destination address.

View File

@ -1311,13 +1311,11 @@ func (s *Stack) FindRoute(id tcpip.NICID, localAddr, remoteAddr tcpip.Address, n
}
r := makeRoute(netProto, ref.address(), remoteAddr, nic.linkEP.LinkAddress(), ref, s.handleLocal && !nic.isLoopback(), multicastLoop && !nic.isLoopback())
r.directedBroadcast = route.Destination.IsBroadcast(remoteAddr)
if len(route.Gateway) > 0 {
if needRoute {
r.NextHop = route.Gateway
}
} else if r.directedBroadcast {
} else if subnet := ref.addrWithPrefix().Subnet(); subnet.IsBroadcast(remoteAddr) {
r.RemoteLinkAddress = header.EthernetBroadcastAddress
}

View File

@ -2344,7 +2344,9 @@ func TestOutgoingSubnetBroadcast(t *testing.T) {
},
},
remoteAddr: remNetSubnetBcast,
requiresBroadcastOpt: true,
// TODO(gvisor.dev/issue/3938): Once we support marking a route as
// broadcast, this test should require the broadcast option to be set.
requiresBroadcastOpt: false,
},
}