diff mbox

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

Message ID 20170610025912.6499-7-Jason@zx2c4.com (mailing list archive)
State Accepted
Delegated to: Johannes Berg
Headers show

Commit Message

Jason A. Donenfeld June 10, 2017, 2:59 a.m. UTC 
Otherwise, we enable all sorts of forgeries via timing attack.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: linux-wireless@vger.kernel.org
Cc: stable@vger.kernel.org
---
 net/mac80211/wpa.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

Comments

Johannes Berg June 13, 2017, 8:20 a.m. UTC | #1 
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
Jason A. Donenfeld June 13, 2017, 1:28 p.m. UTC | #2 
On Tue, Jun 13, 2017 at 10:20 AM, Johannes Berg
<johannes@sipsolutions.net> 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.
diff mbox

Patch

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 <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;
 		}