[Gen-art] Gen-ART Telechat review of draft-ietf-pcp-authentication-13.txt

[Paul Kyzivat <[e-mail]>]

I am the assigned Gen-ART reviewer for this draft. For background on 
Gen-ART, please see the FAQ at <​ 
http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>

Please wait for direction from your document shepherd or AD before 
posting a new version of the draft.

This draft is on the right track but has open issues, described in this 
review.

I've opted to organize my comments in document order. I have tagged 
different comments as BUG/TECHNICAL/EDITORIAL/NIT/etc.

I had difficultly creating my earlier last call review because in a 
number of places I could not discern the intent of the draft. This 
revised version has mostly resolved that problem, and allowed me to be 
more concrete in my comments. I do still have a number of remaining 
concerns.

Disclaimer: I do not claim to be a security expert. The authors are 
vastly more qualified than I on security matters. So I have not tried to 
discern if there are technical security holes in this specification.

     Thanks,
     Paul Kyzivat

* Section 3.1:

[EDITORIAL] In the following sentence:

    ... In the
    message, the Session ID and Sequence Number fields of the
    AUTHENTICATION Opcode are set as 0.

those fields are not not part of the *AUTHENTICATION opcode*. I suggest 
replacing this with:

    In the opcode-specific information of the message, the Session ID
    and Sequence Number fields are set as 0.

[TECHICAL/EDITORIAL] Then, in:

    The PA-Client message SHOULD
    also contain a NONCE option defined in Section 5.3 which consists of
    a random nonce.

Why is this SHOULD rather than MUST? Are there cases where it is 
acceptable to omit the nonce? If so, I would like to see the text 
explain the reasons for and consequences of omitting it, and give 
example(s) of those cases.

[TECHNICAL] The next paragraph calls for the assignment of a *random* 
session identifier. Does it need to be random? Isn't the real 
requirement simply that it be unique from all other sessions between 
this client and server? (I'm guessing that it would be more appropriate 
to replace "random" with "unique", but the key is to achieve whatever 
properties are required for this protocol.)

[EDITORIAL] This paragraph also refers to fields of the opcode. I 
suggest the following replacement:

OLD:
    ... and
    fill the identifier into the Session ID field of the AUTHENTICATION
    Opcode in the PA-Server message.  The Sequence Number field of the
    AUTHENTICATION Opcode is set as 0.

NEW:
    ... and fill the identifier into the Session ID field in the
    server-specific information of the PA-Server message.  The Sequence
    Number field of the message is set as 0.

[EDITORIAL] Also, I'm still troubled by:

    From now on, every PCP message within this PA session MUST contain
    this session identifier.

As I noted previously, this is a truism - it is a restatement of the 
definition of a PA session. Also, "From now on" seems a bit informal and 
vague. I still suggest replacing this sentence with:

    Subsequent PCP messages are included within this PA session by
    attaching an AUTHENTICATION_TAG option containing this session
    identifier.

If you instead mean that once a PCP SA has been established then all PCP 
messages must be sent within this PA session, then you need some 
different words. I talk about this more below.

[EDITORIAL] In the figure in this section (and also in 3.1.2) it would 
be very helpful if you showed the response code for each of the PA 
messages. E.g.

      PCP                                                PCP
      client                                            server
        |-- PA-Initiation-------------------------------->|
        |   (Seq=0, Session ID=0, rc=INITIATION)          |
        |                                                 |
        |<-- PA-Server -----------------------------------|
        |    (Seq=0, Session ID=X, EAP Identity request)  |
        |    (rc=AUTHENTICATION_REQUIRED)                 |
        |                                                 |
        |-- PA-Client ----------------------------------->|
        |    (Seq=1, Session ID=X, EAP Identity response) |
        |    (rc=AUTHENTICATION_REPLY)                    |
        |                                                 |
        |<-- PA-Server -----------------------------------|
        |    (Seq=1, Session ID=X, EAP Identity request)  |
        |    (rc=???)                 |

[TECHNICAL] Note that I find no formal specification of what the return 
code must be in the PA messages following the EAP Identity request and 
its reply. (Hence the ??? above.)

* Section 3.1.2:

[BUG/MAJOR TECHNICAL] (This is my most major concern!!!)

The first sentence is:

    In the scenario where a PCP server receives a common PCP request
    message from a PCP client which needs to be authenticated, the PCP
    server can reply with a PA-Server message to initiate a PA session.

As I read RFC6887, this cannot be viewed as a response to the common PCP 
request!!! The PCP client should still be expecting a response to *that* 
request, containing the same opcode as the request. And it will retry 
until it gets such a response.

Also, if we ignore that, there is no obvious way of matching this 
response to the request. The matching rules on 6887 require the opcodes 
to match, and for some opcode-specific matching rules to be applied to 
the opcode-specific information. (Note that there could be more than one 
outstanding, and some requests might require authentication while others 
do not.)

The only way I see to make this work with 6887 is:
- send an error response to the common PCP message being challenged.
   (With the same opcode as the message. Probably with
   AUTHENTICATION_REQUIRED as the response code.

- To include the new session identifier in that response, use an
   option. The AUTHENTICATION_TAG option seems to work for this.

- Then the client could initiate authentication via PA messages.
   This could be just like in 3.1.1, or perhaps be optimized in some
   way.

If this isn't an acceptable resolution, then it seems necessary for you 
to revise the general PCP response processing in section 8.3 of RFC6887. 
And that would be ugly.

* Also in section 3.1.2:

[TECHNICAL] In the following:

    The PCP client MUST NOT retry the common PCP request until it
    receives AUTHENTICATION_SUCCEEDED result code from the PCP server.

Why MUST NOT? ISTM the point is that the request will not succeed 
outside a PA session. Presumably you could try it again, but would just 
get another request to authenticate. Wouldn't it be better to state that 
the client may assume that it is fruitless to try that request outside 
of a PA session?

* Section 3.1.3:

[TECHNICAL] The following:

    Therefore, after sending out a PA-Server message, the PCP
    server will not send a new PA-Server message until it receives a PA-
    Client message with a proper sequence number from the PCP client, and
    vice versa.

seems overly constrained. AFAICT this only needs to be true within one 
PA session. If there happens to be another PA session, then this 
wouldn't have to apply to messages there. So perhaps this could be 
stated as:

    Therefore, after sending out a PA-Server message, the PCP
    server will not send a new PA-Server message in the same PA session
    until it receives a PA-Client message with a proper sequence number
    from the PCP client, and vice versa.

[TECHNICAL] And then in:

    If a PCP client receives a PA message containing an EAP
    Identity request and cannot generate an EAP Identity response
    immediately ... the PCP device MUST reply with a PA-Acknowledgement
    message ...

This only applies the constraint to an EAP Identity request. IIUC, the 
full EAP negotiation could involve exchange of several EAP messages. I 
presume this restriction must apply to all the PA Server messages 
containing EAP messages, not just the Identity request. If that is so, 
then this sentence needs to be expanded.

(In general, I find that the text insufficiently explains that the 
number of messages in the exchange is variable and how that all works.)

In the following:

    In this approach, PCP client and a PCP server MUST perform a key-
    generating EAP method in authentication.  Particularly, a PCP
    authentication implementation MUST support EAP-TTLS [RFC5281] and
    SHOULD support TEAP [RFC7170].

[EDITORIAL] IIUC, this implies that a non-key-generating EAP method MUST 
NOT be offered, or selected. It would be helpful to point that out.

[TECHNICAL] Then, in the following:

    After the EAP authentication, ... the PCP server MUST generate a PCP
    SA ... the PCP client needs to generate a PCP SA ...

This implies that there is no PCP SA until after the EAP authentication 
has completed. But Section 4 says:

    At the beginning of a PA session, a session MUST generate a PA SA to
    maintain its state information during the session.

That says that the PCP SA is generated at the beginning of the PA 
session, which is before the EAP authentication has begun.

This inconsistency needs to be resolved. Since the PCP SA state 
information listed in section 4 includes the session identifier and 
sequence numbers as well as the security-specific stuff, and since that 
stuff is needed to maintain the PA session, ISTM that the PCP SA must be 
generated as specified in section 4, and so section 3.1.3 needs to be 
updated. For instance, it could say that when the EAP authentication 
succeeds, then the PCP SA needs to be *updated* with the negotiated keys.

[NIT] Also in that paragraph:

    ... From then on, all the PCP
    messages within the session are secured with the transport key and
    the MAC algorithm specified in the PCP SA, unless a re-authentication
    is performed.

ISTM the exception in case of re-authentication is unnecessary.  The PA 
message exchanges used to perform the re-authentication are secured that 
way, and once the re-authentication is complete, the PCP SA is updated 
with the new keys, and then subsequent messages continue to be secured 
with using the info in the PCP SA.

So I think you can simplify this sentence by omitting the part starting 
with ", unless".

[TECHNICAL] At the end of that paragraph:

    ... If PCP
    client sends common PCP request without AUTHENTICATION_TAG option
    then the PCP server rejects the request by returning
    AUTHENTICATION_REQUIRED result code in the PA-server message.

This is new since my prior questions. This effectively says that you 
have made the design choice that once a PA session has been established 
on a particular 5-tuple it MUST be used for all further PCP 
communication between them.

If so, then the consequences of that need to be explored. One 
consequence that comes to my mind is: what if, after establishing a PA 
session, one of the parties restarts and loses its PCP SA state?

If the client lost state, then when it subsequently attempts to send a 
PCP message it will receive a response indicating 
AUTHENTICATION-REQUIRED. But it is then expected to use the existing PCP 
SA session that it has forgotten. Can it attempt to establish a new 
session, as in section 3.1.1? I can't tell if that will work.

Conversely, what if it is the PCP server that loses the PCP SA state? To 
handle this case I think you need a new response code: 
UNKNOWN_SESSION_ID, along with procedures for when to send it and what 
to do when receiving it.

Bottom line - there is more to specify here.

[TECHNICAL] The final (new) paragraph in this section:

    It is possible for independent PCP clients on the host to create
    multiple PA sessions with the PCP server.  It is RECOMMENDED that
    once a PCP client on the host authenticates with the PCP server any
    other PCP clients on the host SHOULD be able to reuse the previously
    negotiated transport key for integrity protection.

is also in response to my earlier comments. It still isn't working for 
me, for a couple of reasons:

First, I don't know what you mean by "independent PCP clients". IIUC 
this is all in the context of a single client and server over a 5-tuple. 
The use case I had in mind was for the aspect of the Advanced Threat 
Model of 6887: "Any implementation that wants to be more permissive in 
authorizing explicit mappings than it is in authorizing implicit 
mappings". Notably where implicitly created mappings can be manipulated 
without authentication, but where other explicit mappings may be 
manipulated with authentication. So it isn't really different clients, 
rather it is code for differing purposes within the same client.

Second, this paragraph is in conflict with my prior comment about 
requiring an established PCP SA to be used. After the first such client 
established a PCP SA, the others wouldn't be able to establish their own.

* Section 3.2:

(This section is much improved. Thanks.)

[EDITORIAL/TECHNICAL] I still have one comment on:

    ... Upon receiving a
    termination-indicating message, the PCP client MUST respond with a
    termination-indicating PA message, and SHOULD then remove the
    associated PA SA.

Why *SHOULD* remove? ISTM this ought to be MUST. Is there any case where 
not removing it is a reasonable thing to do?

* Section 3.3:

[TECHNICAL] This section now says:

    ... The result
    code for PA-Sever message carrying EAP Identity request will be set
    to AUTHENTICATION_REQUIRED and PA-Client message carrying EAP
    Identity response will be set to AUTHENTICATION_REPLY.

This only specifies the response codes used for an EAP Identity request 
and its EAP response. My comments to section 3.1.3 regarding response 
codes for other EAP messages apply here as well.

I also see a case that isn't mentioned here or elsewhere. The session 
lifetime is initialized at the successful completion of EAP 
authentication or re-authentication. It is always selected by the PA 
server, and is *optionally* communicated to the PA client. There is no 
specification of what it is set to prior to this, or what it is set to 
in the client if the server doesn't send the value. ISTM that there 
ought to be a default value that is established at the time the SA 
session is established. That then can govern until/unless a new value is 
chosen by the server and sent to the client.

[TECHNICAL] This section fails to discuss the "glare" case. (Both client 
and server decide to send RE-AUTHENTICATION at the same time.) A 
mechanism needs to be specified to recover from this. (The typical 
mechanism if for both ends to send some distinct error code, and then a 
deterministic way to choose when to retry so that glare will not recur.)

* Section 4:

This section starts with:

    At the beginning of a PA session, a session MUST generate a PA SA to
    maintain its state information during the session.

[EDITORIAL] There is no such term as PA SA. Rather it is PCP SA.

[EDITORIAL] I don't know what it means for a *session* to generate a PA 
SA. I think maybe you mean:

    At the beginning of a new PA session, each PCP device must create
    and initialize state information for a new PA Security Association
    (PCP SA) to maintain its state information for the duration of the
    PA session.

* Section 5.1:

[TECHNICAL] My comment on section 3.3 applies here as well.

[EDITORIAL/TECHNICAL] This section makes a change to the PCP Common 
Request Packet Format defined in section 7.1 of RFC6887, by using a 
portion of the "Reserved" field for an opcode-specific purpose. This 
should be harmless in operation because recipients of this format are 
required to ignore this field. But there could be a problem if a future 
revision of that RFC were to use the same reserved bits for some other 
purpose, without recognizing the conflict with this draft.

At my request during the LC review this document now indicates that it 
updates RFC6887. But there still is no explicit statement that future 
assignment of that field could explicitly impact this document. I would 
suggest explicitly providing a revised version of Figure 2 from RFC6887, 
such as:

       0                   1                   2                   3
       0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |  Version = 2  |R|   Opcode    |   Reserved    |  Opcode-      |
      |               | |             |               |  specific-info|
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |                 Requested Lifetime (32 bits)                  |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |                                                               |
      |            PCP Client's IP Address (128 bits)                 |
      |                                                               |
      |                                                               |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      :                                                               :
      :             (optional) Opcode-specific information            :
      :                                                               :
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      :                                                               :
      :             (optional) PCP Options                            :
      :                                                               :
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+


* Section 5.5:

[EDITORIAL NIT] For consistency with the others, the section heading for 
this section ought to be "PA_AUTHENTICATION_TAG Option".

[EDITORIAL] Also, "because such information is provided in the 
AUTHENTICATION Opcode" ought to be: "because such information is 
provided in the Opcode-specific information".

* Section 6.1:

The first paragraph says:

    If a PCP SA is generated as the result of a successful EAP
    authentication process, every subsequent PCP message within the
    session MUST carry an authentication tag which contains the digest of
    the PCP message for data origin authentication and integrity
    protection.

A couple of things about this:

[TECHNICAL] As I commented earlier, it seems that the PCP SA state needs 
to be established before the EAP authentication process is complete. So 
some other criterion is needed to decide when to start using the PCP SA 
to generate digests of messages.

[NIT] Also "within the session" is unclear. It would be clearer to say 
"within the PA session".

* Section 6.2:

[TECHNICAL] Earlier I raised a question about what happens if one of the 
PCP devices is reset and loses PA SA state. That impacts this section as 
well. If the client loses state, then it will discard any messages 
containing one of the authentication tag options. What will the server 
do in that situation? It isn't evident to me that the two of them will 
be able to get into a functional state again. (By doing authentication 
over again.) That would be bad.

* Section 7:

Thanks, this section is now more readable.

[EDITORIAL] Another change would improve it further: create separate 
subsections for each distinct registry being updated: opcodes, result 
codes, options. Identify the table within the corresponding section. 
(The existing subsections for each opcode would then fall within the 
opcode section.)

This concludes my Gen-ART comments.