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

[<[e-mail]>]

The Port-Session-Limit Attribute is defined as follow:

   This attribute is of type complex [RFC6158] and specifies the limit
   of TCP ports, or UDP ports, or the sum of the two, or ICMP
   identifiers, or the sum of the three, which is configured on a device
   supporting port ranges corresponding to a specific subscriber.

Isn't it the same purpose than the Port-Limit defined in RFC 2865?

Port-Limit

      This Attribute sets the maximum number of ports to be provided to
      the user by the NAS.  This Attribute MAY be sent by the server to
      the client in an Access-Accept packet.  It is intended for use in
      conjunction with Multilink PPP [12] or similar uses.  It MAY also
      be sent by the NAS to the server as a hint that that many ports
      are desired for use, but the server is not required to honor the
      hint.

By the way, I share the comment on Alan about avoiding polymorphic attributes and recommend the use of TLV.
I guess that two different TLV-types will have to be defined, one for external/public address+port (NAT assigned) and the other for internal/private address+port, and both TLV consisting in two sub-attributes, one in the format of "Framed-IP-address" and one ine the format of "Port".

Lionel


-----Message d'origine-----
De : radext [mailto:[e-mail]] De la part de Alan DeKok
Envoyé : mardi 4 mars 2014 10:50
À : [e-mail]
Objet : [radext] Review of draft-cheng-behave-cgn-cfg-radius-ext

  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.

_______________________________________________
radext mailing list
[e-mail]
https://www.ietf.org/mailman/listinfo/radext

_________________________________________________________________________________________________________________________

Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.