From 289b9b7b31bfc8acda4b8d6964446f1ea293904f Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Tue, 26 Feb 2013 12:16:31 +0100 Subject: [PATCH 1/3] Refactor auth_cfg applying to a common function --- src/libcharon/sa/ikev2/tasks/ike_auth.c | 37 ++++++++++++------------- 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/src/libcharon/sa/ikev2/tasks/ike_auth.c b/src/libcharon/sa/ikev2/tasks/ike_auth.c index 70efcd7af..b6e4745c4 100644 --- a/src/libcharon/sa/ikev2/tasks/ike_auth.c +++ b/src/libcharon/sa/ikev2/tasks/ike_auth.c @@ -222,6 +222,18 @@ static auth_cfg_t *get_auth_cfg(private_ike_auth_t *this, bool local) return next; } +/** + * Move the currently active auth config to the auth configs completed + */ +static void apply_auth_cfg(private_ike_auth_t *this, bool local) +{ + auth_cfg_t *cfg; + + cfg = auth_cfg_create(); + cfg->merge(cfg, this->ike_sa->get_auth_cfg(this->ike_sa, local), local); + this->ike_sa->add_auth_cfg(this->ike_sa, local, cfg); +} + /** * Check if we have should initiate another authentication round */ @@ -464,10 +476,7 @@ METHOD(task_t, build_i, status_t, switch (this->my_auth->build(this->my_auth, message)) { case SUCCESS: - /* authentication step complete, reset authenticator */ - cfg = auth_cfg_create(); - cfg->merge(cfg, this->ike_sa->get_auth_cfg(this->ike_sa, TRUE), TRUE); - this->ike_sa->add_auth_cfg(this->ike_sa, TRUE, cfg); + apply_auth_cfg(this, TRUE); this->my_auth->destroy(this->my_auth); this->my_auth = NULL; break; @@ -640,10 +649,7 @@ METHOD(task_t, process_r, status_t, return NEED_MORE; } - /* store authentication information */ - cfg = auth_cfg_create(); - cfg->merge(cfg, this->ike_sa->get_auth_cfg(this->ike_sa, FALSE), FALSE); - this->ike_sa->add_auth_cfg(this->ike_sa, FALSE, cfg); + apply_auth_cfg(this, FALSE); if (!update_cfg_candidates(this, FALSE)) { @@ -778,10 +784,7 @@ METHOD(task_t, build_r, status_t, switch (this->my_auth->build(this->my_auth, message)) { case SUCCESS: - cfg = auth_cfg_create(); - cfg->merge(cfg, this->ike_sa->get_auth_cfg(this->ike_sa, TRUE), - TRUE); - this->ike_sa->add_auth_cfg(this->ike_sa, TRUE, cfg); + apply_auth_cfg(this, TRUE); this->my_auth->destroy(this->my_auth); this->my_auth = NULL; break; @@ -969,10 +972,7 @@ METHOD(task_t, process_i, status_t, goto peer_auth_failed; } - /* store authentication information, reset authenticator */ - cfg = auth_cfg_create(); - cfg->merge(cfg, this->ike_sa->get_auth_cfg(this->ike_sa, FALSE), FALSE); - this->ike_sa->add_auth_cfg(this->ike_sa, FALSE, cfg); + apply_auth_cfg(this, FALSE); } if (this->my_auth) @@ -980,10 +980,7 @@ METHOD(task_t, process_i, status_t, switch (this->my_auth->process(this->my_auth, message)) { case SUCCESS: - cfg = auth_cfg_create(); - cfg->merge(cfg, this->ike_sa->get_auth_cfg(this->ike_sa, TRUE), - TRUE); - this->ike_sa->add_auth_cfg(this->ike_sa, TRUE, cfg); + apply_auth_cfg(this, TRUE); this->my_auth->destroy(this->my_auth); this->my_auth = NULL; this->do_another_auth = do_another_auth(this); From cc787697b8f9cb2bc1480c8e43631e585bb28be7 Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Tue, 26 Feb 2013 12:26:31 +0100 Subject: [PATCH 2/3] Be a little more verbose why a peer_cfg is inacceptable --- src/libcharon/sa/ikev2/tasks/ike_auth.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/src/libcharon/sa/ikev2/tasks/ike_auth.c b/src/libcharon/sa/ikev2/tasks/ike_auth.c index b6e4745c4..970e38cf5 100644 --- a/src/libcharon/sa/ikev2/tasks/ike_auth.c +++ b/src/libcharon/sa/ikev2/tasks/ike_auth.c @@ -319,7 +319,7 @@ static bool update_cfg_candidates(private_ike_auth_t *this, bool strict) { if (this->peer_cfg) { - bool complies = TRUE; + char *comply_error = NULL; enumerator_t *e1, *e2, *tmp; auth_cfg_t *c1, *c2; @@ -336,22 +336,30 @@ static bool update_cfg_candidates(private_ike_auth_t *this, bool strict) while (e1->enumerate(e1, &c1)) { /* check if done authentications comply to configured ones */ - if ((!e2->enumerate(e2, &c2)) || - (!strict && !c1->complies(c1, c2, TRUE)) || - (strict && !c2->complies(c2, c1, TRUE))) + if (!e2->enumerate(e2, &c2)) { - complies = FALSE; + comply_error = "insufficient authentication rounds"; + break; + } + if (!strict && !c1->complies(c1, c2, TRUE)) + { + comply_error = "non-matching authentication done"; + break; + } + if (strict && !c2->complies(c2, c1, TRUE)) + { + comply_error = "constraint checking failed"; break; } } e1->destroy(e1); e2->destroy(e2); - if (complies) + if (!comply_error) { break; } - DBG1(DBG_CFG, "selected peer config '%s' inacceptable", - this->peer_cfg->get_name(this->peer_cfg)); + DBG1(DBG_CFG, "selected peer config '%s' inacceptable: %s", + this->peer_cfg->get_name(this->peer_cfg), comply_error); this->peer_cfg->destroy(this->peer_cfg); } if (this->candidates->remove_first(this->candidates, From d8a94c18c61a8a13b0e400a6f1daa977a43f716b Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Tue, 26 Feb 2013 13:07:11 +0100 Subject: [PATCH 3/3] Apply a mutual EAP auth_cfg not before the EAP method completes --- .../sa/ikev2/authenticators/eap_authenticator.c | 10 ++++++++++ src/libcharon/sa/ikev2/tasks/ike_auth.c | 9 ++++++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/libcharon/sa/ikev2/authenticators/eap_authenticator.c b/src/libcharon/sa/ikev2/authenticators/eap_authenticator.c index aa0644033..b8359cc88 100644 --- a/src/libcharon/sa/ikev2/authenticators/eap_authenticator.c +++ b/src/libcharon/sa/ikev2/authenticators/eap_authenticator.c @@ -667,6 +667,16 @@ METHOD(authenticator_t, build_client, status_t, METHOD(authenticator_t, is_mutual, bool, private_eap_authenticator_t *this) { + if (this->method) + { + u_int32_t vendor; + + if (this->method->get_type(this->method, &vendor) != EAP_IDENTITY || + vendor != 0) + { + return this->method->is_mutual(this->method); + } + } /* we don't know yet, but insist on it after EAP is complete */ this->require_mutual = TRUE; return TRUE; diff --git a/src/libcharon/sa/ikev2/tasks/ike_auth.c b/src/libcharon/sa/ikev2/tasks/ike_auth.c index 970e38cf5..942f97cf5 100644 --- a/src/libcharon/sa/ikev2/tasks/ike_auth.c +++ b/src/libcharon/sa/ikev2/tasks/ike_auth.c @@ -980,7 +980,10 @@ METHOD(task_t, process_i, status_t, goto peer_auth_failed; } - apply_auth_cfg(this, FALSE); + if (!mutual_eap) + { + apply_auth_cfg(this, FALSE); + } } if (this->my_auth) @@ -989,6 +992,10 @@ METHOD(task_t, process_i, status_t, { case SUCCESS: apply_auth_cfg(this, TRUE); + if (this->my_auth->is_mutual(this->my_auth)) + { + apply_auth_cfg(this, FALSE); + } this->my_auth->destroy(this->my_auth); this->my_auth = NULL; this->do_another_auth = do_another_auth(this);