Fix crash when using MACsec without loaded macsec.ko (rh #1497640)

https://bugzilla.redhat.com/show_bug.cgi?id=1497640
This commit is contained in:
Jiří Klimeš 2017-11-01 12:45:57 +01:00
parent fbf8c1b456
commit dadca54170
3 changed files with 194 additions and 1 deletions

View File

@ -0,0 +1,106 @@
From 7612e65b9bdfe03e5a018e3c897f4a3292c42ee4 Mon Sep 17 00:00:00 2001
From: Sabrina Dubroca <sd@queasysnail.net>
Date: Tue, 22 Aug 2017 10:34:19 +0200
Subject: mka: Add error handling for secy_init_macsec() calls
secy_init_macsec() can fail (if ->macsec_init fails), and
ieee802_1x_kay_init() should handle this and not let MKA run any
further, because nothing is going to work anyway.
On failure, ieee802_1x_kay_init() must deinit its kay, which will free
kay->ctx, so ieee802_1x_kay_init callers (only ieee802_1x_alloc_kay_sm)
must not do it. Before this patch there is a double-free of the ctx
argument when ieee802_1x_kay_deinit() was called.
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
src/pae/ieee802_1x_kay.c | 25 ++++++++++++++-----------
wpa_supplicant/wpas_kay.c | 5 ++---
2 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/src/pae/ieee802_1x_kay.c b/src/pae/ieee802_1x_kay.c
index ff55f88..c4bfcbc 100644
--- a/src/pae/ieee802_1x_kay.c
+++ b/src/pae/ieee802_1x_kay.c
@@ -3100,6 +3100,7 @@ ieee802_1x_kay_init(struct ieee802_1x_kay_ctx *ctx, enum macsec_policy policy,
kay = os_zalloc(sizeof(*kay));
if (!kay) {
wpa_printf(MSG_ERROR, "KaY-%s: out of memory", __func__);
+ os_free(ctx);
return NULL;
}
@@ -3134,10 +3135,8 @@ ieee802_1x_kay_init(struct ieee802_1x_kay_ctx *ctx, enum macsec_policy policy,
dl_list_init(&kay->participant_list);
if (policy != DO_NOT_SECURE &&
- secy_get_capability(kay, &kay->macsec_capable) < 0) {
- os_free(kay);
- return NULL;
- }
+ secy_get_capability(kay, &kay->macsec_capable) < 0)
+ goto error;
if (policy == DO_NOT_SECURE ||
kay->macsec_capable == MACSEC_CAP_NOT_IMPLEMENTED) {
@@ -3164,16 +3163,17 @@ ieee802_1x_kay_init(struct ieee802_1x_kay_ctx *ctx, enum macsec_policy policy,
wpa_printf(MSG_DEBUG, "KaY: state machine created");
/* Initialize the SecY must be prio to CP, as CP will control SecY */
- secy_init_macsec(kay);
+ if (secy_init_macsec(kay) < 0) {
+ wpa_printf(MSG_DEBUG, "KaY: Could not initialize MACsec");
+ goto error;
+ }
wpa_printf(MSG_DEBUG, "KaY: secy init macsec done");
/* init CP */
kay->cp = ieee802_1x_cp_sm_init(kay);
- if (kay->cp == NULL) {
- ieee802_1x_kay_deinit(kay);
- return NULL;
- }
+ if (kay->cp == NULL)
+ goto error;
if (policy == DO_NOT_SECURE) {
ieee802_1x_cp_connect_authenticated(kay->cp);
@@ -3184,12 +3184,15 @@ ieee802_1x_kay_init(struct ieee802_1x_kay_ctx *ctx, enum macsec_policy policy,
if (kay->l2_mka == NULL) {
wpa_printf(MSG_WARNING,
"KaY: Failed to initialize L2 packet processing for MKA packet");
- ieee802_1x_kay_deinit(kay);
- return NULL;
+ goto error;
}
}
return kay;
+
+error:
+ ieee802_1x_kay_deinit(kay);
+ return NULL;
}
diff --git a/wpa_supplicant/wpas_kay.c b/wpa_supplicant/wpas_kay.c
index d087e00..587e5c3 100644
--- a/wpa_supplicant/wpas_kay.c
+++ b/wpa_supplicant/wpas_kay.c
@@ -235,10 +235,9 @@ int ieee802_1x_alloc_kay_sm(struct wpa_supplicant *wpa_s, struct wpa_ssid *ssid)
res = ieee802_1x_kay_init(kay_ctx, policy, ssid->macsec_port,
ssid->mka_priority, wpa_s->ifname,
wpa_s->own_addr);
- if (res == NULL) {
- os_free(kay_ctx);
+ /* ieee802_1x_kay_init() frees kay_ctx on failure */
+ if (res == NULL)
return -1;
- }
wpa_s->kay = res;
--
cgit v0.12

View File

@ -0,0 +1,78 @@
From 0ad5893a2f1f521d44712cd395e067ccf0a397c3 Mon Sep 17 00:00:00 2001
From: Michael Braun <michael-dev@fami-braun.de>
Date: Fri, 18 Aug 2017 01:14:28 +0200
Subject: PAE: Validate input before pointer
ieee802_1x_kay_decode_mkpdu() calls ieee802_1x_mka_i_in_peerlist()
before body_len has been checked on all segments.
ieee802_1x_kay_decode_mkpdu() and ieee802_1x_mka_i_in_peerlist() might
continue and thus underflow left_len even if it finds left_len to small
(or before checking).
Additionally, ieee802_1x_mka_dump_peer_body() might perform out of bound
reads in this case.
Fix this by checking left_len and aborting if too small early.
Signed-off-by: Michael Braun <michael-dev@fami-braun.de>
---
src/pae/ieee802_1x_kay.c | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/src/pae/ieee802_1x_kay.c b/src/pae/ieee802_1x_kay.c
index c4bfcbc..cad0292 100644
--- a/src/pae/ieee802_1x_kay.c
+++ b/src/pae/ieee802_1x_kay.c
@@ -964,21 +964,19 @@ ieee802_1x_mka_i_in_peerlist(struct ieee802_1x_mka_participant *participant,
body_len = get_mka_param_body_len(hdr);
body_type = get_mka_param_body_type(hdr);
- if (body_type != MKA_LIVE_PEER_LIST &&
- body_type != MKA_POTENTIAL_PEER_LIST)
- continue;
-
- ieee802_1x_mka_dump_peer_body(
- (struct ieee802_1x_mka_peer_body *)pos);
-
- if (left_len < (MKA_HDR_LEN + body_len + DEFAULT_ICV_LEN)) {
+ if (left_len < (MKA_HDR_LEN + MKA_ALIGN_LENGTH(body_len) + DEFAULT_ICV_LEN)) {
wpa_printf(MSG_ERROR,
"KaY: MKA Peer Packet Body Length (%zu bytes) is less than the Parameter Set Header Length (%zu bytes) + the Parameter Set Body Length (%zu bytes) + %d bytes of ICV",
left_len, MKA_HDR_LEN,
- body_len, DEFAULT_ICV_LEN);
- continue;
+ MKA_ALIGN_LENGTH(body_len),
+ DEFAULT_ICV_LEN);
+ return FALSE;
}
+ if (body_type != MKA_LIVE_PEER_LIST &&
+ body_type != MKA_POTENTIAL_PEER_LIST)
+ continue;
+
if ((body_len % 16) != 0) {
wpa_printf(MSG_ERROR,
"KaY: MKA Peer Packet Body Length (%zu bytes) should be a multiple of 16 octets",
@@ -986,6 +984,9 @@ ieee802_1x_mka_i_in_peerlist(struct ieee802_1x_mka_participant *participant,
continue;
}
+ ieee802_1x_mka_dump_peer_body(
+ (struct ieee802_1x_mka_peer_body *)pos);
+
for (i = 0; i < body_len;
i += sizeof(struct ieee802_1x_mka_peer_id)) {
const struct ieee802_1x_mka_peer_id *peer_mi;
@@ -3018,7 +3019,7 @@ static int ieee802_1x_kay_decode_mkpdu(struct ieee802_1x_kay *kay,
"KaY: MKA Peer Packet Body Length (%zu bytes) is less than the Parameter Set Header Length (%zu bytes) + the Parameter Set Body Length (%zu bytes) + %d bytes of ICV",
left_len, MKA_HDR_LEN,
body_len, DEFAULT_ICV_LEN);
- continue;
+ return -1;
}
if (handled[body_type])
--
cgit v0.12

View File

@ -7,7 +7,7 @@ Summary: WPA/WPA2/IEEE 802.1X Supplicant
Name: wpa_supplicant
Epoch: 1
Version: 2.6
Release: 11%{?dist}
Release: 12%{?dist}
License: BSD
Group: System Environment/Base
Source0: http://w1.fi/releases/%{name}-%{version}%{rcver}%{snapshot}.tar.gz
@ -95,6 +95,10 @@ Patch56: rh1451834-nl80211-Fix-race-condition-in-detecting-MAC-change.patch
Patch57: rh1462262-use-system-openssl-ciphers.patch
Patch58: rh1465138-openssl-Fix-openssl-1-1-private-key-callback.patch
# fixes for crash when using MACsec without loaded macsec.ko (rh #1497640)
Patch59: rh1497640-mka-add-error-handling-for-secy_init_macsec.patch
Patch60: rh1497640-pae-validate-input-before-pointer.patch
URL: http://w1.fi/wpa_supplicant/
%if %{build_gui}
@ -194,6 +198,8 @@ Graphical User Interface for wpa_supplicant written using QT
%patch56 -p1 -b .rh1447073-detect-mac-change
%patch57 -p1 -b .rh1462262-system-ciphers
%patch58 -p1 -b .rh1465138-openssl-cb
%patch59 -p1 -b .rh1487640-mka
%patch60 -p1 -b .rh1487640-pae
%build
pushd wpa_supplicant
@ -294,6 +300,9 @@ chmod -R 0644 %{name}/examples/*.py
%endif
%changelog
* Wed Nov 1 2017 Jiří Klimeš <blueowl@centrum.cz> - 1:2.6-12
- Fix crash when using MACsec without loaded macsec.ko (rh #1497640)
* Mon Oct 16 2017 Lubomir Rintel <lkundrak@v3.sk> - 1:2.6-11
- hostapd: Avoid key reinstallation in FT handshake (CVE-2017-13082)
- Fix PTK rekeying to generate a new ANonce