[PATCH] p54: connect to 11w protected networks

[PATCH] p54: connect to 11w protected networks

[Christian Lamparter]

Previously, it was not possible to connect to
networks which requires 11w to be supported by
the stations.

This implementation relies on the mac80211
stack to do the actual crypto operations, as
there is no support for MFP in the firmware
as far as I can tell.

Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
---
 drivers/net/wireless/p54/main.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/p54/main.c b/drivers/net/wireless/p54/main.c
index 5e91ad0..5333201 100644
--- a/drivers/net/wireless/p54/main.c
+++ b/drivers/net/wireless/p54/main.c
@@ -540,7 +540,8 @@ static int p54_set_key(struct ieee80211_hw *dev, enum set_key_cmd cmd,
 				ret = -EOPNOTSUPP;
 				goto out_unlock;
 			}
-			key->flags |= IEEE80211_KEY_FLAG_GENERATE_IV;
+			key->flags |= IEEE80211_KEY_FLAG_GENERATE_IV |
+				IEEE80211_KEY_FLAG_SW_MGMT;
 			algo = P54_CRYPTO_AESCCMP;
 			break;
 		default:
@@ -736,6 +737,7 @@ struct ieee80211_hw *p54_init_common(size_t priv_data_len)
 	dev->flags = IEEE80211_HW_RX_INCLUDES_FCS |
 		     IEEE80211_HW_SIGNAL_DBM |
 		     IEEE80211_HW_SUPPORTS_PS |
+		     IEEE80211_HW_MFP_CAPABLE |
 		     IEEE80211_HW_PS_NULLFUNC_STACK |
 		     IEEE80211_HW_REPORTS_TX_ACK_STATUS;
 
-- 
1.7.10.4

Re: [PATCH] p54: connect to 11w protected networks

[Johannes Berg]

On Sun, 2012-09-02 at 15:22 +0200, Christian Lamparter wrote:
> Previously, it was not possible to connect to
> networks which requires 11w to be supported by
> the stations.
> 
> This implementation relies on the mac80211
> stack to do the actual crypto operations, as
> there is no support for MFP in the firmware
> as far as I can tell.

Same notes about RX apply here, of course.

johannes

[PATCH v2] p54: connect to 11w protected networks

[Christian Lamparter]

Previously, it was not possible to connect
to networks which requires 11w to be supported
by the stations.

Note:

As all current (and old) firmwares corrupt
incoming, protected management frames, the
decryption offloading needs to be disabled.

This can be done by loading the p54common
module with the nohwcrypt=1 parameter.

Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
---
Johannes,

thank you for making me double-check whenever MFP really works!
Unlike carl9170 which generates and receives a great amount of
addba/delba action frames, p54 - being a legacy product - has
no such "easy to use" debug facilities. 

As you feared: the firmware is corrupting incoming management
frames. However, it does so only when a rxkey was configured.
So as a possible trade-off, MFP will only be supported by p54,
when the firmware crypto offloading is relieved of his duties
(disabled). 

Note: Strictly speaking, it would be enough to no longer upload
any rx key which could affect management frames, as the firmware
does not decrypt any frames unless there is a fitting key candidate.

Regards,
	Chr 
---
 drivers/net/wireless/p54/main.c |   11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/net/wireless/p54/main.c b/drivers/net/wireless/p54/main.c
index 5e91ad0..16b75ec 100644
--- a/drivers/net/wireless/p54/main.c
+++ b/drivers/net/wireless/p54/main.c
@@ -739,6 +739,17 @@ struct ieee80211_hw *p54_init_common(size_t priv_data_len)
 		     IEEE80211_HW_PS_NULLFUNC_STACK |
 		     IEEE80211_HW_REPORTS_TX_ACK_STATUS;
 
+	if (nohwcrypt) {
+		/*
+		 * Only support MFP, if the hardware crypto engine
+		 * is disabled, as the firmware's rx-path corrupts
+		 * incoming CCMP encrypted mgmt frames.
+		 * However, if no rx key is uploaded, the firmware
+		 * passes the unencrypted frame to the driver.
+		 */
+		dev->flags |= IEEE80211_HW_MFP_CAPABLE;
+	}
+
 	dev->wiphy->interface_modes = BIT(NL80211_IFTYPE_STATION) |
 				      BIT(NL80211_IFTYPE_ADHOC) |
 				      BIT(NL80211_IFTYPE_AP) |
-- 
1.7.10.4

Re: [PATCH v2] p54: connect to 11w protected networks

[Johannes Berg]

On Tue, 2012-09-04 at 15:19 +0200, Christian Lamparter wrote:

> As you feared: the firmware is corrupting incoming management
> frames. However, it does so only when a rxkey was configured.
> So as a possible trade-off, MFP will only be supported by p54,
> when the firmware crypto offloading is relieved of his duties
> (disabled). 
> 
> Note: Strictly speaking, it would be enough to no longer upload
> any rx key which could affect management frames, as the firmware
> does not decrypt any frames unless there is a fitting key candidate.

Right, however, that would kinda defeat the point. Unless ... maybe we
can tell the driver if 11w is going to be used at all? If we did that,
we could make it work in the set_key callback.

For now your patch looks good though.

johannes

Re: [PATCH v2] p54: connect to 11w protected networks

[Dan Williams]

On Tue, 2012-09-04 at 16:15 +0200, Johannes Berg wrote:
> On Tue, 2012-09-04 at 15:19 +0200, Christian Lamparter wrote:
> 
> > As you feared: the firmware is corrupting incoming management
> > frames. However, it does so only when a rxkey was configured.
> > So as a possible trade-off, MFP will only be supported by p54,
> > when the firmware crypto offloading is relieved of his duties
> > (disabled). 
> > 
> > Note: Strictly speaking, it would be enough to no longer upload
> > any rx key which could affect management frames, as the firmware
> > does not decrypt any frames unless there is a fitting key candidate.
> 
> Right, however, that would kinda defeat the point. Unless ... maybe we
> can tell the driver if 11w is going to be used at all? If we did that,
> we could make it work in the set_key callback.
> 
> For now your patch looks good though.

We can't get this to happen without a module option?  Is there a way to
turn the crypto offloading off when we know we're connecting to a W
protected AP?

Dan

Re: [PATCH v2] p54: connect to 11w protected networks

[Johannes Berg]

On Tue, 2012-09-04 at 09:33 -0500, Dan Williams wrote:
> On Tue, 2012-09-04 at 16:15 +0200, Johannes Berg wrote:
> > On Tue, 2012-09-04 at 15:19 +0200, Christian Lamparter wrote:
> > 
> > > As you feared: the firmware is corrupting incoming management
> > > frames. However, it does so only when a rxkey was configured.
> > > So as a possible trade-off, MFP will only be supported by p54,
> > > when the firmware crypto offloading is relieved of his duties
> > > (disabled). 
> > > 
> > > Note: Strictly speaking, it would be enough to no longer upload
> > > any rx key which could affect management frames, as the firmware
> > > does not decrypt any frames unless there is a fitting key candidate.
> > 
> > Right, however, that would kinda defeat the point. Unless ... maybe we
> > can tell the driver if 11w is going to be used at all? If we did that,
> > we could make it work in the set_key callback.
> > 
> > For now your patch looks good though.
> 
> We can't get this to happen without a module option?  Is there a way to
> turn the crypto offloading off when we know we're connecting to a W
> protected AP?

Yeah that's what I was thinking. I suppose we *could* somehow do this by
setting some key flag in mac80211 that the driver can check.

johannes

Re: [PATCH v2] p54: connect to 11w protected networks

[Johannes Berg]

On Tue, 2012-09-04 at 16:37 +0200, Johannes Berg wrote:
> On Tue, 2012-09-04 at 09:33 -0500, Dan Williams wrote:
> > On Tue, 2012-09-04 at 16:15 +0200, Johannes Berg wrote:
> > > On Tue, 2012-09-04 at 15:19 +0200, Christian Lamparter wrote:
> > > 
> > > > As you feared: the firmware is corrupting incoming management
> > > > frames. However, it does so only when a rxkey was configured.
> > > > So as a possible trade-off, MFP will only be supported by p54,
> > > > when the firmware crypto offloading is relieved of his duties
> > > > (disabled). 
> > > > 
> > > > Note: Strictly speaking, it would be enough to no longer upload
> > > > any rx key which could affect management frames, as the firmware
> > > > does not decrypt any frames unless there is a fitting key candidate.
> > > 
> > > Right, however, that would kinda defeat the point. Unless ... maybe we
> > > can tell the driver if 11w is going to be used at all? If we did that,
> > > we could make it work in the set_key callback.
> > > 
> > > For now your patch looks good though.
> > 
> > We can't get this to happen without a module option?  Is there a way to
> > turn the crypto offloading off when we know we're connecting to a W
> > protected AP?
> 
> Yeah that's what I was thinking. I suppose we *could* somehow do this by
> setting some key flag in mac80211 that the driver can check.

Something like this perhaps:

http://p.sipsolutions.net/76d17d49f4685cda.txt

But I'd want somebody to check if the mesh code is right and maybe we
need to think about the AP case (have hostapd tell us whether it uses
MFP or not) ... and maybe docs should be improved

johannes

Re: [PATCH v2] p54: connect to 11w protected networks

[Johannes Berg]

On Tue, 2012-09-04 at 16:54 +0200, Johannes Berg wrote:

> > > We can't get this to happen without a module option?  Is there a way to
> > > turn the crypto offloading off when we know we're connecting to a W
> > > protected AP?
> > 
> > Yeah that's what I was thinking. I suppose we *could* somehow do this by
> > setting some key flag in mac80211 that the driver can check.
> 
> Something like this perhaps:
> 
> http://p.sipsolutions.net/76d17d49f4685cda.txt

If you want this for your driver, I'll commit this:

http://p.sipsolutions.net/ea2e3dbe980f5db8.txt

johannes

Re: [PATCH v2] p54: connect to 11w protected networks

[Christian Lamparter]

On Friday 07 September 2012 14:17:14 Johannes Berg wrote:
> On Tue, 2012-09-04 at 16:54 +0200, Johannes Berg wrote:
>
> If you want this for your driver, I'll commit this:
> 
> http://p.sipsolutions.net/ea2e3dbe980f5db8.txt

Won't this totally disable the crypto offload
capabilities in AP mode?

I sort of prefer the nohwcrypt switch as it won't
interfere with running user setups as much (so we
don't burden systems like that weird telekom
streamin-box (Vision S100 - it's name I think)
that came with a p54pci minipci card and are
now used for different purposes by some.

Regards,
	Christian 

Re: [PATCH v2] p54: connect to 11w protected networks

[Johannes Berg]

On Fri, 2012-09-07 at 17:47 +0200, Christian Lamparter wrote:
> On Friday 07 September 2012 14:17:14 Johannes Berg wrote:
> > On Tue, 2012-09-04 at 16:54 +0200, Johannes Berg wrote:
> >
> > If you want this for your driver, I'll commit this:
> > 
> > http://p.sipsolutions.net/ea2e3dbe980f5db8.txt
> 
> Won't this totally disable the crypto offload
> capabilities in AP mode?

Yes, though we could add an nl80211 attribute that allows hostapd to
tell us, and then if it did tell us we could know and do the right thing
here.

> I sort of prefer the nohwcrypt switch as it won't
> interfere with running user setups as much (so we
> don't burden systems like that weird telekom
> streamin-box (Vision S100 - it's name I think)
> that came with a p54pci minipci card and are
> now used for different purposes by some.

I guess it would make sense then to give hostapd the ability to tell us?

It's up to you. I don't need 11w, nor do I have p54 hardware, nor does
it matter much to me ... :)

johannes

Re: [PATCH v2] p54: connect to 11w protected networks

[Christian Lamparter]

On Friday 07 September 2012 17:55:42 Johannes Berg wrote:
> On Fri, 2012-09-07 at 17:47 +0200, Christian Lamparter wrote:
> > On Friday 07 September 2012 14:17:14 Johannes Berg wrote:
> > > On Tue, 2012-09-04 at 16:54 +0200, Johannes Berg wrote:
> > >
> > > If you want this for your driver, I'll commit this:
> > > 
> > > http://p.sipsolutions.net/ea2e3dbe980f5db8.txt
> > 
> > Won't this totally disable the crypto offload
> > capabilities in AP mode?
> 
> Yes, though we could add an nl80211 attribute that
> allows hostapd to tell us, and then if it did tell
> us we could know and do the right thing here.

Well, if we want to do the right thing here, then I
guess we should go for another alternative: a rxkey
manager of some sorts. So, we delete keys from the
firmware context once MFP comes into the game.

This method also has another advantage that with
a proper manager we can use the key usage statistics
and upload only the most-used keys into the firmware
context (most/all? p54 firmwares have usually 4-8 key
slots in total, which is not always enough).
 
> > I sort of prefer the nohwcrypt switch as it won't
> > interfere with running user setups as much (so we
> > don't burden systems like that weird telekom
> > streamin-box (Vision S100 - it's name I think)
> > that came with a p54pci minipci card and are
> > now used for different purposes by some.
> 
> I guess it would make sense then to give hostapd the ability to tell us?
> 
> It's up to you. I don't need 11w, nor do I have p54 hardware, nor does
> it matter much to me ... :)
Let's save that task for a rainy day :-D.

Regards,
	Chr

Re: [PATCH v2] p54: connect to 11w protected networks

[Johannes Berg]

On Fri, 2012-09-07 at 18:10 +0200, Christian Lamparter wrote:
> On Friday 07 September 2012 17:55:42 Johannes Berg wrote:
> > On Fri, 2012-09-07 at 17:47 +0200, Christian Lamparter wrote:
> > > On Friday 07 September 2012 14:17:14 Johannes Berg wrote:
> > > > On Tue, 2012-09-04 at 16:54 +0200, Johannes Berg wrote:
> > > >
> > > > If you want this for your driver, I'll commit this:
> > > > 
> > > > http://p.sipsolutions.net/ea2e3dbe980f5db8.txt
> > > 
> > > Won't this totally disable the crypto offload
> > > capabilities in AP mode?
> > 
> > Yes, though we could add an nl80211 attribute that
> > allows hostapd to tell us, and then if it did tell
> > us we could know and do the right thing here.
> 
> Well, if we want to do the right thing here, then I
> guess we should go for another alternative: a rxkey
> manager of some sorts. So, we delete keys from the
> firmware context once MFP comes into the game.

That's kinda what I do though, the flag tells you (at key installation)
whether MFP will be used or not. It's just that in the AP case, mac80211
doesn't actually know.

johannes

Re: [PATCH v2] p54: connect to 11w protected networks

[Christian Lamparter]

On Friday 07 September 2012 18:15:03 you wrote:
> On Fri, 2012-09-07 at 18:10 +0200, Christian Lamparter wrote:
> > On Friday 07 September 2012 17:55:42 Johannes Berg wrote:
> > > On Fri, 2012-09-07 at 17:47 +0200, Christian Lamparter wrote:
> > > > Won't this totally disable the crypto offload
> > > > capabilities in AP mode?
> > > 
> > > Yes, though we could add an nl80211 attribute that
> > > allows hostapd to tell us, and then if it did tell
> > > us we could know and do the right thing here.
> > 
> > if we want to do the right thing here, then I guess
> > we should go for another alternative: a rxkey manager
> > of some sorts. So, we delete keys from the firmware
> > context once MFP comes into the game.
> 
> That's kinda what I do though, the flag tells you
> (at key installation) whether MFP will be used or
> not. It's just that in the AP case, mac80211 doesn't
> actually know.

Oh no, I mean dynamic reconfiguration of the firmware's
keycache during runtime. But for this, we would need
a way to tell mac80211 when driver wants to delete a key
from the hw/fw cache and when there is room for another
one (Didn't you had a patch for the "we have a empty slot
in the rxkey cache we are not using" case some time ago?
However in this case, we want to tell mac80211 what "exact"
key (by MAC of the peer and key index) we want.)

Of course, I'm well aware of the "amount of work" and the
problems associated with removing and readding keys during
runtime without causing races (or just minor races).

Regards,
        Chr

---
Sorry!

Re: [PATCH v2] p54: connect to 11w protected networks

[Christian Lamparter]

On Friday 07 September 2012 18:27:57 Johannes Berg wrote:
> On Fri, 2012-09-07 at 18:25 +0200, Christian Lamparter wrote:
> 
> > > > if we want to do the right thing here, then I guess
> > > > we should go for another alternative: a rxkey manager
> > > > of some sorts. So, we delete keys from the firmware
> > > > context once MFP comes into the game.
> > > 
> > > That's kinda what I do though, the flag tells you
> > > (at key installation) whether MFP will be used or
> > > not. It's just that in the AP case, mac80211 doesn't
> > > actually know.
> > 
> > Oh no, I mean dynamic reconfiguration of the firmware's
> > keycache during runtime. 
> 
> Right right, but for MFP that doesn't help since in AP mode
> we don't know whether to expect encrypted management frames
> or not.

Hmpf, don't we set the CMAC key in this case? (Note: I haven't
seen much/any of the 11w spec, as 802.11-2012 is AFAICT still not
available for _free_). However, my thinking is/was that when
mac80211 tries to upload the CMAC key, return -EOPNOTSUPP and 
we kick the ccm per-station key, so the firmware won't try to
decrypt incoming (mgmt and data) frames with this combination
anymore.
 
> > But for this, we would need
> > a way to tell mac80211 when driver wants to delete a key
> > from the hw/fw cache and when there is room for another
> > one (Didn't you had a patch for the "we have a empty slot
> > in the rxkey cache we are not using" case some time ago?
> > However in this case, we want to tell mac80211 what "exact"
> > key (by MAC of the peer and key index) we want.)
> 
> I had a patch for the opposite: please remove this key, I can't
> handle it any more. Adding a new one in that case couldn't be
> done.
Ah well, that's too bad. Altough, I thought you removed
it because its prone to cause rx/tx races?!

> > Of course, I'm well aware of the "amount of work" and the
> > problems associated with removing and readding keys during
> > runtime without causing races (or just minor races).
> 
> Actually that's not too difficult, the bigger difficulty is
> actually knowing which key to re-upload I think.
Well, the firmware reports a "CACHE MISS" status in every
encrypted rx frame if no key was found in the rxkey cache.

Of course, being a cache, we can make some sort of LRU 
(not sure about the size, but rxkey cache * 2 would be
a start) and keep track of the rx key usage. This way
when a "rxkey" becomes more popular it will be detected
and "replace" a rxkey that is no longer active... and
so on.

Regards,
	Chr

Re: [PATCH v2] p54: connect to 11w protected networks

[Johannes Berg]

On Fri, 2012-09-07 at 19:01 +0200, Christian Lamparter wrote:

> > > Oh no, I mean dynamic reconfiguration of the firmware's
> > > keycache during runtime. 
> > 
> > Right right, but for MFP that doesn't help since in AP mode
> > we don't know whether to expect encrypted management frames
> > or not.
> 
> Hmpf, don't we set the CMAC key in this case? 

Hmm, I guess we would have that, and actually ... we know MFP will be
used for the *station* so we should in fact know this already. I'll need
to revise my patch. D'oh.

> (Note: I haven't
> seen much/any of the 11w spec, as 802.11-2012 is AFAICT still not
> available for _free_). 

Hm. Well it was published March 29th, so it should be available in 3
weeks if they hold their promise ...

> However, my thinking is/was that when
> mac80211 tries to upload the CMAC key, return -EOPNOTSUPP and 
> we kick the ccm per-station key, so the firmware won't try to
> decrypt incoming (mgmt and data) frames with this combination
> anymore.

CMAC keys aren't per-station, so we'd have to remove them all or
something? But I think we already know about MFP for the station when we
set the key, I'll check this.
 
> > > But for this, we would need
> > > a way to tell mac80211 when driver wants to delete a key
> > > from the hw/fw cache and when there is room for another
> > > one (Didn't you had a patch for the "we have a empty slot
> > > in the rxkey cache we are not using" case some time ago?
> > > However in this case, we want to tell mac80211 what "exact"
> > > key (by MAC of the peer and key index) we want.)
> > 
> > I had a patch for the opposite: please remove this key, I can't
> > handle it any more. Adding a new one in that case couldn't be
> > done.
> Ah well, that's too bad. Altough, I thought you removed
> it because its prone to cause rx/tx races?!

No, just because it was unused. Adding a new key could also be
implemented, or you could iterate the keys in some way maybe.

> > > Of course, I'm well aware of the "amount of work" and the
> > > problems associated with removing and readding keys during
> > > runtime without causing races (or just minor races).
> > 
> > Actually that's not too difficult, the bigger difficulty is
> > actually knowing which key to re-upload I think.
> Well, the firmware reports a "CACHE MISS" status in every
> encrypted rx frame if no key was found in the rxkey cache.
> 
> Of course, being a cache, we can make some sort of LRU 
> (not sure about the size, but rxkey cache * 2 would be
> a start) and keep track of the rx key usage. This way
> when a "rxkey" becomes more popular it will be detected
> and "replace" a rxkey that is no longer active... and
> so on.

Right. Seems pretty simple really, but I guess the devil is in the
details :-)

Anyway I'll revise that patch (after dinner) and see if MFP flag is set
correctly, if so then we can just use that.

johannes

Re: [PATCH v2] p54: connect to 11w protected networks

[Johannes Berg]

On Fri, 2012-09-07 at 19:09 +0200, Johannes Berg wrote:
> On Fri, 2012-09-07 at 19:01 +0200, Christian Lamparter wrote:
> 
> > > > Oh no, I mean dynamic reconfiguration of the firmware's
> > > > keycache during runtime. 
> > > 
> > > Right right, but for MFP that doesn't help since in AP mode
> > > we don't know whether to expect encrypted management frames
> > > or not.
> > 
> > Hmpf, don't we set the CMAC key in this case? 
> 
> Hmm, I guess we would have that, and actually ... we know MFP will be
> used for the *station* so we should in fact know this already. I'll need
> to revise my patch. D'oh.

I just posted a new patch. I haven't really tested it though, but I'm
pretty sure it'll work.

johannes