Subject: [pcp] Review of draft-maglione-pcp-radius-ext-05

From: Alan DeKok <[email-masked]>
Date: Tue, 12 Mar 2013 10:20:58 -0400

This email is a quick review of the draft-maglione-pcp-radius-ext-05
document.  Overall, it looks fairly good.

  The main issue I see is the definition of the PCP-server-name
attribute.  It is defined in reference to draft-ietf-pcp-dhcp, which
says that the attribute is a list of domain names, separated by spaces.
 This encoding does not follow the traditional RADIUS methods.

  Normally, the data would have been sent as a series of attributes of
the same "type", with different content.  e.g. instead of

	PCP-Server-Name = "name1 name2 name3"

  it would be

	PCP-Server-Name = "name1"
	PCP-Server-Name = "name2"
	PCP-Server-Name = "name3"

  I think there is good reason to change the definition.  Keeping it as
a series of space-delimited strings means changing existing RADIUS
systems.  RADIUS systems already have the means to select one of N
attributes.  Searching inside of strings is a little more complex.

  The ASCII art shows this for the definition:

       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
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |     Type      |    Length     |  PCP-Server-Name  ....
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |             Context           |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

  I'm not sure why a fixed-length field "context" comes after a
variable-length field.  In general, the fixed-length fields should be
first, as it assists with decoding the data.

  Further, the document asserts:

   The data type of PCP Server Name is a string with opaque
   encapsulation, according to section 2.1 of [RFC6158]

  I don't see how this is true.  The ietf-pcp-dhcp document does not
define a structure encoding PCP-Server-Name followed by context.  If it
did, that structure should be referenced here.

  It seems to me that both of the fields need to be administered by the
RADIUS server.  Therefore, they are *not* "opaque data" as per the
requirements of RFC 6518.

  I suggest using two attributes instead of one, which has an arbitrary
structure.  I suggest encoding the PCP-Server-Name as a series of
attributes, rather than as space-separated strings.

  Alan DeKok.

---

From: Dean cheng <[email-masked]>
Date: Tue, 12 Mar 2013 14:56:27 +0000

Alan, thanks for your comments that make sense to me, i.e., make the server names as a series of attributes, and move the "context" before the name attribute, allowing Radius server do the processing in a traditional way.
Dean

> -----Original Message-----
> From: [email-masked] [mailto:[email-masked]] On Behalf Of
> Alan DeKok
> Sent: Tuesday, March 12, 2013 7:21 AM
> To: [email-masked]
> Subject: [pcp] Review of draft-maglione-pcp-radius-ext-05
> 
> _______________________________________________
> pcp mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/pcp

---

From: <[email-masked]>
Date: Tue, 12 Mar 2013 17:30:35 +0100

Dear Alan,

Many thanks for the review. 

Most of your comments have been incorporated in -06: http://tools.ietf.org/id/draft-maglione-pcp-radius-ext-06.txt. 
A diff is available here to track the changes: http://www.ietf.org/rfcdiff?url2=draft-maglione-pcp-radius-ext-06. 

-06 does not define Context as a separate attribute as this may complicate the procedure: having this field indicated in each instance of the name attribute makes it easier to associate a context with a name. 

Cheers,
Med

>-----Message d'origine-----
>De : [email-masked] [mailto:[email-masked]] De la 
>part de Alan DeKok
>Envoyé : mardi 12 mars 2013 15:21
>À : [email-masked]
>Objet : [pcp] Review of draft-maglione-pcp-radius-ext-05
>