diff mbox

[5/8] cfg80211: Add KEK/nonces for FILS association frames

Message ID 1477435489-8555-1-git-send-email-jouni@qca.qualcomm.com (mailing list archive)
State Changes Requested
Delegated to: Johannes Berg
Headers show

Commit Message

Jouni Malinen Oct. 25, 2016, 10:44 p.m. UTC
The new nl80211 attributes can be used to provide KEK and nonces to
allow the driver to encrypt and decrypt FILS (Re)Association
Request/Response frames in station mode.

Signed-off-by: Jouni Malinen <jouni@qca.qualcomm.com>
---
 include/linux/ieee80211.h    |  3 +++
 include/net/cfg80211.h       |  9 +++++++++
 include/uapi/linux/nl80211.h |  8 ++++++++
 net/wireless/nl80211.c       | 17 +++++++++++++++++
 4 files changed, 37 insertions(+)

Comments

Johannes Berg Oct. 26, 2016, 5:36 a.m. UTC | #1
> +++ b/net/wireless/nl80211.c
> @@ -414,6 +414,10 @@ enum nl80211_multicast_groups {
>  	[NL80211_ATTR_NAN_MASTER_PREF] = { .type = NLA_U8 },
>  	[NL80211_ATTR_NAN_DUAL] = { .type = NLA_U8 },
>  	[NL80211_ATTR_NAN_FUNC] = { .type = NLA_NESTED },
> +	[NL80211_ATTR_FILS_KEK] = { .type = NLA_BINARY,
> +				    .len = FILS_MAX_KEK_LEN },
> +	[NL80211_ATTR_FILS_NONCES] = { .type = NLA_BINARY,
> +				       .len = 2 * FILS_NONCE_LEN },
>  };

If you remove the type = NLA_BINARY and just leave the type zero, then
you'll get *minimum* length validation, rather than limiting the
maximum length. That seems more appropriate for the nonces?

> +	if (info->attrs[NL80211_ATTR_FILS_NONCES]) {
> +		if (nla_len(info->attrs[NL80211_ATTR_FILS_NONCES])
> !=
> +		    2 * FILS_NONCE_LEN)
> +			return -EINVAL;

You're validating the *exact* length here, which unfortunately nlattr
doesn't support right now, but perhaps we can live with checking that
it's at least that many bytes, and using only 2*nonces? We do that for
most other attributes (like MAC addresses).

Or do we expect to extend this to more than 2 nonces in the future, at
which point we'll need the length?

johannes
Jouni Malinen Oct. 26, 2016, 9:18 a.m. UTC | #2
On Wed, Oct 26, 2016 at 07:36:27AM +0200, Johannes Berg wrote:
> > +++ b/net/wireless/nl80211.c

> > +	[NL80211_ATTR_FILS_KEK] = { .type = NLA_BINARY,

> > +				    .len = FILS_MAX_KEK_LEN },

> > +	[NL80211_ATTR_FILS_NONCES] = { .type = NLA_BINARY,

> > +				       .len = 2 * FILS_NONCE_LEN },


> If you remove the type = NLA_BINARY and just leave the type zero, then

> you'll get *minimum* length validation, rather than limiting the

> maximum length. That seems more appropriate for the nonces?


FILS_KEK is variable length, but FILS_NONCES should be exactly 2 *
FILS_NONCE_LEN. I didn't remember the minimum length case here, but yes,
that sounds reasonable for FILS_NONCES.

> > +	if (info->attrs[NL80211_ATTR_FILS_NONCES]) {

> > +		if (nla_len(info->attrs[NL80211_ATTR_FILS_NONCES])

> > !=

> > +		    2 * FILS_NONCE_LEN)

> > +			return -EINVAL;

> 

> You're validating the *exact* length here, which unfortunately nlattr

> doesn't support right now, but perhaps we can live with checking that

> it's at least that many bytes, and using only 2*nonces? We do that for

> most other attributes (like MAC addresses).


This was because of the .len above ending up allowing shorter values..
Since we do that minimum length check only for number of other
attributes, I guess we can do it here as well and ignore whatever else
the user space might have added incorrectly.

> Or do we expect to extend this to more than 2 nonces in the future, at

> which point we'll need the length?


No, this should remain exactly two nonces and each of those having
exactly 16 octets.

-- 
Jouni Malinen                                            PGP id EFC895FA
diff mbox

Patch

diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index 9a523d9..a39beb9 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -2076,6 +2076,9 @@  enum ieee80211_key_len {
 #define IEEE80211_GCMP_MIC_LEN		16
 #define IEEE80211_GCMP_PN_LEN		6
 
+#define FILS_NONCE_LEN			16
+#define FILS_MAX_KEK_LEN		64
+
 /* Public action codes */
 enum ieee80211_pub_actioncode {
 	WLAN_PUB_ACTION_EXT_CHANSW_ANN = 4,
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 2667917..a3045f1 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1842,6 +1842,12 @@  enum cfg80211_assoc_req_flags {
  * @ht_capa_mask:  The bits of ht_capa which are to be used.
  * @vht_capa: VHT capability override
  * @vht_capa_mask: VHT capability mask indicating which fields to use
+ * @fils_kek: FILS KEK for protecting (Re)Association Request/Response frame or
+ *	%NULL if FILS is not used.
+ * @fils_kek_len: Length of fils_kek in octets
+ * @fils_nonces: FILS nonces (part of AAD) for protecting (Re)Association
+ *	Request/Response frame or %NULL if FILS is not used. This field starts
+ *	with 16 octets of STA Nonce followed by 16 octets of AP Nonce.
  */
 struct cfg80211_assoc_request {
 	struct cfg80211_bss *bss;
@@ -1853,6 +1859,9 @@  struct cfg80211_assoc_request {
 	struct ieee80211_ht_cap ht_capa;
 	struct ieee80211_ht_cap ht_capa_mask;
 	struct ieee80211_vht_cap vht_capa, vht_capa_mask;
+	const u8 *fils_kek;
+	size_t fils_kek_len;
+	const u8 *fils_nonces;
 };
 
 /**
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 5d36e83..b468a89 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -1938,6 +1938,11 @@  enum nl80211_commands {
  *	attribute.
  * @NL80211_ATTR_NAN_MATCH: used to report a match. This is a nested attribute.
  *	See &enum nl80211_nan_match_attributes.
+ * @NL80211_ATTR_FILS_KEK: KEK for FILS (Re)Association Request/Response frame
+ *	protection.
+ * @NL80211_ATTR_FILS_NONCES: Nonces (part of AAD) for FILS (Re)Association
+ *	Request/Response frame protection. This attribute contains the 16 octet
+ *	STA Nonce followed by 16 octets of AP Nonce.
  *
  * @NUM_NL80211_ATTR: total number of nl80211_attrs available
  * @NL80211_ATTR_MAX: highest attribute number currently defined
@@ -2338,6 +2343,9 @@  enum nl80211_attrs {
 	NL80211_ATTR_NAN_FUNC,
 	NL80211_ATTR_NAN_MATCH,
 
+	NL80211_ATTR_FILS_KEK,
+	NL80211_ATTR_FILS_NONCES,
+
 	/* add attributes here, update the policy in nl80211.c */
 
 	__NL80211_ATTR_AFTER_LAST,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index ebd7eaf..a06145c 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -414,6 +414,10 @@  enum nl80211_multicast_groups {
 	[NL80211_ATTR_NAN_MASTER_PREF] = { .type = NLA_U8 },
 	[NL80211_ATTR_NAN_DUAL] = { .type = NLA_U8 },
 	[NL80211_ATTR_NAN_FUNC] = { .type = NLA_NESTED },
+	[NL80211_ATTR_FILS_KEK] = { .type = NLA_BINARY,
+				    .len = FILS_MAX_KEK_LEN },
+	[NL80211_ATTR_FILS_NONCES] = { .type = NLA_BINARY,
+				       .len = 2 * FILS_NONCE_LEN },
 };
 
 /* policy for the key attributes */
@@ -8019,6 +8023,19 @@  static int nl80211_associate(struct sk_buff *skb, struct genl_info *info)
 		req.flags |= ASSOC_REQ_USE_RRM;
 	}
 
+	if (info->attrs[NL80211_ATTR_FILS_KEK]) {
+		req.fils_kek = nla_data(info->attrs[NL80211_ATTR_FILS_KEK]);
+		req.fils_kek_len = nla_len(info->attrs[NL80211_ATTR_FILS_KEK]);
+	}
+
+	if (info->attrs[NL80211_ATTR_FILS_NONCES]) {
+		if (nla_len(info->attrs[NL80211_ATTR_FILS_NONCES]) !=
+		    2 * FILS_NONCE_LEN)
+			return -EINVAL;
+		req.fils_nonces =
+			nla_data(info->attrs[NL80211_ATTR_FILS_NONCES]);
+	}
+
 	err = nl80211_crypto_settings(rdev, info, &req.crypto, 1);
 	if (!err) {
 		wdev_lock(dev->ieee80211_ptr);