eap Mailing List: RE: [Issue 203] Comments on EAP-Peer state machine

[Joseph Salowey (jsalowey]

Hi Pasi,

Comments inline.

Cheers,

Joe

> -----Original Message-----
> From: Pasi.Eronen [at] nokia.com [mailto:Pasi.Eronen [at] nokia.com] 
> Sent: Monday, December 15, 2003 4:54 AM
> To: jsalowey [at] cisco.com; eap [at] frascone.com
> Subject: RE: [eap] [Issue 203] Comments on EAP-Peer state machine
> 
> 
> 
> Thanks for your comments, Joe! (and sorry it took 
> me so long to respond).
> 
> - I think items 1 and 2 are addressed in proposed resolution 
>   of issue 198:
>   
> http://mail.frascone.com/pipermail/public/eap/2003-November/00
> 1852.html
> 
[Joe] I agree

> - Item 3 is a bit more complicated. 
> 
>   Integrity check is currently a separate procedure to show that 
>   when a packet is silently discarded, methodState and decision 
>   (and other state) remain unchanged. You're right in that a 
>   method can produce an error for invalid packets instead...
>   Maybe it would be better if we rename m.integrityCheck() to 
>   m.check() and "intCheck" to "discard"? (and change transition 
>   "!intCheck" to "discard")
>   
>   Combining methodState and decision to a single variable is 
>   certainly possible, but four different states (plus one 
>   indication for IGNORE) are not enough: there are seven different
>   combinations. The reason we originally split it was to make the 
>   transitions to SUCCESS and FAILURE states a bit more compact 
>   (they're quite complex anyway, but become totally unreadable 
>   if the variables are combined...). 
> 

[Joe] I think integrity check is internal to the mechanism and should
not be exposed outside the mechanism. I think there should be a single
call into the method that results in one of n possible results.  In
looking at the state machine I counted 5: IGNORE, CONTINUE,
COND_SUCCESS, SUCCESS, FAILURE.  What are the additional values?

>   One thing we could do is get rid of the "x = FOO | BAR" notation,
>   and instead make m.process() set the variables, and change
>   METHOD pseudocode to something like this:
> 
>     discard = m.check(eapReqData)
>     if (!discard) {
>       (methodState,decision,allowNotifications) = 
>         m.process(eapReqData)
>       /* methodState is CONT, MAY_CONT, or DONE */ 
>       /* decision is FAIL, COND_SUCC or UNCOND_SUCC */
>       eapRespData = m.buildResp(reqId)
>       if (m.isKeyAvailable())
>         eapKeyData = m.getKey()
>     }
> 
>   Would this be better?
>
[Joe] Not really, I don't like breaking up m.check() and m.process().  I
think they should be part of the same method.  I also think the method
state and decision should be combined if possible.  I would rather go
with the proposal pasted from below with additional variables added to
method result if necessary. 

> > 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()
> >    }
> > }

> - Item 4: I think your suggestion (reset timer in INITIALIZE 
>   and SEND_RESPONSE) is ok.
> 
> - Item 5: it might check (reqId != lastReqId) (not "=="), 
>   but I don't think this is necessary, since Success/Failure 
>   are never retransmitted.

[Joe] I've lost the context to this, but I thought that EAP-Success and
EAP-Failure had the same ID as the previous request and response.  From
2284bis Section 4.2:

"Identifier

The Identifier field is one octet and aids in matching replies to
Responses.  The Identifier field MUST match the Identifier field
of the Response packet that it is sent in response to."



> 
> Best regards,
> Pasi
> 
> > -----Original Message-----
> > From: eap-admin [at] frascone.com
> > [mailto:eap-admin [at] frascone.com]On Behalf Of
> > ext Joseph Salowey
> > Sent: Thursday, November 20, 2003 11:27 PM
> > To: eap [at] frascone.com
> > Subject: [eap] [Issue] Comments on EAP-Peer state machine
> > 
> > 
> > Submitter name: Joe Salowey
> > Submitter email address: jsalowey [at] cisco.com
> > 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
> > eap [at] frascone.com http://mail.frascone.com/mailman/listinfo/eap
> > 
>