Subject: [eap] [Issue] Comments on EAP-Peer state machine

From: "Joseph Salowey" <[email-masked]>
Date: Thu, 20 Nov 2003 13:26:51 -0800

Submitter name: Joe Salowey
Submitter email address: [email-masked]
Date first submitted: 11/20/2003
Reference: 
Document:  State Machine
Comment type: 'E'ditorial
Priority: '1' Should fix 
Section: 4.x
Rationale/Explanation of issue:

1. portEnabled

portEnabled seems very .1x/PPP specific.  EAP is being used in cases
such as IKE and PANA where this concept may a bit more abstract.
Perhaps we can change the name of the variable and/or expand its
definition to include other cases.  

Suggestion:
Perhaps "indicates that a lower layer communcation channel has been
established".

2. EAPTunneled

The behavior inside tunnels needs to be defined by the tunnel as there
are severl ways you can handle EAP within a tunnel.  I think we decided
to remove this variable at the meeting, but I want to mae sure we track
it.  In PEAPv2 we started out with a behavior similar to what is
described in the state machine,  but in order to make sure the policy on
the peer and authenticator stayed in sync we introduced itermediate
result indicators which I think change the state machine a little.

Suggestion:
Remove variable.  We could explore this in an apppendix or a separate
document.

3. Interface to method

The interface to the method seems too complicated.  

First I don't think that intCheck should be a different process.
Integrity checking is done as part of the method processing.  Also
methods aren't required to ingore packets, some methods with ignore some
problems and return errors on others.  I think this behavior should be
incorporated into m.process.

On a separate but related note it seems that the method state and
decision is very complex. I don't really see the need for the
independent methodState and decision variables.  M.process() should
return one of serverl possible results: IGNORE, CONTINUE, COND_SUCCESS,
SUCCESS, FAILURE.  MethodState should be able to take on CONTINUE,
COND_SUCCESS, SUCCESS, FAILURE.  I don't think a separate decision
variable is necessary.

Suggestion

In method:

methodResult = m.process(eapRespData)
If (methodResult != IGNORE)
{
   methodState = methodResult
   allowNotifications = TRUE|FALSE
   eapRespData = m.buildResponse(reqID)
   if (methodState == SUCCESS || methodState == COND_SUCCESS)
   {
	eapKeyData = NONE|m.getKey()
   }
}

Transition to discard: (methodResult == IGNORE)
Combine methodState and decision transition conditions

4. Idle timer

It seems that there is a problem with the idle timer.  It would be
possible for the peer to never time out if it keeps on receiving packets
that it discards.  This is not so good.  

Suggestion:

Perhaps the client timeout should be set outside the IDLE state in the
INITIALIZE state and in the METHOD or SEND_RESPONSE state.

5.  rxSuccess and rxFailure

Shouldn't rxSuccess and rxFailure check to see if (reqId == lastReqID)?

_______________________________________________
eap mailing list
[email-masked]
http://mail.frascone.com/mailman/listinfo/eap

---

From: Nick Petroni <[email-masked]>
Date: Wed, 26 Nov 2003 14:01:03 -0500 (EST)

> Perhaps "indicates that a lower layer communcation channel has been
[npetroni] this is fine for me. the name is not really of importance to me
so I would just as soon leave it the same. If others think a name change
is necessary it's fine. I like the definition change.

> Remove variable.  We could explore this in an apppendix or a separate
[npetroni] previously disussed. I agree it should be removed.

> incorporated into m.process.
[npetroni] intCheck is needed because SOMETIMES a method will decide not
to use the packet at all. in this case, intCheck is the way the method
tells the SM it will not be using that packet. Methods with other ways of
dealing with this will just not set intCheck to be true.

> COND_SUCCESS, SUCCESS, FAILURE.  I don't think a separate decision
[npetroni] This is meant to show that a method really has its own state
machine and that these states are separate from the final decision. I
think it provides nice way of separating the two, but I am willing to
discuss this alternative notation.

> that it discards.  This is not so good.
[npetroni] hmm, that seems like a trivial DoS, no? Send enough bad packets
to the Peer and it goes away for a while?

> Shouldn't rxSuccess and rxFailure check to see if (reqId == lastReqID)?
[npetroni] probably so. I think this should be added.