[PATCH] mac80211: do not TX injected frames when not allowed

[PATCH] mac80211: do not TX injected frames when not allowed

[Luis R. Rodriguez]

Monitor mode is able to TX by using injected frames. We should
not allow injected frames to be sent unless allowed by regulatory rules.
Since AP mode uses a monitor interfaces to transmit management frames
we have to take care to not break AP mode as well. Since hostapd
respects 'no-ibss|radar' flags we can make use of the same policy
statically for checking when to disable injected frames. Once AP
mode supports radar detection we should somehow distinguish a
regular monitor mode from an AP's monitor interface.

Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 net/mac80211/tx.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 955f186..276a666 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -899,6 +899,16 @@ __ieee80211_parse_tx_radiotap(struct ieee80211_tx_data *tx,
 
 	sband = tx->local->hw.wiphy->bands[tx->channel->band];
 
+	/* Frame injection is not allowed if beaconing is not allowed
+	 * or if we need radar detection. Beaconing is usually not allowed when
+	 * the mode or operation (Adhoc, AP, Mesh) does not support DFS.
+	 * Since AP mode uses monitor interafaces to inject/TX management
+	 * frames once we have radar detection support we can change this
+	 * here */
+	if ((tx->channel->flags & (IEEE80211_CHAN_NO_IBSS |
+	    IEEE80211_CHAN_RADAR)))
+		return TX_DROP;
+
 	skb->do_not_encrypt = 1;
 	tx->flags &= ~IEEE80211_TX_FRAGMENTED;
 
-- 
1.6.1.rc3.51.g5832d

Re: [PATCH] mac80211: do not TX injected frames when not allowed

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 700 bytes --]

On Fri, 2009-01-16 at 10:52 -0800, Luis R. Rodriguez wrote:
> Monitor mode is able to TX by using injected frames. We should
> not allow injected frames to be sent unless allowed by regulatory rules.
> Since AP mode uses a monitor interfaces to transmit management frames
> we have to take care to not break AP mode as well. Since hostapd
> respects 'no-ibss|radar' flags we can make use of the same policy
> statically for checking when to disable injected frames. Once AP
> mode supports radar detection we should somehow distinguish a
> regular monitor mode from an AP's monitor interface.

Any proposal for "somehow"? I'm not all that worried about packet
injection here.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] mac80211: do not TX injected frames when not allowed

[Luis R. Rodriguez]

On Sun, Jan 18, 2009 at 12:45:39AM -0800, Johannes Berg wrote:
> On Fri, 2009-01-16 at 10:52 -0800, Luis R. Rodriguez wrote:
> > Monitor mode is able to TX by using injected frames. We should
> > not allow injected frames to be sent unless allowed by regulatory rules.
> > Since AP mode uses a monitor interfaces to transmit management frames
> > we have to take care to not break AP mode as well. Since hostapd
> > respects 'no-ibss|radar' flags we can make use of the same policy
> > statically for checking when to disable injected frames. Once AP
> > mode supports radar detection we should somehow distinguish a
> > regular monitor mode from an AP's monitor interface.
> 
> Any proposal for "somehow"? I'm not all that worried about packet
> injection here.

I'll send a patch instead. Keep in mind by not applying this you are
allowing devices to not respect regulatory rules.

  Luis

Re: [PATCH] mac80211: do not TX injected frames when not allowed

[Luis R. Rodriguez]

On Sun, Jan 18, 2009 at 07:32:43AM -0800, Luis Rodriguez wrote:
> On Sun, Jan 18, 2009 at 12:45:39AM -0800, Johannes Berg wrote:
> > On Fri, 2009-01-16 at 10:52 -0800, Luis R. Rodriguez wrote:
> > > Monitor mode is able to TX by using injected frames. We should
> > > not allow injected frames to be sent unless allowed by regulatory rules.
> > > Since AP mode uses a monitor interfaces to transmit management frames
> > > we have to take care to not break AP mode as well. Since hostapd
> > > respects 'no-ibss|radar' flags we can make use of the same policy
> > > statically for checking when to disable injected frames. Once AP
> > > mode supports radar detection we should somehow distinguish a
> > > regular monitor mode from an AP's monitor interface.
> >
> > Any proposal for "somehow"? I'm not all that worried about packet
> > injection here.
> 
> I'll send a patch instead. Keep in mind by not applying this you are
> allowing devices to not respect regulatory rules.

How about a monitor flag? Although I am not sure where we'd use this
except if we'd want to change assumptions on regulatory rules for
the interface.

  Luis

diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
index 4e7a798..7f65d7e 100644
--- a/include/linux/nl80211.h
+++ b/include/linux/nl80211.h
@@ -683,6 +683,8 @@ enum nl80211_reg_rule_flags {
  * @NL80211_MNTR_FLAG_OTHER_BSS: disable BSSID filtering
  * @NL80211_MNTR_FLAG_COOK_FRAMES: report frames after processing.
  *	overrides all other flags.
+ * @NL80211_MNTR_FLAG_AP_MGT: this monitor interface is used for AP mode
+ * 	to be able to inject management frames.
  *
  * @__NL80211_MNTR_FLAG_AFTER_LAST: internal use
  * @NL80211_MNTR_FLAG_MAX: highest possible monitor flag
@@ -694,6 +696,7 @@ enum nl80211_mntr_flags {
 	NL80211_MNTR_FLAG_CONTROL,
 	NL80211_MNTR_FLAG_OTHER_BSS,
 	NL80211_MNTR_FLAG_COOK_FRAMES,
+	NL80211_MNTR_FLAG_AP_MGT,
 
 	/* keep last */
 	__NL80211_MNTR_FLAG_AFTER_LAST,
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 6bab7a1..4b85a20 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -255,6 +255,9 @@ struct station_info {
  * @MONITOR_FLAG_CONTROL: pass control frames
  * @MONITOR_FLAG_OTHER_BSS: disable BSSID filtering
  * @MONITOR_FLAG_COOK_FRAMES: report frames after processing
+ * @MONITOR_FLAG_AP_MGT: informs us this monitor interface is
+ *	used by an driver for AP mode to be able to inject management
+ * 	frames.
  */
 enum monitor_flags {
 	MONITOR_FLAG_FCSFAIL		= 1<<NL80211_MNTR_FLAG_FCSFAIL,
@@ -262,6 +265,7 @@ enum monitor_flags {
 	MONITOR_FLAG_CONTROL		= 1<<NL80211_MNTR_FLAG_CONTROL,
 	MONITOR_FLAG_OTHER_BSS		= 1<<NL80211_MNTR_FLAG_OTHER_BSS,
 	MONITOR_FLAG_COOK_FRAMES	= 1<<NL80211_MNTR_FLAG_COOK_FRAMES,
+	MONITOR_FLAG_AP_MGT		= 1<<NL80211_MNTR_FLAG_AP_MGT,
 };
 
 /**
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 2e7f9eb..90e276c 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -533,6 +533,7 @@ static const struct nla_policy mntr_flags_policy[NL80211_MNTR_FLAG_MAX + 1] = {
 	[NL80211_MNTR_FLAG_CONTROL] = { .type = NLA_FLAG },
 	[NL80211_MNTR_FLAG_OTHER_BSS] = { .type = NLA_FLAG },
 	[NL80211_MNTR_FLAG_COOK_FRAMES] = { .type = NLA_FLAG },
+	[NL80211_MNTR_FLAG_AP_MGT] = { .type = NLA_FLAG },
 };
 
 static int parse_monitor_flags(struct nlattr *nla, u32 *mntrflags)

[PATCH] mac80211: do not TX injected frames when not allowed

[Luis R. Rodriguez]

Monitor mode is able to TX by using injected frames. We should
not allow injected frames to be sent unless allowed by regulatory
rules. Since AP mode uses a monitor interfaces to transmit
management frames we have to take care to not break AP mode as
well while resolving this. We deal with this by allowing compliant
APs solutions to inform mac80211 if their monitor interface is
intended to be used for an AP by setting a cfg80211 flag for the
monitor interface. hostapd, for example, currently does its own
checks to ensure AP mode is not used on channels which require radar
detection. Once such solutions are available it can can enable this
flag.

Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---

This now works, I moved the check as recommended by Johannes to
ieee80211_monitor_start_xmit(), the issues I ran into earlier seem
to be due to a work around set in place for 11w.

 include/linux/nl80211.h |    3 +++
 include/net/cfg80211.h  |    4 ++++
 net/mac80211/tx.c       |   14 ++++++++++++++
 net/wireless/nl80211.c  |    1 +
 4 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
index 76aae3d..3357907 100644
--- a/include/linux/nl80211.h
+++ b/include/linux/nl80211.h
@@ -705,6 +705,8 @@ enum nl80211_reg_rule_flags {
  * @NL80211_MNTR_FLAG_OTHER_BSS: disable BSSID filtering
  * @NL80211_MNTR_FLAG_COOK_FRAMES: report frames after processing.
  *	overrides all other flags.
+ * @NL80211_MNTR_FLAG_AP_MGT: this monitor interface is used for AP mode
+ * 	to be able to inject management frames.
  *
  * @__NL80211_MNTR_FLAG_AFTER_LAST: internal use
  * @NL80211_MNTR_FLAG_MAX: highest possible monitor flag
@@ -716,6 +718,7 @@ enum nl80211_mntr_flags {
 	NL80211_MNTR_FLAG_CONTROL,
 	NL80211_MNTR_FLAG_OTHER_BSS,
 	NL80211_MNTR_FLAG_COOK_FRAMES,
+	NL80211_MNTR_FLAG_AP_MGT,
 
 	/* keep last */
 	__NL80211_MNTR_FLAG_AFTER_LAST,
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index dd1fd51..fe65c64 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -255,6 +255,9 @@ struct station_info {
  * @MONITOR_FLAG_CONTROL: pass control frames
  * @MONITOR_FLAG_OTHER_BSS: disable BSSID filtering
  * @MONITOR_FLAG_COOK_FRAMES: report frames after processing
+ * @MONITOR_FLAG_AP_MGT: informs us this monitor interface is
+ *	used by a driver for AP mode to be able to inject management
+ * 	frames.
  */
 enum monitor_flags {
 	MONITOR_FLAG_FCSFAIL		= 1<<NL80211_MNTR_FLAG_FCSFAIL,
@@ -262,6 +265,7 @@ enum monitor_flags {
 	MONITOR_FLAG_CONTROL		= 1<<NL80211_MNTR_FLAG_CONTROL,
 	MONITOR_FLAG_OTHER_BSS		= 1<<NL80211_MNTR_FLAG_OTHER_BSS,
 	MONITOR_FLAG_COOK_FRAMES	= 1<<NL80211_MNTR_FLAG_COOK_FRAMES,
+	MONITOR_FLAG_AP_MGT		= 1<<NL80211_MNTR_FLAG_AP_MGT,
 };
 
 /**
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 7b013fb..e752f6d 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1432,11 +1432,25 @@ int ieee80211_master_start_xmit(struct sk_buff *skb, struct net_device *dev)
 int ieee80211_monitor_start_xmit(struct sk_buff *skb,
 				 struct net_device *dev)
 {
+	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
 	struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
+	struct ieee80211_channel *chan = local->hw.conf.channel;
 	struct ieee80211_radiotap_header *prthdr =
 		(struct ieee80211_radiotap_header *)skb->data;
 	u16 len_rthdr;
 
+	/* Frame injection is not allowed if beaconing is not allowed
+	 * or if we need radar detection. Beaconing is usually not allowed when
+	 * the mode or operation (Adhoc, AP, Mesh) does not support DFS.
+	 * Since AP mode uses monitor interfaces to inject/TX management
+	 * frames we make AP mode the exception to this rule as its
+	 * implementation can deal with radar detection by itself. */
+	if (WARN_ON(sdata->vif.type != NL80211_IFTYPE_MONITOR))
+		return TX_DROP;
+	if (!(sdata->u.mntr_flags & MONITOR_FLAG_AP_MGT) &&
+	   (chan->flags & (IEEE80211_CHAN_NO_IBSS | IEEE80211_CHAN_RADAR)))
+		return TX_DROP;
+
 	/* check for not even having the fixed radiotap header part */
 	if (unlikely(skb->len < sizeof(struct ieee80211_radiotap_header)))
 		goto fail; /* too short to be possibly valid */
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 4baa875..693cb73 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -537,6 +537,7 @@ static const struct nla_policy mntr_flags_policy[NL80211_MNTR_FLAG_MAX + 1] = {
 	[NL80211_MNTR_FLAG_CONTROL] = { .type = NLA_FLAG },
 	[NL80211_MNTR_FLAG_OTHER_BSS] = { .type = NLA_FLAG },
 	[NL80211_MNTR_FLAG_COOK_FRAMES] = { .type = NLA_FLAG },
+	[NL80211_MNTR_FLAG_AP_MGT] = { .type = NLA_FLAG },
 };
 
 static int parse_monitor_flags(struct nlattr *nla, u32 *mntrflags)
-- 
1.5.6.4

Re: [PATCH] mac80211: do not TX injected frames when not allowed

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 4478 bytes --]

On Mon, 2009-01-26 at 16:22 -0800, Luis R. Rodriguez wrote:
> Monitor mode is able to TX by using injected frames. We should
> not allow injected frames to be sent unless allowed by regulatory
> rules. Since AP mode uses a monitor interfaces to transmit
> management frames we have to take care to not break AP mode as
> well while resolving this. We deal with this by allowing compliant
> APs solutions to inform mac80211 if their monitor interface is
> intended to be used for an AP by setting a cfg80211 flag for the
> monitor interface. hostapd, for example, currently does its own
> checks to ensure AP mode is not used on channels which require radar
> detection. Once such solutions are available it can can enable this
> flag.
> 
> Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
> ---
> 
> This now works, I moved the check as recommended by Johannes to
> ieee80211_monitor_start_xmit(), the issues I ran into earlier seem
> to be due to a work around set in place for 11w.
> 
>  include/linux/nl80211.h |    3 +++
>  include/net/cfg80211.h  |    4 ++++
>  net/mac80211/tx.c       |   14 ++++++++++++++
>  net/wireless/nl80211.c  |    1 +
>  4 files changed, 22 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
> index 76aae3d..3357907 100644
> --- a/include/linux/nl80211.h
> +++ b/include/linux/nl80211.h
> @@ -705,6 +705,8 @@ enum nl80211_reg_rule_flags {
>   * @NL80211_MNTR_FLAG_OTHER_BSS: disable BSSID filtering
>   * @NL80211_MNTR_FLAG_COOK_FRAMES: report frames after processing.
>   *	overrides all other flags.
> + * @NL80211_MNTR_FLAG_AP_MGT: this monitor interface is used for AP mode
> + * 	to be able to inject management frames.
>   *
>   * @__NL80211_MNTR_FLAG_AFTER_LAST: internal use
>   * @NL80211_MNTR_FLAG_MAX: highest possible monitor flag
> @@ -716,6 +718,7 @@ enum nl80211_mntr_flags {
>  	NL80211_MNTR_FLAG_CONTROL,
>  	NL80211_MNTR_FLAG_OTHER_BSS,
>  	NL80211_MNTR_FLAG_COOK_FRAMES,
> +	NL80211_MNTR_FLAG_AP_MGT,
>  
>  	/* keep last */
>  	__NL80211_MNTR_FLAG_AFTER_LAST,
> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> index dd1fd51..fe65c64 100644
> --- a/include/net/cfg80211.h
> +++ b/include/net/cfg80211.h
> @@ -255,6 +255,9 @@ struct station_info {
>   * @MONITOR_FLAG_CONTROL: pass control frames
>   * @MONITOR_FLAG_OTHER_BSS: disable BSSID filtering
>   * @MONITOR_FLAG_COOK_FRAMES: report frames after processing
> + * @MONITOR_FLAG_AP_MGT: informs us this monitor interface is
> + *	used by a driver for AP mode to be able to inject management
> + * 	frames.
>   */
>  enum monitor_flags {
>  	MONITOR_FLAG_FCSFAIL		= 1<<NL80211_MNTR_FLAG_FCSFAIL,
> @@ -262,6 +265,7 @@ enum monitor_flags {
>  	MONITOR_FLAG_CONTROL		= 1<<NL80211_MNTR_FLAG_CONTROL,
>  	MONITOR_FLAG_OTHER_BSS		= 1<<NL80211_MNTR_FLAG_OTHER_BSS,
>  	MONITOR_FLAG_COOK_FRAMES	= 1<<NL80211_MNTR_FLAG_COOK_FRAMES,
> +	MONITOR_FLAG_AP_MGT		= 1<<NL80211_MNTR_FLAG_AP_MGT,
>  };
>  
>  /**
> diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
> index 7b013fb..e752f6d 100644
> --- a/net/mac80211/tx.c
> +++ b/net/mac80211/tx.c
> @@ -1432,11 +1432,25 @@ int ieee80211_master_start_xmit(struct sk_buff *skb, struct net_device *dev)
>  int ieee80211_monitor_start_xmit(struct sk_buff *skb,
>  				 struct net_device *dev)
>  {
> +	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
>  	struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
> +	struct ieee80211_channel *chan = local->hw.conf.channel;
>  	struct ieee80211_radiotap_header *prthdr =
>  		(struct ieee80211_radiotap_header *)skb->data;
>  	u16 len_rthdr;
>  
> +	/* Frame injection is not allowed if beaconing is not allowed

Comment style, /* on a single line please :)

> +	 * or if we need radar detection. Beaconing is usually not allowed when
> +	 * the mode or operation (Adhoc, AP, Mesh) does not support DFS.
> +	 * Since AP mode uses monitor interfaces to inject/TX management
> +	 * frames we make AP mode the exception to this rule as its
> +	 * implementation can deal with radar detection by itself. */

and end with */ on its own line.

> +	if (WARN_ON(sdata->vif.type != NL80211_IFTYPE_MONITOR))
> +		return TX_DROP;

Huh? You're in monitor_start_xmit. If that goes wrong, something is
horribly broken and beyond WARN_ON. Please just remove this.

Other than that, looks good.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]