Revise crypto api lightly to improve group key handling:

o don't pre-assign key index to the global key table entries so device
  has a chance to decide what to use
o make ieee80211_crypto_newkey take the desired flags as an argument
  instead of wacking the key structure directly; this eliminates a
  bunch of code warts
o add a new flag IEEE80211_KEY_GROUP to indicate a key is a WPA Group
  key so devices don't need to guess (temporarily add this flag in the
  ioctl code until we can get wpa_supplicant+hostapd updated)
o shuffle IEEE80211_KEY_* bits to move flags used internally to the high
  nibble of the flags word

Reviewed by:	Tai-hwa Liang

3 files changed, 44 insertions, 33 deletions

diff --git a/sys/net80211/ieee80211_crypto.c b/sys/net80211/ieee80211_crypto.c
index 44663bd..d6678c6 100644
--- a/sys/net80211/ieee80211_crypto.c
+++ b/sys/net80211/ieee80211_crypto.c
@@ -128,7 +128,8 @@ ieee80211_crypto_attach(struct ieee80211com *ic)
 	cs->cs_def_txkey = IEEE80211_KEYIX_NONE;
 	ciphers[IEEE80211_CIPHER_NONE] = &ieee80211_cipher_none;
 	for (i = 0; i < IEEE80211_WEP_NKID; i++)
-		ieee80211_crypto_resetkey(ic, &cs->cs_nw_keys[i], i);
+		ieee80211_crypto_resetkey(ic, &cs->cs_nw_keys[i],
+			IEEE80211_KEYIX_NONE);
 	/*
 	 * Initialize the driver key support routines to noop entries.
 	 * This is useful especially for the cipher test modules.
@@ -206,10 +207,9 @@ static const char *cipher_modnames[] = {
 
 /*
  * Establish a relationship between the specified key and cipher
- * and, if not a global key, allocate a hardware index from the
- * driver.  Note that we may be called for global keys but they
- * should have a key index already setup so the only work done
- * is to setup the cipher reference.
+ * and, if necessary, allocate a hardware index from the driver.
+ * Note that when a fixed key index is required it must be specified
+ * and we blindly assign it w/o consulting the driver (XXX).
  *
  * This must be the first call applied to a key; all the other key
  * routines assume wk_cipher is setup.
@@ -220,7 +220,7 @@ static const char *cipher_modnames[] = {
  */
 int
 ieee80211_crypto_newkey(struct ieee80211com *ic,
-	int cipher, struct ieee80211_key *key)
+	int cipher, int flags, struct ieee80211_key *key)
 {
 #define	N(a)	(sizeof(a) / sizeof(a[0]))
 	const struct ieee80211_cipher *cip;
@@ -268,16 +268,16 @@ ieee80211_crypto_newkey(struct ieee80211com *ic,
 	}
 
 	oflags = key->wk_flags;
+	flags &= IEEE80211_KEY_COMMON;
 	/*
 	 * If the hardware does not support the cipher then
 	 * fallback to a host-based implementation.
 	 */
-	key->wk_flags &= ~(IEEE80211_KEY_SWCRYPT|IEEE80211_KEY_SWMIC);
 	if ((ic->ic_caps & (1<<cipher)) == 0) {
 		IEEE80211_DPRINTF(ic, IEEE80211_MSG_CRYPTO,
 		    "%s: no h/w support for cipher %s, falling back to s/w\n",
 		    __func__, cip->ic_name);
-		key->wk_flags |= IEEE80211_KEY_SWCRYPT;
+		flags |= IEEE80211_KEY_SWCRYPT;
 	}
 	/*
 	 * Hardware TKIP with software MIC is an important
@@ -289,7 +289,7 @@ ieee80211_crypto_newkey(struct ieee80211com *ic,
 		IEEE80211_DPRINTF(ic, IEEE80211_MSG_CRYPTO,
 		    "%s: no h/w support for TKIP MIC, falling back to s/w\n",
 		    __func__);
-		key->wk_flags |= IEEE80211_KEY_SWMIC;
+		flags |= IEEE80211_KEY_SWMIC;
 	}
 
 	/*
@@ -298,8 +298,18 @@ ieee80211_crypto_newkey(struct ieee80211com *ic,
 	 * cipher module can optimize space usage based on
 	 * whether or not it needs to do the cipher work.
 	 */
-	if (key->wk_cipher != cip || key->wk_flags != oflags) {
+	if (key->wk_cipher != cip || key->wk_flags != flags) {
 again:
+		/*
+		 * Fillin the flags so cipher modules can see s/w
+		 * crypto requirements and potentially allocate
+		 * different state and/or attach different method
+		 * pointers.
+		 *
+		 * XXX this is not right when s/w crypto fallback
+		 *     fails and we try to restore previous state.
+		 */
+		key->wk_flags = flags;
 		keyctx = cip->ic_attach(ic, key);
 		if (keyctx == NULL) {
 			IEEE80211_DPRINTF(ic, IEEE80211_MSG_CRYPTO,
@@ -313,6 +323,10 @@ again:
 		key->wk_cipher = cip;		/* XXX refcnt? */
 		key->wk_private = keyctx;
 	}
+	/*
+	 * Commit to requested usage so driver can see the flags.
+	 */
+	key->wk_flags = flags;
 
 	/*
 	 * Ask the driver for a key index if we don't have one.
@@ -340,9 +354,9 @@ again:
 				    "falling back to s/w\n", __func__,
 				    cip->ic_name);
 				oflags = key->wk_flags;
-				key->wk_flags |= IEEE80211_KEY_SWCRYPT;
+				flags |= IEEE80211_KEY_SWCRYPT;
 				if (cipher == IEEE80211_CIPHER_TKIP)
-					key->wk_flags |= IEEE80211_KEY_SWMIC;
+					flags |= IEEE80211_KEY_SWMIC;
 				goto again;
 			}
 			ic->ic_stats.is_crypto_keyfail++;
@@ -388,14 +402,7 @@ _ieee80211_crypto_delkey(struct ieee80211com *ic, struct ieee80211_key *key)
 	}
 	cipher_detach(key);
 	memset(key, 0, sizeof(*key));
-	key->wk_cipher = &ieee80211_cipher_none;
-	key->wk_private = cipher_attach(ic, key);
-	/* NB: cannot depend on key index to decide this */
-	if (&ic->ic_nw_keys[0] <= key &&
-	    key < &ic->ic_nw_keys[IEEE80211_WEP_NKID])
-		key->wk_keyix = keyix;		/* preserve shared key state */
-	else
-		key->wk_keyix = IEEE80211_KEYIX_NONE;
+	ieee80211_crypto_resetkey(ic, key, IEEE80211_KEYIX_NONE);
 	return 1;
 }
 
@@ -480,7 +487,7 @@ ieee80211_crypto_encap(struct ieee80211com *ic,
 	struct ieee80211_key *k;
 	struct ieee80211_frame *wh;
 	const struct ieee80211_cipher *cip;
-	u_int8_t keyix;
+	u_int8_t keyid;
 
 	/*
 	 * Multicast traffic always uses the multicast key.
@@ -499,14 +506,14 @@ ieee80211_crypto_encap(struct ieee80211com *ic,
 			ic->ic_stats.is_tx_nodefkey++;
 			return NULL;
 		}
-		keyix = ic->ic_def_txkey;
+		keyid = ic->ic_def_txkey;
 		k = &ic->ic_nw_keys[ic->ic_def_txkey];
 	} else {
-		keyix = 0;
+		keyid = 0;
 		k = &ni->ni_ucastkey;
 	}
 	cip = k->wk_cipher;
-	return (cip->ic_encap(k, m, keyix<<6) ? k : NULL);
+	return (cip->ic_encap(k, m, keyid<<6) ? k : NULL);
 }
 
 /*
diff --git a/sys/net80211/ieee80211_crypto.h b/sys/net80211/ieee80211_crypto.h
index 7f18c2d..ffb8525 100644
--- a/sys/net80211/ieee80211_crypto.h
+++ b/sys/net80211/ieee80211_crypto.h
@@ -74,8 +74,9 @@ struct ieee80211_key {
 	u_int8_t	wk_flags;
 #define	IEEE80211_KEY_XMIT	0x01	/* key used for xmit */
 #define	IEEE80211_KEY_RECV	0x02	/* key used for recv */
-#define	IEEE80211_KEY_SWCRYPT	0x04	/* host-based encrypt/decrypt */
-#define	IEEE80211_KEY_SWMIC	0x08	/* host-based enmic/demic */
+#define	IEEE80211_KEY_GROUP	0x04	/* key used for WPA group operation */
+#define	IEEE80211_KEY_SWCRYPT	0x10	/* host-based encrypt/decrypt */
+#define	IEEE80211_KEY_SWMIC	0x20	/* host-based enmic/demic */
 	u_int16_t	wk_keyix;	/* key index */
 	u_int8_t	wk_key[IEEE80211_KEYBUF_SIZE+IEEE80211_MICBUF_SIZE];
 #define	wk_txmic	wk_key+IEEE80211_KEYBUF_SIZE+0	/* XXX can't () right */
@@ -85,6 +86,8 @@ struct ieee80211_key {
 	const struct ieee80211_cipher *wk_cipher;
 	void		*wk_private;	/* private cipher state */
 };
+#define	IEEE80211_KEY_COMMON 		/* common flags passed in by apps */\
+	(IEEE80211_KEY_XMIT | IEEE80211_KEY_RECV | IEEE80211_KEY_GROUP)
 
 /*
  * NB: these values are ordered carefully; there are lots of
@@ -133,7 +136,7 @@ struct ieee80211_crypto_state {
 void	ieee80211_crypto_attach(struct ieee80211com *);
 void	ieee80211_crypto_detach(struct ieee80211com *);
 int	ieee80211_crypto_newkey(struct ieee80211com *,
-		int cipher, struct ieee80211_key *);
+		int cipher, int flags, struct ieee80211_key *);
 int	ieee80211_crypto_delkey(struct ieee80211com *,
 		struct ieee80211_key *);
 int	ieee80211_crypto_setkey(struct ieee80211com *,
diff --git a/sys/net80211/ieee80211_ioctl.c b/sys/net80211/ieee80211_ioctl.c
index ce49a59..e40c4af 100644
--- a/sys/net80211/ieee80211_ioctl.c
+++ b/sys/net80211/ieee80211_ioctl.c
@@ -1525,18 +1525,19 @@ ieee80211_ioctl_setkey(struct ieee80211com *ic, struct ieee80211req *ireq)
 			return EINVAL;
 		wk = &ic->ic_nw_keys[kid];
 		ni = NULL;
+		/* XXX auto-add group key flag until applications are updated */
+		if ((ik.ik_flags & IEEE80211_KEY_XMIT) == 0)	/* XXX */
+			ik.ik_flags |= IEEE80211_KEY_GROUP;	/* XXX */
 	}
 	error = 0;
 	ieee80211_key_update_begin(ic);
-	if (ieee80211_crypto_newkey(ic, ik.ik_type, wk)) {
+	if (ieee80211_crypto_newkey(ic, ik.ik_type, ik.ik_flags, wk)) {
 		wk->wk_keylen = ik.ik_keylen;
 		/* NB: MIC presence is implied by cipher type */
 		if (wk->wk_keylen > IEEE80211_KEYBUF_SIZE)
 			wk->wk_keylen = IEEE80211_KEYBUF_SIZE;
 		wk->wk_keyrsc = ik.ik_keyrsc;
 		wk->wk_keytsc = 0;			/* new key, reset */
-		wk->wk_flags |=
-			ik.ik_flags & (IEEE80211_KEY_XMIT|IEEE80211_KEY_RECV);
 		memset(wk->wk_key, 0, sizeof(wk->wk_key));
 		memcpy(wk->wk_key, ik.ik_keydata, ik.ik_keylen);
 		if (!ieee80211_crypto_setkey(ic, wk,
@@ -1958,10 +1959,10 @@ ieee80211_ioctl_set80211(struct ieee80211com *ic, u_long cmd, struct ieee80211re
 		if (error)
 			break;
 		ieee80211_key_update_begin(ic);
-		if (ieee80211_crypto_newkey(ic, IEEE80211_CIPHER_WEP, k)) {
+		k->wk_keyix = kid;	/* NB: force fixed key id */
+		if (ieee80211_crypto_newkey(ic, IEEE80211_CIPHER_WEP,
+		    IEEE80211_KEY_XMIT | IEEE80211_KEY_RECV, k)) {
 			k->wk_keylen = ireq->i_len;
-			k->wk_flags |=
-			    IEEE80211_KEY_XMIT | IEEE80211_KEY_RECV;
 			memcpy(k->wk_key, tmpkey, sizeof(tmpkey));
 			if  (!ieee80211_crypto_setkey(ic, k, ic->ic_myaddr))
 				error = EINVAL;