Re: [PATCH 6/6] mac80211/wpa: use constant time memory comparison for MACs

[Jason A. Donenfeld]

On Tue, Jun 13, 2017 at 10:20 AM, Johannes Berg
 wrote:
> I'm not really sure that this is actually true, since you don't get
> much feedback on your frame that's dropped, especially if you're
> attacking from remote. Basically, I don't see how you can observe the
> timing of this operation?

There have been practical attacks published before that relied on
jitter coming from simultaneous operations.

> Anyway, applied.

Great, thanks.

Re: [PATCH 6/6] mac80211/wpa: use constant time memory comparison for MACs

[Jason A. Donenfeld]

On Tue, Jun 13, 2017 at 10:20 AM, Johannes Berg
 wrote:
> I'm not really sure that this is actually true, since you don't get
> much feedback on your frame that's dropped, especially if you're
> attacking from remote. Basically, I don't see how you can observe the
> timing of this operation?

There have been practical attacks published before that relied on
jitter coming from simultaneous operations.

> Anyway, applied.

Great, thanks.

Re: [PATCH 6/6] mac80211/wpa: use constant time memory comparison for MACs

[Johannes Berg]

On Sat, 2017-06-10 at 04:59 +0200, Jason A. Donenfeld wrote:
> Otherwise, we enable all sorts of forgeries via timing attack.

I'm not really sure that this is actually true, since you don't get
much feedback on your frame that's dropped, especially if you're
attacking from remote. Basically, I don't see how you can observe the
timing of this operation?

Anyway, applied.

johannes

Re: [PATCH 6/6] mac80211/wpa: use constant time memory comparison for MACs

[Johannes Berg]

On Sat, 2017-06-10 at 04:59 +0200, Jason A. Donenfeld wrote:
> Otherwise, we enable all sorts of forgeries via timing attack.

I'm not really sure that this is actually true, since you don't get
much feedback on your frame that's dropped, especially if you're
attacking from remote. Basically, I don't see how you can observe the
timing of this operation?

Anyway, applied.

johannes

[PATCH 6/6] mac80211/wpa: use constant time memory comparison for MACs

[Jason A. Donenfeld]

Otherwise, we enable all sorts of forgeries via timing attack.

Signed-off-by: Jason A. Donenfeld 
Cc: Johannes Berg 
Cc: linux-wirel...@vger.kernel.org
Cc: sta...@vger.kernel.org
---
 net/mac80211/wpa.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/net/mac80211/wpa.c b/net/mac80211/wpa.c
index c1ef22df865f..cc19614ff4e6 100644
--- a/net/mac80211/wpa.c
+++ b/net/mac80211/wpa.c
@@ -17,6 +17,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "ieee80211_i.h"
 #include "michael.h"
@@ -153,7 +154,7 @@ ieee80211_rx_h_michael_mic_verify(struct ieee80211_rx_data 
*rx)
data_len = skb->len - hdrlen - MICHAEL_MIC_LEN;
key = >key->conf.key[NL80211_TKIP_DATA_OFFSET_RX_MIC_KEY];
michael_mic(key, hdr, data, data_len, mic);
-   if (memcmp(mic, data + data_len, MICHAEL_MIC_LEN) != 0)
+   if (crypto_memneq(mic, data + data_len, MICHAEL_MIC_LEN))
goto mic_fail;
 
/* remove Michael MIC from payload */
@@ -1048,7 +1049,7 @@ ieee80211_crypto_aes_cmac_decrypt(struct 
ieee80211_rx_data *rx)
bip_aad(skb, aad);
ieee80211_aes_cmac(key->u.aes_cmac.tfm, aad,
   skb->data + 24, skb->len - 24, mic);
-   if (memcmp(mic, mmie->mic, sizeof(mmie->mic)) != 0) {
+   if (crypto_memneq(mic, mmie->mic, sizeof(mmie->mic))) {
key->u.aes_cmac.icverrors++;
return RX_DROP_UNUSABLE;
}
@@ -1098,7 +1099,7 @@ ieee80211_crypto_aes_cmac_256_decrypt(struct 
ieee80211_rx_data *rx)
bip_aad(skb, aad);
ieee80211_aes_cmac_256(key->u.aes_cmac.tfm, aad,
   skb->data + 24, skb->len - 24, mic);
-   if (memcmp(mic, mmie->mic, sizeof(mmie->mic)) != 0) {
+   if (crypto_memneq(mic, mmie->mic, sizeof(mmie->mic))) {
key->u.aes_cmac.icverrors++;
return RX_DROP_UNUSABLE;
}
@@ -1202,7 +1203,7 @@ ieee80211_crypto_aes_gmac_decrypt(struct 
ieee80211_rx_data *rx)
if (ieee80211_aes_gmac(key->u.aes_gmac.tfm, aad, nonce,
   skb->data + 24, skb->len - 24,
   mic) < 0 ||
-   memcmp(mic, mmie->mic, sizeof(mmie->mic)) != 0) {
+   crypto_memneq(mic, mmie->mic, sizeof(mmie->mic))) {
key->u.aes_gmac.icverrors++;
return RX_DROP_UNUSABLE;
}
-- 
2.13.1

[PATCH 6/6] mac80211/wpa: use constant time memory comparison for MACs

[Jason A. Donenfeld]

Otherwise, we enable all sorts of forgeries via timing attack.

Signed-off-by: Jason A. Donenfeld 
Cc: Johannes Berg 
Cc: linux-wirel...@vger.kernel.org
Cc: sta...@vger.kernel.org
---
 net/mac80211/wpa.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/net/mac80211/wpa.c b/net/mac80211/wpa.c
index c1ef22df865f..cc19614ff4e6 100644
--- a/net/mac80211/wpa.c
+++ b/net/mac80211/wpa.c
@@ -17,6 +17,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "ieee80211_i.h"
 #include "michael.h"
@@ -153,7 +154,7 @@ ieee80211_rx_h_michael_mic_verify(struct ieee80211_rx_data 
*rx)
data_len = skb->len - hdrlen - MICHAEL_MIC_LEN;
key = >key->conf.key[NL80211_TKIP_DATA_OFFSET_RX_MIC_KEY];
michael_mic(key, hdr, data, data_len, mic);
-   if (memcmp(mic, data + data_len, MICHAEL_MIC_LEN) != 0)
+   if (crypto_memneq(mic, data + data_len, MICHAEL_MIC_LEN))
goto mic_fail;
 
/* remove Michael MIC from payload */
@@ -1048,7 +1049,7 @@ ieee80211_crypto_aes_cmac_decrypt(struct 
ieee80211_rx_data *rx)
bip_aad(skb, aad);
ieee80211_aes_cmac(key->u.aes_cmac.tfm, aad,
   skb->data + 24, skb->len - 24, mic);
-   if (memcmp(mic, mmie->mic, sizeof(mmie->mic)) != 0) {
+   if (crypto_memneq(mic, mmie->mic, sizeof(mmie->mic))) {
key->u.aes_cmac.icverrors++;
return RX_DROP_UNUSABLE;
}
@@ -1098,7 +1099,7 @@ ieee80211_crypto_aes_cmac_256_decrypt(struct 
ieee80211_rx_data *rx)
bip_aad(skb, aad);
ieee80211_aes_cmac_256(key->u.aes_cmac.tfm, aad,
   skb->data + 24, skb->len - 24, mic);
-   if (memcmp(mic, mmie->mic, sizeof(mmie->mic)) != 0) {
+   if (crypto_memneq(mic, mmie->mic, sizeof(mmie->mic))) {
key->u.aes_cmac.icverrors++;
return RX_DROP_UNUSABLE;
}
@@ -1202,7 +1203,7 @@ ieee80211_crypto_aes_gmac_decrypt(struct 
ieee80211_rx_data *rx)
if (ieee80211_aes_gmac(key->u.aes_gmac.tfm, aad, nonce,
   skb->data + 24, skb->len - 24,
   mic) < 0 ||
-   memcmp(mic, mmie->mic, sizeof(mmie->mic)) != 0) {
+   crypto_memneq(mic, mmie->mic, sizeof(mmie->mic))) {
key->u.aes_gmac.icverrors++;
return RX_DROP_UNUSABLE;
}
-- 
2.13.1