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

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,

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,

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,

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);
 

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)