From 1549b29ea07482d7b4ccee70c92fd11d06e63890 Mon Sep 17 00:00:00 2001 From: Ben Hutchings Date: Thu, 16 Nov 2017 17:35:46 +0000 Subject: [PATCH] Add follow-up fixes relatd to CVE-2017-13080 --- debian/changelog | 2 + ...ompare-tkip-tx-mic-key-in-reinstall-.patch | 76 +++++++++++++++++++ ...e-constant-time-comparison-with-keys.patch | 36 +++++++++ debian/patches/series | 2 + 4 files changed, 116 insertions(+) create mode 100644 debian/patches/bugfix/all/mac80211-don-t-compare-tkip-tx-mic-key-in-reinstall-.patch create mode 100644 debian/patches/bugfix/all/mac80211-use-constant-time-comparison-with-keys.patch diff --git a/debian/changelog b/debian/changelog index 6d5872240..ba70dfba7 100644 --- a/debian/changelog +++ b/debian/changelog @@ -122,6 +122,8 @@ linux (4.13.13-1) UNRELEASED; urgency=medium profile will fail without it (Closes: #880441) * [powerpc*] kvm: Ignore ABI change in 4.13.6 (fixes FTBFS) * swap: Avoid ABI change in 4.13.12 + * mac80211: use constant time comparison with keys + * mac80211: don't compare TKIP TX MIC key in reinstall prevention -- Salvatore Bonaccorso Sat, 04 Nov 2017 09:54:41 +0100 diff --git a/debian/patches/bugfix/all/mac80211-don-t-compare-tkip-tx-mic-key-in-reinstall-.patch b/debian/patches/bugfix/all/mac80211-don-t-compare-tkip-tx-mic-key-in-reinstall-.patch new file mode 100644 index 000000000..d9b21c398 --- /dev/null +++ b/debian/patches/bugfix/all/mac80211-don-t-compare-tkip-tx-mic-key-in-reinstall-.patch @@ -0,0 +1,76 @@ +From: Johannes Berg +Date: Tue, 24 Oct 2017 21:12:13 +0200 +Subject: mac80211: don't compare TKIP TX MIC key in reinstall prevention +Origin: https://git.kernel.org/linus/cfbb0d90a7abb289edc91833d0905931f8805f12 +Bug-Debian-Security: https://security-tracker.debian.org/tracker/CVE-2017-13080 + +For the reinstall prevention, the code I had added compares the +whole key. It turns out though that iwlwifi firmware doesn't +provide the TKIP TX MIC key as it's not needed in client mode, +and thus the comparison will always return false. + +For client mode, thus always zero out the TX MIC key part before +doing the comparison in order to avoid accepting the reinstall +of the key with identical encryption and RX MIC key, but not the +same TX MIC key (since the supplicant provides the real one.) + +Fixes: fdf7cb4185b6 ("mac80211: accept key reinstall without changing anything") +Signed-off-by: Johannes Berg +--- + net/mac80211/key.c | 36 ++++++++++++++++++++++++++++++++++-- + 1 file changed, 34 insertions(+), 2 deletions(-) + +diff --git a/net/mac80211/key.c b/net/mac80211/key.c +index 035d16fe926e..938049395f90 100644 +--- a/net/mac80211/key.c ++++ b/net/mac80211/key.c +@@ -610,6 +610,39 @@ void ieee80211_key_free_unused(struct ieee80211_key *key) + ieee80211_key_free_common(key); + } + ++static bool ieee80211_key_identical(struct ieee80211_sub_if_data *sdata, ++ struct ieee80211_key *old, ++ struct ieee80211_key *new) ++{ ++ u8 tkip_old[WLAN_KEY_LEN_TKIP], tkip_new[WLAN_KEY_LEN_TKIP]; ++ u8 *tk_old, *tk_new; ++ ++ if (!old || new->conf.keylen != old->conf.keylen) ++ return false; ++ ++ tk_old = old->conf.key; ++ tk_new = new->conf.key; ++ ++ /* ++ * In station mode, don't compare the TX MIC key, as it's never used ++ * and offloaded rekeying may not care to send it to the host. This ++ * is the case in iwlwifi, for example. ++ */ ++ if (sdata->vif.type == NL80211_IFTYPE_STATION && ++ new->conf.cipher == WLAN_CIPHER_SUITE_TKIP && ++ new->conf.keylen == WLAN_KEY_LEN_TKIP && ++ !(new->conf.flags & IEEE80211_KEY_FLAG_PAIRWISE)) { ++ memcpy(tkip_old, tk_old, WLAN_KEY_LEN_TKIP); ++ memcpy(tkip_new, tk_new, WLAN_KEY_LEN_TKIP); ++ memset(tkip_old + NL80211_TKIP_DATA_OFFSET_TX_MIC_KEY, 0, 8); ++ memset(tkip_new + NL80211_TKIP_DATA_OFFSET_TX_MIC_KEY, 0, 8); ++ tk_old = tkip_old; ++ tk_new = tkip_new; ++ } ++ ++ return !crypto_memneq(tk_old, tk_new, new->conf.keylen); ++} ++ + int ieee80211_key_link(struct ieee80211_key *key, + struct ieee80211_sub_if_data *sdata, + struct sta_info *sta) +@@ -635,8 +668,7 @@ int ieee80211_key_link(struct ieee80211_key *key, + * Silently accept key re-installation without really installing the + * new version of the key to avoid nonce reuse or replay issues. + */ +- if (old_key && key->conf.keylen == old_key->conf.keylen && +- !crypto_memneq(key->conf.key, old_key->conf.key, key->conf.keylen)) { ++ if (ieee80211_key_identical(sdata, old_key, key)) { + ieee80211_key_free_unused(key); + ret = 0; + goto out; diff --git a/debian/patches/bugfix/all/mac80211-use-constant-time-comparison-with-keys.patch b/debian/patches/bugfix/all/mac80211-use-constant-time-comparison-with-keys.patch new file mode 100644 index 000000000..437ff9f14 --- /dev/null +++ b/debian/patches/bugfix/all/mac80211-use-constant-time-comparison-with-keys.patch @@ -0,0 +1,36 @@ +From: "Jason A. Donenfeld" +Date: Tue, 17 Oct 2017 20:32:07 +0200 +Subject: mac80211: use constant time comparison with keys +Origin: https://git.kernel.org/linus/2bdd713b92a9cade239d3c7d15205a09f556624d +Bug-Debian-Security: https://security-tracker.debian.org/tracker/CVE-2017-13080 + +Otherwise we risk leaking information via timing side channel. + +Fixes: fdf7cb4185b6 ("mac80211: accept key reinstall without changing anything") +Signed-off-by: Jason A. Donenfeld +Signed-off-by: Johannes Berg +--- + net/mac80211/key.c | 3 ++- + 1 file changed, 2 insertions(+), 1 deletion(-) + +diff --git a/net/mac80211/key.c b/net/mac80211/key.c +index ae995c8480db..035d16fe926e 100644 +--- a/net/mac80211/key.c ++++ b/net/mac80211/key.c +@@ -19,6 +19,7 @@ + #include + #include + #include ++#include + #include + #include "ieee80211_i.h" + #include "driver-ops.h" +@@ -635,7 +636,7 @@ int ieee80211_key_link(struct ieee80211_key *key, + * new version of the key to avoid nonce reuse or replay issues. + */ + if (old_key && key->conf.keylen == old_key->conf.keylen && +- !memcmp(key->conf.key, old_key->conf.key, key->conf.keylen)) { ++ !crypto_memneq(key->conf.key, old_key->conf.key, key->conf.keylen)) { + ieee80211_key_free_unused(key); + ret = 0; + goto out; diff --git a/debian/patches/series b/debian/patches/series index e65756465..90a6f7df5 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -114,6 +114,8 @@ features/all/lockdown/arm64-add-kernel-config-option-to-lock-down-when.patch # Security fixes debian/i386-686-pae-pci-set-pci-nobios-by-default.patch bugfix/all/mac80211-accept-key-reinstall-without-changing-anyth.patch +bugfix/all/mac80211-use-constant-time-comparison-with-keys.patch +bugfix/all/mac80211-don-t-compare-tkip-tx-mic-key-in-reinstall-.patch bugfix/all/sctp-do-not-peel-off-an-assoc-from-one-netns-to-anot.patch # Fix exported symbol versions