| Message ID | 20240331211434.61100-1-mail@david-bauer.net (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] vxlan: drop packets from invalid src-address | expand |
On 2024-04-01 at 02:44:34, David Bauer (mail@david-bauer.net) wrote: > The VXLAN driver currently does not check if the inner layer2 > source-address is valid. > > In case source-address snooping/learning is enabled, a entry in the FDB > for the invalid address is created with the layer3 address of the tunnel > endpoint. what is root cause of creation of invalid MAC from an L3 address ? could you add that as well to commit message. > > If the frame happens to have a non-unicast address set, all this > non-unicast traffic is subsequently not flooded to the tunnel network > but sent to the learnt host in the FDB. To make matters worse, this FDB > entry does not expire. > > Apply the same filtering for packets as it is done for bridges. This not > only drops these invalid packets but avoids them from being learnt into > the FDB. > > Suggested-by: Ido Schimmel <idosch@nvidia.com> > Signed-off-by: David Bauer <mail@david-bauer.net> > --- > drivers/net/vxlan/vxlan_core.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c > index 3495591a5c29..ba319fc21957 100644 > --- a/drivers/net/vxlan/vxlan_core.c > +++ b/drivers/net/vxlan/vxlan_core.c > @@ -1615,6 +1615,10 @@ static bool vxlan_set_mac(struct vxlan_dev *vxlan, > if (ether_addr_equal(eth_hdr(skb)->h_source, vxlan->dev->dev_addr)) > return false; > > + /* Ignore packets from invalid src-address */ > + if (!is_valid_ether_addr(eth_hdr(skb)->h_source)) > + return false; > + > /* Get address from the outer IP header */ > if (vxlan_get_sk_family(vs) == AF_INET) { > saddr.sin.sin_addr.s_addr = ip_hdr(skb)->saddr; > -- > 2.43.0 >
Hello Ratheesh, On 4/1/24 05:04, Ratheesh Kannoth wrote: > On 2024-04-01 at 02:44:34, David Bauer (mail@david-bauer.net) wrote: >> The VXLAN driver currently does not check if the inner layer2 >> source-address is valid. >> >> In case source-address snooping/learning is enabled, a entry in the FDB >> for the invalid address is created with the layer3 address of the tunnel >> endpoint. > what is root cause of creation of invalid MAC from an L3 address ? could you > add that as well to commit message. I sadly can not elaborate on this further as the state happens sporadically after weeks of operation. For more details, see https://lore.kernel.org/all/15ee0cc7-9252-466b-8ce7-5225d605dde8@david-bauer.net/T/ Best David > >> >> If the frame happens to have a non-unicast address set, all this >> non-unicast traffic is subsequently not flooded to the tunnel network >> but sent to the learnt host in the FDB. To make matters worse, this FDB >> entry does not expire. >> >> Apply the same filtering for packets as it is done for bridges. This not >> only drops these invalid packets but avoids them from being learnt into >> the FDB. >> >> Suggested-by: Ido Schimmel <idosch@nvidia.com> >> Signed-off-by: David Bauer <mail@david-bauer.net> >> --- >> drivers/net/vxlan/vxlan_core.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c >> index 3495591a5c29..ba319fc21957 100644 >> --- a/drivers/net/vxlan/vxlan_core.c >> +++ b/drivers/net/vxlan/vxlan_core.c >> @@ -1615,6 +1615,10 @@ static bool vxlan_set_mac(struct vxlan_dev *vxlan, >> if (ether_addr_equal(eth_hdr(skb)->h_source, vxlan->dev->dev_addr)) >> return false; >> >> + /* Ignore packets from invalid src-address */ >> + if (!is_valid_ether_addr(eth_hdr(skb)->h_source)) >> + return false; >> + >> /* Get address from the outer IP header */ >> if (vxlan_get_sk_family(vs) == AF_INET) { >> saddr.sin.sin_addr.s_addr = ip_hdr(skb)->saddr; >> -- >> 2.43.0 >>
On Sun, Mar 31, 2024 at 11:14:34PM +0200, David Bauer wrote: > The VXLAN driver currently does not check if the inner layer2 > source-address is valid. > > In case source-address snooping/learning is enabled, a entry in the FDB > for the invalid address is created with the layer3 address of the tunnel > endpoint. > > If the frame happens to have a non-unicast address set, all this > non-unicast traffic is subsequently not flooded to the tunnel network > but sent to the learnt host in the FDB. To make matters worse, this FDB > entry does not expire. > > Apply the same filtering for packets as it is done for bridges. This not > only drops these invalid packets but avoids them from being learnt into > the FDB. > > Suggested-by: Ido Schimmel <idosch@nvidia.com> > Signed-off-by: David Bauer <mail@david-bauer.net> Hi David and Ido, I wonder if this is an appropriate candidate for 'net', with a Fixes tag. It does seem to address a user-visible problem. ...
On Tue, Apr 02, 2024 at 07:08:48PM +0100, Simon Horman wrote: > On Sun, Mar 31, 2024 at 11:14:34PM +0200, David Bauer wrote: > > The VXLAN driver currently does not check if the inner layer2 > > source-address is valid. > > > > In case source-address snooping/learning is enabled, a entry in the FDB > > for the invalid address is created with the layer3 address of the tunnel > > endpoint. > > > > If the frame happens to have a non-unicast address set, all this > > non-unicast traffic is subsequently not flooded to the tunnel network > > but sent to the learnt host in the FDB. To make matters worse, this FDB > > entry does not expire. > > > > Apply the same filtering for packets as it is done for bridges. This not > > only drops these invalid packets but avoids them from being learnt into > > the FDB. > > > > Suggested-by: Ido Schimmel <idosch@nvidia.com> > > Signed-off-by: David Bauer <mail@david-bauer.net> > > Hi David and Ido, > > I wonder if this is an appropriate candidate for 'net', with a Fixes tag. > It does seem to address a user-visible problem. I'm OK with targeting the patch at 'net'. Looking at git history, the issue seems to be present since initial submission so Fixes tag should be: Fixes: d342894c5d2f ("vxlan: virtual extensible lan") David, can you please re-submit with "[PATCH net]" prefix and the above tag? Thanks
Hi Ido, On 4/3/24 14:45, Ido Schimmel wrote: > On Tue, Apr 02, 2024 at 07:08:48PM +0100, Simon Horman wrote: >> On Sun, Mar 31, 2024 at 11:14:34PM +0200, David Bauer wrote: >>> The VXLAN driver currently does not check if the inner layer2 >>> source-address is valid. >>> >>> In case source-address snooping/learning is enabled, a entry in the FDB >>> for the invalid address is created with the layer3 address of the tunnel >>> endpoint. >>> >>> If the frame happens to have a non-unicast address set, all this >>> non-unicast traffic is subsequently not flooded to the tunnel network >>> but sent to the learnt host in the FDB. To make matters worse, this FDB >>> entry does not expire. >>> >>> Apply the same filtering for packets as it is done for bridges. This not >>> only drops these invalid packets but avoids them from being learnt into >>> the FDB. >>> >>> Suggested-by: Ido Schimmel <idosch@nvidia.com> >>> Signed-off-by: David Bauer <mail@david-bauer.net> >> >> Hi David and Ido, >> >> I wonder if this is an appropriate candidate for 'net', with a Fixes tag. >> It does seem to address a user-visible problem. > > I'm OK with targeting the patch at 'net'. Looking at git history, the > issue seems to be present since initial submission so Fixes tag should > be: > > Fixes: d342894c5d2f ("vxlan: virtual extensible lan") > > David, can you please re-submit with "[PATCH net]" prefix and the above > tag? I can take care of that. Thanks for analyzing the situation. One thing i still have in my head when looking at this: From my understanding, when i manage to send out such a packet from e.g. a VM connected to a vxlan overlay network and manage to send out such malformed packet, this would allow me to break the overlay network created with vxlan doesn't it? Can you comment on my assumption there? I assume you have a better understanding of the inner workings of the vxlan protocol. Best David > > Thanks
On Wed, Apr 03, 2024 at 07:14:14PM +0200, David Bauer wrote: > I can take care of that. Thanks for analyzing the situation. > > One thing i still have in my head when looking at this: > > From my understanding, when i manage to send out such a packet from e.g. a > VM connected to a vxlan overlay network and manage to send out such malformed > packet, this would allow me to break the overlay network created with vxlan doesn't it? > > Can you comment on my assumption there? I'm not sure which assumption you are referring to, but I did verify that before your patch the VXLAN driver will learn an FDB entry with a broadcast MAC if a malformed packet with a broadcast source MAC was processed by it. This will cause the driver to send broadcast packets to the VTEP that sent the malformed packet instead of flooding such packets to all the VTEPs via the all-zeroes FDB entry. This behavior is obviously wrong and I tested that it doesn't happen with your patch.
diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c index 3495591a5c29..ba319fc21957 100644 --- a/drivers/net/vxlan/vxlan_core.c +++ b/drivers/net/vxlan/vxlan_core.c @@ -1615,6 +1615,10 @@ static bool vxlan_set_mac(struct vxlan_dev *vxlan, if (ether_addr_equal(eth_hdr(skb)->h_source, vxlan->dev->dev_addr)) return false; + /* Ignore packets from invalid src-address */ + if (!is_valid_ether_addr(eth_hdr(skb)->h_source)) + return false; + /* Get address from the outer IP header */ if (vxlan_get_sk_family(vs) == AF_INET) { saddr.sin.sin_addr.s_addr = ip_hdr(skb)->saddr;
The VXLAN driver currently does not check if the inner layer2 source-address is valid. In case source-address snooping/learning is enabled, a entry in the FDB for the invalid address is created with the layer3 address of the tunnel endpoint. If the frame happens to have a non-unicast address set, all this non-unicast traffic is subsequently not flooded to the tunnel network but sent to the learnt host in the FDB. To make matters worse, this FDB entry does not expire. Apply the same filtering for packets as it is done for bridges. This not only drops these invalid packets but avoids them from being learnt into the FDB. Suggested-by: Ido Schimmel <idosch@nvidia.com> Signed-off-by: David Bauer <mail@david-bauer.net> --- drivers/net/vxlan/vxlan_core.c | 4 ++++ 1 file changed, 4 insertions(+)