[eapol] Delay EAPoL-Start while waiting for EAP to complete

EAP exchanges may take a long time to reach a final status, especially
when relying upon MAC Authentication Bypass (MAB).  Our current
behaviour of sending EAPoL-Start every few seconds until a final
status is obtained can prevent these exchanges from ever completing.

Fix by redefining the EAP supplicant state to allow EAPoL-Start to be
suppressed: either temporarily (while waiting for a full EAP exchange
to complete, in which case we need to eventually resend EAPoL-Start if
the final Success or Failure packet is lost), or permanently (while
waiting for the potentially very long MAC Authentication Bypass
timeout period).

Signed-off-by: Michael Brown <mcb30@ipxe.org>

src/include/ipxe/eap.h

@@ -51,7 +51,7 @@ union eap_packet {
 	struct eap_request req;
 };
 
-/** Link block timeout
+/** EAP link block timeout
  *
  * We mark the link as blocked upon receiving a Request-Identity, on
  * the basis that this most likely indicates that the switch will not
@@ -64,12 +64,30 @@ union eap_packet {
  */
 #define EAP_BLOCK_TIMEOUT ( 45 * TICKS_PER_SEC )
 
+/** EAP protocol wait timeout
+ *
+ * In the EAP model, the supplicant is a pure responder.  The model
+ * also defines no acknowledgement response for the final Success or
+ * Failure "requests".  This leaves open the possibility that the
+ * final Success or Failure packet is lost, with the supplicant having
+ * no way to determine the final authentication status.
+ *
+ * Sideband mechanisms such as EAPoL-Start may be used to restart the
+ * entire EAP process, as a (crude) workaround for this protocol flaw.
+ * When expecting to receive a further EAP request (e.g. an
+ * authentication challenge), we may wait for some length of time
+ * before triggering this restart.  Choose a duration that is shorter
+ * than the link block timeout, so that there is no period during
+ * which we erroneously leave the link marked as not blocked.
+ */
+#define EAP_WAIT_TIMEOUT ( EAP_BLOCK_TIMEOUT * 7 / 8 )
+
 /** An EAP supplicant */
 struct eap_supplicant {
 	/** Network device */
 	struct net_device *netdev;
-	/** Authentication outcome is final */
+	/** Flags */
-	int done;
+	unsigned int flags;
 	/**
 	 * Transmit EAP response
 	 *
@@ -82,6 +100,23 @@ struct eap_supplicant {
 		       const void *data, size_t len );
 };
 
+/** EAP authentication is in progress
+ *
+ * This indicates that we have received an EAP Request-Identity, but
+ * have not yet received a final EAP Success or EAP Failure.
+ */
+#define EAP_FL_ONGOING 0x0001
+
+/** EAP supplicant is passive
+ *
+ * This indicates that the supplicant should not transmit any futher
+ * unsolicited packets (e.g. EAPoL-Start for a supplicant running over
+ * EAPoL).  This could be because authentication has already
+ * completed, or because we are relying upon MAC Authentication Bypass
+ * (MAB) which may have a very long timeout.
+ */
+#define EAP_FL_PASSIVE 0x0002
+
 extern int eap_rx ( struct eap_supplicant *supplicant,
 		    const void *data, size_t len );
 

src/net/eap.c

@@ -47,6 +47,14 @@ static int eap_rx_request_identity ( struct eap_supplicant *supplicant ) {
 	       netdev->name );
 	netdev_link_block ( netdev, EAP_BLOCK_TIMEOUT );
 
+	/* Mark EAP as in progress */
+	supplicant->flags |= EAP_FL_ONGOING;
+
+	/* We have no identity to offer, so wait until the switch
+	 * times out and switches to MAC Authentication Bypass (MAB).
+	 */
+	supplicant->flags |= EAP_FL_PASSIVE;
+
 	return 0;
 }
 
@@ -69,9 +77,6 @@ static int eap_rx_request ( struct eap_supplicant *supplicant,
 		return -EINVAL;
 	}
 
-	/* Mark authentication as incomplete */
-	supplicant->done = 0;
-
 	/* Handle according to type */
 	switch ( req->type ) {
 	case EAP_TYPE_IDENTITY:
@@ -94,7 +99,7 @@ static int eap_rx_success ( struct eap_supplicant *supplicant ) {
 	struct net_device *netdev = supplicant->netdev;
 
 	/* Mark authentication as complete */
-	supplicant->done = 1;
+	supplicant->flags = EAP_FL_PASSIVE;
 
 	/* Mark link as unblocked */
 	DBGC ( netdev, "EAP %s Success\n", netdev->name );
@@ -113,7 +118,7 @@ static int eap_rx_failure ( struct eap_supplicant *supplicant ) {
 	struct net_device *netdev = supplicant->netdev;
 
 	/* Mark authentication as complete */
-	supplicant->done = 1;
+	supplicant->flags = EAP_FL_PASSIVE;
 
 	/* Record error */
 	DBGC ( netdev, "EAP %s Failure\n", netdev->name );

src/net/eapol.c

@@ -48,37 +48,6 @@ static const uint8_t eapol_mac[ETH_ALEN] = {
 	0x01, 0x80, 0xc2, 0x00, 0x00, 0x03
 };
 
-/**
- * Update EAPoL supplicant state
- *
- * @v supplicant	EAPoL supplicant
- * @v timeout		Timer ticks until next EAPoL-Start (if applicable)
- */
-static void eapol_update ( struct eapol_supplicant *supplicant,
-			   unsigned long timeout ) {
-	struct net_device *netdev = supplicant->eap.netdev;
-
-	/* Check device and EAP state */
-	if ( netdev_is_open ( netdev ) && netdev_link_ok ( netdev ) ) {
-		if ( supplicant->eap.done ) {
-
-			/* EAP has completed: stop sending EAPoL-Start */
-			stop_timer ( &supplicant->timer );
-
-		} else if ( ! timer_running ( &supplicant->timer ) ) {
-
-			/* EAP has not yet begun: start sending EAPoL-Start */
-			start_timer_fixed ( &supplicant->timer, timeout );
-		}
-
-	} else {
-
-		/* Not ready: clear completion and stop sending EAPoL-Start */
-		supplicant->eap.done = 0;
-		stop_timer ( &supplicant->timer );
-	}
-}
-
 /**
  * Process EAPoL packet
  *
@@ -186,8 +155,19 @@ static int eapol_eap_rx ( struct eapol_supplicant *supplicant,
 		goto drop;
 	}
 
-	/* Update supplicant state */
+	/* Update EAPoL-Start transmission timer */
-	eapol_update ( supplicant, EAPOL_START_INTERVAL );
+	if ( supplicant->eap.flags & EAP_FL_PASSIVE ) {
+		/* Stop sending EAPoL-Start */
+		if ( timer_running ( &supplicant->timer ) ) {
+			DBGC ( netdev, "EAPOL %s becoming passive\n",
+			       netdev->name );
+		}
+		stop_timer ( &supplicant->timer );
+	} else if ( supplicant->eap.flags & EAP_FL_ONGOING ) {
+		/* Delay EAPoL-Start until after next expected packet */
+		DBGC ( netdev, "EAPOL %s deferring Start\n", netdev->name );
+		start_timer_fixed ( &supplicant->timer, EAP_WAIT_TIMEOUT );
+	}
 
  drop:
 	free_iob ( iobuf );
@@ -309,15 +289,35 @@ static int eapol_probe ( struct net_device *netdev, void *priv ) {
  * @v netdev		Network device
  * @v priv		Private data
  */
-static void eapol_notify ( struct net_device *netdev __unused, void *priv ) {
+static void eapol_notify ( struct net_device *netdev, void *priv ) {
 	struct eapol_supplicant *supplicant = priv;
 
 	/* Ignore non-EAPoL devices */
 	if ( ! supplicant->eap.netdev )
 		return;
 
-	/* Update supplicant state */
+	/* Terminate and reset EAP when link goes down */
-	eapol_update ( supplicant, 0 );
+	if ( ! ( netdev_is_open ( netdev ) && netdev_link_ok ( netdev ) ) ) {
+		if ( timer_running ( &supplicant->timer ) ) {
+			DBGC ( netdev, "EAPOL %s shutting down\n",
+			       netdev->name );
+		}
+		supplicant->eap.flags = 0;
+		stop_timer ( &supplicant->timer );
+		return;
+	}
+
+	/* Do nothing if EAP is already in progress */
+	if ( timer_running ( &supplicant->timer ) )
+		return;
+
+	/* Do nothing if EAP has already finished transmitting */
+	if ( supplicant->eap.flags & EAP_FL_PASSIVE )
+		return;
+
+	/* Otherwise, start sending EAPoL-Start */
+	start_timer_nodelay ( &supplicant->timer );
+	DBGC ( netdev, "EAPOL %s starting up\n", netdev->name );
 }
 
 /** EAPoL driver */