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

[<[e-mail]>]

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 : [e-mail] [mailto:[e-mail]] De la 
>part de Alan DeKok
>Envoyé : mardi 12 mars 2013 15:21
>À : [e-mail]
>Objet : [pcp] Review of draft-maglione-pcp-radius-ext-05
>
>  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.
>_______________________________________________
>pcp mailing list
>[e-mail]
>https://www.ietf.org/mailman/listinfo/pcp
>