Hi Paul,
Please see inline
-----Original Message-----
From: Paul Kyzivat [mailto:pkyzi...@alum.mit.edu]
Sent: Monday, July 06, 2015 3:24 AM
To: draft-ietf-pcp-authentication....@tools.ietf.org
Cc: General Area Review Team
Subject: Gen-ART Telechat review of draft-ietf-pcp-authentication-13.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 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.
Updated.
[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?
It is not acceptable to omit the nonce, changed from SHOULD to MUST.
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.)
Replaced "random" with "unique".
[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.
NEW:
and fill the identifier into the Session ID field in the opcode-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.
Subsequent PCP messages within the PA session could also be PCP-Auth messages
and these messages will not carry the AUTHENTICATION_TAG option. How about
saying the following instead:
NEW:
Subsequent PCP messages within this PA session MUST contain 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.)
Updated figure. EAP request and responses will also use the same return codes
as EAP identity request and response.
* 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.
NEW:
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 rejects the request
with a AUTHENTICATION_REQUIRED error code and can reply with a unsolicited
PA-Server message to initiate a PA session. The result code field of this
PA-Server message is set to AUTHENTICATION_REQUIRED.
- 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.
Yes.
Presumably you could try it again, but would just get another
request to authenticate.
Yup.
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?
Yes.
NEW:
If the PCP client retries the common request before EAP authentication is
successful then it will receive AUTHENTICATION_REQUIRED error code from the PCP
server.
* 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.
Thanks, updated draft.
[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.
Fixed. I don't see a reason for PCP to distinguish b/w EAP identity request and EAP request (and
the same goes for EAP identity response and EAP response). Updated draft to use "EAP
request" and "EAP response" in all places.
(In general, I find that the text insufficiently explains that the number of
messages in the exchange is variable and how that all works.)
The number of messages exchanged depends on the EAP type.
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.
It is already mentioned in the document that key-generating EAP method MUST be
used. Please clarify why the negative case needs to be discussed. I don't see
any confusion.
[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.
Updated section 3.1.3
1) In this case, before sending out the PA-Server message, the PCP server MUST
update the PCP SA with the MSK and transport key, and use the derived transport
key to generate a digest for the message.
2) In addition, the PCP client needs to update the PCP SA with the MSK and
transport key, and uses the derived transport key to secure the message.
[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".
Updated.
[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.
NEW:
If a PCP server resets or loses the PA SA due to reboot, power
failure, or any reason then it sends unsolicited ANNOUNCE Message as
explained in section 14.1.3 of [RFC6887] to the PCP client. The PCP
client authenticates with the PCP server again as discussed in
Section 3.1.1 and issues new common PCP requests to recreate any lost
mapping state. If the PCP client resets or loses the PA SA due to
reboot, power failure, or any reason and sends common PCP request
then PCP server rejects the request with AUTHENTICATION_REQUIRED
error code and the client authenticates with the server as discussed
in Section 3.1.1.
[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".
"independent PCP clients" means applications on the host using its own PCP
software instance responsible for issuing PCP requests to a PCP server, It is explained
in RFC 6887.
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.
Other PCP clients on the host can also establish a PCP SA on 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?
Agreed, changed to MUST.
* 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.
Yes, updated text.
NEW:
... The result
code for PA-Sever message carrying EAP request will be set
to AUTHENTICATION_REQUIRED and PA-Client message carrying EAP
response will be set to AUTHENTICATION_REPLY.
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.
Good point, modified draft to mandate the use of session lifetime option.
[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.)
Even if both PCP client and PCP trigger re-authentication at the same time
there is no change in the way EAP messages are exchanged. The PCP server has to
send EAP request to the client so that it can re-authenticate. It is mentioned
in the last line of Section 3.3
<snip from Section 3.3>
The sequence of EAP messages exchanged for re-authentication will not change,
regardless of the PCP device triggering re-authentication.
</snip>
* 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.
Fixed.
[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.
Updated.
* Section 5.1:
[TECHNICAL] My comment on section 3.3 applies here as well.
Fixed.
The result code for PA-Sever message carrying EAP request MUST be set to
AUTHENTICATION_REQUIRED.
The result code for PA-Client message carrying EAP response MUST be set to
AUTHENTICATION_REPLY.
[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.1 already has a figure to depict the revised version of the request
format.
* Section 5.5:
[EDITORIAL NIT] For consistency with the others, the section heading for this
section ought to be "PA_AUTHENTICATION_TAG Option".
Updated.
[EDITORIAL] Also, "because such information is provided in the
AUTHENTICATION Opcode" ought to be: "because such information is provided
in the Opcode-specific information".
Updated.
* 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".
Fixed the TECHNICAL and Nit comments.
NEW:
After successful EAP authentication process, every subsequent PCP message
within the PA session MUST carry an authentication tag which contains the
digest of the PCP message for data origin authentication and integrity
protection.
* 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.
My response to the previous comment on PCP server or PCP client losing the PCP
SA should address this comment as well.
* 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.)
The subsections are for each new option. The updated section is in-line with
the details that RFC 6887 in section 19.4 has asked new documents to provide.
This concludes my Gen-ART comments.
Thanks for the detailed review.
Cheers,
-Tiru