Subject: [eap] review of sm-03: authenticator

From: [email-masked] (Jari Arkko)
Date: Mon, 23 Jun 2003 13:24:40 +0300

I'm reviewing the state machine draft version 03
(http://www.cs.umd.edu/~npetroni/EAP/draft-vollbrecht-eap-state-03.ps).
The authenticator state machine in Section 5 appears to be in
very good shape, I like it!

I have a few nits and question marks, however:

1) The three calls to Policy.update have different
    argument lists. It would help the reader if the
    state machine explicitly defined what input goes
    to this function. Looks like it is given eapRespData
    (either in a method or in a NAK), as well as the flags
    that the authenticator has already calculated, e.g.,
    was the response valid. How about

      Policy.update(currentMethod, eapRespData, intCheck)

2) Can't find calculateTimeout from 5.3.

3) I'd rather not see a "..." in the calculateTimeout
    argument list. Can we be more specific? See also
    item 1 above.

4) I'm not sure if the m.buildSuccFail in the diagram
    (state METHOD) and 2284bis Section 4.2 requirements
    are aligned. The latter seems to require that after
    a method specific indication, we still must send
    a Success or Failure. But maybe I'm just confused
    by what "m.buildSuccFail" means since I haven't
    read the passthrough part of the document yet.

5) Similarly, I'm not sure the state machine prevents
    the sending of a canned Success. These seem to be
    forbidden by 2284bis Section 4.2.

6) Should there be initializations of all variables
    in the DISABLED and BACKEND_DISABLED states? Variables
    such as eapFail do not appear to get an initial
    value, at least not in the diagram. The text says
    it is initialized by the lower layer, but why? Wouldn't
    it be easier to treat this variable as an EAP layer
    variable which is only read by the lower layer, not set?

7) Section 5.1.1 s/outside/Outside/

8) Section 5.2 indentation looks funny.

9) Can you explain why BACKEND_DISABLED and DISABLED
    have to be separate states? It looks like they
    could be combined, and the EapBackend variable
    could be checked before branching on to the next
    states. But perhaps this has something to do with
    the IEEE lower layer state machine interface?

10) Why not use the same substructure in 5.2 as is used
     in 5.1? Variables from method->eap layer, eap layer->
     method, etc. Then you could also group 5.4 contents into
     its appropriate place in 5.1/5.2/5.3.

--Jari

---

From: [email-masked] (Jari Arkko)
Date: Tue, 24 Jun 2003 09:51:28 +0300

Additional comments on the  state machine draft version 03
(http://www.cs.umd.edu/~npetroni/EAP/draft-vollbrecht-eap-state-03.ps).
Overall, I like the way that you have represented the passthrough
and direct/AAA authenticators. The separation of the state machines
to small pieces in nicely organized layers is very good design --
thanks!

I do have some comments, though:

1) In the overview figure in Section 6.2, if I have understood
    correctly the two authenticator state machines have been
    instantiated with a different EapBackend value. Perhaps
    this could be shown in the figure.

2) It would be very useful to have the same kind of
    variable and interface descriptions following the passthrough
    and backend state machines as were given for the authenticator.

3) I don't understand why Policy.update needs to be called
    in the AAA_FAILURE state of the passthrough state machine.
    If this happen, we fail completely, right?

4) Similarly, I'm not sure I understand what the policy updates
    do in AAA_REJECT and AAA_ACCEPT. Can you clarify?

5) In state AAA_REQUEST, maybe change
      if (isIdentity(eapRespData))
    to
      if (respId == currentId)
    unless I'm missing something...

6) The passthrough state machine does not appear to contain
    all processing required by RFC 2869bis. For instance,
    Section 2.6.3 about conflicting messages is not described.
    This may be fine, but at least there should be an
    explanation of the scope of the state machine.

7) References need to be split to normative and non-normative.
    I'd say 2284bis and 2869bis are normative along with RFC 2119.
    IEEE is non-normative.

8) Are these all references? I suspect at least the keying
    framework needs to be referenced.

9) The format used for describing the state machine and
    the actions. Do you have a possibility for feeding this
    into a state machine tool for analysis?

    Also, http://www.ietf.org/IESG/STATEMENTS/pseudo-code-in-specs.txt
    mentions that you can use languages such as C. However, the IESG
    statement said that it must be clear whether you just use
    some pseudo code that just looks like C or the language itself.
    I think we have the former case. This should be stated.

    Finally, the IESG statement requires a normative reference
    to the used language or format. In this case I don't think
    we have it, or? If not, maybe we should be more strict in
    defining the language used for the actions, say with BNF,
    including semantics defined in English?

--Jari