diff mbox series

[1/2] Propagate the EAP method error code

Message ID CABo_UfPYjt7Yc=23fZfnCEC_o_VKnTkg6P8RDbTuj_Cgm=-h3Q@mail.gmail.com
State Accepted
Headers show
Series [1/2] Propagate the EAP method error code | expand

Commit Message

Ahmed ElArabawy March 29, 2018, 12:48 a.m. UTC 
In current implementaion, upon an EAP method failure, followed
by an EAP failure, the EAP Status is propagated up in wpa_supplicant
with a general failure parameter string "failure". This parameter is
used for a notification on the dbus.

This patch reports the EAP method failure error code in a separate
callback.  We want this to have visibility into the error code and act
accordingly.

The solution in this patch is generic to all EAP methods, and can be
used by any method that need to pass its error code. However, this
patch only implements the reporting for EAP-SIM, EAP-AKA, and EAP-AKA'
methods.

Signed-off-by: Ahmed ElArabawy <arabawy@google.com>
---
 src/eap_peer/eap.c             | 14 ++++++++++++++
 src/eap_peer/eap.h             |  7 +++++++
 src/eap_peer/eap_aka.c         | 21 +++++++++++++++++++++
 src/eap_peer/eap_i.h           | 13 +++++++++++++
 src/eap_peer/eap_sim.c         | 20 ++++++++++++++++++++
 src/eapol_supp/eapol_supp_sm.c |  8 ++++++++
 src/eapol_supp/eapol_supp_sm.h |  7 +++++++
 wpa_supplicant/notify.c        |  5 +++++
 wpa_supplicant/notify.h        |  1 +
 wpa_supplicant/wpas_glue.c     |  7 +++++++
 10 files changed, 103 insertions(+)

 {
@@ -1115,6 +1121,7 @@ int wpa_supplicant_init_eapol(struct
wpa_supplicant *wpa_s)
    ctx->cert_cb = wpa_supplicant_cert_cb;
    ctx->cert_in_cb = wpa_s->conf->cert_in_cb;
    ctx->status_cb = wpa_supplicant_status_cb;
+   ctx->eap_error_cb = wpa_supplicant_eap_error_cb;
    ctx->set_anon_id = wpa_supplicant_set_anon_id;
    ctx->cb_ctx = wpa_s;
    wpa_s->eapol = eapol_sm_init(ctx);
--
2.17.0.rc1.321.gba9d0f2565-goog

Comments

Jouni Malinen April 1, 2018, 3:24 p.m. UTC | #1 
On Wed, Mar 28, 2018 at 05:48:44PM -0700, Ahmed ElArabawy wrote:
> In current implementaion, upon an EAP method failure, followed
> by an EAP failure, the EAP Status is propagated up in wpa_supplicant
> with a general failure parameter string "failure". This parameter is
> used for a notification on the dbus.
> 
> This patch reports the EAP method failure error code in a separate
> callback.  We want this to have visibility into the error code and act
> accordingly.
> 
> The solution in this patch is generic to all EAP methods, and can be
> used by any method that need to pass its error code. However, this
> patch only implements the reporting for EAP-SIM, EAP-AKA, and EAP-AKA'
> methods.

Thanks, applied with some cleanup and fixes.

Was there supposed to be another patch 2/2 as well? If so, it did not
seem to make it to the mailing list.

> diff --git a/src/eap_peer/eap.c b/src/eap_peer/eap.c
> @@ -2018,6 +2025,13 @@ static void eap_sm_parseEapReq(struct eap_sm
> *sm, const struct wpabuf *req)
>     case EAP_CODE_FAILURE:
>         wpa_printf(MSG_DEBUG, "EAP: Received EAP-Failure");
>         eap_notify_status(sm, "completion", "failure");
> +
> +       /* Get the error code from method */
> +       if (sm->m->get_error_code) {

This could result in segfault due to a NULL pointer dereference since
sm->m can be NULL here if the EAP-Failure is received before selecting
the EAP method.

> diff --git a/wpa_supplicant/notify.c b/wpa_supplicant/notify.c
> +void wpas_notify_eap_error(struct wpa_supplicant *wpa_s, int error_code)
> +{
> +   wpa_dbg(wpa_s, MSG_ERROR,
> +       "EAP Error code = %d", error_code);
> +}

I replaced this a more formally defined prefix in wpa_ctrl.h and
wpa_msg() to avoid the message getting comment out from a build.
diff mbox series

Patch

From 9c86a7f3b4994b1346418f183a9e71c82c87de65 Mon Sep 17 00:00:00 2001
From: Ahmed ElArabawy <arabawy@google.com>
Date: Thu, 15 Mar 2018 09:00:10 -0700
Subject: [PATCH 1/2] Propagate the EAP method error code

In current implementaion, upon an EAP method failure, followed
by an EAP failure, the EAP Status is propagated up in wpa_supplicant
with a general failure parameter string "failure". This parameter is
used for a notification on the dbus.

This commit reports the EAP method failure error code in a separate
callback.
The solution in this commit is generic to all EAP methods, and can be
used by any method that need to pass its error code. However, this
commit only implements the reporting for EAP-SIM and EAP-AKA methods.

Change-Id: I27736fbaa5d92c4d9f0623ccd642177df2c49a0e
Signed-off-by: Ahmed ElArabawy <arabawy@google.com>
---
 src/eap_peer/eap.c             | 14 ++++++++++++++
 src/eap_peer/eap.h             |  7 +++++++
 src/eap_peer/eap_aka.c         | 21 +++++++++++++++++++++
 src/eap_peer/eap_i.h           | 13 +++++++++++++
 src/eap_peer/eap_sim.c         | 20 ++++++++++++++++++++
 src/eapol_supp/eapol_supp_sm.c |  8 ++++++++
 src/eapol_supp/eapol_supp_sm.h |  7 +++++++
 wpa_supplicant/notify.c        |  5 +++++
 wpa_supplicant/notify.h        |  1 +
 wpa_supplicant/wpas_glue.c     |  7 +++++++
 10 files changed, 103 insertions(+)

diff --git a/src/eap_peer/eap.c b/src/eap_peer/eap.c
index 0043707..11d8129 100644
--- a/src/eap_peer/eap.c
+++ b/src/eap_peer/eap.c
@@ -94,6 +94,12 @@  static void eap_notify_status(struct eap_sm *sm, const char *status,
 		sm->eapol_cb->notify_status(sm->eapol_ctx, status, parameter);
 }
 
+static void eap_report_error(struct eap_sm *sm, int error_code)
+{
+	wpa_printf(MSG_DEBUG, "EAP: Error notification: %d", error_code);
+	if (sm->eapol_cb->notify_eap_error)
+		sm->eapol_cb->notify_eap_error(sm->eapol_ctx, error_code);
+}
 
 static void eap_sm_free_key(struct eap_sm *sm)
 {
@@ -1934,6 +1940,7 @@  static void eap_sm_parseEapReq(struct eap_sm *sm, const struct wpabuf *req)
 	const struct eap_hdr *hdr;
 	size_t plen;
 	const u8 *pos;
+        int error_code;
 
 	sm->rxReq = sm->rxResp = sm->rxSuccess = sm->rxFailure = FALSE;
 	sm->reqId = 0;
@@ -2018,6 +2025,13 @@  static void eap_sm_parseEapReq(struct eap_sm *sm, const struct wpabuf *req)
 	case EAP_CODE_FAILURE:
 		wpa_printf(MSG_DEBUG, "EAP: Received EAP-Failure");
 		eap_notify_status(sm, "completion", "failure");
+
+		/* Get the error code from method */
+		if (sm->m->get_error_code) {
+			error_code = sm->m->get_error_code(sm->eap_method_priv);
+			if (error_code != NO_EAP_METHOD_ERROR)
+				eap_report_error(sm, error_code);
+		}
 		sm->rxFailure = TRUE;
 		break;
 	case EAP_CODE_INITIATE:
diff --git a/src/eap_peer/eap.h b/src/eap_peer/eap.h
index b5591a0..d0837e3 100644
--- a/src/eap_peer/eap.h
+++ b/src/eap_peer/eap.h
@@ -246,6 +246,13 @@  struct eapol_callbacks {
 	void (*notify_status)(void *ctx, const char *status,
 			      const char *parameter);
 
+	/**
+	 * notify_eap_error - Report EAP method error code
+	 * @ctx: eapol_ctx from eap_peer_sm_init() call
+	 * @error_code: Error code from the used EAP method
+	 */
+	void (*notify_eap_error)(void *ctx, int error_code);
+
 #ifdef CONFIG_EAP_PROXY
 	/**
 	 * eap_proxy_cb - Callback signifying any updates from eap_proxy
diff --git a/src/eap_peer/eap_aka.c b/src/eap_peer/eap_aka.c
index 7a6bfc9..679f101 100644
--- a/src/eap_peer/eap_aka.c
+++ b/src/eap_peer/eap_aka.c
@@ -56,6 +56,7 @@  struct eap_aka_data {
 	int kdf_negotiation;
 	u16 last_kdf_attrs[EAP_AKA_PRIME_KDF_MAX];
 	size_t last_kdf_count;
+	int error_code;
 };
 
 
@@ -99,6 +100,9 @@  static void * eap_aka_init(struct eap_sm *sm)
 
 	data->eap_method = EAP_TYPE_AKA;
 
+	/* Zero is a valid error code, so we need to initialize */
+	data->error_code = NO_EAP_METHOD_ERROR;
+
 	eap_aka_state(data, CONTINUE);
 	data->prev_id = -1;
 
@@ -1180,6 +1184,7 @@  static struct wpabuf * eap_aka_process_notification(
 
 	eap_sim_report_notification(sm->msg_ctx, attr->notification, 1);
 	if (attr->notification >= 0 && attr->notification < 32768) {
+		data->error_code = attr->notification;
 		eap_aka_state(data, FAILURE);
 	} else if (attr->notification == EAP_SIM_SUCCESS &&
 		   data->state == RESULT_SUCCESS)
@@ -1523,6 +1528,20 @@  static u8 * eap_aka_get_emsk(struct eap_sm *sm, void *priv, size_t *len)
 	return key;
 }
 
+static int eap_aka_get_error_code(void *priv)
+{
+	struct eap_aka_data *data = priv;
+
+	if (!data)
+		return NO_EAP_METHOD_ERROR;
+
+	int current_data_error = data->error_code;
+
+	/* Now reset for next transaction */
+	data->error_code = NO_EAP_METHOD_ERROR;
+
+	return current_data_error;
+}
 
 int eap_peer_aka_register(void)
 {
@@ -1544,6 +1563,7 @@  int eap_peer_aka_register(void)
 	eap->init_for_reauth = eap_aka_init_for_reauth;
 	eap->get_identity = eap_aka_get_identity;
 	eap->get_emsk = eap_aka_get_emsk;
+	eap->get_error_code = eap_aka_get_error_code;
 
 	return eap_peer_method_register(eap);
 }
@@ -1571,6 +1591,7 @@  int eap_peer_aka_prime_register(void)
 	eap->init_for_reauth = eap_aka_init_for_reauth;
 	eap->get_identity = eap_aka_get_identity;
 	eap->get_emsk = eap_aka_get_emsk;
+	eap->get_error_code = eap_aka_get_error_code;
 
 	return eap_peer_method_register(eap);
 }
diff --git a/src/eap_peer/eap_i.h b/src/eap_peer/eap_i.h
index 6ab2483..5b38969 100644
--- a/src/eap_peer/eap_i.h
+++ b/src/eap_peer/eap_i.h
@@ -14,6 +14,8 @@ 
 #include "eap_peer/eap.h"
 #include "eap_common/eap_common.h"
 
+#define NO_EAP_METHOD_ERROR (-1)
+
 /* RFC 4137 - EAP Peer state machine */
 
 typedef enum {
@@ -205,6 +207,17 @@  struct eap_method {
 	 */
 	const u8 * (*get_identity)(struct eap_sm *sm, void *priv, size_t *len);
 
+	/**
+	 * get_error_code - Get latest EAP method error code
+	 * @priv: Pointer to private EAP method data from eap_method::init()
+	 * Returns: An int for the EAP Method Error code if exists or
+	 * NO_EAP_METHOD_ERROR otherwise
+	 *
+	 * This method is an optional handler that only EAP methods that need to
+	 * report their error code need to implement.
+	 */
+	int (*get_error_code)(void *priv);
+
 	/**
 	 * free - Free EAP method data
 	 * @method: Pointer to the method data registered with
diff --git a/src/eap_peer/eap_sim.c b/src/eap_peer/eap_sim.c
index cd687cb..c0896aa 100644
--- a/src/eap_peer/eap_sim.c
+++ b/src/eap_peer/eap_sim.c
@@ -47,6 +47,7 @@  struct eap_sim_data {
 	} state;
 	int result_ind, use_result_ind;
 	int use_pseudonym;
+	int error_code;
 };
 
 
@@ -94,6 +95,9 @@  static void * eap_sim_init(struct eap_sm *sm)
 		return NULL;
 	}
 
+	/* Zero is a valid error code, so we need to initialize */
+	data->error_code = NO_EAP_METHOD_ERROR;
+
 	data->min_num_chal = 2;
 	if (config && config->phase1) {
 		char *pos = os_strstr(config->phase1, "sim_min_num_chal=");
@@ -920,6 +924,7 @@  static struct wpabuf * eap_sim_process_notification(
 
 	eap_sim_report_notification(sm->msg_ctx, attr->notification, 0);
 	if (attr->notification >= 0 && attr->notification < 32768) {
+		data->error_code = attr->notification;
 		eap_sim_state(data, FAILURE);
 	} else if (attr->notification == EAP_SIM_SUCCESS &&
 		   data->state == RESULT_SUCCESS)
@@ -1243,6 +1248,20 @@  static u8 * eap_sim_get_emsk(struct eap_sm *sm, void *priv, size_t *len)
 	return key;
 }
 
+static int eap_sim_get_error_code(void *priv)
+{
+	struct eap_sim_data *data = priv;
+
+	if (!data)
+		return NO_EAP_METHOD_ERROR;
+
+	int current_data_error = data->error_code;
+
+	/* Now reset for next transaction */
+	data->error_code = NO_EAP_METHOD_ERROR;
+
+	return current_data_error;
+}
 
 int eap_peer_sim_register(void)
 {
@@ -1264,6 +1283,7 @@  int eap_peer_sim_register(void)
 	eap->init_for_reauth = eap_sim_init_for_reauth;
 	eap->get_identity = eap_sim_get_identity;
 	eap->get_emsk = eap_sim_get_emsk;
+	eap->get_error_code = eap_sim_get_error_code;
 
 	return eap_peer_method_register(eap);
 }
diff --git a/src/eapol_supp/eapol_supp_sm.c b/src/eapol_supp/eapol_supp_sm.c
index 8e4f0e4..cb463ef 100644
--- a/src/eapol_supp/eapol_supp_sm.c
+++ b/src/eapol_supp/eapol_supp_sm.c
@@ -2014,6 +2014,13 @@  static void eapol_sm_notify_status(void *ctx, const char *status,
 		sm->ctx->status_cb(sm->ctx->ctx, status, parameter);
 }
 
+static void eapol_sm_notify_eap_error(void *ctx, int error_code)
+{
+	struct eapol_sm *sm = ctx;
+
+	if (sm->ctx->eap_error_cb)
+		sm->ctx->eap_error_cb(sm->ctx->ctx, error_code);
+}
 
 #ifdef CONFIG_EAP_PROXY
 
@@ -2062,6 +2069,7 @@  static const struct eapol_callbacks eapol_cb =
 	eapol_sm_eap_param_needed,
 	eapol_sm_notify_cert,
 	eapol_sm_notify_status,
+	eapol_sm_notify_eap_error,
 #ifdef CONFIG_EAP_PROXY
 	eapol_sm_eap_proxy_cb,
 	eapol_sm_eap_proxy_notify_sim_status,
diff --git a/src/eapol_supp/eapol_supp_sm.h b/src/eapol_supp/eapol_supp_sm.h
index a25c799..74f40bb 100644
--- a/src/eapol_supp/eapol_supp_sm.h
+++ b/src/eapol_supp/eapol_supp_sm.h
@@ -271,6 +271,13 @@  struct eapol_ctx {
 	void (*status_cb)(void *ctx, const char *status,
 			  const char *parameter);
 
+	/**
+	 * eap_error_cb - Notification of EAP method error
+	 * @ctx: Callback context (ctx)
+	 * @error_code: EAP method error code
+	 */
+	void (*eap_error_cb)(void *ctx, int error_code);
+
 #ifdef CONFIG_EAP_PROXY
 	/**
 	 * eap_proxy_cb - Callback signifying any updates from eap_proxy
diff --git a/wpa_supplicant/notify.c b/wpa_supplicant/notify.c
index a5db82c..8b62a6d 100644
--- a/wpa_supplicant/notify.c
+++ b/wpa_supplicant/notify.c
@@ -887,6 +887,11 @@  void wpas_notify_eap_status(struct wpa_supplicant *wpa_s, const char *status,
 		     status, parameter);
 }
 
+void wpas_notify_eap_error(struct wpa_supplicant *wpa_s, int error_code)
+{
+	wpa_dbg(wpa_s, MSG_ERROR,
+		"EAP Error code = %d", error_code);
+}
 
 void wpas_notify_network_bssid_set_changed(struct wpa_supplicant *wpa_s,
 					   struct wpa_ssid *ssid)
diff --git a/wpa_supplicant/notify.h b/wpa_supplicant/notify.h
index 26b07f5..c3ac3d1 100644
--- a/wpa_supplicant/notify.h
+++ b/wpa_supplicant/notify.h
@@ -138,6 +138,7 @@  void wpas_notify_preq(struct wpa_supplicant *wpa_s,
 		      const u8 *ie, size_t ie_len, u32 ssi_signal);
 void wpas_notify_eap_status(struct wpa_supplicant *wpa_s, const char *status,
 			    const char *parameter);
+void wpas_notify_eap_error(struct wpa_supplicant *wpa_s, int error_code);
 void wpas_notify_network_bssid_set_changed(struct wpa_supplicant *wpa_s,
 					   struct wpa_ssid *ssid);
 void wpas_notify_network_type_changed(struct wpa_supplicant *wpa_s,
diff --git a/wpa_supplicant/wpas_glue.c b/wpa_supplicant/wpas_glue.c
index e44f6af..e29f13b 100644
--- a/wpa_supplicant/wpas_glue.c
+++ b/wpa_supplicant/wpas_glue.c
@@ -1037,6 +1037,12 @@  static void wpa_supplicant_status_cb(void *ctx, const char *status,
 	wpas_notify_eap_status(wpa_s, status, parameter);
 }
 
+static void wpa_supplicant_eap_error_cb(void *ctx, int error_code)
+{
+	struct wpa_supplicant *wpa_s = ctx;
+
+	wpas_notify_eap_error(wpa_s, error_code);
+}
 
 static void wpa_supplicant_set_anon_id(void *ctx, const u8 *id, size_t len)
 {
@@ -1115,6 +1121,7 @@  int wpa_supplicant_init_eapol(struct wpa_supplicant *wpa_s)
 	ctx->cert_cb = wpa_supplicant_cert_cb;
 	ctx->cert_in_cb = wpa_s->conf->cert_in_cb;
 	ctx->status_cb = wpa_supplicant_status_cb;
+	ctx->eap_error_cb = wpa_supplicant_eap_error_cb;
 	ctx->set_anon_id = wpa_supplicant_set_anon_id;
 	ctx->cb_ctx = wpa_s;
 	wpa_s->eapol = eapol_sm_init(ctx);
-- 
2.17.0.rc1.321.gba9d0f2565-goog