eap Mailing List: Re: Issue 193: Peer SM review

[Nick Petroni (npetroni]

Sorry I am just getting around to commenting. Comments inline.

>
> "pass EAP messages to a Backend Server where the real Authentication
> Method resides"
>
> As is made clear later, the pass-through authenticator does *not* forward
> all EAP messages to the backend authentication server, only EAP-Responses.
> Rephrase to:
>
> "pass EAP-Response messages to a Backend Server where the Authentication
> Method resides"

ok.

> Third paragraph:
>
> The implication is the EAP method demultiplexing depends on the lower
> layer. This is not the case because all EAP implementations on all media
> must support  demultiplexing (whether they support an authenticator or
> peer listener is another issue).
>
> Suggest replacing the paragraph with:
>
> "As noted in RFC 2284bis, Section 2.3, the EAP layer demultiplexes
> incoming EAP packets. Packets with Code=2 (Response) are delivered to the
> Authenticator state  machine, while packets with Code=1 (Request), 3
> (Success) or 4 (Failure) are  delivered to the Peer state machine."

ok.

> Page 9, Figure 3.
>
> The notation used later (Figure 6) is more clear than the use of the "|"
> operator as in Figure 3. I'd recommend adopting the Figure 6 notation
> everywhere.

sure, it doesn't matter to me. The notation is clearly stated, but it's
fine if the second way is more readable.

> It would appear to me that methodState is a variable that needs to be
> exported from the peer to the lower layer. This is so that lower layer
> state  machines such as 802.1X can know whether their own state machines
> should be  affected or not.

already discussed, I agree with Yoshihiro.

>
> The INITIALIZE state does not initialize all variables. Non-initialized
> variables
> include:
>
> eapKeyData = NONE
> eapKeyAvailable = FALSE
> eapSuccess = FALSE
> eapFail = FALSE
> methodState = NONE
> selectedMethod = NONE
> decision = FAIL
>
> The initialization values seem important since transitions can be based on
> them before they are initialized. Re-initializing variables in the
> INITIALIZE  state is a bit different from giving the variables initial
> values before they are ever used.

agreed.

>
> Not sure what "decision = FAIL | COND_SUCC" means in the INITIALIZE state.
> I assume this means that this variable can be assigned one of two values,
> at the  implementation's descretion. I think we want this variable
> initialized to FAIL.

The problem here is that while the default value is to NOT accept a canned
success, there are protocols wehre canned success is used. For example,
figure 8-10 of 802.1X-REV contains a txCannedSuccess() function. If we don
not handle this on the EAP side, they will have to explicitly check for it
themselves. Perhaps we should note that the preferred vale is FAIL, but
that COND_SUCC is also possible for those willing to accept canned succes.

> There is a transition from the IDLE state to the SUCCESS and FAILURE
> states. This occurs without appearing to require receipt of a packet. I
> think that this can't  be the case;  something has to be received at some
> point for the peer to leave idleWhile  OR it is a situation where the peer
> is testing whether the authenticator is  EAP-capable (e.g.
> a wired LAN application).
perhaps this should be a separate issue. It seems that if a canned success
is allowed, a canned alternative success also should be,but this reduces
to a no-EAP-message protocol. Sounds funny.

> The transition to FAILURE state occurs when an alternative indication of
> Rejection occurs (e.g. Disassociate); when the idleWhile timer runs out
> and the decision is not an unconditional success; when an alternative
> indication of success is available but the decision is FAIL.
>
> There is also a transition from the IDLE state to the SUCCESS state.
> This can occur when the idleWhile timer runs out and the decision is
> an unconditional success; or when an alternative indication of success
> is available and the decision is not FAILURE.
>
> It seems that the INITIALIZE state will need to have the correct default
> values or these transitions may not occur correctly.

yes. agree the initailizations should be explicit.