LinuxLists.cc - [PATCH] ath5k: add support of HW encryption in management frames

[permalink] [raw]

Subject: Re: [PATCH] ath5k: add support of HW encryption in management frames

Hi, Adrian

Appreciate your testing on this.

---
Chun-Yeow

On Fri, Aug 31, 2012 at 2:58 PM, Adrian Chadd <[email protected]> wrote:
> Hiya,
>
> This is cool stuff - but as I don't think MFP is enabled in any
> hardware pre-AR9280, I think it's worth doing a lot more testing
> before flipping this on.
>
> But very cool indeed. I'll see what it'll take to get it enabled in
> the FreeBSD atheros HAL for these chips.
>
>
>
> Adrian
>
> On 28 August 2012 02:34, Chun-Yeow Yeoh <[email protected]> wrote:
>> This patch provides the support of hardware encyrption for
>> management frame, including the support of AES CMAC. This
>> patch is tested with the following chipsets:
>> - AR5213A
>> - AR5413
>> - AR2413/AR2414
>>
>> Beside, this patch also extends the IBSS RSN to mesh.
>>
>> Signed-off-by: Chun-Yeow Yeoh <[email protected]>
>> ---
>> drivers/net/wireless/ath/ath5k/base.c | 4 +++-
>> drivers/net/wireless/ath/ath5k/mac80211-ops.c | 7 +++++--
>> 2 files changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c
>> index a0a202d..084162e 100644
>> --- a/drivers/net/wireless/ath/ath5k/base.c
>> +++ b/drivers/net/wireless/ath/ath5k/base.c
>> @@ -2446,7 +2446,9 @@ ath5k_init_ah(struct ath5k_hw *ah, const struct ath_bus_ops *bus_ops)
>> hw->flags = IEEE80211_HW_RX_INCLUDES_FCS |
>> IEEE80211_HW_HOST_BROADCAST_PS_BUFFERING |
>> IEEE80211_HW_SIGNAL_DBM |
>> - IEEE80211_HW_REPORTS_TX_ACK_STATUS;
>> + IEEE80211_HW_REPORTS_TX_ACK_STATUS |
>> + IEEE80211_HW_MFP_CAPABLE |
>> + IEEE80211_HW_SUPPORTS_PER_STA_GTK;
>>
>> hw->wiphy->interface_modes =
>> BIT(NL80211_IFTYPE_AP) |
>> diff --git a/drivers/net/wireless/ath/ath5k/mac80211-ops.c b/drivers/net/wireless/ath/ath5k/mac80211-ops.c
>> index df61a09..7f78caa 100644
>> --- a/drivers/net/wireless/ath/ath5k/mac80211-ops.c
>> +++ b/drivers/net/wireless/ath/ath5k/mac80211-ops.c
>> @@ -489,7 +489,8 @@ ath5k_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>> if (ath5k_modparam_nohwcrypt)
>> return -EOPNOTSUPP;
>>
>> - if (vif->type == NL80211_IFTYPE_ADHOC &&
>> + if ((vif->type == NL80211_IFTYPE_ADHOC ||
>> + vif->type == NL80211_IFTYPE_MESH_POINT) &&
>> (key->cipher == WLAN_CIPHER_SUITE_TKIP ||
>> key->cipher == WLAN_CIPHER_SUITE_CCMP) &&
>> !(key->flags & IEEE80211_KEY_FLAG_PAIRWISE)) {
>> @@ -502,6 +503,7 @@ ath5k_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>> case WLAN_CIPHER_SUITE_WEP104:
>> case WLAN_CIPHER_SUITE_TKIP:
>> break;
>> + case WLAN_CIPHER_SUITE_AES_CMAC:
>> case WLAN_CIPHER_SUITE_CCMP:
>> if (common->crypt_caps & ATH_CRYPT_CAP_CIPHER_AESCCM)
>> break;
>> @@ -522,7 +524,8 @@ ath5k_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>> key->flags |= IEEE80211_KEY_FLAG_GENERATE_IV;
>> if (key->cipher == WLAN_CIPHER_SUITE_TKIP)
>> key->flags |= IEEE80211_KEY_FLAG_GENERATE_MMIC;
>> - if (key->cipher == WLAN_CIPHER_SUITE_CCMP)
>> + if (key->cipher == WLAN_CIPHER_SUITE_CCMP &&
>> + !(common->crypt_caps & ATH_CRYPT_CAP_CIPHER_AESCCM))
>> key->flags |= IEEE80211_KEY_FLAG_SW_MGMT;
>> ret = 0;
>> }
>> --
>> 1.7.0.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html

2012-08-31 06:58:40

[permalink] [raw]

Subject: Re: [PATCH] ath5k: add support of HW encryption in management frames

Hiya,

This is cool stuff - but as I don't think MFP is enabled in any
hardware pre-AR9280, I think it's worth doing a lot more testing
before flipping this on.

But very cool indeed. I'll see what it'll take to get it enabled in
the FreeBSD atheros HAL for these chips.

Adrian

On 28 August 2012 02:34, Chun-Yeow Yeoh <[email protected]> wrote:
> This patch provides the support of hardware encyrption for
> management frame, including the support of AES CMAC. This
> patch is tested with the following chipsets:
> - AR5213A
> - AR5413
> - AR2413/AR2414
>
> Beside, this patch also extends the IBSS RSN to mesh.
>
> Signed-off-by: Chun-Yeow Yeoh <[email protected]>
> ---
> drivers/net/wireless/ath/ath5k/base.c | 4 +++-
> drivers/net/wireless/ath/ath5k/mac80211-ops.c | 7 +++++--
> 2 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c
> index a0a202d..084162e 100644
> --- a/drivers/net/wireless/ath/ath5k/base.c
> +++ b/drivers/net/wireless/ath/ath5k/base.c
> @@ -2446,7 +2446,9 @@ ath5k_init_ah(struct ath5k_hw *ah, const struct ath_bus_ops *bus_ops)
> hw->flags = IEEE80211_HW_RX_INCLUDES_FCS |
> IEEE80211_HW_HOST_BROADCAST_PS_BUFFERING |
> IEEE80211_HW_SIGNAL_DBM |
> - IEEE80211_HW_REPORTS_TX_ACK_STATUS;
> + IEEE80211_HW_REPORTS_TX_ACK_STATUS |
> + IEEE80211_HW_MFP_CAPABLE |
> + IEEE80211_HW_SUPPORTS_PER_STA_GTK;
>
> hw->wiphy->interface_modes =
> BIT(NL80211_IFTYPE_AP) |
> diff --git a/drivers/net/wireless/ath/ath5k/mac80211-ops.c b/drivers/net/wireless/ath/ath5k/mac80211-ops.c
> index df61a09..7f78caa 100644
> --- a/drivers/net/wireless/ath/ath5k/mac80211-ops.c
> +++ b/drivers/net/wireless/ath/ath5k/mac80211-ops.c
> @@ -489,7 +489,8 @@ ath5k_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
> if (ath5k_modparam_nohwcrypt)
> return -EOPNOTSUPP;
>
> - if (vif->type == NL80211_IFTYPE_ADHOC &&
> + if ((vif->type == NL80211_IFTYPE_ADHOC ||
> + vif->type == NL80211_IFTYPE_MESH_POINT) &&
> (key->cipher == WLAN_CIPHER_SUITE_TKIP ||
> key->cipher == WLAN_CIPHER_SUITE_CCMP) &&
> !(key->flags & IEEE80211_KEY_FLAG_PAIRWISE)) {
> @@ -502,6 +503,7 @@ ath5k_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
> case WLAN_CIPHER_SUITE_WEP104:
> case WLAN_CIPHER_SUITE_TKIP:
> break;
> + case WLAN_CIPHER_SUITE_AES_CMAC:
> case WLAN_CIPHER_SUITE_CCMP:
> if (common->crypt_caps & ATH_CRYPT_CAP_CIPHER_AESCCM)
> break;
> @@ -522,7 +524,8 @@ ath5k_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
> key->flags |= IEEE80211_KEY_FLAG_GENERATE_IV;
> if (key->cipher == WLAN_CIPHER_SUITE_TKIP)
> key->flags |= IEEE80211_KEY_FLAG_GENERATE_MMIC;
> - if (key->cipher == WLAN_CIPHER_SUITE_CCMP)
> + if (key->cipher == WLAN_CIPHER_SUITE_CCMP &&
> + !(common->crypt_caps & ATH_CRYPT_CAP_CIPHER_AESCCM))
> key->flags |= IEEE80211_KEY_FLAG_SW_MGMT;
> ret = 0;
> }
> --
> 1.7.0.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2012-09-09 01:55:01

[permalink] [raw]

Subject: Re: [PATCH] ath5k: add support of HW encryption in management frames

Hiya,

Yeoh - can you please email me privately with a summary of what you
implemented, what you've tested and what worked / what didn't work?

I can do some digging into what changed with the encryption stuff and
see if I can figure it out in my (limited) spare time. I can try to do
it sometime next week.

I've been meaning to do MFP for FreeBSD's ath/net80211 driver so I
guess now it's a fine time to dig into the older chipset code and
figure out what's going on.

Thanks,

Adrian

2012-09-10 13:13:08

by Kalle Valo

[permalink] [raw]

Subject: Re: [PATCH] ath5k: add support of HW encryption in management frames

Adrian Chadd <[email protected]> writes:

> Yeoh - can you please email me privately with a summary of what you
> implemented, what you've tested and what worked / what didn't work?

Why privately? Better to have all the information public, you never know
if someone finds the info from the web and picks up the work.

--
Kalle Valo

2012-09-11 16:06:08

[permalink] [raw]

Subject: Re: [PATCH] ath5k: add support of HW encryption in management frames

2012/9/11 Yeoh Chun-Yeow <[email protected]>:
> Hi, Nick
>
>> Nope, with nohwcrypt we don't initialize the hw engine at all, we just
>> tell hw that data is not encrypted and that nothing should be done
>> about it. With these hw options from what I understand from docs we
>> can initialize the engine but use only part of it, eg. use only hw
>> encryption or only hw decryption. It might be useful in your case.
>
> The problem of ath5k right seems to be that: '
> If CCMP HW encryption is enabled, HW encryption for unicast data frame
> and SW encryption for unicast management frame are possible for TX.
> But for RX, both the unicast data frame and unicast management frame
> are decrypted by the HW instead of doing only SW decryption for
> unicast management frame.
>
> With the help of PCU DIAG register, possible to separate this?
>

Its possible to tell hw to disable decryption so that you can handle
that only on sw, I haven't tested it but that's what docs say.

--
GPG ID: 0xEE878588
As you read this post global entropy rises. Have Fun ;-)
Nick

2012-09-10 15:11:09

[permalink] [raw]

Subject: Re: [PATCH] ath5k: add support of HW encryption in management frames

Hi, all

For your information, my submitted patch has allowed me to do the
following and mainly to setup the secured mesh 802.11s using authsae:

1. Key installations for the following:
/* key to protect integrity of multicast mgmt frames tx*/
install_key(nlcfg, NULL, CIPHER_AES_CMAC, NL80211_KEYTYPE_GROUP, 4, mgtk_tx);
/* key to encrypt multicast data traffic */
install_key(nlcfg, NULL, CIPHER_CCMP, NL80211_KEYTYPE_GROUP, 0, mgtk_tx);
/* key to encrypt/decrypt unicast data AND mgmt traffic to/from this peer */
install_key(&nlcfg, peer, CIPHER_CCMP, NL80211_KEYTYPE_PAIRWISE, 0, mtk);
/* key to decrypt multicast data traffic from this peer */
install_key(&nlcfg, peer, CIPHER_CCMP, NL80211_KEYTYPE_GROUP, 0, peer_mgtk);
/* to check integrity of multicast mgmt frames from this peer */
install_key(&nlcfg, peer, CIPHER_AES_CMAC, NL80211_KEYTYPE_GROUP, 4, peer_mgtk);

2. By using the submitted patch, how ever as Jouni has pointed out
that testing with ath5k implementation alone may not be correct, due
to the following statement:
If the CCMP processing is done incorrectly, they could both mangle the
results in the same way to hide the issue.

thus I revert back by not disabling the IEEE80211_KEY_FLAG_SW_MGMT.
However, with this, it has showed that robust unicast management frame
is encrypted in SW but is decrypted wrongly in SW (perhaps HW decrypt
it due to the HW accl enabling for unicast data frame).

Hope this help.

Thanks.

Regards,
Chun-Yeow

On Mon, Sep 10, 2012 at 9:13 PM, Kalle Valo <[email protected]> wrote:
> Adrian Chadd <[email protected]> writes:
>
>> Yeoh - can you please email me privately with a summary of what you
>> implemented, what you've tested and what worked / what didn't work?
>
> Why privately? Better to have all the information public, you never know
> if someone finds the info from the web and picks up the work.
>
> --
> Kalle Valo

2012-09-11 09:42:53

[permalink] [raw]

Subject: Re: [PATCH] ath5k: add support of HW encryption in management frames

Hi, Nick

> Have you tried disabing RX or TX only encryption/decryption on hw
> trough PCU DIAG register ?

Nope. Is that same with loading the ath5k with nohwcrypt=1 to do
purely SW encryption/decryption?

----
Chun-Yeow

2012-09-04 16:56:54

[permalink] [raw]

Subject: Re: [PATCH] ath5k: add support of HW encryption in management frames

On Tue, 2012-09-04 at 18:55 +0200, Christian Lamparter wrote:

> > By the way, current secured mesh requires the AES CMAC to be enabled.
> > But without enabling IEEE80211_HW_MFP_CAPABLE, the key cannot be added
> > since this cipher suite is considered not supported. But actually AES
> > CMAC can be done in software. Any work around on this?
> >
> I think you can override this by supplying your own custom set
> of available ciphers. This is done by setting
> hw.wiphy->cipher_suites
> hw.wiphy->n_cipher_suites
> during the initalization of the driver (i.e.: before
> ieee80211_register_hw(hw) is called).

Yes, that's the way you could do it, but my argument that it's pointless
still stands, so what good would it do?

johannes

2012-09-11 16:36:27

[permalink] [raw]

Subject: Re: [PATCH] ath5k: add support of HW encryption in management frames

2012/9/11 Yeoh Chun-Yeow <[email protected]>:
>> To be more clear: you can tell the hw to *only* disable decryption and
>> keep doing encryption.
>
> But the question here is that during Rx, need to do SW decryption for
> management frame and then HW decryption for unicast data frame for all
> the frame coming from the same STA. I still not so sure how to do
> that.
>
> Jouni said that the workaround is to re-encrypt(incorrectly) received
> robust unicast management frames if hwaccel for CCMP was configured
> for the transmitting STA (this is to undo the incorrect decryption
> done by the hardware) and then pass the encrypted frame to mac80211
> for software decryption.
>

How about disabling acks on hw completely and handle them on sw ? This
might keep the engine running ok for unicast frames.

#define AR5K_DIAG_SW_DIS_ACK 0x00000002 /* Disable ACKs */
#define AR5K_DIAG_SW_DIS_CTS 0x00000004 /* Disable CTSs */

--
GPG ID: 0xEE878588
As you read this post global entropy rises. Have Fun ;-)
Nick

2012-09-05 07:17:08

[permalink] [raw]

Subject: Re: [PATCH] ath5k: add support of HW encryption in management frames

On Wed, Sep 05, 2012 at 12:41:16AM +0800, Yeoh Chun-Yeow wrote:
> By the way, current secured mesh requires the AES CMAC to be enabled.
> But without enabling IEEE80211_HW_MFP_CAPABLE, the key cannot be added
> since this cipher suite is considered not supported. But actually AES
> CMAC can be done in software. Any work around on this?

Could you please describe what exactly you mean with "current secured
mesh requires the AES CMAC to be enabled" and what is that claim based
on? Any pointers to the specific standard clause(s) that say that?

--
Jouni Malinen PGP id EFC895FA

2012-09-04 11:25:39

[permalink] [raw]

Subject: Re: [PATCH] ath5k: add support of HW encryption in management frames

Hi, Jouni

I have retested with the following:

case 1 with submitted patch:
mesh1: ath5k nohwcrypt=1
mesh2: ath5k (no IEEE80211_KEY_FLAG_SW_MGMT)
Result: Both of them are not able to ping each other. Also, the PREP
action frame is not able to decrypted by another node.

case 2 with submitted patch:
mesh1: ath5k nohwcrypt=1
mesh2: ath5k nohwcrypt=1
Result: They are able to ping each other.

case 3 with submitted patch but enable IEEE80211_KEY_FLAG_SW_MGMT:
mesh1: ath5k nohwcrypt=1
mesh2: ath5k (with IEEE80211_KEY_FLAG_SW_MGMT)
Result: They are able to ping each other

case 4 with submitted patch but enable IEEE80211_KEY_FLAG_SW_MGMT:
mesh1: ath5k (with IEEE80211_KEY_FLAG_SW_MGMT)
mesh2: ath5k (with IEEE80211_KEY_FLAG_SW_MGMT)
Result: Both of them are not able to ping each other. Also, the PREP
action frame is not able to decrypted by another node.

So can we conclude that hardware encryption is not working for both
robust unicast management frame and also unicast
data frame? I am confuse why on case 4, it is not working?

Please advice. Thanks

----
Chun-Yeow

On Tue, Sep 4, 2012 at 7:21 PM, Yeoh Chun-Yeow <[email protected]> wrote:
> Hi, Jouni
>
>> Depends on what those nodes were.. If they were both using the same
>> ath5k implementation, then no, that would not be enough.
> Both of nodes are using ath5k.
>
>> so working with AR2414 or even AR5213 does not necessarily mean that this would work
>> with AR5210, AR5211, or AR5212.
> Noted.
>
>> I would also verify that unicast data frames get processed in hardware and robust
>> management frames in software.
> I have retested with the following:
>
> mesh1: ath5k nohwcrypt=1
> mesh2: ath5k (no IEEE80211_KEY_FLAG_SW_MGMT)
> Result: Both of them are not able to ping each other. Also, the PREP
> action frame is not able to decrypted by another node.
>
> mesh1: ath5k nohwcrypt=1
> mesh2: ath5k nohwcrypt=1
> Result: They are able to ping each other.
>
> mesh1: ath5k nohwcrypt=1
> mesh2: ath5k (with IEEE80211_KEY_FLAG_SW_MGMT)
> Result: They are able to ping each other
>
> mesh1:
> mesh2I also try to enable back the IEEE80211_KEY_FLAG_SW_MGMT flags.
> Also it does not work. So can we conclude that hardware encryption is
> not working for both robust unicast management frame and also unicast
> data frame?
>
> Please advice. Thanks
>
> ----
> Chun-Yeow

2012-09-11 16:30:38

[permalink] [raw]

Subject: Re: [PATCH] ath5k: add support of HW encryption in management frames

This is why I asked for a bit of a summary.

What specific behaviour are you chasing from the keycache,
source/destination MAC address matching and encryption engine?

There've apparently been some keycache matching bugs in the past ..

Adrian

2012-09-11 15:05:35

[permalink] [raw]

Subject: Re: [PATCH] ath5k: add support of HW encryption in management frames

2012/9/11 Yeoh Chun-Yeow <[email protected]>:
> Hi, Nick
>
>> Have you tried disabing RX or TX only encryption/decryption on hw
>> trough PCU DIAG register ?
>
> Nope. Is that same with loading the ath5k with nohwcrypt=1 to do
> purely SW encryption/decryption?
>
> ----
> Chun-Yeow

Nope, with nohwcrypt we don't initialize the hw engine at all, we just
tell hw that data is not encrypted and that nothing should be done
about it. With these hw options from what I understand from docs we
can initialize the engine but use only part of it, eg. use only hw
encryption or only hw decryption. It might be useful in your case.

--
GPG ID: 0xEE878588
As you read this post global entropy rises. Have Fun ;-)
Nick

2012-09-04 11:34:46

[permalink] [raw]

Subject: Re: [PATCH] ath5k: add support of HW encryption in management frames

On Tue, 2012-09-04 at 19:25 +0800, Yeoh Chun-Yeow wrote:
> Hi, Jouni
>
> I have retested with the following:
>
> case 1 with submitted patch:
> mesh1: ath5k nohwcrypt=1
> mesh2: ath5k (no IEEE80211_KEY_FLAG_SW_MGMT)
> Result: Both of them are not able to ping each other. Also, the PREP
> action frame is not able to decrypted by another node.
>
> case 2 with submitted patch:
> mesh1: ath5k nohwcrypt=1
> mesh2: ath5k nohwcrypt=1
> Result: They are able to ping each other.
>
> case 3 with submitted patch but enable IEEE80211_KEY_FLAG_SW_MGMT:
> mesh1: ath5k nohwcrypt=1
> mesh2: ath5k (with IEEE80211_KEY_FLAG_SW_MGMT)
> Result: They are able to ping each other
>
> case 4 with submitted patch but enable IEEE80211_KEY_FLAG_SW_MGMT:
> mesh1: ath5k (with IEEE80211_KEY_FLAG_SW_MGMT)
> mesh2: ath5k (with IEEE80211_KEY_FLAG_SW_MGMT)
> Result: Both of them are not able to ping each other. Also, the PREP
> action frame is not able to decrypted by another node.
>
> So can we conclude that hardware encryption is not working for both
> robust unicast management frame and also unicast
> data frame? I am confuse why on case 4, it is not working?

I would guess that hardware *decryption* is faulty, maybe only one
action frame needs to be correct and so if one of them is nohwcrypt=1 it
still works?

johannes

2012-09-11 16:07:21

[permalink] [raw]

Subject: Re: [PATCH] ath5k: add support of HW encryption in management frames

2012/9/11 Nick Kossifidis <[email protected]>:
> 2012/9/11 Yeoh Chun-Yeow <[email protected]>:
>> Hi, Nick
>>
>>> Nope, with nohwcrypt we don't initialize the hw engine at all, we just
>>> tell hw that data is not encrypted and that nothing should be done
>>> about it. With these hw options from what I understand from docs we
>>> can initialize the engine but use only part of it, eg. use only hw
>>> encryption or only hw decryption. It might be useful in your case.
>>
>> The problem of ath5k right seems to be that: '
>> If CCMP HW encryption is enabled, HW encryption for unicast data frame
>> and SW encryption for unicast management frame are possible for TX.
>> But for RX, both the unicast data frame and unicast management frame
>> are decrypted by the HW instead of doing only SW decryption for
>> unicast management frame.
>>
>> With the help of PCU DIAG register, possible to separate this?
>>
>
> Its possible to tell hw to disable decryption so that you can handle
> that only on sw, I haven't tested it but that's what docs say.
>

To be more clear: you can tell the hw to *only* disable decryption and
keep doing encryption.

--
GPG ID: 0xEE878588
As you read this post global entropy rises. Have Fun ;-)
Nick

2012-09-04 09:28:41

[permalink] [raw]

Subject: Re: [PATCH] ath5k: add support of HW encryption in management frames

Hi, Johannes

> _How_ did you test this? Did you test that management frames are
> properly encrypted using AES CCM, and not mangled when decrypted?

I have setup the two mesh nodes using the secured mesh with the
following key installation:

/* key to encrypt/decrypt unicast data AND mgmt traffic to/from this peer */
install_key(&nlcfg, peer, CIPHER_CCMP, NL80211_KEYTYPE_PAIRWISE, 0, mtk);

I confirm that the hardware key for CCMP is set and
IEEE80211_KEY_FLAG_SW_MGMT is not enabled in mac80211-ops.c. Both
nodes are able to ping each others. Is this enough?

Please advice. Thanks

---
Chun-Yeow

2012-09-11 17:31:34

by Christian Lamparter

[permalink] [raw]

Subject: Re: [PATCH] ath5k: add support of HW encryption in management frames

On Tuesday 11 September 2012 18:36:27 Nick Kossifidis wrote:
> 2012/9/11 Yeoh Chun-Yeow <[email protected]>:
> >> To be more clear: you can tell the hw to *only* disable decryption and
> >> keep doing encryption.
> >
> > But the question here is that during Rx, need to do SW decryption for
> > management frame and then HW decryption for unicast data frame for all
> > the frame coming from the same STA. I still not so sure how to do
> > that.
> >
> > Jouni said that the workaround is to re-encrypt(incorrectly) received
> > robust unicast management frames if hwaccel for CCMP was configured
> > for the transmitting STA (this is to undo the incorrect decryption
> > done by the hardware) and then pass the encrypted frame to mac80211
> > for software decryption.
> >
>
> How about disabling acks on hw completely and handle them on sw ?
> This might keep the engine running ok for unicast frames.
>
> #define AR5K_DIAG_SW_DIS_ACK 0x00000002 /* Disable ACKs */
> #define AR5K_DIAG_SW_DIS_CTS 0x00000004 /* Disable CTSs */

No, CTS and ACKs are control frames and not management frames.

2012-09-11 17:48:48

[permalink] [raw]

Subject: Re: [PATCH] ath5k: add support of HW encryption in management frames

2012/9/11 Christian Lamparter <[email protected]>:
> On Tuesday 11 September 2012 18:36:27 Nick Kossifidis wrote:
>> 2012/9/11 Yeoh Chun-Yeow <[email protected]>:
>> >> To be more clear: you can tell the hw to *only* disable decryption and
>> >> keep doing encryption.
>> >
>> > But the question here is that during Rx, need to do SW decryption for
>> > management frame and then HW decryption for unicast data frame for all
>> > the frame coming from the same STA. I still not so sure how to do
>> > that.
>> >
>> > Jouni said that the workaround is to re-encrypt(incorrectly) received
>> > robust unicast management frames if hwaccel for CCMP was configured
>> > for the transmitting STA (this is to undo the incorrect decryption
>> > done by the hardware) and then pass the encrypted frame to mac80211
>> > for software decryption.
>> >
>>
>> How about disabling acks on hw completely and handle them on sw ?
>> This might keep the engine running ok for unicast frames.
>>
>> #define AR5K_DIAG_SW_DIS_ACK 0x00000002 /* Disable ACKs */
>> #define AR5K_DIAG_SW_DIS_CTS 0x00000004 /* Disable CTSs */
>
> No, CTS and ACKs are control frames and not management frames.

ACK :-)

--
GPG ID: 0xEE878588
As you read this post global entropy rises. Have Fun ;-)
Nick

2012-09-04 16:53:52

[permalink] [raw]

Subject: Re: [PATCH] ath5k: add support of HW encryption in management frames

Hi,

> > I would guess that hardware *decryption* is faulty, maybe only one
> > action frame needs to be correct and so if one of them is nohwcrypt=1 it
> > still works?
> Yes, you are correct. Case 3 is working only accidentally not always
> if the mesh node loaded with nohwcrypt=1 is reboot. So, with following
> case is also not working.
>
> mesh1: ath5k loaded without nohwcrypt=1 (with
> IEEE80211_KEY_FLAG_SW_MGMT enabled)
> mesh2: ath5k loaded without nohwcrypt=1 (with
> IEEE80211_KEY_FLAG_SW_MGMT enabled)
>
> Can we conclude that unicast data frames get processed in hardware and
> robust unicast management frames get processed in software for CCMP
> are not working.

I'm sure robust management frames in software *are* working, but
obviously the KEY_FLAG_SW_MGMT only affects transmit, not receive, so
receiving may still be broken.

> By the way, current secured mesh requires the AES CMAC to be enabled.
> But without enabling IEEE80211_HW_MFP_CAPABLE, the key cannot be added
> since this cipher suite is considered not supported. But actually AES
> CMAC can be done in software. Any work around on this?

Well, that's a separate question. Of course we could enable it, but what
would the point be? You don't have CCM for management frames right now,
so CMAC is pretty useless? And if you had CCM for management frames, say
with the things discussed in the p54 thread, then you could just enable
both, right?

johannes

2012-09-11 15:51:48

[permalink] [raw]

Subject: Re: [PATCH] ath5k: add support of HW encryption in management frames

Hi, Nick

> Nope, with nohwcrypt we don't initialize the hw engine at all, we just
> tell hw that data is not encrypted and that nothing should be done
> about it. With these hw options from what I understand from docs we
> can initialize the engine but use only part of it, eg. use only hw
> encryption or only hw decryption. It might be useful in your case.

The problem of ath5k right seems to be that: '
If CCMP HW encryption is enabled, HW encryption for unicast data frame
and SW encryption for unicast management frame are possible for TX.
But for RX, both the unicast data frame and unicast management frame
are decrypted by the HW instead of doing only SW decryption for
unicast management frame.

With the help of PCU DIAG register, possible to separate this?

-----
Chun-Yeow

> --
> GPG ID: 0xEE878588
> As you read this post global entropy rises. Have Fun ;-)
> Nick

2012-09-04 01:03:07

[permalink] [raw]

Subject: Re: [PATCH] ath5k: add support of HW encryption in management frames

Hi, Adrian

I have tested with secured mesh on AR5213A, AR5413 and AR2413/AR2414.
It seems work for me.

> I suggest that you at least wrap this in a run-time configuration item
> that defaults to off or something.
Yes, I do agree on this. But if more peoples are able to test it and
confirm, it would be better.

Thanks

Regards,
Chun-Yeow

On Tue, Sep 4, 2012 at 3:24 AM, Adrian Chadd <[email protected]> wrote:
> On 31 August 2012 08:24, Yeoh Chun-Yeow <[email protected]> wrote:
>> Hi, Adrian
>>
>> Appreciate your testing on this.
>>
>
> Hi,
>
> I don't have time to test ath5k stuff. I'm just an Atheros employee
> who hacks on FreeBSD in his spare time. I'm lurking to provide
> feedback. :-)
>
> I suggest that you at least wrap this in a run-time configuration item
> that defaults to off or something. i'd be really worried about this
> stuff having subtle bugs that aren't well documented; it may work for
> you but it may induce weird behaviour in other setups.
>
>
>
> Adrian

2012-09-11 16:39:24

[permalink] [raw]

Subject: Re: [PATCH] ath5k: add support of HW encryption in management frames

On Tue, 2012-09-11 at 19:36 +0300, Nick Kossifidis wrote:

> > Jouni said that the workaround is to re-encrypt(incorrectly) received
> > robust unicast management frames if hwaccel for CCMP was configured
> > for the transmitting STA (this is to undo the incorrect decryption
> > done by the hardware) and then pass the encrypted frame to mac80211
> > for software decryption.
> >
>
> How about disabling acks on hw completely and handle them on sw ? This
> might keep the engine running ok for unicast frames.

That would achieve what? Even if you could do something with that ...
the cure would be worse than the disease for sure!

(I'd also appreciate if you guys could drop me from this thread, I have
little interest in ath5k I'm just the person pointing out that it isn't
as easy to enable MFP as it seems)

johannes

2012-09-04 09:31:05

[permalink] [raw]

Subject: Re: [PATCH] ath5k: add support of HW encryption in management frames

On Tue, 2012-09-04 at 17:28 +0800, Yeoh Chun-Yeow wrote:
> Hi, Johannes
>
> > _How_ did you test this? Did you test that management frames are
> > properly encrypted using AES CCM, and not mangled when decrypted?
>
> I have setup the two mesh nodes using the secured mesh with the
> following key installation:
>
> /* key to encrypt/decrypt unicast data AND mgmt traffic to/from this peer */
> install_key(&nlcfg, peer, CIPHER_CCMP, NL80211_KEYTYPE_PAIRWISE, 0, mtk);
>
> I confirm that the hardware key for CCMP is set and
> IEEE80211_KEY_FLAG_SW_MGMT is not enabled in mac80211-ops.c. Both
> nodes are able to ping each others. Is this enough?

I don't know? Did this encrypt any management frames with AES-CCM and
rely on them being exchanged properly?

johannes

2012-09-11 06:47:00

[permalink] [raw]

Subject: Re: [PATCH] ath5k: add support of HW encryption in management frames

2012/9/10 Yeoh Chun-Yeow <[email protected]>:
> Hi, all
>
> For your information, my submitted patch has allowed me to do the
> following and mainly to setup the secured mesh 802.11s using authsae:
>
> 1. Key installations for the following:
> /* key to protect integrity of multicast mgmt frames tx*/
> install_key(nlcfg, NULL, CIPHER_AES_CMAC, NL80211_KEYTYPE_GROUP, 4, mgtk_tx);
> /* key to encrypt multicast data traffic */
> install_key(nlcfg, NULL, CIPHER_CCMP, NL80211_KEYTYPE_GROUP, 0, mgtk_tx);
> /* key to encrypt/decrypt unicast data AND mgmt traffic to/from this peer */
> install_key(&nlcfg, peer, CIPHER_CCMP, NL80211_KEYTYPE_PAIRWISE, 0, mtk);
> /* key to decrypt multicast data traffic from this peer */
> install_key(&nlcfg, peer, CIPHER_CCMP, NL80211_KEYTYPE_GROUP, 0, peer_mgtk);
> /* to check integrity of multicast mgmt frames from this peer */
> install_key(&nlcfg, peer, CIPHER_AES_CMAC, NL80211_KEYTYPE_GROUP, 4, peer_mgtk);
>
> 2. By using the submitted patch, how ever as Jouni has pointed out
> that testing with ath5k implementation alone may not be correct, due
> to the following statement:
> If the CCMP processing is done incorrectly, they could both mangle the
> results in the same way to hide the issue.
>
> thus I revert back by not disabling the IEEE80211_KEY_FLAG_SW_MGMT.
> However, with this, it has showed that robust unicast management frame
> is encrypted in SW but is decrypted wrongly in SW (perhaps HW decrypt
> it due to the HW accl enabling for unicast data frame).
>
> Hope this help.
>
> Thanks.
>
> Regards,
> Chun-Yeow
>
> On Mon, Sep 10, 2012 at 9:13 PM, Kalle Valo <[email protected]> wrote:
>> Adrian Chadd <[email protected]> writes:
>>
>>> Yeoh - can you please email me privately with a summary of what you
>>> implemented, what you've tested and what worked / what didn't work?
>>
>> Why privately? Better to have all the information public, you never know
>> if someone finds the info from the web and picks up the work.
>>
>> --
>> Kalle Valo

Have you tried disabing RX or TX only encryption/decryption on hw
trough PCU DIAG register ?

--
GPG ID: 0xEE878588
As you read this post global entropy rises. Have Fun ;-)
Nick

2012-09-04 11:06:03

[permalink] [raw]

Subject: Re: [PATCH] ath5k: add support of HW encryption in management frames

On Tue, Sep 04, 2012 at 05:28:40PM +0800, Yeoh Chun-Yeow wrote:
> Hi, Johannes
>
> > _How_ did you test this? Did you test that management frames are
> > properly encrypted using AES CCM, and not mangled when decrypted?
>
> I have setup the two mesh nodes using the secured mesh with the
> following key installation:
>
> /* key to encrypt/decrypt unicast data AND mgmt traffic to/from this peer */
> install_key(&nlcfg, peer, CIPHER_CCMP, NL80211_KEYTYPE_PAIRWISE, 0, mtk);
>
> I confirm that the hardware key for CCMP is set and
> IEEE80211_KEY_FLAG_SW_MGMT is not enabled in mac80211-ops.c. Both
> nodes are able to ping each others. Is this enough?

Depends on what those nodes were.. If they were both using the same
ath5k implementation, then no, that would not be enough. If the CCMP
processing is done incorrectly, they could both mangle the results in
the same way to hide the issue. It should also be noted that there has
been key cache changes between hardware revisions, so working with
AR2414 or even AR5213 does not necessarily mean that this would work
with AR5210, AR5211, or AR5212.

You would need to test an ath5k-based device with another device that is
known to handle unicast robust management frame protection correctly. If
you do not have a suitable other device for this, it should be possible
to force one of the devices to use software encryption for everything
(i.e., make sure it does not configure any CCMP keys in the hardware key
cache) and then run a test that exchanges robust unicast management
frames (both TX and RX using the modified ath5k driver). I would also
verify that unicast data frames get processed in hardware and robust
management frames in software.

--
Jouni Malinen PGP id EFC895FA

2012-09-05 08:04:22

[permalink] [raw]

Subject: Re: [PATCH] ath5k: add support of HW encryption in management frames

On Wed, Sep 05, 2012 at 03:31:08PM +0800, Yeoh Chun-Yeow wrote:
> I am based on the authsae source code for secured mesh setup which can
> be found at:
> https://github.com/cozybit/authsae/blob/master/linux/meshd-nl80211.c

It looks like this particular implementation is hardcoded to use MFP..

> > Any pointers to the specific standard clause(s) that say that?
> I have not gone through the standard on this.

.. while the standard does not actually have such requirement as far as
I can tell. I have nothing against adding support for MFP in general,
but just wanted to understand where this assumed requirement came from.

So yes, if you want to enable support for MFP, you cannot do it unless
the driver is able to handle both CCMP and BIP protection for robust
management frames. In case of ath5k, I would assume there are two
options:
- only enable MFP if software encryption is used for all frames (i.e.,
no hwaccel even for data frames)
- implement workaround to re-encrypt(incorrectly) received robust
unicast management frames if hwaccel for CCMP was configured for the
transmitting STA (this is to undo the incorrect decryption done by the
hardware) and then pass the encrypted frame to mac80211 for software
decryption; with this option, you could advertise MFP support even
with CCMP hwaccel enabled

--
Jouni Malinen PGP id EFC895FA

2012-09-04 08:11:06

[permalink] [raw]

Subject: Re: [PATCH] ath5k: add support of HW encryption in management frames

On Tue, 2012-08-28 at 17:34 +0800, Chun-Yeow Yeoh wrote:
> This patch provides the support of hardware encyrption for
> management frame, including the support of AES CMAC. This
> patch is tested with the following chipsets:
> - AR5213A
> - AR5413
> - AR2413/AR2414

_How_ did you test this? Did you test that management frames are
properly encrypted using AES CCM, and not mangled when decrypted? That's
the concern, see my other mail to Christian on his carl9170 patch.

johannes

2012-09-04 16:41:18

[permalink] [raw]

Subject: Re: [PATCH] ath5k: add support of HW encryption in management frames

Hi, Johannes

> I would guess that hardware *decryption* is faulty, maybe only one
> action frame needs to be correct and so if one of them is nohwcrypt=1 it
> still works?
Yes, you are correct. Case 3 is working only accidentally not always
if the mesh node loaded with nohwcrypt=1 is reboot. So, with following
case is also not working.

mesh1: ath5k loaded without nohwcrypt=1 (with
IEEE80211_KEY_FLAG_SW_MGMT enabled)
mesh2: ath5k loaded without nohwcrypt=1 (with
IEEE80211_KEY_FLAG_SW_MGMT enabled)

Can we conclude that unicast data frames get processed in hardware and
robust unicast management frames get processed in software for CCMP
are not working.

By the way, current secured mesh requires the AES CMAC to be enabled.
But without enabling IEEE80211_HW_MFP_CAPABLE, the key cannot be added
since this cipher suite is considered not supported. But actually AES
CMAC can be done in software. Any work around on this?

---
Chun-Yeow

2012-09-05 06:52:09

[permalink] [raw]

Subject: Re: [PATCH] ath5k: add support of HW encryption in management frames

Hi, Johannes

> Well, that's a separate question. Of course we could enable it, but what
> would the point be? You don't have CCM for management frames right now,
> so CMAC is pretty useless? And if you had CCM for management frames, say
> with the things discussed in the p54 thread, then you could just enable
> both, right?

We will load the ath5k with nohwcrypt=, so no hardware encryption at
all. But to proceed with key installation using AES CMAC to be
processed in software, we need somethings related to:
http://www.spinics.net/lists/linux-wireless/msg96187.html

Otherwise, we may need to bypass the cfg80211_supported_cipher_suite checking:
http://www.spinics.net/lists/linux-wireless/msg95951.html

Is this relevant?

---
Chun-Yeow

2012-09-11 16:20:03

[permalink] [raw]

Subject: Re: [PATCH] ath5k: add support of HW encryption in management frames

> To be more clear: you can tell the hw to *only* disable decryption and
> keep doing encryption.

But the question here is that during Rx, need to do SW decryption for
management frame and then HW decryption for unicast data frame for all
the frame coming from the same STA. I still not so sure how to do
that.

Jouni said that the workaround is to re-encrypt(incorrectly) received
robust unicast management frames if hwaccel for CCMP was configured
for the transmitting STA (this is to undo the incorrect decryption
done by the hardware) and then pass the encrypted frame to mac80211
for software decryption.

----
Chun-Yeow

2012-09-05 07:31:09

[permalink] [raw]

Subject: Re: [PATCH] ath5k: add support of HW encryption in management frames

Hi, Jouni

> Could you please describe what exactly you mean with "current secured
> mesh requires the AES CMAC to be enabled" and what is that claim based
> on?
I am based on the authsae source code for secured mesh setup which can
be found at:
https://github.com/cozybit/authsae/blob/master/linux/meshd-nl80211.c

There are total 5 keys for secured mesh:

/* key to protect integrity of multicast mgmt frames tx*/
install_key(nlcfg, NULL, CIPHER_AES_CMAC, NL80211_KEYTYPE_GROUP, 4, mgtk_tx);
/* key to encrypt multicast data traffic */
install_key(nlcfg, NULL, CIPHER_CCMP, NL80211_KEYTYPE_GROUP, 0, mgtk_tx);
/* key to encrypt/decrypt unicast data AND mgmt traffic to/from this peer */
install_key(&nlcfg, peer, CIPHER_CCMP, NL80211_KEYTYPE_PAIRWISE, 0, mtk);
/* key to decrypt multicast data traffic from this peer */
install_key(&nlcfg, peer, CIPHER_CCMP, NL80211_KEYTYPE_GROUP, 0, peer_mgtk);
/* to check integrity of multicast mgmt frames from this peer */
install_key(&nlcfg, peer, CIPHER_AES_CMAC, NL80211_KEYTYPE_GROUP, 4, peer_mgtk);

> Any pointers to the specific standard clause(s) that say that?
I have not gone through the standard on this.

-----
Chun-Yeow

2012-09-07 03:51:02

[permalink] [raw]

Subject: Re: [PATCH] ath5k: add support of HW encryption in management frames

> So yes, if you want to enable support for MFP, you cannot do it unless
> the driver is able to handle both CCMP and BIP protection for robust
> management frames. In case of ath5k, I would assume there are two
> options:
> - only enable MFP if software encryption is used for all frames (i.e.,
> no hwaccel even for data frames)
> - implement workaround to re-encrypt(incorrectly) received robust
> unicast management frames if hwaccel for CCMP was configured for the
> transmitting STA (this is to undo the incorrect decryption done by the
> hardware) and then pass the encrypted frame to mac80211 for software
> decryption; with this option, you could advertise MFP support even
> with CCMP hwaccel enabled

No luck and still cannot work out how to re-encrypt the received
robust unicast management frames if hwaccel for CCMP was configured.
Need to stick with first option then.

---
Chun-Yeow

2012-09-05 07:12:01

[permalink] [raw]

Subject: Re: [PATCH] ath5k: add support of HW encryption in management frames

On Tue, Sep 04, 2012 at 01:35:21PM +0200, Johannes Berg wrote:
> I would guess that hardware *decryption* is faulty, maybe only one
> action frame needs to be correct and so if one of them is nohwcrypt=1 it
> still works?

Yes, I was assuming that receiving robust unicast management frames
would fail with ath5k. This is exactly the reason why I adding
IEEE80211_HW_MFP_CAPABLE conditionally in ath9k only for the hardware
revisions that support the new CCMP rules.

It should be possible to work around this if someone really cares, but
that require the driver to re-encrypt the incorrectly decrypted frame
(i.e., using the pre-802.11w rules on CCMP exactly in the way the
hardware did it) and then pass that end result for masc80211 to decrypt
it correctly. I considered doing that for a while, but did not find
enough justification to work with that.

--
Jouni Malinen PGP id EFC895FA

2012-09-04 16:55:25

by Christian Lamparter

[permalink] [raw]

Subject: Re: [PATCH] ath5k: add support of HW encryption in management frames

On Tuesday 04 September 2012 18:41:16 Yeoh Chun-Yeow wrote:
> Hi, Johannes
>
> > I would guess that hardware *decryption* is faulty, maybe only one
> > action frame needs to be correct and so if one of them is nohwcrypt=1 it
> > still works?
> Yes, you are correct. Case 3 is working only accidentally not always
> if the mesh node loaded with nohwcrypt=1 is reboot. So, with following
> case is also not working.
>
> mesh1: ath5k loaded without nohwcrypt=1 (with
> IEEE80211_KEY_FLAG_SW_MGMT enabled)
> mesh2: ath5k loaded without nohwcrypt=1 (with
> IEEE80211_KEY_FLAG_SW_MGMT enabled)
>
> Can we conclude that unicast data frames get processed in hardware and
> robust unicast management frames get processed in software for CCMP
> are not working.
>
> By the way, current secured mesh requires the AES CMAC to be enabled.
> But without enabling IEEE80211_HW_MFP_CAPABLE, the key cannot be added
> since this cipher suite is considered not supported. But actually AES
> CMAC can be done in software. Any work around on this?
>
I think you can override this by supplying your own custom set
of available ciphers. This is done by setting
hw.wiphy->cipher_suites
hw.wiphy->n_cipher_suites
during the initalization of the driver (i.e.: before
ieee80211_register_hw(hw) is called).

Regards,
Chr

2012-09-03 19:24:13

[permalink] [raw]

Subject: Re: [PATCH] ath5k: add support of HW encryption in management frames

On 31 August 2012 08:24, Yeoh Chun-Yeow <[email protected]> wrote:
> Hi, Adrian
>
> Appreciate your testing on this.
>

Hi,

I don't have time to test ath5k stuff. I'm just an Atheros employee
who hacks on FreeBSD in his spare time. I'm lurking to provide
feedback. :-)

I suggest that you at least wrap this in a run-time configuration item
that defaults to off or something. i'd be really worried about this
stuff having subtle bugs that aren't well documented; it may work for
you but it may induce weird behaviour in other setups.

Adrian

2012-09-04 11:21:24