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@xxxxxxxxx
> > 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