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

From: [email-masked] ([email-masked])
Date: Wed, 25 Jun 2003 15:53:40 +0300

Thanks for your review! See below for comments to=20
both of your emails:

Jari Arkko wrote (23.6.):
>
> 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!
>=20
> I have a few nits and question marks, however:
>=20
> 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)

The diagram was done back when sequences were still allowed...=20
The idea was that Policy is independent of methods, and the methods
just feed somethign to the Policy (e.g. "I've authenticated the peer
as [email-masked]") and the Policy would decide what to do next
(e.g. permit or deny access, follow with some other method, etc.).

I guess the real problem here is that the "pseudo-code" in METHOD
isn't really C-like pseudo-code at all -- it just illustrates that the
method should feed something to the policy.  Nothing really restricts
what sort of information is communicated here.  In theory, Policy
could have access to everything that can be deduced from eapRespData,
but this would require the Policy knows how to parse it.

One alternative would be to replace the Policy.update call with
"m.process(eapRespData, Policy)", meaning that m.process would
feed something to the Policy. =20
Or "Policy.update(m.getPolicyInformation())"?

> 2) Can't find calculateTimeout from 5.3.

OK, will be added.

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

2284bis is quite vague on this.  I guess we could specify=20
RFC2988 style behavior here: add two new variables
(eapSRTT, eapRTTVAR) that are set by the lower layer,=20
and give them to calculateTimeout.  Would this be better?

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

m.buildSuccFail actually just takes the Success/Failure packet=20
from the Access-Accept/Reject message.  Maybe we should rename it=20
to something more understandable (m.getSuccFail) or at least
document it better...

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

You're right, it does not prevent this: if Policy.isSatisfied()=20
always returns TRUE, the authenticator just sends a "canned"=20
EAP Success packet.

(There are other things Policy shouldn't do, e.g. sequences,
contradict method-specific result indications, etc.)

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

I just checked 802.1aa/D6.1, and they certainly don't initialize=20
the variables... I guess there are two possible solutions:

- Add statement saying "All boolean variables are initialized=20
  to FALSE before the state machine execution begins."
  I would really prefer this option.
 =20
- If variables have undetermined values before they are assigned,
  then we need a lot of initializations.  The problem is that
  e.g. eapFail has to be initialized in lower layer, because
  otherwise we get synchronization issues (the lower layer
  could read it before the initialization block in EAP state=20
  machine has been executed). Figuring out what goes where
  is a lot of work..

I guess this should be checked with the 802.1aa folks as well.
(John, are you reading this?)

> 7) Section 5.1.1 s/outside/Outside/

OK.

> 8) Section 5.2 indentation looks funny.

I agree.
=20
> 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?

They could be combined (and were combined in an earlier version),=20
but we thought it's easier to read this way.

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

Most of 5.2 is just descriptive text, which is more difficult to
group.  Also, the interface to the lower layer is quite well-defined,
but the interface to methods isn't that clear (or another way to put
it would be that "many difficult details are hidden in Policy
calls").

Then your next message:

Jari Arkko wrote (24.6.):
>
> 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!
>=20
> I do have some comments, though:
>=20
> 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.

OK.

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

Agreed. Will be added.

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

Yes, we fail, but before the authenticator state machine reaches=20
the FAILURE state, it calls Policy.isSatisfied() and
Policy.getNextMethod().  The Policy.update call just informs=20
the policy so it knows to return the correct values (FALSE and NONE).

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

Same as above.=20

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

Here "identity" actually refers to contents of Identity Response,
not ID number field.  (Yes, you're right, we need a list of
variables/procedures for these :-)

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

I think the backend authenticator+adapter can't ever send conflicting
messages (EAP Success inside Access-Reject or vice versa).  Do you
think the pass-through authenticator+forwarder needs something=20
special for this?

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

Ok (but see below; we might want to make IEEE normative).

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

Yes, we actually mention the keying framework in the text,
so we should add a reference.

> 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?
>=20
>     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.
>=20
>     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?

No, we don't have any analysis tool for this (also, it's not clear
what results any tool could give for these diagrams alone?).

The pseudocode is defined in 802.1X-2001 Maybe we should just add=20
a normative reference there, and remove Section 3.1 (so we don't=20
have to copy the pseudo-code definition as well)?

Best regards,
Pasi

---

From: [email-masked] (John Vollbrecht)
Date: Wed, 25 Jun 2003 15:41:30 -0400

Thanks for the review -- some comments in line - John

--On Wednesday, June 25, 2003 3:53 PM +0300 [email-masked] wrote:

> Thanks for your review! See below for comments to
> both of your emails:
>

thanks !

> The diagram was done back when sequences were still allowed...

We may have a difference of bias here - my opinion is that this should not 
allow sequences outside tunnels but they are ok inside.  This means that if 
EAP will work with both tunnelled and untunnelled methods, sequences must 
be possible.  In untunnelled methods they are  not allowed but in others 
they are.  There are a number of things that are to be worked out, but I 
believe we agree on this.

> feed something to the Policy.
I think what we have is fine.  The others could work if others feel 
strongly.

> 2284bis is quite vague on this.  I guess we could specify
> (eapSRTT, eapRTTVAR) that are set by the lower layer,
> m.buildSuccFail actually just takes the Success/Failure packet
> from the Access-Accept/Reject message.  Maybe we should rename it
> You're right, it does not prevent this: if Policy.isSatisfied()
> always returns TRUE, the authenticator just sends a "canned"
> EAP Success packet.
>
This is interesting.  Policy.update decides if the method 
succeeds/fails/etc.  Perhaps a policy variable that is initialized nul or 
false should be set that prohibits creating a canned Success or Failure if 
no method has been called.

> I just checked 802.1aa/D6.1, and they certainly don't initialize
> - Add statement saying "All boolean variables are initialized
me too.

>   could read it before the initialization block in EAP state
I will check with Paul and Jim on this.

> They could be combined (and were combined in an earlier version),

true :)

> Yes, we fail, but before the authenticator state machine reaches
> Policy.getNextMethod().  The Policy.update call just informs
> Same as above.
> >       if (respId == currentId)
> think the pass-through authenticator+forwarder needs something
> special for this?
>

seems like it might be possible but would be an error by a method?  Perhaps 
I am missing something?

> Ok (but see below; we might want to make IEEE normative).

I am not sure what normative means, but I like IEEE being normative if 
possible.

> The pseudocode is defined in 802.1X-2001 Maybe we should just add
> a normative reference there, and remove Section 3.1 (so we don't
> _______________________________________________
> eap mailing list
> [email-masked]
> http://mail.frascone.com/mailman/listinfo/eap

---

From: [email-masked] (Jari Arkko)
Date: Sun, 29 Jun 2003 10:45:05 +0300

> and give them to calculateTimeout.  Would this be better?

I wonder if it would be possible to leave all the vagueness
in the "calculateTimeout" function, but specify the input
to this function completely?

> to something more understandable (m.getSuccFail) or at least

Ok.

> contradict method-specific result indications, etc.)

I think there's two correct ways of handling this:
either the 2284bis requirements are coded
to the state machines (which may increase their size).
Or we explicitly specify that the policy object must
take care of the 2284bis requirements X, Y, and Z.

>   I would really prefer this option.

Ok. The nature of the variables should be made very clear
in the document, it is OK for the lower layer to set a variable
to FALSE and the higher layer to TRUE. So you may explain that
initialization part also as something that happens at the
lower layer.

> it would be that "many difficult details are hidden in Policy

I think its fine to have descriptive text. But I still
think interface to the methods would be clearer, if
you listed and classified all variables, constants,
function calls.. and included the descriptive text.

> the policy so it knows to return the correct values (FALSE and NONE).

Ok. Maybe it would be better to have separate calls for
the different aspects of policy, such as method selection
and success/failure determination.

> variables/procedures for these :-)

Ok.

> think the pass-through authenticator+forwarder needs something 

I think so, because RFC 2869bis did not assume the backend
always sent correct messages. It is true that put together,
your state machine components will work flawlessly. But still,
our protocol specifications are expected to work even with
non-perfect components, and that's why 2869bis had such
safety checks.

> what results any tool could give for these diagrams alone?).

Yes, if we don't have a tool then I guess we can't do much about
it. (A tool might reveal issues such as undefined variables,
spelling errors, inconsistent formulas, missing events etc. even
if it didn't "understand" the state machine in any deeper sense.)

> have to copy the pseudo-code definition as well)?

Sounds good!

--Jari