[Pana] AD review of draft-ietf-pana-statemachine

[Jari Arkko <jari.arkko at piuha.net>]

Apologies for taking some time to do this. I have reviewed the draft and 
I think it is in very good shape. There are one or two technical 
problems that require the draft to be revised, unless I missed 
something. Please discuss this with me and, if needed, revise the draft 
accordingly.

Here are my comments:

> 7.2. Constants
>
>    FAILED_SESS_TIMEOUT
>
>       Configurable value ...

Small editorial problem: Something is inconsistent here.... maybe call 
the section "7.2. Configurable Values"?

>    State: WAIT_PNA

Three technical problems:

1. You enter this state in two ways, either because you sent a PING, or 
because you wanted to re-authenticate. However, coming back from the 
state you go to re-authentication or ping processing depending on the 
flag in the answer message. So, if the peer responds with the wrong flag 
value, you might accidentally go into, say, OPEN state even if you 
really were going to do re-authentication...

The fix is is to either note the limitation or split WAIT_PNA into two 
different states.

2. What happens if you want to initiate re-auth while waiting for ping 
response? Perhaps you could state that the relevant variables are not 
supposed to be set until you are in OPEN... or you could do some more 
radical change.

3. More seriously, I think you need to support the case that you are 
doing ping on one side while the other guy initiates re-authentication. 
WAIT_PNA does not support reception of the relevant messages. This needs 
to be fixed somehow.

Generalizing the last point a little bit, what do you do when there's 
termination / ping / re-authentication / initialization in parallel with 
something else that the other peer is doing? At the very least, you need 
to explicitly decide to discard undesired events in states (and show 
that its still according to the RFC). Or, some other type of a change is 
needed in the draft.

>    ------------------------------
>    State: INITIAL (Initial State)
>    ------------------------------
>
>    Initialization Action:
>
>      OPTIMIZED_INIT=Set|Unset;
>      NONCE_SENT=Unset;
>      RTX_COUNTER=0;
>      RtxTimerStop();
>
>    ...
>
>    - - - - - - - - - - - - - - (PAN Handling)  - - - - - - - - - -
>    Rx:PAN[S] &&             if (PAN.exist_avp          WAIT_EAP_MSG
>    ((OPTIMIZED_INIT ==         ("EAP-Payload"))
>      Unset) ||                TxEAP();
>    PAN.exist_avp            else {
>      ("EAP-Payload"))         EAP_Restart();
>                               SessionTimerReStart
>                                (FAILED_SESS_TIMEOUT);
>                             }
>
>    Rx:PAN[S] &&             None();                    WAIT_PAN_OR_PAR
>    (OPTIMIZED_INIT ==
>      Set) &&
>    ! PAN.exist_avp
>     ("EAP-Payload")

Observation: the two actions do not cover all possible combinations, 
e.g., not Rx:PAN[S] && OPTIMIZED_INIT == Unset && 
!PAN.exist_avp("EAP-Payload").

You are setting OPTIMIZED_INIT to a value at the time that the state 
machine is created. What if it disagrees with the contents of the PAN 
that you receive from the network?

Jari