FAILED: patch "[PATCH] mac80211/wpa: use constant time memory comparison for MACs" failed to apply to 3.18-stable tree

PATCH Superseded [email protected] 4 participants 6 messages Jun 18, 2017 – Jun 27, 2017

Subsystems (2)

Stable branch mac80211

lore

Share & tools

lore

[email protected]

author maintainer

2017-06-18 00:24 UTC #0 lore raw

The patch below does not apply to the 3.18-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

Quoted 1 line

From 98c67d187db7808b1f3c95f2110dd4392d034182 Mon Sep 17 00:00:00 2001

From: "Jason A. Donenfeld" <[email protected]>
Date: Sat, 10 Jun 2017 04:59:12 +0200
Subject: [PATCH] mac80211/wpa: use constant time memory comparison for MACs

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

Signed-off-by Jason A. Donenfeld <[email protected]>

Cc Johannes Berg <[email protected]>

Cc [email protected]

Signed-off-by Johannes Berg <[email protected]>

net/mac80211/wpa.c +5 -4

index c1ef22df865f..cc19614ff4e6 100644
@@ -17,6 +17,7 @@
 #include <asm/unaligned.h>
 #include <net/mac80211.h>
 #include <crypto/aes.h>
+#include <crypto/algapi.h>
 
 #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 = &rx->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;
 		}

Jason A. Donenfeld

2017-06-18 19:19 UTC #1 lore raw

PATCH 1/1 mac80211/wpa: use constant time memory comparison for MACs

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

Signed-off-by Jason A. Donenfeld <[email protected]>

Cc Johannes Berg <[email protected]>

Cc [email protected]

Signed-off-by Johannes Berg <[email protected]>

---
Here's the backport for 3.18.

 net/mac80211/wpa.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

net/mac80211/wpa.c +4 -3

index 983527a4c1ab..49592c7e4199 100644
@@ -16,6 +16,7 @@
 #include <asm/unaligned.h>
 #include <net/mac80211.h>
 #include <crypto/aes.h>
+#include <crypto/algapi.h>
 
 #include "ieee80211_i.h"
 #include "michael.h"

@@ -150,7 +151,7 @@ ieee80211_rx_h_michael_mic_verify(struct ieee80211_rx_data *rx)
 	data_len = skb->len - hdrlen - MICHAEL_MIC_LEN;
 	key = &rx->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) != 0)
 		goto mic_fail;
 
 	/* remove Michael MIC from payload */

@@ -520,7 +521,7 @@ ieee80211_crypto_ccmp_decrypt(struct ieee80211_rx_data *rx)
 
 	queue = rx->security_idx;
 
-	if (memcmp(pn, key->u.ccmp.rx_pn[queue], IEEE80211_CCMP_PN_LEN) <= 0) {
+	if (crypto_memneq(pn, key->u.ccmp.rx_pn[queue], IEEE80211_CCMP_PN_LEN) <= 0) {
 		key->u.ccmp.replays++;
 		return RX_DROP_UNUSABLE;
 	}

@@ -771,7 +772,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)) != 0) {
 			key->u.aes_cmac.icverrors++;
 			return RX_DROP_UNUSABLE;
 		}

Signature · 1 line

2.13.1

Emmanuel Grumbach

2017-06-18 20:31 UTC #2 lore raw

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

Quoted 1 line · Jason A. Donenfeld

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

crypto_memneq's description says:

Returns 0 when data is equal, 1 otherwise.

Clearly this is not suitable here. You are allowing replay attacks... For network drivers, this is worse than timing attacks. You still need to explain how you can exploit timing attacks *on a remote system*. On your local system, threads are impacted etc... Fine. On a remote system (you are in Rx path here..) how do you exploit them?

Quoted 54 lines

Signed-off-by Jason A. Donenfeld <[email protected]>

Cc Johannes Berg <[email protected]>

Cc [email protected]

Signed-off-by Johannes Berg <[email protected]>

---
Here's the backport for 3.18.

 net/mac80211/wpa.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

net/mac80211/wpa.c +4 -4

index 983527a4c1ab..49592c7e4199 100644
@@ -16,6 +16,7 @@
 #include <asm/unaligned.h>
 #include <net/mac80211.h>
 #include <crypto/aes.h>
+#include <crypto/algapi.h>
 
 #include "ieee80211_i.h"
 #include "michael.h"

@@ -150,7 +151,7 @@ ieee80211_rx_h_michael_mic_verify(struct ieee80211_rx_data *rx)
        data_len = skb->len - hdrlen - MICHAEL_MIC_LEN;
        key = &rx->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) != 0)
                goto mic_fail;
 
        /* remove Michael MIC from payload */

@@ -520,7 +521,7 @@ ieee80211_crypto_ccmp_decrypt(struct ieee80211_rx_data *rx)
 
        queue = rx->security_idx;
 
-       if (memcmp(pn, key->u.ccmp.rx_pn[queue], IEEE80211_CCMP_PN_LEN) <= 0) {
+       if (crypto_memneq(pn, key->u.ccmp.rx_pn[queue], IEEE80211_CCMP_PN_LEN) <= 0) {
                key->u.ccmp.replays++;
                return RX_DROP_UNUSABLE;
        }

@@ -771,7 +772,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)) != 0) {
                        key->u.aes_cmac.icverrors++;
                        return RX_DROP_UNUSABLE;
                }
--

2.13.1

Johannes Berg

maintainer

2017-06-18 20:44 UTC #3 lore raw

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

Quoted 5 lines · Emmanuel Grumbach

On Sun, Jun 18, 2017 at 10:18 PM, Jason A. Donenfeld <[email protected]
Quoted 2 lines
wrote:
Otherwise, we enable all sorts of forgeries via timing attack.
crypto_memneq's description says:

[...]

Quoted 2 lines

Quoted 2 lines
---
Here's the backport for 3.18.

Yeah, not sure what happened here, but ...

Quoted 6 lines

Quoted 6 lines
#include "ieee80211_i.h"
#include "michael.h"
(quoted diff) +0 -0
@@ -150,7 +151,7 @@ ieee80211_rx_h_michael_mic_verify(struct
 ieee80211_rx_data *rx)
         data_len = skb->len - hdrlen - MICHAEL_MIC_LEN;
key = &rx->key-

Quoted 6 lines

Quoted 6 lines
Quoted 1 line
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) !=
0)
                goto mic_fail;

This is obviously wrong and not like that in the original,

Quoted 6 lines

/* remove Michael MIC from payload */

net/mac80211/wpa.c +0 -0

@@ -520,7 +521,7 @@ ieee80211_crypto_ccmp_decrypt(struct
 ieee80211_rx_data *rx)
 
         queue = rx->security_idx;

Quoted 7 lines

(quoted diff) +1 -1

-       if (memcmp(pn, key->u.ccmp.rx_pn[queue],
 IEEE80211_CCMP_PN_LEN) <= 0) {
+       if (crypto_memneq(pn, key->u.ccmp.rx_pn[queue],
 IEEE80211_CCMP_PN_LEN) <= 0) {

                key->u.ccmp.replays++;
                return RX_DROP_UNUSABLE;
        }

this isn't in the original at all, and clearly shouldn't be here,

Quoted 10 lines

(quoted diff) +1 -1

@@ -771,7 +772,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-

Quoted 1 line

mic)) != 0) {

and this is just as wrong as the first one.

johannes

Jason A. Donenfeld

2017-06-19 16:46 UTC #4 lore raw

PATCH v2 1/1 mac80211/wpa: use constant time memory comparison for MACs

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

Signed-off-by Jason A. Donenfeld <[email protected]>

Cc Johannes Berg <[email protected]>

Cc [email protected]

Signed-off-by Johannes Berg <[email protected]>

---
This is for 3.18. Tested this, and it works as intended.

 net/mac80211/wpa.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

net/mac80211/wpa.c +3 -2

index 983527a4c1ab..bf87de469c03 100644
@@ -16,6 +16,7 @@
 #include <asm/unaligned.h>
 #include <net/mac80211.h>
 #include <crypto/aes.h>
+#include <crypto/algapi.h>
 
 #include "ieee80211_i.h"
 #include "michael.h"

@@ -150,7 +151,7 @@ ieee80211_rx_h_michael_mic_verify(struct ieee80211_rx_data *rx)
 	data_len = skb->len - hdrlen - MICHAEL_MIC_LEN;
 	key = &rx->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 */

@@ -771,7 +772,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;
 		}

Signature · 1 line

2.13.1

Greg KH

author maintainer

2017-06-27 11:32 UTC #5 lore raw

Re: [PATCH v2 3.18-stable] mac80211/wpa: use constant time memory comparison for MACs

Quoted 9 lines · Jason A. Donenfeld

Otherwise, we enable all sorts of forgeries via timing attack.
Signed-off-by Jason A. Donenfeld <[email protected]>
Cc Johannes Berg <[email protected]>
Cc [email protected]
Cc [email protected]
Signed-off-by Johannes Berg <[email protected]>
---
This is for 3.18. Tested this, and it works as intended.

Thanks, now applied.

greg k-h