* [PATCH] mac80211/wpa: use constant time memory comparison for MACs [not found] <1497720871224217@kroah.com> @ 2017-06-18 19:18 ` Jason A. Donenfeld 2017-06-18 20:31 ` Emmanuel Grumbach 0 siblings, 1 reply; 5+ messages in thread From: Jason A. Donenfeld @ 2017-06-18 19:18 UTC (permalink / raw) To: gregkh Cc: Jason A. Donenfeld, Johannes Berg, linux-wireless, stable, Johannes Berg 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 Signed-off-by: Johannes Berg <johannes.berg@intel.com> --- Here's the backport for 3.18. net/mac80211/wpa.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/net/mac80211/wpa.c b/net/mac80211/wpa.c index 983527a4c1ab..49592c7e4199 100644 --- a/net/mac80211/wpa.c +++ b/net/mac80211/wpa.c @@ -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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] mac80211/wpa: use constant time memory comparison for MACs 2017-06-18 19:18 ` [PATCH] mac80211/wpa: use constant time memory comparison for MACs Jason A. Donenfeld @ 2017-06-18 20:31 ` Emmanuel Grumbach 2017-06-18 20:44 ` Johannes Berg 0 siblings, 1 reply; 5+ messages in thread From: Emmanuel Grumbach @ 2017-06-18 20:31 UTC (permalink / raw) To: Jason A. Donenfeld Cc: Greg KH, Johannes Berg, linux-wireless, stable@vger.kernel.org, Johannes Berg On Sun, Jun 18, 2017 at 10:18 PM, Jason A. Donenfeld <Jason@zx2c4.com> wrote: > 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? > > 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 > Signed-off-by: Johannes Berg <johannes.berg@intel.com> > --- > Here's the backport for 3.18. > > net/mac80211/wpa.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/net/mac80211/wpa.c b/net/mac80211/wpa.c > index 983527a4c1ab..49592c7e4199 100644 > --- a/net/mac80211/wpa.c > +++ b/net/mac80211/wpa.c > @@ -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 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mac80211/wpa: use constant time memory comparison for MACs 2017-06-18 20:31 ` Emmanuel Grumbach @ 2017-06-18 20:44 ` Johannes Berg 2017-06-19 16:44 ` [PATCH v2 3.18-stable] " Jason A. Donenfeld 0 siblings, 1 reply; 5+ messages in thread From: Johannes Berg @ 2017-06-18 20:44 UTC (permalink / raw) To: Emmanuel Grumbach, Jason A. Donenfeld Cc: Greg KH, linux-wireless, stable@vger.kernel.org On Sun, 2017-06-18 at 23:31 +0300, Emmanuel Grumbach wrote: > On Sun, Jun 18, 2017 at 10:18 PM, Jason A. Donenfeld <Jason@zx2c4.com > > wrote: > > Otherwise, we enable all sorts of forgeries via timing attack. > > crypto_memneq's description says: [...] > > --- > > Here's the backport for 3.18. Yeah, not sure what happened here, but ... > > #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; This is obviously wrong and not like that in the original, > > /* 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; > > } this isn't in the original at all, and clearly shouldn't be here, > > @@ -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) { and this is just as wrong as the first one. johannes ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 3.18-stable] mac80211/wpa: use constant time memory comparison for MACs 2017-06-18 20:44 ` Johannes Berg @ 2017-06-19 16:44 ` Jason A. Donenfeld 2017-06-27 11:32 ` Greg KH 0 siblings, 1 reply; 5+ messages in thread From: Jason A. Donenfeld @ 2017-06-19 16:44 UTC (permalink / raw) To: gregkh Cc: Jason A. Donenfeld, Johannes Berg, linux-wireless, stable, Johannes Berg 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 Signed-off-by: Johannes Berg <johannes.berg@intel.com> --- This is for 3.18. Tested this, and it works as intended. net/mac80211/wpa.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/net/mac80211/wpa.c b/net/mac80211/wpa.c index 983527a4c1ab..bf87de469c03 100644 --- a/net/mac80211/wpa.c +++ b/net/mac80211/wpa.c @@ -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; } -- 2.13.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 3.18-stable] mac80211/wpa: use constant time memory comparison for MACs 2017-06-19 16:44 ` [PATCH v2 3.18-stable] " Jason A. Donenfeld @ 2017-06-27 11:32 ` Greg KH 0 siblings, 0 replies; 5+ messages in thread From: Greg KH @ 2017-06-27 11:32 UTC (permalink / raw) To: Jason A. Donenfeld; +Cc: Johannes Berg, linux-wireless, stable, Johannes Berg On Mon, Jun 19, 2017 at 06:44:06PM +0200, Jason A. Donenfeld wrote: > 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 > Signed-off-by: Johannes Berg <johannes.berg@intel.com> > --- > This is for 3.18. Tested this, and it works as intended. Thanks, now applied. greg k-h ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-06-27 11:32 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1497720871224217@kroah.com>
2017-06-18 19:18 ` [PATCH] mac80211/wpa: use constant time memory comparison for MACs Jason A. Donenfeld
2017-06-18 20:31 ` Emmanuel Grumbach
2017-06-18 20:44 ` Johannes Berg
2017-06-19 16:44 ` [PATCH v2 3.18-stable] " Jason A. Donenfeld
2017-06-27 11:32 ` Greg KH
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).