This patch is not runtime tested, as I did not implement tkip
support in bcm43xx, yet.
--
This fixes TKIP phase1 key mixing for hwcrypto on BigEndian
platforms.
Casting an u8 array to u16* is wrong and will only work
on le platforms.
Make it explicit and expect an u8* parameter for
ieee80211_tkip_gen_phase1key(). The function will take
care to return an u8 array, instead of an u16 array, as
that's what drivers assume.
Signed-off-by: Michael Buesch <[email protected]>
Index: bu3sch-wireless-dev/net/d80211/tkip.c
===================================================================
--- bu3sch-wireless-dev.orig/net/d80211/tkip.c 2007-01-11 19:09:43.000000000 +0100
+++ bu3sch-wireless-dev/net/d80211/tkip.c 2007-02-03 18:23:52.000000000 +0100
@@ -192,10 +192,15 @@ u8 * ieee80211_tkip_add_iv(u8 *pos, stru
void ieee80211_tkip_gen_phase1key(struct ieee80211_key *key, u8 *ta,
- u16 *phase1key)
+ u8 *phase1key)
{
+ __le16 *k = (__le16 *)phase1key;
+ int i;
+
tkip_mixing_phase1(ta, &key->key[ALG_TKIP_TEMP_ENCR_KEY],
- key->u.tkip.iv32, phase1key);
+ key->u.tkip.iv32, (u16 *)k);
+ for (i = 0; i < 5; i++)
+ k[i] = cpu_to_le16(k[i]);
}
void ieee80211_tkip_gen_rc4key(struct ieee80211_key *key, u8 *ta,
Index: bu3sch-wireless-dev/net/d80211/tkip.h
===================================================================
--- bu3sch-wireless-dev.orig/net/d80211/tkip.h 2007-01-11 19:09:43.000000000 +0100
+++ bu3sch-wireless-dev/net/d80211/tkip.h 2007-02-03 18:19:18.000000000 +0100
@@ -16,7 +16,7 @@
u8 * ieee80211_tkip_add_iv(u8 *pos, struct ieee80211_key *key,
u8 iv0, u8 iv1, u8 iv2);
void ieee80211_tkip_gen_phase1key(struct ieee80211_key *key, u8 *ta,
- u16 *phase1key);
+ u8 *phase1key);
void ieee80211_tkip_gen_rc4key(struct ieee80211_key *key, u8 *ta,
u8 *rc4key);
void ieee80211_tkip_encrypt_data(struct crypto_blkcipher *tfm,
Index: bu3sch-wireless-dev/net/d80211/wpa.c
===================================================================
--- bu3sch-wireless-dev.orig/net/d80211/wpa.c 2007-01-11 19:09:43.000000000 +0100
+++ bu3sch-wireless-dev/net/d80211/wpa.c 2007-02-03 18:18:50.000000000 +0100
@@ -349,7 +349,7 @@ skip_iv_inc:
if (key->u.tkip.iv16 == 0 ||
!key->u.tkip.tx_initialized) {
ieee80211_tkip_gen_phase1key(key, hdr->addr2,
- (u16 *)tx->u.tx.control->tkip_key);
+ tx->u.tx.control->tkip_key);
key->u.tkip.tx_initialized = 1;
tx->u.tx.control->flags |=
IEEE80211_TXCTL_TKIP_NEW_PHASE1_KEY;
--
Greetings Michael.
On Monday 05 February 2007 22:19, Jiri Benc wrote:
> On Sat, 3 Feb 2007 18:32:48 +0100, Michael Buesch wrote:
> > @@ -192,10 +192,15 @@ u8 * ieee80211_tkip_add_iv(u8 *pos, stru
> >
> >
> > void ieee80211_tkip_gen_phase1key(struct ieee80211_key *key, u8 *ta,
> > - u16 *phase1key)
> > + u8 *phase1key)
> > {
> > + __le16 *k = (__le16 *)phase1key;
> > + int i;
> > +
> > tkip_mixing_phase1(ta, &key->key[ALG_TKIP_TEMP_ENCR_KEY],
> > - key->u.tkip.iv32, phase1key);
> > + key->u.tkip.iv32, (u16 *)k);
> > + for (i = 0; i < 5; i++)
> > + k[i] = cpu_to_le16(k[i]);
> > }
>
> Maybe a slightly better type checking but still looks ugly:
Hm, well. I don't really see how typechecking is better in this case,
but if you like it more, I'm ok with it. ;)
> @@ -192,10 +192,16 @@ u8 * ieee80211_tkip_add_iv(u8 *pos, stru
>
>
> void ieee80211_tkip_gen_phase1key(struct ieee80211_key *key, u8 *ta,
> - u16 *phase1key)
> + u8 *phase1key)
> {
> + u16 *tmp_result = (u16 *)phase1key;
> + __le16 *k = (__le16 *)phase1key;
> + int i;
> +
> tkip_mixing_phase1(ta, &key->key[ALG_TKIP_TEMP_ENCR_KEY],
> - key->u.tkip.iv32, phase1key);
> + key->u.tkip.iv32, tmp_result);
> + for (i = 0; i < 5; i++)
> + k[i] = cpu_to_le16(tmp_result[i]);
> }
>
> void ieee80211_tkip_gen_rc4key(struct ieee80211_key *key, u8 *ta,
>
>
> Moreover, I'm not sure if GCC is able to optimize out the for loop in
> this case :-(
Yeah, I was going to check this and was going to add #ifdefs if it doesn't.
But that was not my major concern at this point.
It was more that people agree to me that it _is_ broken on BE platforms.
(I cannot test it, yet, as tkip has other problems for bcm43xx).
--
Greetings Michael.
On Sat, 3 Feb 2007 18:32:48 +0100, Michael Buesch wrote:
> @@ -192,10 +192,15 @@ u8 * ieee80211_tkip_add_iv(u8 *pos, stru
>
>
> void ieee80211_tkip_gen_phase1key(struct ieee80211_key *key, u8 *ta,
> - u16 *phase1key)
> + u8 *phase1key)
> {
> + __le16 *k = (__le16 *)phase1key;
> + int i;
> +
> tkip_mixing_phase1(ta, &key->key[ALG_TKIP_TEMP_ENCR_KEY],
> - key->u.tkip.iv32, phase1key);
> + key->u.tkip.iv32, (u16 *)k);
> + for (i = 0; i < 5; i++)
> + k[i] = cpu_to_le16(k[i]);
> }
Maybe a slightly better type checking but still looks ugly:
@@ -192,10 +192,16 @@ u8 * ieee80211_tkip_add_iv(u8 *pos, stru
void ieee80211_tkip_gen_phase1key(struct ieee80211_key *key, u8 *ta,
- u16 *phase1key)
+ u8 *phase1key)
{
+ u16 *tmp_result = (u16 *)phase1key;
+ __le16 *k = (__le16 *)phase1key;
+ int i;
+
tkip_mixing_phase1(ta, &key->key[ALG_TKIP_TEMP_ENCR_KEY],
- key->u.tkip.iv32, phase1key);
+ key->u.tkip.iv32, tmp_result);
+ for (i = 0; i < 5; i++)
+ k[i] = cpu_to_le16(tmp_result[i]);
}
void ieee80211_tkip_gen_rc4key(struct ieee80211_key *key, u8 *ta,
Moreover, I'm not sure if GCC is able to optimize out the for loop in
this case :-(
Jiri
--
Jiri Benc
SUSE Labs
On Thu, 8 Feb 2007 18:15:27 +0100, Michael Buesch wrote:
> On Tuesday 06 February 2007 16:33, Michael Wu wrote:
> > d80211: Fix TKIP key type
> >
> > Avoid the messy typecasting and let drivers handle byteordering.
> >
> > Signed-off-by: Michael Wu <[email protected]>
>
> Jiri, can you avoid pulling this patch, yet?
> I am trying to get a better solution for the whole TKIP stuff
> that actually works with bcm43xx. My patch will fix this issue then, too.
I wasn't going to apply it anyway as it doesn't seem to solve the
problem - it just shifts it to someone else.
Jiri
--
Jiri Benc
SUSE Labs
On Tue, 6 Feb 2007 16:02:45 +0100, Michael Buesch wrote:
> On Monday 05 February 2007 22:19, Jiri Benc wrote:
> > On Sat, 3 Feb 2007 18:32:48 +0100, Michael Buesch wrote:
> > > @@ -192,10 +192,15 @@ u8 * ieee80211_tkip_add_iv(u8 *pos, stru
> > >
> > >
> > > void ieee80211_tkip_gen_phase1key(struct ieee80211_key *key, u8 *ta,
> > > - u16 *phase1key)
> > > + u8 *phase1key)
> > > {
> > > + __le16 *k = (__le16 *)phase1key;
> > > + int i;
> > > +
> > > tkip_mixing_phase1(ta, &key->key[ALG_TKIP_TEMP_ENCR_KEY],
> > > - key->u.tkip.iv32, phase1key);
> > > + key->u.tkip.iv32, (u16 *)k);
> > > + for (i = 0; i < 5; i++)
> > > + k[i] = cpu_to_le16(k[i]);
> > > }
> >
> > Maybe a slightly better type checking but still looks ugly:
>
> Hm, well. I don't really see how typechecking is better in this case,
cpu_to_le16 with a __le16 variable as a parameter always looks
suspicious. In the version I sent it is at least clear that it's
intended.
> but if you like it more, I'm ok with it. ;)
I dislike both of them but have no better idea.
> Yeah, I was going to check this and was going to add #ifdefs if it doesn't.
> But that was not my major concern at this point.
> It was more that people agree to me that it _is_ broken on BE platforms.
Looks like it is.
Thanks,
Jiri
--
Jiri Benc
SUSE Labs
d80211: Fix TKIP key type
Avoid the messy typecasting and let drivers handle byteordering.
Signed-off-by: Michael Wu <[email protected]>
diff --git a/include/net/d80211.h b/include/net/d80211.h
index 65a5d36..0bd6b15 100644
--- a/include/net/d80211.h
+++ b/include/net/d80211.h
@@ -202,7 +202,10 @@ struct ieee80211_tx_control {
* hw->set_key() */
u8 icv_len; /* length of the ICV/MIC field in octets */
u8 iv_len; /* length of the IV field in octets */
- u8 tkip_key[16]; /* generated phase2/phase1 key for hw TKIP */
+ union {
+ u16 phase1[5];
+ u8 phase2[16];
+ } tkip; /* generated phase2/phase1 key for hw TKIP */
u8 queue; /* hardware queue to use for this frame;
* 0 = highest, hw->queues-1 = lowest */
u8 sw_retry_attempt; /* number of times hw has tried to
diff --git a/net/d80211/ieee80211_i.h b/net/d80211/ieee80211_i.h
diff --git a/net/d80211/ieee80211_ioctl.c b/net/d80211/ieee80211_ioctl.c
diff --git a/net/d80211/ieee80211_sta.c b/net/d80211/ieee80211_sta.c
diff --git a/net/d80211/wpa.c b/net/d80211/wpa.c
index 7b64abf..d5ef61a 100644
--- a/net/d80211/wpa.c
+++ b/net/d80211/wpa.c
@@ -344,12 +344,12 @@ skip_iv_inc:
if (flags & IEEE80211_HW_TKIP_REQ_PHASE2_KEY)
ieee80211_tkip_gen_rc4key(key, hdr->addr2,
- tx->u.tx.control->tkip_key);
+ tx->u.tx.control->tkip.phase2);
else if (flags & IEEE80211_HW_TKIP_REQ_PHASE1_KEY) {
if (key->u.tkip.iv16 == 0 ||
!key->u.tkip.tx_initialized) {
ieee80211_tkip_gen_phase1key(key, hdr->addr2,
- (u16 *)tx->u.tx.control->tkip_key);
+ tx->u.tx.control->tkip.phase1);
key->u.tkip.tx_initialized = 1;
tx->u.tx.control->flags |=
IEEE80211_TXCTL_TKIP_NEW_PHASE1_KEY;
On Tue, 2007-02-06 at 16:44 +0100, Michael Buesch wrote:
> So I think we need a callback or something that sets the key in HW.
> Maybe we can do that in the already existing set_key callback.
Yes, your original patch with the library functions should be fine, if
the driver needs it it can request it either in set_key or during tx (if
the TSC cycled or whatever)
johannes
On Tuesday 06 February 2007 16:33, Michael Wu wrote:
> d80211: Fix TKIP key type
>
> Avoid the messy typecasting and let drivers handle byteordering.
>
> Signed-off-by: Michael Wu <[email protected]>
Jiri, can you avoid pulling this patch, yet?
I am trying to get a better solution for the whole TKIP stuff
that actually works with bcm43xx. My patch will fix this issue then, too.
> diff --git a/include/net/d80211.h b/include/net/d80211.h
> index 65a5d36..0bd6b15 100644
> --- a/include/net/d80211.h
> +++ b/include/net/d80211.h
> @@ -202,7 +202,10 @@ struct ieee80211_tx_control {
> * hw->set_key() */
> u8 icv_len; /* length of the ICV/MIC field in octets */
> u8 iv_len; /* length of the IV field in octets */
> - u8 tkip_key[16]; /* generated phase2/phase1 key for hw TKIP */
> + union {
> + u16 phase1[5];
> + u8 phase2[16];
> + } tkip; /* generated phase2/phase1 key for hw TKIP */
> u8 queue; /* hardware queue to use for this frame;
> * 0 = highest, hw->queues-1 = lowest */
> u8 sw_retry_attempt; /* number of times hw has tried to
> diff --git a/net/d80211/ieee80211_i.h b/net/d80211/ieee80211_i.h
> diff --git a/net/d80211/ieee80211_ioctl.c b/net/d80211/ieee80211_ioctl.c
> diff --git a/net/d80211/ieee80211_sta.c b/net/d80211/ieee80211_sta.c
> diff --git a/net/d80211/wpa.c b/net/d80211/wpa.c
> index 7b64abf..d5ef61a 100644
> --- a/net/d80211/wpa.c
> +++ b/net/d80211/wpa.c
> @@ -344,12 +344,12 @@ skip_iv_inc:
>
> if (flags & IEEE80211_HW_TKIP_REQ_PHASE2_KEY)
> ieee80211_tkip_gen_rc4key(key, hdr->addr2,
> - tx->u.tx.control->tkip_key);
> + tx->u.tx.control->tkip.phase2);
> else if (flags & IEEE80211_HW_TKIP_REQ_PHASE1_KEY) {
> if (key->u.tkip.iv16 == 0 ||
> !key->u.tkip.tx_initialized) {
> ieee80211_tkip_gen_phase1key(key, hdr->addr2,
> - (u16 *)tx->u.tx.control->tkip_key);
> + tx->u.tx.control->tkip.phase1);
> key->u.tkip.tx_initialized = 1;
> tx->u.tx.control->flags |=
> IEEE80211_TXCTL_TKIP_NEW_PHASE1_KEY;
>
--
Greetings Michael.
On Tuesday 06 February 2007 16:33, Michael Wu wrote:
> d80211: Fix TKIP key type
>
> Avoid the messy typecasting and let drivers handle byteordering.
Well, ok.
We need to remove all the tkip stuff from tx_control anyway,
as it's broken. It does not work to pass tkip keys along with
TX packets, as we also need the key for RX (on bcm43xx at least).
So I think we need a callback or something that sets the key in HW.
Maybe we can do that in the already existing set_key callback.
Dunno yet.
> Signed-off-by: Michael Wu <[email protected]>
>
> diff --git a/include/net/d80211.h b/include/net/d80211.h
> index 65a5d36..0bd6b15 100644
> --- a/include/net/d80211.h
> +++ b/include/net/d80211.h
> @@ -202,7 +202,10 @@ struct ieee80211_tx_control {
> * hw->set_key() */
> u8 icv_len; /* length of the ICV/MIC field in octets */
> u8 iv_len; /* length of the IV field in octets */
> - u8 tkip_key[16]; /* generated phase2/phase1 key for hw TKIP */
> + union {
> + u16 phase1[5];
> + u8 phase2[16];
> + } tkip; /* generated phase2/phase1 key for hw TKIP */
> u8 queue; /* hardware queue to use for this frame;
> * 0 = highest, hw->queues-1 = lowest */
> u8 sw_retry_attempt; /* number of times hw has tried to
> diff --git a/net/d80211/ieee80211_i.h b/net/d80211/ieee80211_i.h
> diff --git a/net/d80211/ieee80211_ioctl.c b/net/d80211/ieee80211_ioctl.c
> diff --git a/net/d80211/ieee80211_sta.c b/net/d80211/ieee80211_sta.c
> diff --git a/net/d80211/wpa.c b/net/d80211/wpa.c
> index 7b64abf..d5ef61a 100644
> --- a/net/d80211/wpa.c
> +++ b/net/d80211/wpa.c
> @@ -344,12 +344,12 @@ skip_iv_inc:
>
> if (flags & IEEE80211_HW_TKIP_REQ_PHASE2_KEY)
> ieee80211_tkip_gen_rc4key(key, hdr->addr2,
> - tx->u.tx.control->tkip_key);
> + tx->u.tx.control->tkip.phase2);
> else if (flags & IEEE80211_HW_TKIP_REQ_PHASE1_KEY) {
> if (key->u.tkip.iv16 == 0 ||
> !key->u.tkip.tx_initialized) {
> ieee80211_tkip_gen_phase1key(key, hdr->addr2,
> - (u16 *)tx->u.tx.control->tkip_key);
> + tx->u.tx.control->tkip.phase1);
> key->u.tkip.tx_initialized = 1;
> tx->u.tx.control->flags |=
> IEEE80211_TXCTL_TKIP_NEW_PHASE1_KEY;
>
--
Greetings Michael.