Re: [Gen-art] Gen-ART Last Call review of draft-ietf-pcp-authentication-11.txt

["Tirumaleswar Reddy (tireddy)" <[e-mail]>]

> -----Original Message-----
> From: Paul Kyzivat [mailto:[e-mail]]
> Sent: Monday, June 29, 2015 11:30 PM
> To: Tirumaleswar Reddy (tireddy); draft-ietf-pcp-
> [e-mail]
> Cc: General Area Review Team
> Subject: Re: Gen-ART Last Call review of draft-ietf-pcp-authentication-11.txt
> 
> Hi Tiru,
> 
> You have resolved many of my concerns.
> 
> I raised a number of questions, and in many cases you responded with an
> explanation of how you intend for things to work. That is good - in most cases
> the answer makes sense to me. But my point in raising the questions was that I
> don't think the -11 version is clear and unambiguous about these points. Your
> answers to me don't resolve that.
> What I am hoping for is that you will make draft revisions that resolve the
> ambiguity in the way you intend.

Yes. I am also updating the draft, published version 12 to address some of your comments; will address pending comments in the next revision. Please see inline

> 
> More inline.
> 
> On 6/29/15 4:21 AM, Tirumaleswar Reddy (tireddy) wrote:
> > Hi Paul,
> >
> > Thanks for the detailed review. Please see inline
> >
> >> -----Original Message-----
> >> From: Paul Kyzivat [mailto:[e-mail]]
> >> Sent: Saturday, June 27, 2015 2:32 AM
> >> To: [e-mail]
> >> Cc: General Area Review Team
> >> Subject: Gen-ART Last Call review of
> >> draft-ietf-pcp-authentication-11.txt
> >>
> >> 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 resolve these comments along with any other comments you may
> >> receive.
> >>
> >> This draft is on the right track but has open issues, described in this review.
> >>
> >> For the most part my issues are with confusing and/or ambiguous
> >> language and under-specification. This means some clued in
> >> implementers will be able to create functional, interoperable,
> >> implementations. But others, working just from the specification, may
> >> build arguably conforming implementations that fail to interoperate.
> >>
> >> I've opted not to split major and minor issues out separately,
> >> because I thought it better to put things in document order. But
> >> please take special note of my comments on sections 3.1.1 and 3.1.2
> >> that highlight what I believe to be a true protocol errors. I did note those
> points I consider nits.
> >>
> >> 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.
> >>
> >> I’ll start with some general comments, and then follow with specific
> >> comments on particular parts of the document.
> >>
> >> 	Thanks,
> >> 	Paul Kyzivat
> >>
> >> * General Comments:
> >>
> >> I assume that a reader of this document is reasonably familiar with
> >> the RFCs that specify PCP and EAP. (When I began this review, I was
> >> not, so I read them before starting this review.) I apologize if I
> >> have made technical mistakes in my understanding of those protocols.
> >>
> >> There is a fundamental inconsistency of approach between PCP and EAP.
> >> Both are client/server protocols, but for most of the expected usage
> >> a PCP client will be the EAP server and visa versa. So this draft
> >> works hard to reconcile this difference. I *think* it has, for the
> >> most part, done so *technically*, but not so well with the
> >> terminology and language. For example, while reading the draft it was
> >> a constant struggle to understand whether a request is a PCP request
> >> containing response pertaining to EAP, or a request pertaining to EAP
> contained in a PCP response.
> >>
> >> I have another concern about the big picture of the protocol. There
> >> is much material about message level details, but not much about how
> >> they all play together. Some things I had trouble with:
> >>
> >> - Once a PA session is initiated, may common PCP messages that are
> >> not part of that session continue to be exchanged ?
> >
> > No, otherwise PCP messages will not get integrity protection.
> 
> My question was whether it was allowed, not whether it was recommended.
> I find nothing that forbids it.
> 
> Also, if one wanted to establish more than one concurrent SA (if that is to be
> allowed) then I think, after establishing one SA, that to create another it is
> necessary to send some messages that are not within the first SA.
> 
> My point is simply that whatever behaviors you want to allow and forbid need
> to be clarified normatively in the text.

Added the following lines to make it clear.
NEW:
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 it permissible to have more than one concurrent PA session
> >> active between the same PCP client and server?
> >
> > Yes, it is possible but typically PCP authentication only happens once for a
> host and multiple applications on the host will use same SA.
> 
> As noted above, if allowed then there must be some way to establish.
> 
> >> These might be multiple sessions created with the same credentials,
> >> or using different credentials. (E.g.
> >> to support both a "regular" and "superuser" level of privilege.)
> >
> > No, typically single session will created for a PCP client. PCP client will be an
> endpoint and will not have different credentials in the same administrative
> domain.
> 
> I guess if multiple SAs are allowed, and possible to establish, then they will be
> usable to accomplish this kind of usage.
> 
> >> - If multiple sessions are permitted, how to avoid accumulating many
> >> active sessions ?
> >
> > PCP server rejects session request from the client, error codes like
> (USER_EX_QUOTA) are discussed in RFC 6887.
> 
> OK. I guess that is probably sufficient.
> 
> >> To address this, I would hope to see some state models and some
> >> example message sequence diagrams that show how this plays out in
> practice.
> >>
> >> (Consider: PCP client starts without credentials. At some point it
> >> tries a request that requires authentication. It does so using "normal"
> >> credentials. Later, within that PA Session it tries a request that
> >> requires "superuser" authentication. How would this be indicated? How
> >> would the new authentication take place? How would the client know what
> credentials to use?
> >> How to revert back to "normal" privilege?)
> >
> > PCP client will typically not be using multiple credentials with the PCP server.
> PCP server which has PCP authentication enabled will permit MAP/PEER only
> after authentication is successful.
> 
> OK. I guess that if you don't consider this to be an expected usage there is no
> need to discuss how to do it.
> 
> >> Another thing: it appears to me that this document makes a change to
> >> the PCP protocol that was not anticipated as an extension point in RFC6887.
> >> Specifically, it uses part of the "Reserved" portion of the PCP request
> message.
> >> There should be no technical problem with doing this, but I think
> >> this means that this document should be marked as updating RFC6887.
> >> Ideally 6887 would be revised, splitting off the last 8 bits of the
> >> Reserved field as a separate field that is available for
> >> opcode-specific use. That would eliminate the possibility that some future
> revision of 6687 would assign that field for some other use.
> >
> > This document uses the reserved portion only for the Authentication opcode
> and will not impact existing opcodes or future opcodes.
> 
> It would be a problem if the base PCP was revised in the future to use the
> portion of the response code for something else, that applied to
> *all* opcodes.
> 
> So this use precludes such a change in the future. My point was to make this
> clear to everybody - even those who don't know or care about this extension to
> PCP.

Got it, modified document that it updates RFC 6887.

> 
> >> Regarding lifetime: All PCP messages carry a lifetime field. But this
> >> seems to differ from the "session lifetime" that is passed in an option.
> >> I find *no* mention of the PCP lifetime in this document. IMO there
> >> should be
> >> *some* statement of what to do with that.
> >> ISTM that there are two distinct but
> >> similar things here:
> >>
> >> - The lifetime of a PA session (validity of the session ID).
> >> Presumably it has to become valid upon the first PA message from the
> >> PA-Client, and it remains valid through initial authentication (or
> >> until failure of
> >> that) and through any and all re-authentications. The PA-Server can’t
> >> send PA messages to the PA-Client without having this session. So it
> >> needs to know how long it will remain valid. And this lifetime should
> >> be bounded so that an authentication attempt that never completed doesn’t
> leave a session forever.
> >> This is quite analogous to the lifetime of a mapping, and could
> >> conceivably be managed the same way, using the lifetime field in the PCP
> messages.
> >>
> >> - The lifetime of a PA SA and associated keys. Presumably this begins
> >> at the successful completion of the initial EAP negotiation, and is
> >> revised upon a successful re-authentication. This is needed for
> >> authenticating Common PCP messages within the PA session.
> >
> > This document is only discussing lifetime of a PA SA and PCP lifetime
> discussed in https://tools.ietf.org/html/rfc6887#section-7.2 is the lifetime of
> the mapping created on PCP-aware firewall and NAT devices using MAP or
> PEER opcodes.
> >
> > Added the following lines to make it clear:
> > The Requested Lifetime field of PA-Client message and Lifetime field of PA-
> Server message are both set to 0 on transmission and ignored on reception.
> 
> OK.
> 
> >> Currently these two seem to be conflated. Even if you are happy
> >> bundling these into one concept then still *something* should be said
> >> about how the PCP lifetime field is used.
> >>
> >> * Section 2:
> >>
> >> Re PCP-Authentication (PA) message:
> >>
> >>      PCP-Authentication (PA) message: A PCP message containing an
> >>      Authentication Opcode.  Particularly, a PA message sent from a PCP
> >>      server to a PCP client is referred to as a PA-Server, while a PA
> >>      message sent from a PCP client to a PCP server is referred to as a
> >>      PA-Client.  Therefore, a PA-Server is actually a PCP response message
> >>      specified in [RFC6887], and a PA-Client is a PCP request message.
> >>
> >> This terminology seems odd. It seems to say that the term "PA-Server"
> >> describes a *message*. But a name like that should seemingly describe
> >> a type of server, not a message. I suggest not doing this – instead,
> >> use "PA-Client message" and "PA-Server message" when referring to
> >> such messages. Instead, define and use "PA-Client" and "PA-Server"
> >> analogously to PCP Client and PCP Server.
> >
> > Updated.
> >
> >>
> >> (Could there be cases where the PCP Client also needs to authenticate
> >> the PCP Server, and the EAP method used doesn’t support mutual
> >> authentication? I don’t think the existing text contemplates that
> >> possibility, even though
> >> RFC3748 does. Supporting that would require a lot of changes.)
> >
> > No, the mandatory to implement EAP method (EAP-TTLS) discussed in the
> draft does support mutual authentication.
> 
> OK.
> 
> >> Re Common PCP message:
> >>
> >>      Common PCP message: A PCP message which does not contain an
> >>      Authentication Opcode.  This document specifies an Authentication Tag
> >>      Option to provide integrity protection and message origin
> >>      authentication for the common PCP messages.
> >>
> >> It isn’t clear to me if this term is intended to cover all PCP
> >> messages not containing the Authentication opcode, or only messages
> >> that contain the Authentication Tag Option. The usages in the
> >> document seem to make different assumptions about this. I’ve assumed
> >> that it covers messages both with and without that option unless
> >> explicitly qualified. It would be helpful to have distinct terms for those with
> and without.
> >
> > All PCP messages without Authentication opcode will contain the
> Authentication Tag Option.
> 
> I don't see that stated anywhere. If that is the intent, then it needs to be.
> 
> Also, it isn't literally true, since those before the first PA message won't. And I
> guess if the SA is terminated, then any after that won't.
> 
> It might be easier if you had separate terms, such as:
> - Unauthenticated PCP messages
> - Authenticated PCP messages
> 
> Then if you want this restriction, then you could say that when an SA is in effect
> that no Unauthenticated PCP messages may be sent.
> 
> But note that making this restriction then prevents establishing multiple SAs
> with different credentials.

Added the following line should make it clear. 
NEW:
If PCP client sends common PCP requests without AUTHENTICATION_TAG option then the PCP server rejects the request by returning AUTHENTICATION_REQUIRED result code in the PA-server message.

> 
> >> PCP device: This term is not defined in this terminology section, or
> >> anywhere else I can find, in this document or in RFC6887. But it is
> >> used frequently. IIUC it means "PCP Client or PCP Server" and is closely
> related to Session Partner.
> >> There ought to be a definition of it, or if appropriate, usages could
> >> be replaced with Session Partner.
> >
> > Updated terminology.
> >
> >>
> >> * Section 3.1:
> >>
> >>      ... Each PA message is attached with an Authentication Opcode ...
> >>
> >> This language is awkward/confusing. I *think* you mean:
> >>
> >>      ... Each PA message MUST contain an Authentication Opcode
> >
> > Updated.
> >
> >>
> >> (Actually, all occurrences of "attached with" deserve a look.)
> >>
> >> Then the following:
> >>
> >>      ... The Authentication Opcode consists of two fields: Session ID and
> >>      Sequence Number. ...
> >>
> >> Again this language seems odd: a PCP opcode doesn’t contain fields
> >> like this, though it may be *accompanied* by such fields. After
> >> reading ahead, I guess you mean:
> >>
> >>      ... The opcode-specific information of each PA message contains
> >>      two fields: Session ID and Sequence Number. ...".
> >
> > Changed.
> >
> >>
> >> * Section 3.1.1:
> >>
> >> In the following:
> >>
> >>      When a PCP client intends to proactively initiate a PA session with a
> >>      PCP server, it sends a PA-Initiation message (a PA-Client message
> >>      with the result code "INITIATION") to the PCP server.
> >>
> >> It seems odd to have a "result code" in a *request*. (I would expect
> >> it to be in a
> >> *response*.) It does make *some* sense when what is being sent is a
> >> response to a prior EAP request, but that is not the case here. Here
> >> it is serving as a sub- opcode.
> >>
> >> ISTM that using the PCP result code in PCP requests is confusing, in
> >> addition to being contrary to the definition of the PCP request message.
> >> I suggest refining the terminology and language around this. E.g.,
> >> define appropriate new names for these fields and alias them to the PCP
> fields.
> >>
> >> Then:
> >>
> >>      ... From now on, every PCP message within this session
> >>      will be attached with this session identifier. ...
> >>
> >> Again the language usage is odd, and apparently non-normative. And
> >> what does "within this session" mean? This document defines PA
> >> Session, so it could mean that. Or it could mean PCP Session – except
> >> that AFAICT PCP has no notion of a PCP session.
> >>
> >> If it means PA Session, then the statement is odd because it is a
> >> truism. If that is the intent, then I suggest rewording it as:
> >>
> >>      ... Subsequent PCP messages to be included in this PA Session
> >> MUST contain this session identifier. ...
> >
> > Yes, updated.
> >
> >>
> >> If this is intended to mean that once a PA Session is established
> >> then
> >> *all* messages must be within it, then more substantial document
> >> changes are needed.
> >>
> >> Then:
> >>
> >>      ... If the PCP client intends to simplify the
> >>      authentication process, it MAY append an EAP Identity Response
> >>      message within the PA-Initiation message so as to inform the PCP
> >>      server that it would like to perform EAP authentication and skip the
> >>      step of waiting for the EAP Identity Request.
> >>
> >> I don’t understand how this can work. IIUC the intent is to tunnel
> >> EAP within PCP, not to change EAP. AFAICT EAP has no provision for
> >> sending an EAP response message without having first received a
> >> corresponding request. The response message will have to contain an
> >> EAP Identifier, which is normally assigned by the Authenticator. In
> >> this case that wouldn’t be available, so the PCP client would have to make
> up a value.
> >> The authenticator won’t be expecting a response for this identifier.
> >> (Or worse, it may have just happened to send a request using this
> >> identifier.)
> >>
> >> If this behavior is desired, then I think an extension to EAP is
> >> needed to allow an EAP peer to preemptively provide identity. (I
> >> apologize if I have misunderstood EAP on this point.)
> >
> > Yes, it is mentioned as "MAY" in case if EAP supports this mechanism in
> future.
> 
> As the statement is worded it implies that this is valid usage. If you are simply
> future proofing for an anticipated EAP extension then IMO it would be wise to
> explain that is what you are doing, and that you are not encouraging invalid
> usage.

I have removed the above lines from the document to avoid any confusion (as it is not supported by any existing EAP type).

> 
> >> Then, regarding the message sequence diagram:
> >>
> >> (nit): the message sequence diagram uses "EAP request" and "EAP response"
> >> while the text uses "EAP identity request" and "EAP identity response".
> >> Consistent terminology should be used.
> >
> > Updated.
> >
> >>
> >> * Section 3.1.2:
> >>
> >> I have been studying the message sequence diagram in this section
> >> along with RFC6887. As best I can understand, this is not a valid sequence.
> >>
> >> I see the PCP client sending a Common PCP request, but I see no
> >> matching PCP response message for that. (This should trigger
> >> retransmission of the request.) Instead, I see the PCP server sending a PA-
> Server message.
> >> That will have a different opcode, and so can’t be considered a
> >> response to the Common PCP message. If the PCP client has not
> >> previously sent a PA message (the case here) it is supposed to ignore this
> message.
> >
> > No, PCP allows server to send unsolicited response. For example refer
> > to the usage of ANNOUNCE explained in
> > https://tools.ietf.org/html/rfc6887#section-14.1.3
> 
> I just reviewed 6887 again on this point. I agree that it does explicitly allow
> sending of unsolicited *ANNOUNCE* responses. It certainly appears to be a
> very narrow exception. OTOH, I guess it does establish a precedent that such
> exceptions are possible, so I will grant that you can declare the usage in this
> draft to be another exception.
> 
> BUT, that doesn't make the PA response serve as a response to the Common
> PCP request that triggered it. That request still needs a response of its own.
> And I don't see that mentioned anywhere. I can't even guess what is intended.
> Do you intend that the response is simply deferred until after the SA is
> negotiated? Or is the client intended to *retry* that request after the SA is
> established?

The client is intended to retry the request after the SA is established. Added the following line to make it clear.
NEW:
The PCP client MUST NOT retry the common PCP request until it receives AUTHENTICATION_SUCCEEDED result code from the PCP server.
> 
> Deferring the response seems problematic. The normal processing of that
> request by the client will trigger resending it periodically. And if the SA
> negotiation takes long enough the retry process may time out.
> 
> ISTM that the obvious conclusion is that that request needs an error response.
> Potentially the session ID could be carried in an option on that response. In that
> case an unsolicited PA response wouldn't be needed.
> 
> >> I was *expecting* to see the PCP server send a PCP response to the
> >> Common PCP message, with result AUTHENTICATION-REQUIRED. Then, the
> >> PCP client to send a PA-Client message to initiate the PA session.
> >> That could be exactly like the sequence in 3.1.1, or it might be optimized
> somehow.
> >>
> >> (I defer commenting on the text in this section pending clarification
> >> of these
> >> issues.)
> >>
> >> * Section 3.1.3:
> >>
> >> IIUC, PCP requires that the PCP client send a PA-client message
> >> before the PCP server can send any PA-server messages. Once that has
> >> been done the PCP server is allowed to send an arbitrary number of
> >> "responses" to that, enabling the exchange of EAP messages. The
> >> server messages must "match" the client message by opcode and
> >> opcode-specific data according to opcode-specific rules. (I think the
> >> intent for PA messages is that they must match on the session
> >> ID.)
> >>
> >> If I’m right about this, it would be good to explain that before the
> >> content of this section. (It took me a lot of reading before I
> >> figured this out.)
> >>
> >> Then:
> >>
> >>      ... If a PCP device receives a PA message from its partner and
> >>      cannot generate an EAP response immediately due to certain reasons
> >>      (e.g., waiting for human input to construct a EAP message or waiting
> >>      for the additional PA messages in order to construct a complete EAP
> >>      message), the PCP device MUST reply with a PA-Acknowledgement
> message
> >>      (PA message with a Received Packet Option) ...
> >>
> >> The use of "PCP device" suggests this is intended to apply to both
> >> the PCP client and server. But the use of "generate an EAP response"
> >> implies that this must be the PA-Client, since only it sends EAP
> >> responses. And that is consistent with sending a PA-Acknowledgement
> (request) message.
> >> If I’m right about this, then perhaps the following would be a
> >> clearer wording of the above:
> >>
> >>      ... If a PA-Client receives a PA message containing an EAP request
> >>      and cannot generate an EAP response immediately due to certain
> >>      reasons (e.g., waiting for human input to construct a EAP message
> >>      or waiting for the additional PA messages in order to construct a
> >>      complete EAP message), the PA-Client MUST send a PA-
> Acknowledgement
> >>      message (PA message with a Received Packet Option) ...
> >
> > Updated.
> >
> >>
> >> Then:
> >>
> >>      In this approach, it is mandated for a PCP client and a PCP server to
> >>      perform a key-generating EAP method in authentication.  Particularly,
> >>      a PCP authentication implementation MUST support EAP-TTLS [RFC5281]
> >>      and SHOULD support TEAP [RFC7170].  Therefore, after a successful
> >>      authentication procedure, a Master Session Key (MSK) will be
> >>      generated.  If the PCP client and the PCP server want to generate a
> >>      transport key using the MSK, they need to agree upon a Pseudo-Random
> >>      Function (PRF) for the transport key derivation and a MAC algorithm
> >>      to provide data origin authentication for subsequent PCP messages.
> >>      In order to do this, the PCP server needs to append a set of PRF
> >>      Options and MAC Algorithm Options to the initial PA-Server message.
> >>      Each PRF Option contains a PRF that the PCP server supports, and each
> >>      MAC Algorithm Option contains a MAC (Message Authentication Code)
> >>      algorithm that the PCP server supports.  Moreover, in the first PA-
> >>      Server message, the server MAY also attach an ID Indicator Option
> >>      defined in Section 5.11 to direct the client to choose correct
> >>      credentials.  After receiving the options, the PCP client selects the
> >>      PRF and the MAC algorithm which it would like to use, and then adds
> >>      the associated PRF and MAC Algorithm Options to the next PA-Client
> >>      message.
> >>
> >> In the above there appears to be quite a bit of normative behavior
> >> that has no
> >> 2119 language. (E.g., "it is mandated", "will be generated", "the PCP
> >> client selects ... then adds".) IMO this should be tightened up with
> >> 2119 language.
> >
> > Thanks, fixed.
> >
> >>
> >> The last two paragraphs of this section explain, in part, the use of
> >> messages with result codes AUTHENTICATION-SUCCEEDED,
> >> AUTHENTICATION-FAILED, and DOWGRADE-ATTACK-DETECTED. Section
> 3.1.1
> >> explains the use of AUTHENTICATION-REQUIRED. Elsewhere is explanation
> >> of SESSION- TERMINATED. Of those, it appears that only
> >> AUTHENTICATION-REQUIRED is used with messages carrying EAP messages,
> >> and then I only see mention of it being used with the *first* EAP message.
> >> Is AUTHENTICATION-REQUIRED to be used with all PA messages that
> >> contain EAP messages?
> >
> > Yes.
> > NEW:
> > The result code for PA-Sever message carrying EAP Identity request will be set
> to AUTHENTICATION_REQUIRED.
> 
> What about the other EAP messages? What PCP result code should be used for
> them? Someplace should specify that.

Yes, added the following lines.
The result code for PA-Sever message carrying EAP Identity request MUST be set to AUTHENTICATION_REQUIRED. The result code for PA-Client message carrying EAP Identity response MUST be set to AUTHENTICATION_REPLY.

> 
> >> If so, that should be specified. If not, then what should be used
> >> with the others?
> >>
> >> Those two paragraphs also say to "terminate the session" in some error
> cases.
> >> Then the following section 3.2 talks about Session Termination. I
> >> *guess* that in the error cases you mean that local session state is
> >> dropped without sending a SESSION-TERMINATED request. It would be
> >> good to be clear about that.
> >>
> >> * Section 3.2:
> >>
> >> This says that upon receiving a SESSION-TERMINATED from a partner one
> >> must also send a SESSION-TERMINATED. Why? That could lead to an
> >> infinite exchange of messages. Also, once the PA SA is removed, any
> >> subsequently received PA message won’t match any session and so will
> >> presumably be dropped. More clarity here would help.
> >
> > It was added because PCP messages are exchanged over UDP and the PCP
> device needs to know if it's partner has received the termination-indicating PA
> message or not.
> 
> OK. But in what cases is that needed?
> 
> If the PCP Client is the first to send SESSION-TERMINATED, then I presume a
> response is to be expected, and it will be retried until a response is received. So
> that doesn't require a special rule.
> 
> The case where it matters is when the PCP Server is the first to send an
> unsolicited response with SESSION-TERMINATED. In that case it may indeed be
> lost. So that is the case when sending one the other way makes sense.
> Here is a possible rewording:
> 
>     A PA session can be explicitly terminated by either session partner.
>     A PCP Server may explicitly request termination of the session by
>     sending an unsolicited termination-indicating PA response (a PA
>     response with a result code "SESSION-TERMINATED"). Upon receiving
>     a termination-indicating request, the PCP Server MUST send a
>     response to the request, and SHOULD then remove the associated PA
>     SA. A PCP Client may explicitly request termination of the session by
>     sending a termination-indicating PA request (a PA request with a
>     result code "SESSION-TERMINATED").  It MUST then remove the
>     associated PA SA upon receiving any response to that request.
> 
> This results in the minimal number of messages, and ensures that the SA will
> remain long enough to process those messages.

Thanks, updated text.
NEW:
A PA session can be explicitly terminated by either session partner. A PCP Server may explicitly request termination of the session by  sending an unsolicited termination-indicating PA response (a PA response with a result code "SESSION-TERMINATED"). Upon receiving a termination-indicating request, the PCP client MUST respond with a termination-Indicating PA message, and SHOULD then remove the associated PA SA. To accommodate packet loss, the PCP server device MAY transmit the termination-indicating PA response up to ten times (with an appropriate Epoch Time value in each to reflect the passage of time between transmissions) provided that the interval between the first two notifications is at least 250 ms, and the interval between subsequent notification at least doubles.

A PCP client may explicitly request termination of the session by sending a termination-indicating PA request (a PA request with a result code "SESSION-TERMINATED"). After receiving a termination-indicating message from the PCP client, a PCP server MUST respond with a termination-indicating PA message and remove the PA SA immediately. When the PCP client receives the termination-indicating PA message, it MUST remove the associated PA SA immediately.

>

> 
> >> * Section 3.3:
> >>
> >>      A session partner may select to perform EAP re-authentication if it
> >>      would like to update the PCP SA without initiating a new PA session.
> >>      A re-authentication procedure could be triggered for the following
> >>      reasons:
> >>
> >>      o  The session lifetime needs to be extended.
> >>
> >>      o  The sequence number is going to reach the maximum value.
> >>         Specifically, when the sequence number reaches 2**32 - 2**16, the
> >>         session partner MUST trigger re-authentication.
> >>
> >> (nit): IMO this would read better if you replace "select" with "elect".
> >>
> >> Also, it is unclear (to me) whether the stated reasons are intended
> >> to be exhaustive, or are simply examples. I *guess* that these are
> >> examples, and that re-authentication can also be requested at whim.
> >> Would be good to be more explicit about this.
> >
> > Okay, mentioned those are only examples.
> >
> >>
> >> Also, what happens if re-authentication is not attempted and the
> >> session lifetime is reached? Is there a requirement to silently terminate the
> session ?
> >
> > Yes, server must attempt re-authentication. This is to handle the case where
> PCP client could have left the network without terminating the session. In this
> case re-authentication fails and server has to silently terminate the session.
> 
> The text doesn't require this. It says "A session partner *may* select to perform
> EAP re-authentication".
> 
> And of course it may be that a partner might not be able to do so. (May not be
> able to send *any* more messages.) So ISTM that it would best to specify what
> does happen if the reauthentication is not attempted. (I think the right thing is
> that the SA is silently terminated.)

NEW:
If for some unknown reason re-authentication is not performed and session lifetime has expired then PA session MUST be terminated immediately.

> 
> >> Do the PA messages containing RE-AUTHENTICATION contain EAP messages
> ?
> >
> > No.
> 
> Would be good to say that.
> 
> >> Or do they trigger sending EAP messages within PA messages having
> >> some other reason code ?
> >
> > No, otherwise it will complicate the mechanism.
> 
> I'm confused. This must trigger sending EAP messages *somehow*. If not in the
> RE-AUTHENTICATION message, then it must be some other. I presume it should
> be the same as is done for the initial authentication. 

Yes.

> But it should be specified.

Yes, updated section 3.3
NEW:
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.

> 
> >> What if "glare" occurs? (Both client and server decide to send RE-
> >> AUTHENTICATION at the same time ?)
> >
> > I guess then one of the partners can back-off (preferably PCP client) and let
> PCP server  proceed with re-authentication.
> 
> I think it is better if the implementer doesn't have to guess. :-)
> 
> There needs to be some specification of the required behavior.

Added the following the line to make it clear.
NEW:
The sequence of EAP messages exchanged for re-authentication will not change, regardless of the PCP device triggering re-authentication.

> 
> >> * Section 4:
> >>
> >>      At the beginning of a PA session, a session SHOULD generate a PA SA
> >>      to maintain its state information during the session.  The parameters
> >>      of a PA SA are listed as follows:
> >>
> >> Why SHOULD rather than MUST? How can any of this work if the SHOULD
> >> is violated? ISTM the only case would be if the PA-Server refuses the
> >> initial PA request.
> >
> > Updated to MUST.
> >
> >>
> >> And I'll restate something I mentioned above: is it permissible for a
> >> PCP session to have more than one concurrently active PA sessions? If
> >> so, each will have its own state.
> >>
> >> Included in the listed state are *four* sequence numbers. But there
> >> is little mention of these elsewhere in the document. Many places
> >> simply say things
> >> like: "A sequence number needs to be incremented", "The sequence
> >> number of the last received PCP message". (Sections 6.4 and 6.5 are
> >> more explicit.) It would be helpful for the places that are vague about this to
> be more explicit.
> >
> > Please mention the places where you find it is vague.
> 
> At least sections 5.2, 5.4, and 5.10.

Thanks, updated those sections.

-Tiru

> 
> >> * Section 5.1:
> >>
> >> I'll restate that it seems confusing to define "response codes" that
> >> are not identifying *responses*.
> >>
> >> I guess that because the PA messages containing EAP messages invert
> >> the roles of client and server (wrt Common PCP messages), it might
> >> make sense to have response codes in PCP request messages that happen
> >> to be EAP response messages. But then it is wrong terminology for PCP
> >> response messages that contain EAP requests.
> >>
> >> I'm not quite sure how to recommend clarifying this. The whole
> >> situation is set up for confusion of roles, so it does require considerable care
> to make clear.
> >> (*I* would be inclined to use separate fields for PCP response codes,
> >> EAP response codes, and for "sub-opcodes" of PCP opcodes. But you
> >> have thought about it more than I have.)
> >>
> >> * Section 5.2:
> >>
> >> The section title is inappropriate – the section is not about the
> >> Authentication Opcode, it is about information that is specific and
> >> common to all messages (requests and responses) containing the
> >> Authentication opcode. I suggest retitling this "Opcode-specific information
> of PCP Auth Messages".
> >
> > Updated.
> >
> >>
> >> Also, it says:
> >>
> >>         ... A
> >>         sequence number needs to be incremented on every new (non-
> >>         retransmission) outgoing message in order to provide an ordering
> >>         guarantee for PCP messages.
> >>
> >> This could be clearer – stating that the sequence number to use is
> >> different for requests and responses. (And name which ones.)
> >
> > It is discussed in
> > https://tools.ietf.org/html/draft-ietf-pcp-authentication-11#section-6
> > .4
> 
> Yes it does. I was eventually able to figure it out. But it took me two readings
> before I understood it. It isn't essential to fix, but it would be nice to help out
> the first time reader.
> 
> >> * Section 5.3:
> >>
> >> Is the following intended to be normative?
> >>
> >>      If the PA-Server message does not carry the correct nonce,
> >>      the message will be discarded silently.
> >>
> >> If so it should use 2119 language. Perhaps this section isn’t the
> >> proper place for that normative language, but I don’t find it anywhere else.
> >
> > Okay, updated.
> > NEW:
> > If the PA-Server message does not carry the correct nonce, the message
> MUST be discarded silently.
> 
> OK.
> 
> >> * Section 5.4:
> >>
> >> As in section 5.2, this would benefit from describing which sequence
> >> numbers are to be used.
> >
> > Outgoing and incoming message have their respective sequence numbers
> > and it's usage is explained in
> > https://tools.ietf.org/html/draft-ietf-pcp-authentication-11#section-6
> > .5
> 
> My earlier comment applies.
> 
> >> * Section 5.11:
> >>
> >>         ... The two indicator
> >>         strings are to be considered equivalent by the client if they are
> >>         an exact octet-for-octet match.
> >>
> >> Is this intended to mean "if *and only if* they are an exact match"?
> >> Or is the client being given the option of accepting a looser match
> >> if it wishes? Right now that seems ambiguous, so it would be good to
> >> tighten up the language to be explicit about what you mean.
> >
> > Fixed. NEW:
> > The two indicator strings are to be considered equivalent by the client if and
> only if they are an exact octet-for-octet match.
> >
> >>
> >> * Section 6.1:
> >>
> >> In this section there are separate bullets for PA messages and Common
> >> PCP messages. Both talk about appending an "Authentication Tag Option".
> >> But there are different options for PA messages and common messages.
> >> ISTM the text here should reiterate that.
> >
> > Yup, updated section to make it clear.
> >
> >>
> >> * Section 6.6:
> >>
> >> This says that EAP lower layers indicate to EAP methods the MTU of
> >> the lower layer. But doesn’t *this* protocol provide the "lower
> >> layer" that EAP is running over? So isn’t this protocol responsible
> >> for determining the MTU of the layer
> >> *it* runs over, adjust that for the overhead it adds, and providing
> >> that to the EAP layer ?
> >
> > No, PCP does not determine the path MTU.
> 
> It is a small point, and I won't pursue it. But I don't see how this works. To
> determine when to fragment, the EAP lower layers need to know both the path
> MTU and the overhead of any intervening layers that are using up part of that
> packet size, including the PCP layer.
> 
> >> * Section 7:
> >>
> >> All opcodes, result codes, and options currently registered with IANA
> >> have names formatted as upper case tokens that are legal C-language
> identifiers.
> >> (Using underscore rather than dash.) For consistency the new ones
> >> defined in this document ought to follow that form. The new response
> >> codes are close, but use dashes. The others are currently phrases.
> >> The new option names read more like descriptions. (Those may be
> >> appropriate in the Purpose field.) If this change is made, then the
> >> mnemonics can be used throughout the text. That would be clearer.
> >
> > Yes, updated draft.
> >
> >>
> >> All of the options registered here are to be allocated in the
> >> mandatory-to- process range. Yet I *guess* that everything in this
> >> draft is optional for PCP. I guess that there is a backward
> >> compatibility requirement – that implementations PCP with this
> >> extension interoperate with basic 6887 implementations. Doesn’t that
> >> mean that all the things registered here should be optional to
> >> process by PCP (though still mandatory to process for those
> >> implementing this specification.)
> >
> > No, mandatory-to-process means that PCP server returns UNSUPP_OPTION if
> the option is unrecognized, unimplemented, or disabled. For more info please
> refer to https://tools.ietf.org/html/rfc6887#section-7.3.  For PCP
> authentication to work all the new PCP options introduced in the document are
> mandatory for PCP client and server to implement.
> 
> Do you mean that you intend for this extension to be mandatory for *all*
> implementations of PCP? And that old implementations that don't support it
> are not to interwork with new ones that do? I am guessing that is not the case.
> So lets call PCP implementations that support it Authenticating PCP
> clients/servers, and those that don't Unauthenticating PCP clients/servers. And
> then consider the cases:
> 
> 1) Authenticating client, Authenticating Server
> 
> In this cae you want both to process all the new options
> 
> 2) Unauthenticating client, Unauthenticating server
> 
> The options won't be used here so it doesn't matter
> 
> 3) Authenticating client, Unauthenticating server
> 
> This will be like (2), except in case where the client sends a PA-Initiation
> request. While this has options, presumably the opcode will be rejected and so
> the processing of options is irrelevant. A smart client will decide that the failure
> of the PA-Initiation should be ignored.
> 
> 4) Unauthenticating client, Authenticating server
> 
> This case is entangled with my concerns about section 3.1.2 of the draft. The
> first issue will be when the client receives a PA-response to one of its Common
> PCP requests. The client can be expected to ignore it.
> It will be left retrying the request repeatedly until giving up, and will think the
> server is broken.
> 
> The consequences depend a bit on how the server is using authentication.
> If it unconditionally requires authentication, then it doesn't matter much,
> because it will never be able to work with an unauthenticating client.
> 
> But if it only requires authentication for certain sorts of requests, while
> permitting others without authentication, then this could be a problem. But it
> isn't really a problem with *options*.
> 
> However this depends on if/how you end up dealing with the issues I have
> earlier raised on this. If you sent a response to the message requiring
> authentication, and it had an option containing the session id, then it matters,
> because the client should deal with the response code (as one it can't deal
> with) and not get upset if it can't process a mandatory option it doesn't know.
> 
> Having gone through all these cases, I guess it having these be mandatory
> doesn't hurt much.
> 
> 	Thanks,
> 	Paul
> 
> >> It would be good to have numbered subsections for each distinct
> >> registry, and maybe for each distinct value, so that they show up in
> >> the TOC. It would also be nice to format these so that it is clear
> >> where one element ends and the next one starts. (Right now it is very
> >> hard to read.)
> >
> > Sure, added sub-sections.
> >
> > Cheers,
> > -Tiru
> >
> >>
> >
>