eap-authenticator: Enforce failure if MSK generation fails

Without this, the authentication succeeded if the server sent an early
EAP-Success message for mutual, key-generating EAP methods like EAP-TLS,
which may be used in EAP-only scenarios but would complete without server
or client authentication.  For clients configured for such EAP-only
scenarios, a rogue server could capture traffic after the tunnel is
established or even access hosts behind the client.  For non-mutual EAP
methods, public key server authentication has been enforced for a while.

A server previously could also crash a client by sending an EAP-Success
immediately without initiating an actual EAP method.

Fixes: 0706c39cda52 ("added support for EAP methods not establishing an MSK")
Fixes: CVE-2021-45079

diff --git a/src/libcharon/plugins/eap_gtc/eap_gtc.c b/src/libcharon/plugins/eap_gtc/eap_gtc.c
index 95ba090b79ced2843369ab629cbb5e7d3ddd2566..cffb6222c2f830ce4f47d098f5e369a7c31348e5 100644 (file)
--- a/src/libcharon/plugins/eap_gtc/eap_gtc.c
+++ b/src/libcharon/plugins/eap_gtc/eap_gtc.c
@@ -195,7 +195,7 @@ METHOD(eap_method_t, get_type, eap_type_t,
 METHOD(eap_method_t, get_msk, status_t,
        private_eap_gtc_t *this, chunk_t *msk)
 {
-       return FAILED;
+       return NOT_SUPPORTED;
 }
 
 METHOD(eap_method_t, get_identifier, uint8_t,

diff --git a/src/libcharon/plugins/eap_md5/eap_md5.c b/src/libcharon/plugins/eap_md5/eap_md5.c
index ab5f7ff6a8231a972e08d47f2a4fdd4b619462df..3a92ad7c0a04fcb1b0a8a25cce63472fbc630f9e 100644 (file)
--- a/src/libcharon/plugins/eap_md5/eap_md5.c
+++ b/src/libcharon/plugins/eap_md5/eap_md5.c
@@ -213,7 +213,7 @@ METHOD(eap_method_t, get_type, eap_type_t,
 METHOD(eap_method_t, get_msk, status_t,
        private_eap_md5_t *this, chunk_t *msk)
 {
-       return FAILED;
+       return NOT_SUPPORTED;
 }
 
 METHOD(eap_method_t, is_mutual, bool,

diff --git a/src/libcharon/plugins/eap_radius/eap_radius.c b/src/libcharon/plugins/eap_radius/eap_radius.c
index 2dc7a423e702fc2dbc4f1a61713cadaa4f9ddf84..5336dead13d9fe1777e302fd65ad90158b8175c7 100644 (file)
--- a/src/libcharon/plugins/eap_radius/eap_radius.c
+++ b/src/libcharon/plugins/eap_radius/eap_radius.c
@@ -733,7 +733,9 @@ METHOD(eap_method_t, get_msk, status_t,
                *out = msk;
                return SUCCESS;
        }
-       return FAILED;
+       /* we assume the selected method did not establish an MSK, if it failed
+        * to establish one, process() would have failed */
+       return NOT_SUPPORTED;
 }
 
 METHOD(eap_method_t, get_identifier, uint8_t,

diff --git a/src/libcharon/sa/eap/eap_method.h b/src/libcharon/sa/eap/eap_method.h
index 0b5218dfec155817ffb8384624a169394afa92c8..33564831f86e352ab52ce84a71219028cc6de540 100644 (file)
--- a/src/libcharon/sa/eap/eap_method.h
+++ b/src/libcharon/sa/eap/eap_method.h
@@ -114,10 +114,16 @@ struct eap_method_t {
         * Not all EAP methods establish a shared secret. For implementations of
         * the EAP-Identity method, get_msk() returns the received identity.
         *
+        * @note Returning NOT_SUPPORTED is important for implementations of EAP
+        * methods that don't establish an MSK.  In particular as client because
+        * key-generating EAP methods MUST fail to process EAP-Success messages if
+        * no MSK is established.
+        *
         * @param msk                   chunk receiving internal stored MSK
         * @return
-        *                                              - SUCCESS, or
+        *                                              - SUCCESS, if MSK is established
         *                                              - FAILED, if MSK not established (yet)
+        *                                              - NOT_SUPPORTED, for non-MSK-establishing methods
         */
        status_t (*get_msk) (eap_method_t *this, chunk_t *msk);
 

diff --git a/src/libcharon/sa/ikev2/authenticators/eap_authenticator.c b/src/libcharon/sa/ikev2/authenticators/eap_authenticator.c
index e1e6cd7ee6f30ef332a585a24dfed97f7cc57ac3..87548fc471a6cb0b7f021a2ad0e5206d17dfa561 100644 (file)
--- a/src/libcharon/sa/ikev2/authenticators/eap_authenticator.c
+++ b/src/libcharon/sa/ikev2/authenticators/eap_authenticator.c
@@ -305,9 +305,17 @@ static eap_payload_t* server_process_eap(private_eap_authenticator_t *this,
                                this->method->destroy(this->method);
                                return server_initiate_eap(this, FALSE);
                        }
-                       if (this->method->get_msk(this->method, &this->msk) == SUCCESS)
+                       switch (this->method->get_msk(this->method, &this->msk))
                        {
-                               this->msk = chunk_clone(this->msk);
+                               case SUCCESS:
+                                       this->msk = chunk_clone(this->msk);
+                                       break;
+                               case NOT_SUPPORTED:
+                                       break;
+                               case FAILED:
+                               default:
+                                       DBG1(DBG_IKE, "failed to establish MSK");
+                                       goto failure;
                        }
                        if (vendor)
                        {
@@ -326,6 +334,7 @@ static eap_payload_t* server_process_eap(private_eap_authenticator_t *this,
                        return eap_payload_create_code(EAP_SUCCESS, in->get_identifier(in));
                case FAILED:
                default:
+failure:
                        /* type might have changed for virtual methods */
                        type = this->method->get_type(this->method, &vendor);
                        if (vendor)
@@ -661,9 +670,24 @@ METHOD(authenticator_t, process_client, status_t,
                                uint32_t vendor;
                                auth_cfg_t *cfg;
 
-                               if (this->method->get_msk(this->method, &this->msk) == SUCCESS)
+                               if (!this->method)
                                {
-                                       this->msk = chunk_clone(this->msk);
+                                       DBG1(DBG_IKE, "received unexpected %N",
+                                                eap_code_names, eap_payload->get_code(eap_payload));
+                                       return FAILED;
+                               }
+                               switch (this->method->get_msk(this->method, &this->msk))
+                               {
+                                       case SUCCESS:
+                                               this->msk = chunk_clone(this->msk);
+                                               break;
+                                       case NOT_SUPPORTED:
+                                               break;
+                                       case FAILED:
+                                       default:
+                                               DBG1(DBG_IKE, "received %N but failed to establish MSK",
+                                                        eap_code_names, eap_payload->get_code(eap_payload));
+                                               return FAILED;
                                }
                                type = this->method->get_type(this->method, &vendor);
                                if (vendor)