| Message ID | 1428652454-1224-3-git-send-email-johannes@sipsolutions.net (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Kalle Valo |
| Headers | show |
Hello. On 4/10/2015 10:54 AM, Johannes Berg wrote: > From: Johannes Berg <johannes.berg@intel.com> > In certain 802.11 wireless deployments, there will be ARP proxies > that use knowledge of the network to correctly answer requests. > To prevent gratuitous ARP frames on the shared medium from being > a problem, on such deployments wireless needs to drop them. > Enable this by providing an option called "drop_gratuitous_arp". > Signed-off-by: Johannes Berg <johannes.berg@intel.com> > --- > Documentation/networking/ip-sysctl.txt | 6 ++++++ > include/uapi/linux/ip.h | 1 + > net/ipv4/arp.c | 8 ++++++++ > net/ipv4/devinet.c | 2 ++ > 4 files changed, 17 insertions(+) [...] > diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c > index 5f5c674e130a..5487d5e5191e 100644 > --- a/net/ipv4/arp.c > +++ b/net/ipv4/arp.c > @@ -715,6 +715,14 @@ static int arp_process(struct sk_buff *skb) > (!IN_DEV_ROUTE_LOCALNET(in_dev) && ipv4_is_loopback(tip))) > goto out; > > + /* > + * For some 802.11 wireless deployments (and possibly other networks), > + * there will be an ARP proxy and gratuitous ARP frames are attacks > + * and thus should not be accepted. > + */ Hm, why this strange indentation? > + if (IN_DEV_CONF_GET(in_dev, DROP_GRATUITOUS_ARP) && sip == tip) > + goto out; > + > /* > * Special case: We must set Frame Relay source Q.922 address > */ [...] WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2015-04-10 at 15:56 +0300, Sergei Shtylyov wrote: > > diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c > > index 5f5c674e130a..5487d5e5191e 100644 > > --- a/net/ipv4/arp.c > > +++ b/net/ipv4/arp.c > > @@ -715,6 +715,14 @@ static int arp_process(struct sk_buff *skb) > > (!IN_DEV_ROUTE_LOCALNET(in_dev) && ipv4_is_loopback(tip))) > > goto out; > > > > + /* > > + * For some 802.11 wireless deployments (and possibly other networks), > > + * there will be an ARP proxy and gratuitous ARP frames are attacks > > + * and thus should not be accepted. > > + */ > > Hm, why this strange indentation? > > > + if (IN_DEV_CONF_GET(in_dev, DROP_GRATUITOUS_ARP) && sip == tip) > > + goto out; > > + > > /* > > * Special case: We must set Frame Relay source Q.922 address > > */ > [...] Well, because of the context. All the comments in this file are that way, so it seemed nicer to keep it like that rather than add one "modern" one to it... johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello, On Fri, 10 Apr 2015, Johannes Berg wrote: > From: Johannes Berg <johannes.berg@intel.com> > > In certain 802.11 wireless deployments, there will be ARP proxies > that use knowledge of the network to correctly answer requests. > To prevent gratuitous ARP frames on the shared medium from being > a problem, on such deployments wireless needs to drop them. > > Enable this by providing an option called "drop_gratuitous_arp". > > Signed-off-by: Johannes Berg <johannes.berg@intel.com> > diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c > index 5f5c674e130a..5487d5e5191e 100644 > --- a/net/ipv4/arp.c > +++ b/net/ipv4/arp.c > @@ -715,6 +715,14 @@ static int arp_process(struct sk_buff *skb) > (!IN_DEV_ROUTE_LOCALNET(in_dev) && ipv4_is_loopback(tip))) > goto out; > > + /* > + * For some 802.11 wireless deployments (and possibly other networks), > + * there will be an ARP proxy and gratuitous ARP frames are attacks > + * and thus should not be accepted. > + */ > + if (IN_DEV_CONF_GET(in_dev, DROP_GRATUITOUS_ARP) && sip == tip) > + goto out; Does it happen for any pkt_type? IN_DEV_ARP_ACCEPT is not ON by default, so new entries are not created but update can happen at any time, even with simple request like who-has OURIP tell PROXYIP and sha=hacker_mac sent by attackers. Is that the only gap that needs to be protected with this patch? May be only arptable_filter can help here to protect ARP? Regards -- Julian Anastasov <ja@ssi.bg> -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, 2015-04-11 at 13:59 +0300, Julian Anastasov wrote: > > + /* > > + * For some 802.11 wireless deployments (and possibly other networks), > > + * there will be an ARP proxy and gratuitous ARP frames are attacks > > + * and thus should not be accepted. > > + */ > > + if (IN_DEV_CONF_GET(in_dev, DROP_GRATUITOUS_ARP) && sip == tip) > > + goto out; > > Does it happen for any pkt_type? Yes, it's supposed to. > IN_DEV_ARP_ACCEPT > is not ON by default, so new entries are not created but Correct, this protects against "gratuitous updates" in a way. > update can happen at any time, even with simple request like > who-has OURIP tell PROXYIP and sha=hacker_mac sent by > attackers. Is that the only gap that needs to be protected > with this patch? Realistically, I'd expect networks that deploy this to implement other things that prevent clients from messing up the network. I'd expect, for example, that ARP packets are simple dropped in the AP bridge if it implements proxy service and wants to control the network tightly. It can still be desirable to not let gratuitous ARP packets update the cache entry though. IPv6 for example automatically marks such updated entries stale, IIRC, so there I had even bigger issues with testing and I need to check if I even need the 4th patch in this series. However, there's also a compliance test here that requires this behaviour, and checks specifically that a gratuitous ARP doesn't update an existing cache entry. > May be only arptable_filter can help here to > protect ARP? That could be possible, I'll check. Thanks! johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, 2015-04-11 at 13:59 +0300, Julian Anastasov wrote: > > May be only arptable_filter can help here to > protect ARP? > Finally reviving an ancient thread ... I checked, butI don't see a way to match tip==sip. You can match on each, but not against each other. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt index 0a4715253ac2..f6f32c21edaf 100644 --- a/Documentation/networking/ip-sysctl.txt +++ b/Documentation/networking/ip-sysctl.txt @@ -1172,6 +1172,12 @@ drop_unicast_in_l2_multicast - BOOLEAN 1122, but is disabled by default for compatibility reasons. Default: off (0) +drop_gratuitous_arp - BOOLEAN + Drop all gratuitous ARP frames, for example if there's a known + good ARP proxy on the network and such frames need not be used + (or in the case of 802.11, must not be used to prevent attacks.) + Default: off (0) + tag - INTEGER Allows you to write a number, which can be used as required. diff --git a/include/uapi/linux/ip.h b/include/uapi/linux/ip.h index c0e594b209ff..fa0dd3a7e0f1 100644 --- a/include/uapi/linux/ip.h +++ b/include/uapi/linux/ip.h @@ -165,6 +165,7 @@ enum IPV4_DEVCONF_IGMPV2_UNSOLICITED_REPORT_INTERVAL, IPV4_DEVCONF_IGMPV3_UNSOLICITED_REPORT_INTERVAL, IPV4_DEVCONF_DROP_UNICAST_IN_L2_MULTICAST, + IPV4_DEVCONF_DROP_GRATUITOUS_ARP, __IPV4_DEVCONF_MAX }; diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c index 5f5c674e130a..5487d5e5191e 100644 --- a/net/ipv4/arp.c +++ b/net/ipv4/arp.c @@ -715,6 +715,14 @@ static int arp_process(struct sk_buff *skb) (!IN_DEV_ROUTE_LOCALNET(in_dev) && ipv4_is_loopback(tip))) goto out; + /* + * For some 802.11 wireless deployments (and possibly other networks), + * there will be an ARP proxy and gratuitous ARP frames are attacks + * and thus should not be accepted. + */ + if (IN_DEV_CONF_GET(in_dev, DROP_GRATUITOUS_ARP) && sip == tip) + goto out; + /* * Special case: We must set Frame Relay source Q.922 address */ diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c index b608407f96e7..3f2bd37e3d7e 100644 --- a/net/ipv4/devinet.c +++ b/net/ipv4/devinet.c @@ -2169,6 +2169,8 @@ static struct devinet_sysctl_table { "igmpv2_unsolicited_report_interval"), DEVINET_SYSCTL_RW_ENTRY(IGMPV3_UNSOLICITED_REPORT_INTERVAL, "igmpv3_unsolicited_report_interval"), + DEVINET_SYSCTL_RW_ENTRY(DROP_GRATUITOUS_ARP, + "drop_gratuitous_arp"), DEVINET_SYSCTL_FLUSHING_ENTRY(NOXFRM, "disable_xfrm"), DEVINET_SYSCTL_FLUSHING_ENTRY(NOPOLICY, "disable_policy"),