[radext] Review of draft-cheng-behave-cgn-cfg-radius-ext

[Alan DeKok <[e-mail]>]

  This is a quick review of the document prior to RADEXT WG meeting.


   This document proposes three new RADIUS attributes as RADIUS
   protocol's extensions, and they are used for separate purposes as
   follows:

  Any draft is a RADIUS protocol extension.  I'd suggest changing this
text to:

   This document proposes three new attributes as follows:


  The following paragraphs could be re-organized.  They currently give a
description first, followed by the attribute name.  The text also seems
redundant:

   The session limit is carried by a new RADIUS attribute ...

  Yes, most drafts define a new attribute.  I would just say:

  o Port-Session-Limit

    Sent in Access-Accept or CoA-Request.  Limits the of total number of
    TCP/UDP ports and/or ICMP identifiers which a subscriber can use.

  i.e. give the name, which packets it can occur in, and a short
description of its meaning.  The later sections will expand on these
descriptions, so there's no need to give complicated definitions here.


     o  Session Limit - This is the maximum number of TCP ports, or UDP

  Is this the same as "Port-Session-Limit" above?  If so, I suggest
using the same name.

  Also, all authorization attributes in RADIUS are for a particular
session.  So there's no need to call them "Session" limits.  By
definition, they're for one session, and only one session.


      [Discussion: should these attributes be allocated from the
      extended RADIUS attribute code space?]

  I don't think it matters.  They could be allocated from the standard
space.


3.1 Port-Session-Limit

   This attribute is of type complex [RFC6158] ...

  There is no reason to make these attributes type "complex".  Everyone
would be better served by using TLVs, as defined in RFC 6929.



3.2 Port-Session-Range

   ...
   The port range follows the encoding specified in [RFC6431];

  RFC 6431 defines a data structure of 4 fields.  This attribute re-uses
that, but inserts it in the middle of other complex fields.  So it does
not appear to follow the requirements to be an "opaque data type" as
given in RFC 6158 Section A.1.3.


3.3.  Port-Forwarding-Map Attribute

   ... In addition, the attribute may contain a
   32-bit IPv4 address or a 128-bit IPv6 address, respectively, as their
   respective NAT mappings internal IP address.

  This definition violates RFC 6158 Section 3.4, which prohibits
polymorphic attributes.


7.2.  Name Spaces

   This document establishes a new name space for Session Type (see
   Section 3.1 for the initial reservation of values.  The allocation of
   future values is according to RFC Required policy [RFC5226]

  I don't think any previous RADIUS specification defined an IANA name
space for sub-fields of an attribute.  All of the existing enumerated
types are of data type "integer".


  Over all, the goal of the document seems reasonable.  But the encoding
of the attributes violates most of RFC 6158, without giving any reason
as to why.  Implementing it as-is would impose undue burden on implementors.

  I suggest using TLVs instead of these complex data types.  That would
avoid the need for polymorphic attributes, too.

  Alan DeKok.