Skip to contentSkip to conversation

mac80211: Fix TKIP replay protection immediately after key setup

cip-dev Pavel Machek 3 participants 4 messages Feb 15, 2020 – Mar 20, 2020 mac80211

lore
Share & tools
lore

Pavel Machek

author
#0 lore raw

Hi!

So... this is first backport patch. I'll need to reformat a changelog.

The patch should pass our tests on gitlab, but I somehow don't think those tests involved wifi at all... At least it compiles.

Can someone test it easily? Should I just submit it to stable explaining I did not test it?

Do you have other patches that should go to 4.4/4.19?

Best regards,
Pavel

commit 911e21ed055f6700fa80d0f7a818ba223999bb2a

Author: Pavel Machek <[email protected]>
Date:   Thu Feb 13 22:56:46 2020 +0100

Author: Jouni Malinen <[email protected]>
Date: Tue Jan 7 17:35:45 2020 +0200

commit fa73f24d1b119b85b32cd8f217a73d108888097e

mac80211: Fix TKIP replay protection immediately after key setup

TKIP replay protection was skipped for the very first frame received after a new key is configured. While this is potentially needed to avoid dropping a frame in some cases, this does leave a window for replay attacks with group-addressed frames at the station side. Any earlier frame sent by the AP using the same key would be accepted as a valid frame and the internal RSC would then be updated to the TSC from that frame. This would allow multiple previously transmitted group-addressed frames to be replayed until the next valid new group-addressed frame from the AP is received by the station.

Fix this by limiting the no-replay-protection exception to apply only for the case where TSC=0, i.e., when this is for the very first frame protected using the new key, and the local RSC had not been set to a higher value when configuring the key (which may happen with GTK).

Signed-off-by Jouni Malinen <[email protected]>
Link https://lore.kernel.org/r/20200107153545.10934-1-j at w1.fi
Signed-off-by Johannes Berg <[email protected]>
[pavel at ucw.cz: port to 4.4]
Signed-off-by Pavel Machek <[email protected]>
net/mac80211/tkip.c +15 -4
index 0ae207771a58..d09d24d04f8a 100644
@@ -265,10 +265,21 @@ int ieee80211_tkip_decrypt_data(struct crypto_cipher *tfm,
 	if ((keyid >> 6) != key->conf.keyidx)
 		return TKIP_DECRYPT_INVALID_KEYIDX;
 
-	if (key->u.tkip.rx[queue].state != TKIP_STATE_NOT_INIT &&
-	    (iv32 < key->u.tkip.rx[queue].iv32 ||
-	     (iv32 == key->u.tkip.rx[queue].iv32 &&
-	      iv16 <= key->u.tkip.rx[queue].iv16)))
+	/* Reject replays if the received TSC is smaller than or equal to the
+	 * last received value in a valid message, but with an exception for
+	 * the case where a new key has been set and no valid frame using that
+	 * key has yet received and the local RSC was initialized to 0. This
+	 * exception allows the very first frame sent by the transmitter to be
+	 * accepted even if that transmitter were to use TSC 0 (IEEE 802.11
+	 * described TSC to be initialized to 1 whenever a new key is taken into
+	 * use).
+	 */
+	if (iv32 < key->u.tkip.rx[queue].iv32 ||
+	    (iv32 == key->u.tkip.rx[queue].iv32 &&
+	     (iv16 < key->u.tkip.rx[queue].iv16 ||
+	      (iv16 == key->u.tkip.rx[queue].iv16 &&
+	       (key->u.tkip.rx[queue].iv32 || key->u.tkip.rx[queue].iv16 ||
+		key->u.tkip.rx[queue].state != TKIP_STATE_NOT_INIT)))))
 		return TKIP_DECRYPT_REPLAY;
 
 	if (only_iv) {
Signature · 9 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://lists.cip-project.org/pipermail/cip-dev/attachments/20200215/0eba15fd/attachment.sig>
#1 lore raw

Hi Pavel,

Quoted 10 lines

-----Original Message-----
From: cip-dev [mailto:cip-dev-bounces at lists.cip-project.org] On Behalf Of Pavel Machek
Sent: Sunday, February 16, 2020 4:54 AM
To: cip-dev at lists.cip-project.org; Chris.Paterson2 at renesas.com
Subject: [cip-dev] [backport 4.4] mac80211: Fix TKIP replay protection immediately after key setup

Hi!

Quoted 7 lines

So... this is first backport patch. I'll need to reformat a changelog.

The patch should pass our tests on gitlab, but I somehow don't think those tests involved wifi at all... At least it compiles.

Can someone test it easily? Should I just submit it to stable explaining I did not test it?

If testing is difficult, how about sending a patch to stable ML as RFC? We may get reviews from the patch authors.

Quoted 3 lines

Do you have other patches that should go to 4.4/4.19?

I don't think there are other patches.

Quoted 3 lines

Best regards,
Pavel

Best regards,
Nobuhiro

Quoted 88 lines

commit 911e21ed055f6700fa80d0f7a818ba223999bb2a

Author: Pavel Machek <[email protected]>
Date:   Thu Feb 13 22:56:46 2020 +0100

Author: Jouni Malinen <[email protected]>
Date: Tue Jan 7 17:35:45 2020 +0200

commit fa73f24d1b119b85b32cd8f217a73d108888097e

mac80211: Fix TKIP replay protection immediately after key setup

TKIP replay protection was skipped for the very first frame
received
after a new key is configured. While this is potentially
needed to avoid
dropping a frame in some cases, this does leave a window for
replay
attacks with group-addressed frames at the station side. Any
earlier
frame sent by the AP using the same key would be accepted
as a valid
frame and the internal RSC would then be updated to the TSC
from that
frame. This would allow multiple previously transmitted
group-addressed
frames to be replayed until the next valid new
group-addressed frame
from the AP is received by the station.

Fix this by limiting the no-replay-protection exception to
apply only
for the case where TSC=0, i.e., when this is for the very
first frame
protected using the new key, and the local RSC had not been
set to a
higher value when configuring the key (which may happen with
GTK).

Signed-off-by Jouni Malinen <[email protected]>
Link https://lore.kernel.org/r/20200107153545.10934-1-j at w1.fi
Signed-off-by Johannes Berg <[email protected]>
[pavel at ucw.cz: port to 4.4]
Signed-off-by Pavel Machek <[email protected]>
net/mac80211/tkip.c index renamed from net/mac80211/tkip.c +0 -0

0ae207771a58..d09d24d04f8a 100644

net/mac80211/tkip.c +1 -4
@@ -265,10 +265,21 @@ int ieee80211_tkip_decrypt_data(struct
 crypto_cipher *tfm,
 	if ((keyid >> 6) != key->conf.keyidx)
 		return TKIP_DECRYPT_INVALID_KEYIDX;
 
-	if (key->u.tkip.rx[queue].state != TKIP_STATE_NOT_INIT &&
-	    (iv32 < key->u.tkip.rx[queue].iv32 ||
-	     (iv32 == key->u.tkip.rx[queue].iv32 &&
-	      iv16 <= key->u.tkip.rx[queue].iv16)))
+	/* Reject replays if the received TSC is smaller than or equal

to the

net/mac80211/tkip.c +1 -0
+	 * last received value in a valid message, but with an exception

for

net/mac80211/tkip.c +1 -0
+	 * the case where a new key has been set and no valid frame using

that

net/mac80211/tkip.c +1 -0
+	 * key has yet received and the local RSC was initialized to 0.

This

net/mac80211/tkip.c +1 -0
+	 * exception allows the very first frame sent by the transmitter

to be

net/mac80211/tkip.c +2 -0
+	 * accepted even if that transmitter were to use TSC 0 (IEEE 802.11
+	 * described TSC to be initialized to 1 whenever a new key is

taken into

net/mac80211/tkip.c +7 -0
+	 * use).
+	 */
+	if (iv32 < key->u.tkip.rx[queue].iv32 ||
+	    (iv32 == key->u.tkip.rx[queue].iv32 &&
+	     (iv16 < key->u.tkip.rx[queue].iv16 ||
+	      (iv16 == key->u.tkip.rx[queue].iv16 &&
+	       (key->u.tkip.rx[queue].iv32 ||

key->u.tkip.rx[queue].iv16 ||

net/mac80211/tkip.c +1 -0
+		key->u.tkip.rx[queue].state !=

TKIP_STATE_NOT_INIT)))))

net/mac80211/tkip.c +0 -1
 		return TKIP_DECRYPT_REPLAY;
 
 	if (only_iv) {
 
--

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

#3 lore raw
In reply to Pavel Machek
Quoted 9 lines · Pavel Machek

Hi!

So... this is first backport patch. I'll need to reformat a changelog.

The patch should pass our tests on gitlab, but I somehow don't think those tests involved wifi at all... At least it compiles.

Can someone test it easily? Should I just submit it to stable explaining I did not test it?

That's what I would do.

Quoted 5 lines

Do you have other patches that should go to 4.4/4.19?

Best regards,
Pavel

Quoted 8 lines

commit 911e21ed055f6700fa80d0f7a818ba223999bb2a

Author: Pavel Machek <[email protected]>
Date:   Thu Feb 13 22:56:46 2020 +0100
    Author: Jouni Malinen <[email protected]>
    Date:   Tue Jan 7 17:35:45 2020 +0200

    commit fa73f24d1b119b85b32cd8f217a73d108888097e

This reference is wrong; the upstream commit is 6f601265215a421f425ba3a4850a35861d024643. Also the usual format for this reference has "upstream." after the commit hash.

[...]

Quoted 18 lines
net/mac80211/tkip.c +8 -4
@@ -265,10 +265,21 @@ int ieee80211_tkip_decrypt_data(struct crypto_cipher *tfm,
 	if ((keyid >> 6) != key->conf.keyidx)
 		return TKIP_DECRYPT_INVALID_KEYIDX;
 
-	if (key->u.tkip.rx[queue].state != TKIP_STATE_NOT_INIT &&
-	    (iv32 < key->u.tkip.rx[queue].iv32 ||
-	     (iv32 == key->u.tkip.rx[queue].iv32 &&
-	      iv16 <= key->u.tkip.rx[queue].iv16)))
+	/* Reject replays if the received TSC is smaller than or equal to the
+	 * last received value in a valid message, but with an exception for
+	 * the case where a new key has been set and no valid frame using that
+	 * key has yet received and the local RSC was initialized to 0. This
+	 * exception allows the very first frame sent by the transmitter to be
+	 * accepted even if that transmitter were to use TSC 0 (IEEE 802.11
+	 * described TSC to be initialized to 1 whenever a new key is taken into
+	 * use).

net/mac80211/tkip.c int ieee80211_tkip_decrypt_data(struct crypto_cipher *tfm, · line 285

Quoted 10 lines
net/mac80211/tkip.c +7 -0
+	 */
+	if (iv32 < key->u.tkip.rx[queue].iv32 ||
+	    (iv32 == key->u.tkip.rx[queue].iv32 &&
+	     (iv16 < key->u.tkip.rx[queue].iv16 ||
+	      (iv16 == key->u.tkip.rx[queue].iv16 &&
+	       (key->u.tkip.rx[queue].iv32 || key->u.tkip.rx[queue].iv16 ||
+		key->u.tkip.rx[queue].state != TKIP_STATE_NOT_INIT)))))
 		return TKIP_DECRYPT_REPLAY;
 
 	if (only_iv) {

This backport makes sense to me. Please can you send it to the stable list, with the fixed commit message?

Ben.

Signature · 8 lines
Ben Hutchings, Software Developer                         Codethink Ltd
https://www.codethink.co.uk/                 Dale House, 35 Dale Street
                                     Manchester, M1 2HF, United Kingdom

_______________________________________________
cip-dev mailing list
[email protected]
https://lists.cip-project.org/mailman/listinfo/cip-dev
#2 lore raw
In reply to nobuhiro1.iwamatsu at toshiba.co.jp
Quoted 20 lines · On Mon, 2020-02-17 at 23:14 +0000, [email protected] wrote:

Hi Pavel,

Quoted 10 lines

-----Original Message-----
From: cip-dev [mailto:[email protected]] On Behalf Of Pavel Machek
Sent: Sunday, February 16, 2020 4:54 AM
To: [email protected]; [email protected]
Subject: [cip-dev] [backport 4.4] mac80211: Fix TKIP replay protection immediately after key setup

Hi!

Quoted 7 lines

So... this is first backport patch. I'll need to reformat a changelog.

The patch should pass our tests on gitlab, but I somehow don't think those tests involved wifi at all... At least it compiles.

Can someone test it easily? Should I just submit it to stable explaining I did not test it?

Quoted 7 lines

If testing is difficult, how about sending a patch to stable ML as RFC? We may get reviews from the patch authors.

Quoted 2 lines

Do you have other patches that should go to 4.4/4.19?

I don't think there are other patches.

The security tracker shows a lot of fixes missing from 4.4.

Ben.

Signature · 8 lines
Ben Hutchings, Software Developer                         Codethink Ltd
https://www.codethink.co.uk/                 Dale House, 35 Dale Street
                                     Manchester, M1 2HF, United Kingdom

_______________________________________________
cip-dev mailing list
[email protected]
https://lists.cip-project.org/mailman/listinfo/cip-dev