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

From: <[email-masked]>
Date: Mon, 15 Dec 2003 14:54:18 +0200

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/001852.html

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

  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?

- 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.

Best regards,
Pasi

> -----Original Message-----
> From: [email-masked] 
> [mailto:[email-masked]]On Behalf Of
> ext Joseph Salowey
> Sent: Thursday, November 20, 2003 11:27 PM
> To: [email-masked]
> Subject: [eap] [Issue] Comments on EAP-Peer state machine
> 
> 
> 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
> 
_______________________________________________
eap mailing list
[email-masked]
http://mail.frascone.com/mailman/listinfo/eap

---

From: Nick Petroni <[email-masked]>
Date: Mon, 15 Dec 2003 08:05:46 -0500 (EST)

Hi Pasi, just one quick note-

>   but I don't think this is necessary, since Success/Failure
It's still a violation of the protocol though for the packet to have the
same ID as the previous one. I agree the change won't affect how one
transitions in the state machine since eventually you will go to success
anyway, but it seems if the identity were not new the packet should be
discarded.

Regards,
nick

---

From: "Joseph Salowey" <[email-masked]>
Date: Mon, 15 Dec 2003 20:10:12 -0800

Hi Pasi,

Comments inline.

Cheers,

Joe

> From: [email-masked] [mailto:[email-masked]] 
> Sent: Monday, December 15, 2003 4:54 AM
> To: [email-masked]; [email-masked]
> Subject: RE: [eap] [Issue 203] Comments on EAP-Peer state machine
> 
> 
> 
[Joe] I agree

>   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?

>       eapRespData = m.buildResp(reqId)
[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. 

>   but I don't think this is necessary, since Success/Failure 

[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."

> > 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
> > 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.
> > 
> > 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:
> > 
> > possible for the peer to never time out if it keeps on receiving 
> > packets that it discards.  This is not so good.
> > 
> > Suggestion:
> > 
> > [email-masked] http://mail.frascone.com/mailman/listinfo/eap
> > 
>

---

From: Nick Petroni <[email-masked]>
Date: Tue, 16 Dec 2003 02:28:54 -0500 (EST)

Hi Joe,

> of the Response packet that it is sent in response to."

Sorry, I lead us down the wrong path. I misunderstood how this works. I am
still confused why it works this way, but either way you are correct
-- the Success should match the last Request/Response ID.

Regards,
nick