linux-kernel - RE: [PATCH 4.9 094/206] nl80211: Use different attrs for BSSID and random MAC addr in scan req

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aa353c91377f40388ba6e817482400f8@aphydexm01b.ap.qualcomm.com>
Date:   Fri, 21 Jul 2017 09:40:02 +0000
From:   Krishna Vamsi <vamsin@....qualcomm.com>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC:     "stable@...r.kernel.org" <stable@...r.kernel.org>,
        Jouni Malinen <jouni@....qualcomm.com>,
        Johannes Berg <johannes.berg@...el.com>
Subject: RE: [PATCH 4.9 094/206] nl80211: Use different attrs for BSSID and
 random MAC addr in scan req

Hello Greg,

Really sorry that I couldn't find this earlier but there is one issue with this cherry-pick. In "enum nl80211_commands", there are more variables between NL80211_ATTR_NAN_MATCH and NL80211_ATTR_BSSID. But they are missing in this cherry-pick. The missing variables are 
NL80211_ATTR_FILS_KEK, NL80211_ATTR_FILS_NONCES andNL80211_ATTR_MULTICAST_TO_UNICAST_ENABLED. Because of this NL80211_ATTR_BSSID value might not be consistent across different releases. Could you please let me know the next steps on this?

Thanks,
Vamsi

> -----Original Message-----
> From: Greg Kroah-Hartman [mailto:gregkh@...uxfoundation.org]
> Sent: Tuesday, January 10, 2017 7:06 PM
> To: linux-kernel@...r.kernel.org
> Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>;
> stable@...r.kernel.org; Vamsi, Krishna <vamsin@....qualcomm.com>;
> Malinen, Jouni <jouni@....qualcomm.com>; Johannes Berg
> <johannes.berg@...el.com>
> Subject: [PATCH 4.9 094/206] nl80211: Use different attrs for BSSID and
> random MAC addr in scan req
> 
> 4.9-stable review patch.  If anyone has any objections, please let me know.
> 
> ------------------
> 
> From: Vamsi Krishna <vamsin@....qualcomm.com>
> 
> commit 2fa436b3a2a7009c11a3bc03fe0ff4c26e80fd87 upstream.
> 
> NL80211_ATTR_MAC was used to set both the specific BSSID to be scanned and
> the random MAC address to be used when privacy is enabled. When both the
> features are enabled, both the BSSID and the local MAC address were getting
> same value causing Probe Request frames to go with unintended DA. Hence,
> this has been fixed by using a different NL80211_ATTR_BSSID attribute to set
> the specific BSSID (which was the more recent addition in cfg80211) for a scan.
> 
> Backwards compatibility with old userspace software is maintained to some
> extent by allowing NL80211_ATTR_MAC to be used to set the specific BSSID
> when scanning without enabling random MAC address use.
> 
> Scanning with random source MAC address was introduced by commit
> ad2b26abc157 ("cfg80211: allow drivers to support random MAC addresses for
> scan") and the issue was introduced with the addition of the second user for the
> same attribute in commit 818965d39177 ("cfg80211: Allow a scan request for a
> specific BSSID").
> 
> Fixes: 818965d39177 ("cfg80211: Allow a scan request for a specific BSSID")
> Signed-off-by: Vamsi Krishna <vamsin@....qualcomm.com>
> Signed-off-by: Jouni Malinen <jouni@....qualcomm.com>
> Signed-off-by: Johannes Berg <johannes.berg@...el.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> 
> ---
>  include/uapi/linux/nl80211.h |    7 ++++++-
>  net/wireless/nl80211.c       |   16 +++++++++++++++-
>  2 files changed, 21 insertions(+), 2 deletions(-)
> 
> --- a/include/uapi/linux/nl80211.h
> +++ b/include/uapi/linux/nl80211.h
> @@ -323,7 +323,7 @@
>   * @NL80211_CMD_GET_SCAN: get scan results
>   * @NL80211_CMD_TRIGGER_SCAN: trigger a new scan with the given
> parameters
>   *	%NL80211_ATTR_TX_NO_CCK_RATE is used to decide whether to send
> the
> - *	probe requests at CCK rate or not. %NL80211_ATTR_MAC can be used
> to
> + *	probe requests at CCK rate or not. %NL80211_ATTR_BSSID can be used
> to
>   *	specify a BSSID to scan for; if not included, the wildcard BSSID will
>   *	be used.
>   * @NL80211_CMD_NEW_SCAN_RESULTS: scan notification (as a reply to @@
> -1937,6 +1937,9 @@ enum nl80211_commands {
>   * @NL80211_ATTR_NAN_MATCH: used to report a match. This is a nested
> attribute.
>   *	See &enum nl80211_nan_match_attributes.
>   *
> + * @NL80211_ATTR_BSSID: The BSSID of the AP. Note that
> %NL80211_ATTR_MAC is also
> + *	used in various commands/events for specifying the BSSID.
> + *
>   * @NUM_NL80211_ATTR: total number of nl80211_attrs available
>   * @NL80211_ATTR_MAX: highest attribute number currently defined
>   * @__NL80211_ATTR_AFTER_LAST: internal use @@ -2336,6 +2339,8 @@
> enum nl80211_attrs {
>  	NL80211_ATTR_NAN_FUNC,
>  	NL80211_ATTR_NAN_MATCH,
> 
> +	NL80211_ATTR_BSSID,
> +
>  	/* add attributes here, update the policy in nl80211.c */
> 
>  	__NL80211_ATTR_AFTER_LAST,
> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c
> @@ -414,6 +414,7 @@ static const struct nla_policy nl80211_p
>  	[NL80211_ATTR_NAN_MASTER_PREF] = { .type = NLA_U8 },
>  	[NL80211_ATTR_NAN_DUAL] = { .type = NLA_U8 },
>  	[NL80211_ATTR_NAN_FUNC] = { .type = NLA_NESTED },
> +	[NL80211_ATTR_BSSID] = { .len = ETH_ALEN },
>  };
> 
>  /* policy for the key attributes */
> @@ -6677,7 +6678,20 @@ static int nl80211_trigger_scan(struct s
>  	request->no_cck =
>  		nla_get_flag(info->attrs[NL80211_ATTR_TX_NO_CCK_RATE]);
> 
> -	if (info->attrs[NL80211_ATTR_MAC])
> +	/* Initial implementation used NL80211_ATTR_MAC to set the specific
> +	 * BSSID to scan for. This was problematic because that same attribute
> +	 * was already used for another purpose (local random MAC address).
> The
> +	 * NL80211_ATTR_BSSID attribute was added to fix this. For backwards
> +	 * compatibility with older userspace components, also use the
> +	 * NL80211_ATTR_MAC value here if it can be determined to be used
> for
> +	 * the specific BSSID use case instead of the random MAC address
> +	 * (NL80211_ATTR_SCAN_FLAGS is used to enable random MAC
> address use).
> +	 */
> +	if (info->attrs[NL80211_ATTR_BSSID])
> +		memcpy(request->bssid,
> +		       nla_data(info->attrs[NL80211_ATTR_BSSID]), ETH_ALEN);
> +	else if (!(request->flags & NL80211_SCAN_FLAG_RANDOM_ADDR) &&
> +		 info->attrs[NL80211_ATTR_MAC])
>  		memcpy(request->bssid, nla_data(info-
> >attrs[NL80211_ATTR_MAC]),
>  		       ETH_ALEN);
>  	else
>