eap Mailing List: [Issue 247] Comments on EAP state machine v4

[Nick Petroni (npetroni]

Florent,
Sorry for the delayed response, some comments are inline. I will respond
to the other issues and hope to hear back from others on these.

Thanks,
nick

> I know that these comments may:
> * Come much too late (since IETF last call has ended 2004-05-13 so it
> shouldn't be the time IINM to propose any "real" changes to this document)
> * Be completely naive or off the rocket
> * Echo some comments that have already been made (although I tried my
> best to reread - and understand ;-) - the EAP state machine issues
> tracked by Bernard
It's fine, I think the document should be the best it can be. Thanks for
taking the time.


> Comment #1 - Editorial
>
> In section 3.2 state machine symbols, we can read:
> "+ Arithmetic addition operator.
> - Arithmetic subtraction operator."
>
> I find these definitions useless (IINM we never see the notation + in
> the state machines and the notation ++ which is perhaps why this was
> included should either be defined directly or assumed understandable as
> it is said in section 3.1 "The interpretation of the special symbols and
> operators used in the state diagrams is as defined in Section 3.2; these
> symbols and operators are derived from the notation of the C++
> programming language, ISO/IEC 14882.") BTW, I cannot find any use of -
> or -- in the document. And anyway, I think that in case + and - are
> however necessary for the comprehension of the document, it should be
> stated on which space they operate (N, Z/2**16Z, Z/2**32Z...)
That description was taken from IEEE 802.1X-REV and the notation
was chosen so that both documents could be easily read together. It is
true that the current version of SM document does not use "+" or "-", but
I can assure you that at certain points during the document's lifecycle it
did. Now that things are basically stable in the document I would be fine
removing these, but if it requires outside approval (and if that process
is overly difficult) I would just as soon
leave them for the sake of expediency. IMHO they are not *that* confusing,
even on the issue of space of operation, to most implementers.

Since you brough up the issue of operators, I also noticed that '>=' is
not used but is defined, while '<=' is used but is not defined. Perhaps we
should clean up all of these at one time and just identify symbols that
are used? Are we missing any others?

> Comment #2 - Editorial
>
> This is about portEnabled and eapRestart.
>
> This discussion for what regards portEnabled is a follow-up on issues
> 198 and 203. I do still find the name of this variable too .1X centric
> and, in the light of the recent debates on corner cases on EAP starts
> and restarts, I'd prefer a more explicit name (like Yoshi had proposed
I have no opinion on the name, whatever the group agrees on is fine with
me. I think it was indecision that left us with portEnabled last time, but
I could be remembering incorrectly.

> for instance). Also I'd like the current definition (e.g. in section
> 4.1.1 "portEnabled (boolean) Indicates that the EAP peer state machine
> should be ready for communication. This is set to TRUE
> when the EAP conversation is started by the lower layer. If at any point
> the communication port or session is not available, portEnabled is set
> to FALSE and the state machine transitions to DISABLED.") clarified (and
> why not - two - example instantiations given, one .1x and one IKEv2 for
> instance) to take the "new"? problems into account (e.g. section 7.12 of
> RFC 3748b "In IEEE 802.11, a "link down" indication is an unreliable
> indication of link failure, since wireless signal strength can come and
> go and may be influenced by radio frequency interference generated by an
> attacker. To avoid unnecessary resets, it is advisable to damp these
> indications, rather than passing them directly to the EAP. Since EAP
> supports retransmission, it is robust against transient connectivity
> losses. " )
IMHO, this is not an EAP issue nor a problem with the state machine. I
think it is up to the lower layer to wisely use our interface. From my
point of view the lower layer can determine when "the communication port
or session is not available" and if it wants to be fault tolerant that's
great. Perhaps we could reword it to say "If at any point the lower layer
decides communication is no longer available..." ?

>
> For eapRestart, my problem is much the same, two concrete examples (.1X,
> PPP or IKEv2 for instance) would considerably help understand what this
> variable stands for.
You are asking to include these examples in the definition? Personally, I
think the definition is fine, but I could be missing something. From my
POV, the definition is describing some functionality that EAP provides to
the lower layer, namely restarting, when eapRestart is set. Any decision
process for setting that variable is up to the lower layer.


> Comment #3 - Editorial
>
> This is about the initialization of lastID which is not done in the EAP
> peer state machine. This had been pointed out in issue 229 but
> apparently not taken into account.
> Also specify, e.g. in section 4.3.1 that lastId may take the value "NONE"
Sorry, I guess this was an oversight. I agree this should be changed.

> Comment #4 - Editorial
>
> This is about idleWhile. From my understanding, this timer steadily
> decreases and when it reaches 0, the peer may time out. Clearly, knowing
> the initial value of this timer, by a simple subtraction, one can get to
> know how long the peer has been waiting.
> So, contrary to the definition in section 4.1.1 ("idleWhile (integer)
> Outside timer used to indicate how long the peer has waited for a new
> (valid) request."), I'd rather say that this timer indicates how long
> remains before the peer may time out.
I agree that by simple subtraction you can arrive at the definition, but
changing it to yours seems more correct.

> Comment #7 - Editorial
>
> I do not find any definition of m.buildResp (that is used in Figure 3
> EAP peer state machine) in section 4.4 Peer state machine procedures).
> Moreover, i would find it clearer if m.buildresp somehow indicated that
> it does not only depend on ReqId but also on some internal method state
> that has been calculated by m.process.
I agree that the definition should be added.

As for method state, IMHO it is clear that the method is using internal
state in addition to the information provided from the EAP layer. I think
we have just chosen to show what is explicitly provided to the method by
EAP. There have been a number of debates on the best way to show the
method/EAP separation and how much to model the method itself. I would
prefer to leave it as is, but of course am open to more conversation on
the matter.

> Comment #8 - Editorial
>
> Although in section 4.4, it is said parseEapReq() "checks that the
> lengthfield is not longer than the received packet", I do not find it
> completely straightforward from Figure 3 what the peer does in case
> there is a parse error due to the length (or an invalid code type or...).
I think the "else" transition catches this condition, but the text could
be better. Anyone want to suggest something? I think the idea is that
rxReq, rxSuccess, and rxFailure would all be FALSE, thereby catching
"else".

> Comment #11 - Editorial
>
> eapTimeout does not seem to be defined in the text.
yes! that's true. good catch. I would say the definition goes in the
sections for "EAP to lower layer". Thoughts?

> Comment #13 - Editorial
>
> Why call section 5 "standalone authenticator"? I bet this is the old
> story of the glass half full or half empty, because I'd rather have
> standalone EAP server.
Since EAP is based on the 2-party "peer" and "authenticator" it seems a
little odd to me to only have a "peer" and an "EAP server" in the
2-party case.

> Comment #15 - Editorial
>
> The title of Section 6 is "EAP Backend Authenticator" which I find quite
> strange. I'd rather suggest "backend authentication server".
This diagram is meant to show the backend of the state machine (the
authenticator portion), which is implemented on the AAA server. "backend
authentication server" to me would describe the entire AAA server
including protocols outside of EAP. Anyway, I think the text describing
what this entity does is fine, even if the name is not ;). Are there
others out there who feel strongly on this issue?

> Comment #16 - Editorial
>
> In section 6.2 we find: "The only difference is that some methods on the
> backend may support "picking up" a conversation started by the
> pass-through. That is, the EAP Request packet was sent by the
> pass-through, but the backend must process the corresponding EAP
> Response. Usually only the Identity method supports this, but others are
> possible"
> Would it be possible to explain whether this possibility is explicitly
> left open by some document (in this case, which one) or is implicitly
> allowed (and in this case, whether there are yet
> settings/implementations which use such a possibility).
I think this is explicitly left open by the fact that the protocol is not
dependent on implementation- you can put any piece of it anywhere you
want, as long as it looks like EAP to the other end. I don't think it's
necessary to say any more, but I could be wrong.

> Comment #18 - Editorial
>
> If the purpose of aaaIdentity is to allow encapsulation of the peer's
> identity in e.g. RADIUS User-Name attribute, I'd rather have aaaIdentity
> be directly the identity of the peer than a whole EAP packet
I am not a AAA expert, but I think this may have something to do with
parsing the actual packet at the AAA server instead of at the AP. Is this
not correct?