All of lore.kernel.org
 help / color / mirror / Atom feed
* d80211: current TKIP hwcrypto implementation seems to be broken
@ 2007-02-04 12:44 Michael Buesch
  2007-02-05 20:43 ` Jiri Benc
  2007-02-12 18:30 ` Jouni Malinen
  0 siblings, 2 replies; 23+ messages in thread
From: Michael Buesch @ 2007-02-04 12:44 UTC (permalink / raw)
  To: Jiri Benc; +Cc: linux-wireless

Current TKIP hwcrypto phase1+2 key support in d80211
seems to be broken. (At least for bcm43xx).

Let me explain what happens:
We need the phase1 for bcm43xx. We need to upload it to
card memory and we need to pass it on every TX on the DMA.

So, currently we receive the phase1 key on the first
encrypted TX. That's too late, as we already receive encrypted
packets before that. bcm43xx needs the phase1 key (and the iv32)
on RX. It uses the one uploaded into the card memory. But it
is not uploaded, yet, as we did not TX any encrypted packet.

I'd say the only solution to this is to implement the earlier
suggested way of having a library function call to generate
the keys. Of course, that needs some bookkeeping about the IVs
and stuff.
That library function would be called by bcm43xx before any
traffic to get an initial phase1 key (and iv32) uploaded.

I currently don't really know how to implement this. Any suggestion?

As a sideeffect, it would shrink the txcontrol by 16 bytes, again.

-- 
Greetings Michael.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: d80211: current TKIP hwcrypto implementation seems to be broken
  2007-02-04 12:44 d80211: current TKIP hwcrypto implementation seems to be broken Michael Buesch
@ 2007-02-05 20:43 ` Jiri Benc
  2007-02-05 21:38   ` Johannes Berg
  2007-02-06 14:55   ` Michael Buesch
  2007-02-12 18:30 ` Jouni Malinen
  1 sibling, 2 replies; 23+ messages in thread
From: Jiri Benc @ 2007-02-05 20:43 UTC (permalink / raw)
  To: Michael Buesch; +Cc: linux-wireless

On Sun, 4 Feb 2007 13:44:18 +0100, Michael Buesch wrote:
> We need the phase1 for bcm43xx. We need to upload it to
> card memory and we need to pass it on every TX on the DMA.
> 
> So, currently we receive the phase1 key on the first
> encrypted TX. That's too late, as we already receive encrypted
> packets before that. bcm43xx needs the phase1 key (and the iv32)
> on RX. It uses the one uploaded into the card memory. But it
> is not uploaded, yet, as we did not TX any encrypted packet.
> 
> I'd say the only solution to this is to implement the earlier
> suggested way of having a library function call to generate
> the keys. Of course, that needs some bookkeeping about the IVs
> and stuff.
> That library function would be called by bcm43xx before any
> traffic to get an initial phase1 key (and iv32) uploaded.

What if the key is changed by the user space? You won't know that and
won't call the library function then. I think it needs to be designed
in the opposite way - the stack will call a driver's callback.

Or am I missing something?

 Jiri

-- 
Jiri Benc
SUSE Labs

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: d80211: current TKIP hwcrypto implementation seems to be broken
  2007-02-05 20:43 ` Jiri Benc
@ 2007-02-05 21:38   ` Johannes Berg
  2007-02-05 21:45     ` Johannes Berg
  2007-02-06 14:55   ` Michael Buesch
  1 sibling, 1 reply; 23+ messages in thread
From: Johannes Berg @ 2007-02-05 21:38 UTC (permalink / raw)
  To: Jiri Benc; +Cc: Michael Buesch, linux-wireless

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

On Mon, 2007-02-05 at 21:43 +0100, Jiri Benc wrote:

> What if the key is changed by the user space? You won't know that and
> won't call the library function then. I think it needs to be designed
> in the opposite way - the stack will call a driver's callback.
> 
> Or am I missing something?

Wouldn't you have to reset the TSC to zero in that case?

johannes

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: d80211: current TKIP hwcrypto implementation seems to be broken
  2007-02-05 21:38   ` Johannes Berg
@ 2007-02-05 21:45     ` Johannes Berg
  2007-02-05 21:48       ` Johannes Berg
  0 siblings, 1 reply; 23+ messages in thread
From: Johannes Berg @ 2007-02-05 21:45 UTC (permalink / raw)
  To: Jiri Benc; +Cc: Michael Buesch, linux-wireless

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

On Mon, 2007-02-05 at 22:38 +0100, Johannes Berg wrote:
> On Mon, 2007-02-05 at 21:43 +0100, Jiri Benc wrote:
> 
> > What if the key is changed by the user space? You won't know that and
> > won't call the library function then. I think it needs to be designed
> > in the opposite way - the stack will call a driver's callback.
> > 
> > Or am I missing something?
> 
> Wouldn't you have to reset the TSC to zero in that case?

Ah, but the driver will need to know for the RX path. Sorry about the
confusion.

johannes

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: d80211: current TKIP hwcrypto implementation seems to be broken
  2007-02-05 21:45     ` Johannes Berg
@ 2007-02-05 21:48       ` Johannes Berg
  0 siblings, 0 replies; 23+ messages in thread
From: Johannes Berg @ 2007-02-05 21:48 UTC (permalink / raw)
  To: Jiri Benc; +Cc: Michael Buesch, linux-wireless

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

On Mon, 2007-02-05 at 22:45 +0100, Johannes Berg wrote:
> On Mon, 2007-02-05 at 22:38 +0100, Johannes Berg wrote:
> > On Mon, 2007-02-05 at 21:43 +0100, Jiri Benc wrote:
> > 
> > > What if the key is changed by the user space? You won't know that and
> > > won't call the library function then. I think it needs to be designed
> > > in the opposite way - the stack will call a driver's callback.
> > > 
> > > Or am I missing something?
> > 
> > Wouldn't you have to reset the TSC to zero in that case?
> 
> Ah, but the driver will need to know for the RX path. Sorry about the
> confusion.

And of course, it is told by way of the set_key callback.. we're talking
hw crypto after all.

johannes

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: d80211: current TKIP hwcrypto implementation seems to be broken
  2007-02-05 20:43 ` Jiri Benc
  2007-02-05 21:38   ` Johannes Berg
@ 2007-02-06 14:55   ` Michael Buesch
  1 sibling, 0 replies; 23+ messages in thread
From: Michael Buesch @ 2007-02-06 14:55 UTC (permalink / raw)
  To: Jiri Benc; +Cc: linux-wireless

On Monday 05 February 2007 21:43, Jiri Benc wrote:
> On Sun, 4 Feb 2007 13:44:18 +0100, Michael Buesch wrote:
> > We need the phase1 for bcm43xx. We need to upload it to
> > card memory and we need to pass it on every TX on the DMA.
> > 
> > So, currently we receive the phase1 key on the first
> > encrypted TX. That's too late, as we already receive encrypted
> > packets before that. bcm43xx needs the phase1 key (and the iv32)
> > on RX. It uses the one uploaded into the card memory. But it
> > is not uploaded, yet, as we did not TX any encrypted packet.
> > 
> > I'd say the only solution to this is to implement the earlier
> > suggested way of having a library function call to generate
> > the keys. Of course, that needs some bookkeeping about the IVs
> > and stuff.
> > That library function would be called by bcm43xx before any
> > traffic to get an initial phase1 key (and iv32) uploaded.
> 
> What if the key is changed by the user space? You won't know that and
> won't call the library function then. I think it needs to be designed
> in the opposite way - the stack will call a driver's callback.
> 
> Or am I missing something?

Yeah, peobably. I don't really know in detail how
tkip works. I just know that current implementation in
d80211 does not work, as I tried it. ;)

I need to read the specs about it first.

-- 
Greetings Michael.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: d80211: current TKIP hwcrypto implementation seems to be broken
  2007-02-04 12:44 d80211: current TKIP hwcrypto implementation seems to be broken Michael Buesch
  2007-02-05 20:43 ` Jiri Benc
@ 2007-02-12 18:30 ` Jouni Malinen
  2007-02-12 21:39   ` Michael Buesch
  2007-02-13 16:18   ` Johannes Berg
  1 sibling, 2 replies; 23+ messages in thread
From: Jouni Malinen @ 2007-02-12 18:30 UTC (permalink / raw)
  To: Michael Buesch; +Cc: Jiri Benc, linux-wireless

On Sun, Feb 04, 2007 at 01:44:18PM +0100, Michael Buesch wrote:

> We need the phase1 for bcm43xx. We need to upload it to
> card memory and we need to pass it on every TX on the DMA.

> So, currently we receive the phase1 key on the first
> encrypted TX. That's too late, as we already receive encrypted
> packets before that. bcm43xx needs the phase1 key (and the iv32)
> on RX. It uses the one uploaded into the card memory. But it
> is not uploaded, yet, as we did not TX any encrypted packet.

How exactly is this supposed to work for RX? Phase1 needs to be
calculated after 65536 frames (whenever iv32 changes), but the
exact time for RX case is unclear due to possible loss of frames and
retransmissions etc. Is bcm43xx just going drop couple of packets
whenever the phase1 value is changed? Or is there some kind of
mechanism for the hardware/firmware request a new phase1 value? Or is
this supposed to be recovered from in software (which is something that
d80211 is able to do if the radio driver notifies that the frame was not
decrypted)?

-- 
Jouni Malinen                                            PGP id EFC895FA

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: d80211: current TKIP hwcrypto implementation seems to be broken
  2007-02-12 18:30 ` Jouni Malinen
@ 2007-02-12 21:39   ` Michael Buesch
  2007-02-12 23:15     ` Tomas Winkler
  2007-02-12 23:53     ` Jouni Malinen
  2007-02-13 16:18   ` Johannes Berg
  1 sibling, 2 replies; 23+ messages in thread
From: Michael Buesch @ 2007-02-12 21:39 UTC (permalink / raw)
  To: Jouni Malinen; +Cc: Jiri Benc, linux-wireless

On Monday 12 February 2007 19:30, Jouni Malinen wrote:
> On Sun, Feb 04, 2007 at 01:44:18PM +0100, Michael Buesch wrote:
> 
> > We need the phase1 for bcm43xx. We need to upload it to
> > card memory and we need to pass it on every TX on the DMA.
> 
> > So, currently we receive the phase1 key on the first
> > encrypted TX. That's too late, as we already receive encrypted
> > packets before that. bcm43xx needs the phase1 key (and the iv32)
> > on RX. It uses the one uploaded into the card memory. But it
> > is not uploaded, yet, as we did not TX any encrypted packet.
> 
> How exactly is this supposed to work for RX?

I misunderstood it, so my explaination was not 100% correct.

It works the following way: We need the phase1 for TX and for RX.
The key for TX is passed along with the TX frame. We can take the
key from the tx_control (I'd still like to get rid of the
array in the tx_control, though. Working on it...)
For RX we have the phase1 uploaded to some device memory.

What happens on RX when the iv32 wraps. Well, it simply won't decrypt
the frame. What we must do then is upload a new key as fast as possible.
The undecrypted frame can be either decrypted in SW (which I think d80211
can do. At least with minor additional changes), or we drop it.

I started to implement an API to calculate the phase1 key on demand.
It doesn't quite work, yet and I'm stuck in more important work
that should be done before the d80211 merge, so it will take some time
until I can continue debugging the stuff.

-- 
Greetings Michael.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: d80211: current TKIP hwcrypto implementation seems to be broken
  2007-02-12 21:39   ` Michael Buesch
@ 2007-02-12 23:15     ` Tomas Winkler
  2007-02-12 23:23       ` Michael Buesch
  2007-02-12 23:53     ` Jouni Malinen
  1 sibling, 1 reply; 23+ messages in thread
From: Tomas Winkler @ 2007-02-12 23:15 UTC (permalink / raw)
  To: Michael Buesch; +Cc: Jouni Malinen, Jiri Benc, linux-wireless

I fetching this from my "dusty" memory so I hope I'm right about it.
TKIP phase1 one can be precomputed in advance. There's no runtime
dependency after key exchange.
Cached keys can be provided by supplicant. Usually u want to keep 2 or
3 cached TKIP phase1 for smooth decryption. As in QoS sequence
counters are kept per AC, you might get into gentle state of receiving
packets for the old key. If  you hold more then one  phase1 you might
be able decrypt the packets that doesn't match current phase1 in
software.


On 2/12/07, Michael Buesch <mb@bu3sch.de> wrote:
> On Monday 12 February 2007 19:30, Jouni Malinen wrote:
> > On Sun, Feb 04, 2007 at 01:44:18PM +0100, Michael Buesch wrote:
> >
> > > We need the phase1 for bcm43xx. We need to upload it to
> > > card memory and we need to pass it on every TX on the DMA.
> >
> > > So, currently we receive the phase1 key on the first
> > > encrypted TX. That's too late, as we already receive encrypted
> > > packets before that. bcm43xx needs the phase1 key (and the iv32)
> > > on RX. It uses the one uploaded into the card memory. But it
> > > is not uploaded, yet, as we did not TX any encrypted packet.
> >
> > How exactly is this supposed to work for RX?
>
> I misunderstood it, so my explaination was not 100% correct.
>
> It works the following way: We need the phase1 for TX and for RX.
> The key for TX is passed along with the TX frame. We can take the
> key from the tx_control (I'd still like to get rid of the
> array in the tx_control, though. Working on it...)
> For RX we have the phase1 uploaded to some device memory.
>
> What happens on RX when the iv32 wraps. Well, it simply won't decrypt
> the frame. What we must do then is upload a new key as fast as possible.
> The undecrypted frame can be either decrypted in SW (which I think d80211
> can do. At least with minor additional changes), or we drop it.
>
> I started to implement an API to calculate the phase1 key on demand.
> It doesn't quite work, yet and I'm stuck in more important work
> that should be done before the d80211 merge, so it will take some time
> until I can continue debugging the stuff.
>
> --
> Greetings Michael.
> -
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: d80211: current TKIP hwcrypto implementation seems to be broken
  2007-02-12 23:15     ` Tomas Winkler
@ 2007-02-12 23:23       ` Michael Buesch
  2007-02-12 23:54         ` Tomas Winkler
  0 siblings, 1 reply; 23+ messages in thread
From: Michael Buesch @ 2007-02-12 23:23 UTC (permalink / raw)
  To: Tomas Winkler; +Cc: Jouni Malinen, Jiri Benc, linux-wireless

On Tuesday 13 February 2007 00:15, Tomas Winkler wrote:
> I fetching this from my "dusty" memory so I hope I'm right about it.
> TKIP phase1 one can be precomputed in advance. There's no runtime

Phase1 key changes on iv32 wrap.

> dependency after key exchange.
> Cached keys can be provided by supplicant. Usually u want to keep 2 or
> 3 cached TKIP phase1 for smooth decryption. As in QoS sequence

Hardware (bcm43xx at least) can only have one key at a time.
There's no point in caching, as phase1 key calculation is cheap
and can be done on-the-run. The expensive part it uploading it to HW.

> counters are kept per AC, you might get into gentle state of receiving
> packets for the old key. If  you hold more then one  phase1 you might
> be able decrypt the packets that doesn't match current phase1 in
> software.

You can generate the new key then. There is no point in wasting memory
by precomputation of the keys.

-- 
Greetings Michael.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: d80211: current TKIP hwcrypto implementation seems to be broken
  2007-02-12 21:39   ` Michael Buesch
  2007-02-12 23:15     ` Tomas Winkler
@ 2007-02-12 23:53     ` Jouni Malinen
  2007-02-13  0:06       ` Michael Buesch
  1 sibling, 1 reply; 23+ messages in thread
From: Jouni Malinen @ 2007-02-12 23:53 UTC (permalink / raw)
  To: Michael Buesch; +Cc: Jiri Benc, linux-wireless

On Mon, Feb 12, 2007 at 10:39:31PM +0100, Michael Buesch wrote:

> What happens on RX when the iv32 wraps. Well, it simply won't decrypt
> the frame. What we must do then is upload a new key as fast as possible.
> The undecrypted frame can be either decrypted in SW (which I think d80211
> can do. At least with minor additional changes), or we drop it.

That "iv32 wrap" should have been "iv32 changes" (phase1 key changes
whenever iv16 changes, i.e., when iv32 is incremented by one). d80211
does indeed support software decryption for this case, so it is fine to
just pass the undecrypted frame up and marking it as such. This was
needed for some of the TKIP cases with Atheros.

> I started to implement an API to calculate the phase1 key on demand.
> It doesn't quite work, yet and I'm stuck in more important work
> that should be done before the d80211 merge, so it will take some time
> until I can continue debugging the stuff.

OK.

-- 
Jouni Malinen                                            PGP id EFC895FA

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: d80211: current TKIP hwcrypto implementation seems to be broken
  2007-02-12 23:23       ` Michael Buesch
@ 2007-02-12 23:54         ` Tomas Winkler
  2007-02-13  0:10           ` Michael Buesch
  0 siblings, 1 reply; 23+ messages in thread
From: Tomas Winkler @ 2007-02-12 23:54 UTC (permalink / raw)
  To: Michael Buesch; +Cc: Jouni Malinen, Jiri Benc, linux-wireless

On 2/13/07, Michael Buesch <mb@bu3sch.de> wrote:
> On Tuesday 13 February 2007 00:15, Tomas Winkler wrote:
> > I fetching this from my "dusty" memory so I hope I'm right about it.
> > TKIP phase1 one can be precomputed in advance. There's no runtime
>
> Phase1 key changes on iv32 wrap.

That's correct, this depends on packet sequence counter,  but you can
compute next TKIP phase1 even before iv32 wrap has occurred. You can
compute 100 of them in advance.

> > dependency after key exchange.
> > Cached keys can be provided by supplicant. Usually u want to keep 2 or
> > 3 cached TKIP phase1 for smooth decryption. As in QoS sequence
>
> Hardware (bcm43xx at least) can only have one key at a time.
> There's no point in caching, as phase1 key calculation is cheap
> and can be done on-the-run. The expensive part it uploading it to HW.

True, but when there is a packet from past with tkip phase1 that
doesn't match currently configured phase1 tkip you might be able to
decrypt it, of course f hardware doesn't scramble it with wrong key.

> > counters are kept per AC, you might get into gentle state of receiving
> > packets for the old key. If  you hold more then one  phase1 you might
> > be able decrypt the packets that doesn't match current phase1 in
> > software.
>
> You can generate the new key then. There is no point in wasting memory
> by precomputation of the keys.

These few bytes doesn't hurt you even if you're running on a cell phone.

> --
> Greetings Michael.
>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: d80211: current TKIP hwcrypto implementation seems to be broken
  2007-02-12 23:53     ` Jouni Malinen
@ 2007-02-13  0:06       ` Michael Buesch
  2007-02-13  0:16         ` Michael Buesch
  0 siblings, 1 reply; 23+ messages in thread
From: Michael Buesch @ 2007-02-13  0:06 UTC (permalink / raw)
  To: Jouni Malinen; +Cc: Jiri Benc, linux-wireless

On Tuesday 13 February 2007 00:53, Jouni Malinen wrote:
> On Mon, Feb 12, 2007 at 10:39:31PM +0100, Michael Buesch wrote:
> 
> > What happens on RX when the iv32 wraps. Well, it simply won't decrypt
> > the frame. What we must do then is upload a new key as fast as possible.
> > The undecrypted frame can be either decrypted in SW (which I think d80211
> > can do. At least with minor additional changes), or we drop it.
> 
> That "iv32 wrap" should have been "iv32 changes" (phase1 key changes

Yep, I'm sorry. It should be iv32, of course.

> whenever iv16 changes, i.e., when iv32 is incremented by one). d80211
> does indeed support software decryption for this case, so it is fine to
> just pass the undecrypted frame up and marking it as such. This was
> needed for some of the TKIP cases with Atheros.

That's really cool.

-- 
Greetings Michael.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: d80211: current TKIP hwcrypto implementation seems to be broken
  2007-02-12 23:54         ` Tomas Winkler
@ 2007-02-13  0:10           ` Michael Buesch
  2007-02-13  0:19             ` Tomas Winkler
  0 siblings, 1 reply; 23+ messages in thread
From: Michael Buesch @ 2007-02-13  0:10 UTC (permalink / raw)
  To: Tomas Winkler; +Cc: Jouni Malinen, Jiri Benc, linux-wireless

On Tuesday 13 February 2007 00:54, Tomas Winkler wrote:
> On 2/13/07, Michael Buesch <mb@bu3sch.de> wrote:
> > On Tuesday 13 February 2007 00:15, Tomas Winkler wrote:
> > > I fetching this from my "dusty" memory so I hope I'm right about it.
> > > TKIP phase1 one can be precomputed in advance. There's no runtime
> >
> > Phase1 key changes on iv32 wrap.
> 
> That's correct, this depends on packet sequence counter,  but you can
> compute next TKIP phase1 even before iv32 wrap has occurred. You can
> compute 100 of them in advance.

Actually it should be "iv32 changes" instead of "iv32 wraps". Typo.
But my point still remains. I see no point in precomputing it.

> > > dependency after key exchange.
> > > Cached keys can be provided by supplicant. Usually u want to keep 2 or
> > > 3 cached TKIP phase1 for smooth decryption. As in QoS sequence
> >
> > Hardware (bcm43xx at least) can only have one key at a time.
> > There's no point in caching, as phase1 key calculation is cheap
> > and can be done on-the-run. The expensive part it uploading it to HW.
> 
> True, but when there is a packet from past with tkip phase1 that
> doesn't match currently configured phase1 tkip you might be able to
> decrypt it, of course f hardware doesn't scramble it with wrong key.

HW doesn't scramble it. So when d80211 receives the packet it can
_then_ generate the key. There's no point in precomputing it.

> > > counters are kept per AC, you might get into gentle state of receiving
> > > packets for the old key. If  you hold more then one  phase1 you might
> > > be able decrypt the packets that doesn't match current phase1 in
> > > software.
> >
> > You can generate the new key then. There is no point in wasting memory
> > by precomputation of the keys.
> 
> These few bytes doesn't hurt you even if you're running on a cell phone.

But they don't gain you anything too, IMO.
So better not waste the memory and not have the extra complexity in code.

-- 
Greetings Michael.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: d80211: current TKIP hwcrypto implementation seems to be broken
  2007-02-13  0:06       ` Michael Buesch
@ 2007-02-13  0:16         ` Michael Buesch
  0 siblings, 0 replies; 23+ messages in thread
From: Michael Buesch @ 2007-02-13  0:16 UTC (permalink / raw)
  To: Jouni Malinen; +Cc: Jiri Benc, linux-wireless

On Tuesday 13 February 2007 01:06, Michael Buesch wrote:
> On Tuesday 13 February 2007 00:53, Jouni Malinen wrote:
> > On Mon, Feb 12, 2007 at 10:39:31PM +0100, Michael Buesch wrote:
> > 
> > > What happens on RX when the iv32 wraps. Well, it simply won't decrypt
> > > the frame. What we must do then is upload a new key as fast as possible.
> > > The undecrypted frame can be either decrypted in SW (which I think d80211
> > > can do. At least with minor additional changes), or we drop it.
> > 
> > That "iv32 wrap" should have been "iv32 changes" (phase1 key changes
> 
> Yep, I'm sorry. It should be iv32, of course.

Ah, crap. I should really go to bed. ;)

-- 
Greetings Michael.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: d80211: current TKIP hwcrypto implementation seems to be broken
  2007-02-13  0:10           ` Michael Buesch
@ 2007-02-13  0:19             ` Tomas Winkler
  2007-02-13  0:28               ` Jouni Malinen
  2007-02-13  0:33               ` Michael Buesch
  0 siblings, 2 replies; 23+ messages in thread
From: Tomas Winkler @ 2007-02-13  0:19 UTC (permalink / raw)
  To: Michael Buesch; +Cc: Jouni Malinen, Jiri Benc, linux-wireless

On 2/13/07, Michael Buesch <mb@bu3sch.de> wrote:
> On Tuesday 13 February 2007 00:54, Tomas Winkler wrote:
> > On 2/13/07, Michael Buesch <mb@bu3sch.de> wrote:
> > > On Tuesday 13 February 2007 00:15, Tomas Winkler wrote:
> > > > I fetching this from my "dusty" memory so I hope I'm right about it.
> > > > TKIP phase1 one can be precomputed in advance. There's no runtime
> > >
> > > Phase1 key changes on iv32 wrap.
> >
> > That's correct, this depends on packet sequence counter,  but you can
> > compute next TKIP phase1 even before iv32 wrap has occurred. You can
> > compute 100 of them in advance.
>
> Actually it should be "iv32 changes" instead of "iv32 wraps". Typo.
> But my point still remains. I see no point in precomputing it.
>
> > > > dependency after key exchange.
> > > > Cached keys can be provided by supplicant. Usually u want to keep 2 or
> > > > 3 cached TKIP phase1 for smooth decryption. As in QoS sequence
> > >
> > > Hardware (bcm43xx at least) can only have one key at a time.
> > > There's no point in caching, as phase1 key calculation is cheap
> > > and can be done on-the-run. The expensive part it uploading it to HW.
> >
> > True, but when there is a packet from past with tkip phase1 that
> > doesn't match currently configured phase1 tkip you might be able to
> > decrypt it, of course f hardware doesn't scramble it with wrong key.
>
> HW doesn't scramble it. So when d80211 receives the packet it can
> _then_ generate the key. There's no point in precomputing it.

For past packets you've already computed it so why do  it again? So
you keep 2 phas1 keys one old and one current. When sequence numbers
advance sufficiently you discard the old one and compute the new one
in a spare time. Something like double buffer.

> > > > counters are kept per AC, you might get into gentle state of receiving
> > > > packets for the old key. If  you hold more then one  phase1 you might
> > > > be able decrypt the packets that doesn't match current phase1 in
> > > > software.
> > >
> > > You can generate the new key then. There is no point in wasting memory
> > > by precomputation of the keys.
> >
> > These few bytes doesn't hurt you even if you're running on a cell phone.
>
> But they don't gain you anything too, IMO.
> So better not waste the memory and not have the extra complexity in code.
>
I didn't measured performance but I thought  it can be precomputed by
the supplicant rather then by kernel code.    I might be wrong.
> --
> Greetings Michael.
>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: d80211: current TKIP hwcrypto implementation seems to be broken
  2007-02-13  0:19             ` Tomas Winkler
@ 2007-02-13  0:28               ` Jouni Malinen
  2007-02-13  1:08                 ` Tomas Winkler
  2007-02-13  0:33               ` Michael Buesch
  1 sibling, 1 reply; 23+ messages in thread
From: Jouni Malinen @ 2007-02-13  0:28 UTC (permalink / raw)
  To: Tomas Winkler; +Cc: Michael Buesch, Jiri Benc, linux-wireless

On Tue, Feb 13, 2007 at 02:19:15AM +0200, Tomas Winkler wrote:

> For past packets you've already computed it so why do  it again? So
> you keep 2 phas1 keys one old and one current. When sequence numbers
> advance sufficiently you discard the old one and compute the new one
> in a spare time. Something like double buffer.

Well, you can, but how likely is this case to really happen? If someone
can show this happening frequently, I could consider adding support for
it, but I see not much point in adding the extra complexity without a
clear case where this would be of benefit. P1K is not really that slow
after all and figuring out when the kernel code has "spare time" and all
the extra handling of pre-calculation and storing the values is not
really something I would like to add into d80211 unless this can really
be shown to provide noticeable throughput improvement.

> I didn't measured performance but I thought  it can be precomputed by
> the supplicant rather then by kernel code.    I might be wrong.

P1K is calculated by whoever is using the TKIP TK and in case of all the
implementations that I'm aware of, this is not done in the supplicant.
Sure, it could be done there, but I think the current design of doing
all TKIP encryption/decryption (including phase 1 and 2 key derivation)
in the kernel is better.

-- 
Jouni Malinen                                            PGP id EFC895FA

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: d80211: current TKIP hwcrypto implementation seems to be broken
  2007-02-13  0:19             ` Tomas Winkler
  2007-02-13  0:28               ` Jouni Malinen
@ 2007-02-13  0:33               ` Michael Buesch
  2007-02-13  1:09                 ` Tomas Winkler
  1 sibling, 1 reply; 23+ messages in thread
From: Michael Buesch @ 2007-02-13  0:33 UTC (permalink / raw)
  To: Tomas Winkler; +Cc: Jouni Malinen, Jiri Benc, linux-wireless

On Tuesday 13 February 2007 01:19, Tomas Winkler wrote:
> > HW doesn't scramble it. So when d80211 receives the packet it can
> > _then_ generate the key. There's no point in precomputing it.
> 
> For past packets you've already computed it so why do  it again? So
> you keep 2 phas1 keys one old and one current. When sequence numbers
> advance sufficiently you discard the old one and compute the new one
> in a spare time. Something like double buffer.

Ah, now I see. We are talking about two different things.
You are talking about the TX key and I am talking about
the RX key. We could probably cache the TX key to avoid the
key computation on every TX. That's right. But I don't even have
it working, yet, so optimization is not my worry, yet. :)
But yes, that could be an optimization to be done.

-- 
Greetings Michael.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: d80211: current TKIP hwcrypto implementation seems to be broken
  2007-02-13  0:28               ` Jouni Malinen
@ 2007-02-13  1:08                 ` Tomas Winkler
  2007-02-13  1:20                   ` Jouni Malinen
  0 siblings, 1 reply; 23+ messages in thread
From: Tomas Winkler @ 2007-02-13  1:08 UTC (permalink / raw)
  To: Jouni Malinen; +Cc: Michael Buesch, Jiri Benc, linux-wireless

On 2/13/07, Jouni Malinen <jkm@devicescape.com> wrote:
> On Tue, Feb 13, 2007 at 02:19:15AM +0200, Tomas Winkler wrote:
>
> > For past packets you've already computed it so why do  it again? So
> > you keep 2 phas1 keys one old and one current. When sequence numbers
> > advance sufficiently you discard the old one and compute the new one
> > in a spare time. Something like double buffer.
>
> Well, you can, but how likely is this case to really happen? If someone
> can show this happening frequently, I could consider adding support for
> it, but I see not much point in adding the extra complexity without a
> clear case where this would be of benefit. P1K is not really that slow
> after all and figuring out when the kernel code has "spare time" and all
> the extra handling of pre-calculation and storing the values is not
> really something I would like to add into d80211 unless this can really
> be shown to provide noticeable throughput improvement.

There are real cases when this happens. Try to two different AC for
example  VoIP and have FTP in parallel.  Packets for low priority AC
can be stalled encrypted and scheduled in a AP TX queue while high
priority packets are already sent with the new key.
Maybe the new key can be computed on demand but it's good thing to
preserve the old key for while.


>
> > I didn't measured performance but I thought  it can be precomputed by
> > the supplicant rather then by kernel code.    I might be wrong.
>
> P1K is calculated by whoever is using the TKIP TK and in case of all the
> implementations that I'm aware of, this is not done in the supplicant.
> Sure, it could be done there, but I think the current design of doing
> all TKIP encryption/decryption (including phase 1 and 2 key derivation)
> in the kernel is better.
>
You might be correct here.
> --
> Jouni Malinen                                            PGP id EFC895FA
>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: d80211: current TKIP hwcrypto implementation seems to be broken
  2007-02-13  0:33               ` Michael Buesch
@ 2007-02-13  1:09                 ` Tomas Winkler
  0 siblings, 0 replies; 23+ messages in thread
From: Tomas Winkler @ 2007-02-13  1:09 UTC (permalink / raw)
  To: Michael Buesch; +Cc: Jouni Malinen, Jiri Benc, linux-wireless

On 2/13/07, Michael Buesch <mb@bu3sch.de> wrote:
> On Tuesday 13 February 2007 01:19, Tomas Winkler wrote:
> > > HW doesn't scramble it. So when d80211 receives the packet it can
> > > _then_ generate the key. There's no point in precomputing it.
> >
> > For past packets you've already computed it so why do  it again? So
> > you keep 2 phas1 keys one old and one current. When sequence numbers
> > advance sufficiently you discard the old one and compute the new one
> > in a spare time. Something like double buffer.
>
> Ah, now I see. We are talking about two different things.
> You are talking about the TX key and I am talking about
> the RX key. We could probably cache the TX key to avoid the
> key computation on every TX. That's right. But I don't even have
> it working, yet, so optimization is not my worry, yet. :)
> But yes, that could be an optimization to be done.

No RX. phase1 is not computed on every packet, phase2 is but this is
usually done be HW.
> --
> Greetings Michael.
>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: d80211: current TKIP hwcrypto implementation seems to be broken
  2007-02-13  1:08                 ` Tomas Winkler
@ 2007-02-13  1:20                   ` Jouni Malinen
  2007-02-15 16:58                     ` Tomas Winkler
  0 siblings, 1 reply; 23+ messages in thread
From: Jouni Malinen @ 2007-02-13  1:20 UTC (permalink / raw)
  To: Tomas Winkler; +Cc: Michael Buesch, Jiri Benc, linux-wireless

On Tue, Feb 13, 2007 at 03:08:03AM +0200, Tomas Winkler wrote:

> There are real cases when this happens. Try to two different AC for
> example  VoIP and have FTP in parallel.  Packets for low priority AC
> can be stalled encrypted and scheduled in a AP TX queue while high
> priority packets are already sent with the new key.
> Maybe the new key can be computed on demand but it's good thing to
> preserve the old key for while.

I don't know the particular hardware design well enough to comment on
this, but d80211 software implementation keeps a separate RX P1K for
each AC (actually, each TID). In other words, this kind of issue does
not show up there. If the hardware implementation is limited to only one
P1K for RX, there may be some benefit on storing old keys in some cases,
but I'm not sure how that would work if the key has to be configured
before the frame is actually received.

In other words, in this case, the next frame after an FTP packet could
well be from voice and not background and reconfiguring the old P1K
value could have caused more latency on the higher priority frame at
this point.. In other word, if there is only one RX P1K, the benefits
for throughput may be requiring compromise on latency for higher
priority traffic and that may or may not be acceptable.

-- 
Jouni Malinen                                            PGP id EFC895FA

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: d80211: current TKIP hwcrypto implementation seems to be broken
  2007-02-12 18:30 ` Jouni Malinen
  2007-02-12 21:39   ` Michael Buesch
@ 2007-02-13 16:18   ` Johannes Berg
  1 sibling, 0 replies; 23+ messages in thread
From: Johannes Berg @ 2007-02-13 16:18 UTC (permalink / raw)
  To: Jouni Malinen; +Cc: Michael Buesch, Jiri Benc, linux-wireless

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

On Mon, 2007-02-12 at 10:30 -0800, Jouni Malinen wrote:

> How exactly is this supposed to work for RX? Phase1 needs to be
> calculated after 65536 frames (whenever iv32 changes), but the
> exact time for RX case is unclear due to possible loss of frames and
> retransmissions etc. Is bcm43xx just going drop couple of packets
> whenever the phase1 value is changed?

I think it's been said before, but I'll explain it (again). bcm43xx hw
will simply not decrypt the frame if the iv32 is different from what it
has the phase1 key for, so then it passes it up undecrypted.

>  Or is there some kind of
> mechanism for the hardware/firmware request a new phase1 value? Or is
> this supposed to be recovered from in software (which is something that
> d80211 is able to do if the radio driver notifies that the frame was not
> decrypted)?

Yeah, you notice the frame wasn't decrypted and then give the hw the new
phase1 key and also recover in software (or just drop the frame if you
want, I guess).

johannes

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: d80211: current TKIP hwcrypto implementation seems to be broken
  2007-02-13  1:20                   ` Jouni Malinen
@ 2007-02-15 16:58                     ` Tomas Winkler
  0 siblings, 0 replies; 23+ messages in thread
From: Tomas Winkler @ 2007-02-15 16:58 UTC (permalink / raw)
  To: Jouni Malinen; +Cc: Michael Buesch, Jiri Benc, linux-wireless

On 2/13/07, Jouni Malinen <jkm@devicescape.com> wrote:
> On Tue, Feb 13, 2007 at 03:08:03AM +0200, Tomas Winkler wrote:
>
> > There are real cases when this happens. Try to two different AC for
> > example  VoIP and have FTP in parallel.  Packets for low priority AC
> > can be stalled encrypted and scheduled in a AP TX queue while high
> > priority packets are already sent with the new key.
> > Maybe the new key can be computed on demand but it's good thing to
> > preserve the old key for while.
>
> I don't know the particular hardware design well enough to comment on
> this, but d80211 software implementation keeps a separate RX P1K for
> each AC (actually, each TID). In other words, this kind of issue does
> not show up there. If the hardware implementation is limited to only one
> P1K for RX, there may be some benefit on storing old keys in some cases,
> but I'm not sure how that would work if the key has to be configured
> before the frame is actually received.


Since TX  can use only on sequence counter regardless of AC also on
receive side it's enough to use one phase1 key. To keep phase1 key for
each AC is possible but it wast of memory. You need to keep just old
key till all AC wraps.

> In other words, in this case, the next frame after an FTP packet could
> well be from voice and not background and reconfiguring the old P1K
> value could have caused more latency on the higher priority frame at
> this point.. In other word, if there is only one RX P1K, the benefits
> for throughput may be requiring compromise on latency for higher
> priority traffic and that may or may not be acceptable.
>

You decrypt with old phase1 only in software, usually it is only few
packets (since TX counter is global). It will be a bug to configure HW
with the old key.

> Jouni Malinen                                            PGP id EFC895FA
>

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2007-02-15 16:58 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-02-04 12:44 d80211: current TKIP hwcrypto implementation seems to be broken Michael Buesch
2007-02-05 20:43 ` Jiri Benc
2007-02-05 21:38   ` Johannes Berg
2007-02-05 21:45     ` Johannes Berg
2007-02-05 21:48       ` Johannes Berg
2007-02-06 14:55   ` Michael Buesch
2007-02-12 18:30 ` Jouni Malinen
2007-02-12 21:39   ` Michael Buesch
2007-02-12 23:15     ` Tomas Winkler
2007-02-12 23:23       ` Michael Buesch
2007-02-12 23:54         ` Tomas Winkler
2007-02-13  0:10           ` Michael Buesch
2007-02-13  0:19             ` Tomas Winkler
2007-02-13  0:28               ` Jouni Malinen
2007-02-13  1:08                 ` Tomas Winkler
2007-02-13  1:20                   ` Jouni Malinen
2007-02-15 16:58                     ` Tomas Winkler
2007-02-13  0:33               ` Michael Buesch
2007-02-13  1:09                 ` Tomas Winkler
2007-02-12 23:53     ` Jouni Malinen
2007-02-13  0:06       ` Michael Buesch
2007-02-13  0:16         ` Michael Buesch
2007-02-13 16:18   ` Johannes Berg

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.