2015-04-10 07:54:25

by Johannes Berg

[permalink] [raw]
Subject: [PATCH 1/4] ipv4: add option to drop unicast encapsulated in L2 multicast

From: Johannes Berg <[email protected]>

In order to solve a problem with 802.11, the so-called hole-196 attack,
add an option (sysctl) called "drop_unicast_in_l2_multicast" which, if
enabled, causes the stack to drop IPv4 unicast packets encapsulated in
link-layer multi- or broadcast frames. Such frames can (as an attack)
be created by any member of the same wireless network and transmitted
as valid encrypted frames since the symmetric key for broadcast frames
is shared between all stations.

Additionally, enabling this option provides compliance with a SHOULD
clause of RFC 1122.

Signed-off-by: Johannes Berg <[email protected]>
---
Documentation/networking/ip-sysctl.txt | 7 +++++++
include/uapi/linux/ip.h | 1 +
net/ipv4/devinet.c | 2 ++
net/ipv4/route.c | 20 ++++++++++++++++++++
4 files changed, 30 insertions(+)

diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index 071fb18dc57c..f97ad76e6f82 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -1165,6 +1165,13 @@ promote_secondaries - BOOLEAN
promote a corresponding secondary IP address instead of
removing all the corresponding secondary IP addresses.

+drop_unicast_in_l2_multicast - BOOLEAN
+ Drop any unicast IP packets that are received in link-layer
+ multicast (or broadcast) frames.
+ This behavior (for multicast) is actually a SHOULD in RFC
+ 1122, but is disabled by default for compatibility reasons.
+ 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 411959405ab6..c0e594b209ff 100644
--- a/include/uapi/linux/ip.h
+++ b/include/uapi/linux/ip.h
@@ -164,6 +164,7 @@ enum
IPV4_DEVCONF_ROUTE_LOCALNET,
IPV4_DEVCONF_IGMPV2_UNSOLICITED_REPORT_INTERVAL,
IPV4_DEVCONF_IGMPV3_UNSOLICITED_REPORT_INTERVAL,
+ IPV4_DEVCONF_DROP_UNICAST_IN_L2_MULTICAST,
__IPV4_DEVCONF_MAX
};

diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index c6473f365ad1..b608407f96e7 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -2176,6 +2176,8 @@ static struct devinet_sysctl_table {
"promote_secondaries"),
DEVINET_SYSCTL_FLUSHING_ENTRY(ROUTE_LOCALNET,
"route_localnet"),
+ DEVINET_SYSCTL_FLUSHING_ENTRY(DROP_UNICAST_IN_L2_MULTICAST,
+ "drop_unicast_in_l2_multicast"),
},
};

diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 652b92ebd7ba..5d5e03f9697b 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1727,6 +1727,26 @@ static int ip_route_input_slow(struct sk_buff *skb, __be32 daddr, __be32 saddr,
if (res.type == RTN_BROADCAST)
goto brd_input;

+ /* RFC 1122 3.3.6:
+ *
+ * When a host sends a datagram to a link-layer broadcast address,
+ * the IP destination address MUST be a legal IP broadcast or IP
+ * multicast address.
+ *
+ * A host SHOULD silently discard a datagram that is received via
+ * a link-layer broadcast (see Section 2.4) but does not specify
+ * an IP multicast or broadcast destination address.
+ *
+ * This doesn't explicitly say L2 *broadcast*, but broadcast is in a
+ * way a form of multicast and the most common use case for this is
+ * 802.11 protecting against cross-station spoofing (the so-called
+ * "hole-196" attack) so do it for both.
+ */
+ if (IN_DEV_CONF_GET(in_dev, DROP_UNICAST_IN_L2_MULTICAST) &&
+ (skb->pkt_type == PACKET_BROADCAST ||
+ skb->pkt_type == PACKET_MULTICAST))
+ goto e_inval;
+
if (res.type == RTN_LOCAL) {
err = fib_validate_source(skb, saddr, daddr, tos,
0, dev, in_dev, &itag);
--
2.1.4



2015-04-13 11:17:30

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 3/4] ipv4: add option to drop gratuitous ARP packets

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



2015-04-10 07:54:25

by Johannes Berg

[permalink] [raw]
Subject: [PATCH 2/4] ipv6: add option to drop unicast encapsulated in L2 multicast

From: Johannes Berg <[email protected]>

In order to solve a problem with 802.11, the so-called hole-196 attack,
add an option (sysctl) called "drop_unicast_in_l2_multicast" which, if
enabled, causes the stack to drop IPv6 unicast packets encapsulated in
link-layer multi- or broadcast frames. Such frames can (as an attack)
be created by any member of the same wireless network and transmitted
as valid encrypted frames since the symmetric key for broadcast frames
is shared between all stations.

Signed-off-by: Johannes Berg <[email protected]>
---
Documentation/networking/ip-sysctl.txt | 6 ++++++
include/linux/ipv6.h | 1 +
include/uapi/linux/ipv6.h | 1 +
net/ipv6/addrconf.c | 8 ++++++++
net/ipv6/ip6_input.c | 10 ++++++++++
5 files changed, 26 insertions(+)

diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index f97ad76e6f82..0a4715253ac2 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -1572,6 +1572,12 @@ stable_secret - IPv6 address

By default the stable secret is unset.

+drop_unicast_in_l2_multicast - BOOLEAN
+ Drop any unicast IPv6 packets that are received in link-layer
+ multicast (or broadcast) frames.
+
+ By default this is turned off.
+
icmp/*:
ratelimit - INTEGER
Limit the maximal rates for sending ICMPv6 packets.
diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
index 82806c60aa42..1ec287a37e1d 100644
--- a/include/linux/ipv6.h
+++ b/include/linux/ipv6.h
@@ -48,6 +48,7 @@ struct ipv6_devconf {
__s32 mc_forwarding;
#endif
__s32 disable_ipv6;
+ __s32 drop_unicast_in_l2_multicast;
__s32 accept_dad;
__s32 force_tllao;
__s32 ndisc_notify;
diff --git a/include/uapi/linux/ipv6.h b/include/uapi/linux/ipv6.h
index 5efa54ae567c..5a7061d690cb 100644
--- a/include/uapi/linux/ipv6.h
+++ b/include/uapi/linux/ipv6.h
@@ -171,6 +171,7 @@ enum {
DEVCONF_USE_OPTIMISTIC,
DEVCONF_ACCEPT_RA_MTU,
DEVCONF_STABLE_SECRET,
+ DEVCONF_DROP_UNICAST_IN_L2_MULTICAST,
DEVCONF_MAX
};

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 5c9e94cb1b2c..c017edf04f30 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -4584,6 +4584,7 @@ static inline void ipv6_store_devconf(struct ipv6_devconf *cnf,
array[DEVCONF_ACCEPT_RA_FROM_LOCAL] = cnf->accept_ra_from_local;
array[DEVCONF_ACCEPT_RA_MTU] = cnf->accept_ra_mtu;
/* we omit DEVCONF_STABLE_SECRET for now */
+ array[DEVCONF_DROP_UNICAST_IN_L2_MULTICAST] = cnf->drop_unicast_in_l2_multicast;
}

static inline size_t inet6_ifla6_size(void)
@@ -5583,6 +5584,13 @@ static struct addrconf_sysctl_table
.proc_handler = addrconf_sysctl_stable_secret,
},
{
+ .procname = "drop_unicast_in_l2_multicast",
+ .data = &ipv6_devconf.drop_unicast_in_l2_multicast,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec,
+ },
+ {
/* sentinel */
}
},
diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
index fb97f7f8d4ed..70066fd2d685 100644
--- a/net/ipv6/ip6_input.c
+++ b/net/ipv6/ip6_input.c
@@ -134,6 +134,16 @@ int ipv6_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt
IPV6_ADDR_MC_SCOPE(&hdr->daddr) == 1)
goto err;

+ /* If enabled, drop unicast packets that were encapsulated in link-layer
+ * multicast or broadcast to protected against the so-called "hole-196"
+ * attack in 802.11 wireless.
+ */
+ if (!ipv6_addr_is_multicast(&hdr->daddr) &&
+ (skb->pkt_type == PACKET_BROADCAST ||
+ skb->pkt_type == PACKET_MULTICAST) &&
+ idev->cnf.drop_unicast_in_l2_multicast)
+ goto err;
+
/* RFC4291 2.7
* Nodes must not originate a packet to a multicast address whose scope
* field contains the reserved value 0; if such a packet is received, it
--
2.1.4


2015-04-11 11:01:37

by Julian Anastasov

[permalink] [raw]
Subject: Re: [PATCH 3/4] ipv4: add option to drop gratuitous ARP packets


Hello,

On Fri, 10 Apr 2015, Johannes Berg wrote:

> From: Johannes Berg <[email protected]>
>
> 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 <[email protected]>

> 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 <[email protected]>

2015-04-10 07:54:26

by Johannes Berg

[permalink] [raw]
Subject: [PATCH 4/4] ipv6: add option to drop unsolicited neighbor advertisements

From: Johannes Berg <[email protected]>

In certain 802.11 wireless deployments, there will be NA proxies
that use knowledge of the network to correctly answer requests.
To prevent unsolicitd advertisements on the shared medium from
being a problem, on such deployments wireless needs to drop them.

Enable this by providing an option called "drop_unsolicited_na".

Signed-off-by: Johannes Berg <[email protected]>
---
Documentation/networking/ip-sysctl.txt | 7 +++++++
include/linux/ipv6.h | 1 +
include/uapi/linux/ipv6.h | 1 +
net/ipv6/addrconf.c | 8 ++++++++
net/ipv6/ndisc.c | 9 +++++++++
5 files changed, 26 insertions(+)

diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index f6f32c21edaf..b0981c586fc4 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -1584,6 +1584,13 @@ drop_unicast_in_l2_multicast - BOOLEAN

By default this is turned off.

+drop_unsolicited_na - BOOLEAN
+ Drop all unsolicited neighbor advertisements, for example if there's
+ a known good NA 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.)
+
+ By default this is turned off.
+
icmp/*:
ratelimit - INTEGER
Limit the maximal rates for sending ICMPv6 packets.
diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
index 1ec287a37e1d..cef258173a9d 100644
--- a/include/linux/ipv6.h
+++ b/include/linux/ipv6.h
@@ -54,6 +54,7 @@ struct ipv6_devconf {
__s32 ndisc_notify;
__s32 suppress_frag_ndisc;
__s32 accept_ra_mtu;
+ __s32 drop_unsolicited_na;
struct ipv6_stable_secret {
bool initialized;
struct in6_addr secret;
diff --git a/include/uapi/linux/ipv6.h b/include/uapi/linux/ipv6.h
index 5a7061d690cb..73d7f1df6fe1 100644
--- a/include/uapi/linux/ipv6.h
+++ b/include/uapi/linux/ipv6.h
@@ -172,6 +172,7 @@ enum {
DEVCONF_ACCEPT_RA_MTU,
DEVCONF_STABLE_SECRET,
DEVCONF_DROP_UNICAST_IN_L2_MULTICAST,
+ DEVCONF_DROP_UNSOLICITED_NA,
DEVCONF_MAX
};

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index c017edf04f30..6d5a680085e8 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -4585,6 +4585,7 @@ static inline void ipv6_store_devconf(struct ipv6_devconf *cnf,
array[DEVCONF_ACCEPT_RA_MTU] = cnf->accept_ra_mtu;
/* we omit DEVCONF_STABLE_SECRET for now */
array[DEVCONF_DROP_UNICAST_IN_L2_MULTICAST] = cnf->drop_unicast_in_l2_multicast;
+ array[DEVCONF_DROP_UNSOLICITED_NA] = cnf->drop_unsolicited_na;
}

static inline size_t inet6_ifla6_size(void)
@@ -5591,6 +5592,13 @@ static struct addrconf_sysctl_table
.proc_handler = proc_dointvec,
},
{
+ .procname = "drop_unsolicited_na",
+ .data = &ipv6_devconf.drop_unsolicited_na,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec,
+ },
+ {
/* sentinel */
}
},
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index c283827d60e2..7291b91e359b 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -868,6 +868,7 @@ static void ndisc_recv_na(struct sk_buff *skb)
offsetof(struct nd_msg, opt));
struct ndisc_options ndopts;
struct net_device *dev = skb->dev;
+ struct inet6_dev *idev = __in6_dev_get(dev);
struct inet6_ifaddr *ifp;
struct neighbour *neigh;

@@ -887,6 +888,14 @@ static void ndisc_recv_na(struct sk_buff *skb)
return;
}

+ /* For some 802.11 wireless deployments (and possibly other networks),
+ * there will be a NA proxy and unsolicitd packets are attacks
+ * and thus should not be accepted.
+ */
+ if (idev && idev->cnf.drop_unsolicited_na &&
+ !msg->icmph.icmp6_solicited)
+ return;
+
if (!ndisc_parse_options(msg->opt, ndoptlen, &ndopts)) {
ND_PRINTK(2, warn, "NS: invalid ND option\n");
return;
--
2.1.4


2015-04-13 12:06:36

by Julian Anastasov

[permalink] [raw]
Subject: Re: [PATCH 1/4] ipv4: add option to drop unicast encapsulated in L2 multicast


Hello,

On Mon, 13 Apr 2015, Johannes Berg wrote:

> > - CLUSTERIP works in LOCAL_IN (after ip_rcv_finish), LOCAL_IN
> > is here: ip_rcv_finish->dst_input->ip_local_deliver->
> > ip_local_deliver_finish
>
> It's just that the later this is, the more nervous I get about it being
> really effective. :)
>
> I'm willing to discount the CLUSTERIP case, it seems insane to want to
> run CLUSTERIP over wifi on a network that explicitly limits multicast.
>
> Other than that, do you see any reason for not putting it in
> ip_rcv_finish()?

ok, I don't remember for other cases.

Regards

--
Julian Anastasov <[email protected]>

2015-04-10 13:10:44

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 1/4] ipv4: add option to drop unicast encapsulated in L2 multicast

Hi
>
> > + if (IN_DEV_CONF_GET(in_dev, DROP_UNICAST_IN_L2_MULTICAST) &&
>
> For this flag IN_DEV_ORCONF can be used, by this way
> all/drop_unicast_in_l2_multicast=1 can enable it for all
> interfaces.

Good point. I need to look into that for the other patches in this
series as well.

> So, this is the same patch as the 2014-Aug version
> but this time with flag? But how the previous problems were
> addressed? May be something is changed in kernel afterwards?
>
> So, if your are back at step 1 can you check again
> the problems with this implementation?:

I think I am, sorry about that. I had focused on the CLUSTERIP problem
in your email, and decided that was solved by simply having the admin
for CLUSTERIP networks never set this flag.

> - no way to select correct skb->pkt_type in inet_rtm_getroute
> before ip_route_input, this is a chiken-egg problem, of course,
> skb->pkt_type = PACKET_HOST can work for now

This is interesting. Why does that even build an skb and route it, if it
really just cares about the struct rtable? Anyway, you're right, using
PACKET_HOST would work. I'm not really sure I see the chicken-and-egg
problem though, it's not like the result of this is used to route the
skb again or something, afaict?

> - ip_route_input is called also for ARP, so incoming ARP
> broadcasts are not replied anymore

Hm. I didn't see this in my testing so far, but I only enable this
temporarily, so perhaps by that time it was already done.

This looks like another case of using this function to not really route
anything but to just look up the route, similar to the
inet_rtm_getroute() function you pointed out. Perhaps that would warrant
splitting out somewhat.

On the other hand, there's not all that much reason not to put this into
ip_rcv_finish(), we already touch the rt->rt_type there for MIB
counters. I'll look into that.

> - CLUSTERIP

See above.

Thanks,
johannes



2015-04-11 10:40:55

by Julian Anastasov

[permalink] [raw]
Subject: Re: [PATCH 1/4] ipv4: add option to drop unicast encapsulated in L2 multicast


Hello,

On Fri, 10 Apr 2015, Johannes Berg wrote:

> > - no way to select correct skb->pkt_type in inet_rtm_getroute
> > before ip_route_input, this is a chiken-egg problem, of course,
> > skb->pkt_type = PACKET_HOST can work for now
>
> This is interesting. Why does that even build an skb and route it, if it
> really just cares about the struct rtable? Anyway, you're right, using
> PACKET_HOST would work. I'm not really sure I see the chicken-and-egg
> problem though, it's not like the result of this is used to route the
> skb again or something, afaict?

If we want to set correct pkt_type here we should
know the type of daddr (multicast => PACKET_MULTICAST,
broadcast => PACKET_BROADCAST, etc) and we can know it only
after routing (rt_type). I hope we will not do other checks
with pkt_type, so we can put some comment that
skb->pkt_type = PACKET_HOST is just to make
DROP_UNICAST_IN_L2_MULTICAST check happy.

> > - ip_route_input is called also for ARP, so incoming ARP
> > broadcasts are not replied anymore
>
> Hm. I didn't see this in my testing so far, but I only enable this
> temporarily, so perhaps by that time it was already done.

I didn't tested it myself, may be you can try:

other_host_on_LAN# arp -d patched_box
other_host_on_LAN# arping -c 1 -I eth0 patched_box

There must be reply for broadcast requests.

> This looks like another case of using this function to not really route
> anything but to just look up the route, similar to the
> inet_rtm_getroute() function you pointed out. Perhaps that would warrant
> splitting out somewhat.

Yes, ARP essentially performs rp_filter check (note
that arp_filter check is limited only to local targets, not
to proxy_arp) and target address type check.

The solution would be to add skb->protocol == htons(ETH_P_IP)
check.

> On the other hand, there's not all that much reason not to put this into
> ip_rcv_finish(), we already touch the rt->rt_type there for MIB
> counters. I'll look into that.

May be in ip_local_deliver_finish because:

- ip_forward() already has PACKET_HOST check.

- for broadcast/multicast dests we do not care

- CLUSTERIP works in LOCAL_IN (after ip_rcv_finish), LOCAL_IN
is here: ip_rcv_finish->dst_input->ip_local_deliver->
ip_local_deliver_finish

Regards

--
Julian Anastasov <[email protected]>

2015-04-13 10:57:31

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 1/4] ipv4: add option to drop unicast encapsulated in L2 multicast

Hi Julian,

> > Hm. I didn't see this in my testing so far, but I only enable this
> > temporarily, so perhaps by that time it was already done.
>
> I didn't tested it myself, may be you can try:
>
> other_host_on_LAN# arp -d patched_box
> other_host_on_LAN# arping -c 1 -I eth0 patched_box
>
> There must be reply for broadcast requests.

Sure; I would have expected this to have broken my network but as I
said, it was only enabled temporarily.

> > On the other hand, there's not all that much reason not to put this into
> > ip_rcv_finish(), we already touch the rt->rt_type there for MIB
> > counters. I'll look into that.
>
> May be in ip_local_deliver_finish because:
>
> - ip_forward() already has PACKET_HOST check.
>
> - for broadcast/multicast dests we do not care
>
> - CLUSTERIP works in LOCAL_IN (after ip_rcv_finish), LOCAL_IN
> is here: ip_rcv_finish->dst_input->ip_local_deliver->
> ip_local_deliver_finish

It's just that the later this is, the more nervous I get about it being
really effective. :)

I'm willing to discount the CLUSTERIP case, it seems insane to want to
run CLUSTERIP over wifi on a network that explicitly limits multicast.

Other than that, do you see any reason for not putting it in
ip_rcv_finish()?

johannes


2015-04-10 13:11:22

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 3/4] ipv4: add option to drop gratuitous ARP packets

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


2015-04-10 12:26:54

by Julian Anastasov

[permalink] [raw]
Subject: Re: [PATCH 1/4] ipv4: add option to drop unicast encapsulated in L2 multicast


Hello,

On Fri, 10 Apr 2015, Johannes Berg wrote:

> From: Johannes Berg <[email protected]>
>
> In order to solve a problem with 802.11, the so-called hole-196 attack,
> add an option (sysctl) called "drop_unicast_in_l2_multicast" which, if
> enabled, causes the stack to drop IPv4 unicast packets encapsulated in
> link-layer multi- or broadcast frames. Such frames can (as an attack)
> be created by any member of the same wireless network and transmitted
> as valid encrypted frames since the symmetric key for broadcast frames
> is shared between all stations.
>
> Additionally, enabling this option provides compliance with a SHOULD
> clause of RFC 1122.
>

> +++ b/net/ipv4/route.c
> @@ -1727,6 +1727,26 @@ static int ip_route_input_slow(struct sk_buff *skb, __be32 daddr, __be32 saddr,
> if (res.type == RTN_BROADCAST)
> goto brd_input;
>
> + /* RFC 1122 3.3.6:
> + *
> + * When a host sends a datagram to a link-layer broadcast address,
> + * the IP destination address MUST be a legal IP broadcast or IP
> + * multicast address.
> + *
> + * A host SHOULD silently discard a datagram that is received via
> + * a link-layer broadcast (see Section 2.4) but does not specify
> + * an IP multicast or broadcast destination address.
> + *
> + * This doesn't explicitly say L2 *broadcast*, but broadcast is in a
> + * way a form of multicast and the most common use case for this is
> + * 802.11 protecting against cross-station spoofing (the so-called
> + * "hole-196" attack) so do it for both.
> + */
> + if (IN_DEV_CONF_GET(in_dev, DROP_UNICAST_IN_L2_MULTICAST) &&

For this flag IN_DEV_ORCONF can be used, by this way
all/drop_unicast_in_l2_multicast=1 can enable it for all
interfaces.

> + (skb->pkt_type == PACKET_BROADCAST ||
> + skb->pkt_type == PACKET_MULTICAST))
> + goto e_inval;
> +

So, this is the same patch as the 2014-Aug version
but this time with flag? But how the previous problems were
addressed? May be something is changed in kernel afterwards?

So, if your are back at step 1 can you check again
the problems with this implementation?:

http://marc.info/?l=linux-netdev&m=140865079120355&w=2

Thread:
http://marc.info/?t=140864197300004&r=1&w=2

In short:

- no way to select correct skb->pkt_type in inet_rtm_getroute
before ip_route_input, this is a chiken-egg problem, of course,
skb->pkt_type = PACKET_HOST can work for now

- ip_route_input is called also for ARP, so incoming ARP
broadcasts are not replied anymore

- CLUSTERIP

Regards

--
Julian Anastasov <[email protected]>

2015-04-10 07:54:26

by Johannes Berg

[permalink] [raw]
Subject: [PATCH 3/4] ipv4: add option to drop gratuitous ARP packets

From: Johannes Berg <[email protected]>

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 <[email protected]>
---
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/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"),
--
2.1.4


2015-04-10 12:56:26

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH 3/4] ipv4: add option to drop gratuitous ARP packets

Hello.

On 4/10/2015 10:54 AM, Johannes Berg wrote:

> From: Johannes Berg <[email protected]>

> 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 <[email protected]>
> ---
> 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


2015-11-04 16:19:56

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 3/4] ipv4: add option to drop gratuitous ARP packets

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