Subject: [Int-dir] Int-Dir Review of draft-ietf-pcp-authentication-09

From: "Carlos Pignataro (cpignata)" <[email-masked]>
Date: Mon, 8 Jun 2015 15:38:35 +0000

Hi,

I am an assigned INT directorate reviewer for draft-ietf-pcp-authentication-09.

These comments were written primarily for the benefit of the Internet
Area Directors. Document editors and shepherd(s) should treat these
comments just like they would treat comments from any other IETF
contributors and resolve them along with any other Last Call comments
that have been received. For more details on the INT Directorate,
see http://www.ietf.org/iesg/directorate.html.

Summary:

This document describes an EAP-based Authentication mechanism for the Port Control Protocol (PCP), meeting the security requirements of the Advanced Threat Model in the base PCP specification. Its intended status is Standards Track.

This is a very well written document. I only found minor issues (e.g., option-lengths), editorials and nits.

Major Issues:

None.

Minor Issues, Nits, Editorials:

I hope you find these useful:

General:

CMP: There are variations of “session ID” vs. “Session-ID” vs. “Session_ID” and others. Same with “opcode” vs. “Opcode” and others, which should be normalized.

1.  Introduction

   This document proposes a PCP security
   extension which enables PCP servers to authenticate their clients
   with Extensible Authentication Protocol (EAP).

CMP: s/which/that/

CMP: Does this document “proposes” or “describes” / “defines"?

2.  Terminology

   PCP Client: A PCP software instance which is responsible for issuing

CMP: s/which/that/ [there are a couple more instances of this]

5.X.  For all the 5.X sections, starting from 5.3:

CMP: Reserved — could we specify the treatment of Reserved bits (e.g., MBZ)? Like “Reserved: MUST be zero on transmission and MUST be ignored on reception."

CMP: What follows is some potential issues with Option-Lengths. I might be missing something.

5.3.  Nonce Option

      Option-Length: The length of the Nonce Option (in octets),
      including the 4 octet fixed header and the variable length of the
      authentication data.

CMP: The format of the figure does not show any Authentication Data. Is this Option-Length always 4?

5.4.  Authentication Tag Option for Common PCP Messages

      Option-Length: The length of the Authentication Tag Option for
      Common PCP (in octets), including the 12 octet fixed header and
      the variable length of the authentication data.

CMP: Is it necessary to clarify that when the Auth Data is padded, the length still reflects the unpadded data length?

5.5.  Authentication Tag Option for PA Messages

      Option-Length: The length of the Authentication Tag Option for PCP
      Auth (in octet), including the 12 octet fixed header and the
      variable length of the authentication data.

CMP: This one seems to only include 4 (and not 12) octets of fixed header.

5.6.  EAP Payload Option

      Option-Length: The length of the EAP Payload Option (in octets),
      including the 4 octet fixed header and the variable length of the
      EAP message.

CMP: What is the fixed header in this one?

5.7.  PRF Option

   Option-Length: The length of the PRF Option (in octets), including
   the 4 octet fixed header and the variable length of the EAP message.

CMP: What is the variable length in this one? Is it always 4?

7.  IANA Considerations

   Opcodes is maintained in http://www.iana.org/ assignments/pcp-
   parameters):

CMP: The URLs pointing to pcp-parameters are broken.

   TBA Authentication Opcode.

CMP: An editorial suggestion, it help with readability of this section to indent the actual assignments for PCP Opcode and PCP Option Codes, such as it is done with PCP result codes:

      TBA INITIATION: The client indication to the server for
      authentication.

   Maximum occurences:  1.

CMP: s/occurences/occurrences/g

References:

CMP: Idnits finds these two:

  ** Downref: Normative reference to an Informational RFC: RFC 5281

  ** Obsolete normative reference: RFC 5996 (Obsoleted by RFC 7296)

Thanks!

— Carlos.

---

From: "Tirumaleswar Reddy (tireddy)" <[email-masked]>
Date: Tue, 9 Jun 2015 07:19:29 +0000

Hi Carlos,

Thanks for the review. Please see inline

> -----Original Message-----
> From: Carlos Pignataro (cpignata)
> Sent: Monday, June 08, 2015 9:09 PM
> To: [email-masked]; [email-masked]
> Cc: [email-masked]; [email-masked]
> Subject: Int-Dir Review of draft-ietf-pcp-authentication-09
> 
> Hi,
> 
> These comments were written primarily for the benefit of the Internet Area
> Directors. Document editors and shepherd(s) should treat these comments just
> like they would treat comments from any other IETF contributors and resolve
> them along with any other Last Call comments that have been received. For
> more details on the INT Directorate, see
> http://www.ietf.org/iesg/directorate.html.
> 
> Summary:
> 
> This document describes an EAP-based Authentication mechanism for the Port
> Control Protocol (PCP), meeting the security requirements of the Advanced
> Threat Model in the base PCP specification. Its intended status is Standards
> Track.
> 
> CMP: There are variations of “session ID” vs. “Session-ID” vs. “Session_ID” and
> others. Same with “opcode” vs. “Opcode” and others, which should be
> normalized.

Fixed.

>    with Extensible Authentication Protocol (EAP).

Fixed.

> CMP: Does this document “proposes” or “describes” / “defines"?

"defines" seems to fits better.

> CMP: s/which/that/ [there are a couple more instances of this]

Fixed.

> CMP: Reserved — could we specify the treatment of Reserved bits (e.g., MBZ)?
> Like “Reserved: MUST be zero on transmission and MUST be ignored on
> reception."

Yes, updated.

> CMP: The format of the figure does not show any Authentication Data. Is this

It was a cut and paste mistake, corrected option length field for all options.
Thanks for catching it.

> still reflects the unpadded data length?

No, it is already discussed in http://tools.ietf.org/html/rfc6887#section-7.3 

> CMP: This one seems to only include 4 (and not 12) octets of fixed header.

Yes, corrected.

> CMP: What is the fixed header in this one?

There is no fixed header, corrected text.

> CMP: What is the variable length in this one? Is it always 4?

Yes, it is always 4.

> CMP: The URLs pointing to pcp-parameters are broken.

Fixed.

> CMP: An editorial suggestion, it help with readability of this section to indent
> the actual assignments for PCP Opcode and PCP Option Codes, such as it is
> done with PCP result codes:
> 

Replaced.

>   ** Downref: Normative reference to an Informational RFC: RFC 5281

Yes, it was intentional. Please refer to the explanation given by Dave Thaler co-author of PCP WG
in the Shepard write-up

<snip>

(15) Are there downward normative references (see RFC 3967)?

    Yes.  The WG agreed that having a mandatory-to-implement EAP method
    was required for interoperability.  The WG surveyed EAP implementers/experts
    as noted in Q5 above and found that there is no standards track EAP method that
    is available in all the relevant EAP implementations.  As such, the EAP expert
    reviewers advice was followed, which was to require RFC 5281 (widely implemented
    but only an Informational RFC) while making the new RFC 7170 (proposed
    standard, but not implemented) a SHOULD.  The alternative would be to
    require RFC 7170, which the feedback indicated would block deployment
    of this draft for some time to come.  Having security for PCP sooner than
    later was determined to be important, and so the WG would like to request
    approval for this downward reference.

</snip>

>   ** Obsolete normative reference: RFC 5996 (Obsoleted by RFC 7296)

Fixed.

Cheers,
-Tiru

---

From: "Carlos Pignataro (cpignata)" <[email-masked]>
Date: Tue, 9 Jun 2015 16:04:59 +0000

Thanks, Tiru!

> On Jun 9, 2015, at 3:19 AM, Tirumaleswar Reddy (tireddy) <[email-masked]> wrote:
> 
> Hi Carlos,
> 
> No, it is already discussed in http://tools.ietf.org/html/rfc6887#section-7.3 <http://tools.ietf.org/html/rfc6887#section-7.3>
> 
>> 
>>   Opcodes is maintained in http://www.iana.org/ <http://www.iana.org/> assignments/pcp-
>>   parameters):
>>