From 96d02c1a6c9a4cf9eedbc5f0bcc1c176b55de05f Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Wed, 3 Jun 2015 10:52:34 +0200 Subject: [PATCH] ikev2: Enforce remote authentication config before proceeding with own authentication Previously the constraints in the authentication configuration of an initiator were enforced only after all authentication rounds were complete. This posed a problem if an initiator used EAP or PSK authentication while the responder was authenticated with a certificate and if a rogue server was able to authenticate itself with a valid certificate issued by any CA the initiator trusted. Because any constraints for the responder's identity (rightid) or other aspects of the authentication (e.g. rightca) the initiator had were not enforced until the initiator itself finished its authentication such a rogue responder was able to acquire usernames and password hashes from the client. And if a client supported EAP-GTC it was even possible to trick it into sending plaintext passwords. This patch enforces the configured constraints right after the responder's authentication successfully finished for each round and before the initiator starts with its own authentication. Fixes CVE-2015-4171. --- src/charon/sa/tasks/ike_auth.c | 69 ++++++++++++++++++++++++++++++------------ 1 file changed, 49 insertions(+), 20 deletions(-) diff --git a/src/charon/sa/tasks/ike_auth.c b/src/charon/sa/tasks/ike_auth.c index 2c15ac765ebb..0efc5402c16b 100644 --- a/src/charon/sa/tasks/ike_auth.c +++ b/src/charon/sa/tasks/ike_auth.c @@ -745,6 +745,27 @@ static status_t build_r(private_ike_auth_t *this, message_t *message) } /** + * Check if strict constraint fullfillment required to continue current auth + */ +static bool require_strict(private_ike_auth_t *this) +{ + auth_cfg_t *cfg; + + cfg = this->ike_sa->get_auth_cfg(this->ike_sa, TRUE); + switch ((uintptr_t)cfg->get(cfg, AUTH_RULE_AUTH_CLASS)) + { + case AUTH_CLASS_EAP: + return TRUE; + case AUTH_CLASS_PSK: + return TRUE; + case AUTH_CLASS_PUBKEY: + case AUTH_CLASS_ANY: + default: + return FALSE; + } +} + +/** * Implementation of task_t.process for initiator */ static status_t process_i(private_ike_auth_t *this, message_t *message) @@ -812,26 +833,6 @@ static status_t process_i(private_ike_auth_t *this, message_t *message) } enumerator->destroy(enumerator); - if (this->my_auth) - { - 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->my_cfgs->insert_last(this->my_cfgs, cfg); - this->my_auth->destroy(this->my_auth); - this->my_auth = NULL; - this->do_another_auth = do_another_auth(this); - break; - case NEED_MORE: - break; - default: - return FAILED; - } - } - if (this->expect_another_auth) { if (this->other_auth == NULL) @@ -894,6 +895,34 @@ static status_t process_i(private_ike_auth_t *this, message_t *message) } } + if (require_strict(this)) + { + if (!update_cfg_candidates(this, TRUE)) + { + return FAILED; + } + } + + if (this->my_auth) + { + 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->my_cfgs->insert_last(this->my_cfgs, cfg); + this->my_auth->destroy(this->my_auth); + this->my_auth = NULL; + this->do_another_auth = do_another_auth(this); + break; + case NEED_MORE: + break; + default: + return FAILED; + } + } + if (message->get_notify(message, ANOTHER_AUTH_FOLLOWS) == NULL) { this->expect_another_auth = FALSE; -- 1.9.1