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

[Alan DeKok <[e-mail]>]

  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.