[radext] Benoit Claise's Discuss on draft-ietf-radext-ip-port-radius-ext-11: (with DISCUSS and COMMENT)

["Benoit Claise" <[e-mail]>]

Benoit Claise has entered the following ballot position for
draft-ietf-radext-ip-port-radius-ext-11: Discuss

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)


Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-radext-ip-port-radius-ext/



----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

I'm late for my review here. If some points in my DISCUSS/COMMENT have
been discussed already let me know.

I'll flag this as a DISCUSS to follow up on the IPFIX issues.
First of, I applause the attempt to reuse/map/combine data models (IPFIX
information element and RADIUS attribute).

Now, there are a couple of issues.
The first set of issues has been sent via the IPFIX experts, copied here
for tracking purposes.

    Dear Authors,

    The experts for the IPFIX IE registry have returned the following
    review:

    In general, the Information Elements in draft-ietf-radext-ip-port-
    radius-ext are so underspecified as to be unimplementable. They
should
    not be added to the registry in their present form. The authors are
    advised to read RFC 7013, especially Section 4, which provides
useful
    information on defining Information Elements. Specifically:

    The Information Element transportType is underspecified: (a) I
presume
    this is in reference to sourceTransportPort and
    destinationTransportPort, but the description must say this if it is
    the case; (b) It's not clear at all from the description in what
    context this distinction is useful; (c) What's an ICMP identifier?

    In addition, the description of transportType appears to create a
    table which should probably be handled as a subregistry. See See
    RFC7013 section 4.7. for advice on the creation of tables without
    subregistries (in short, "don't".)

    The Information Element natTransportLimit has an inappropriate name;
    it does not describe that which it (presumably) is supposed to
    represent (see RFC 7013 section 4.1). In addition, it is
    underspecified. It is impossible to implement from the description.
Is
    the field IPv4 specific, or is IPv6 supported as well? (If not, why
    not?)

    The Information Element localID has an inappropriate name; it is far
    too general (see RFC 7013 section 4.1). It uses an inappropriate
    abstract data type (addresses should never be represented as UTF-8
    strings in IPFIX, see RFC 7013 section 4.2). It is underspecified as
    well as poorly designed. Without the ability to disambiguate the
type
    of information in the field, this is not a useful Information
Element.
    Without a complete enumeration of possible types (n.b. 'etc.' in the
    description), it is not a useful Information Element. Its purpose is
    unclear from its description; further, it appears to violate the
    following guidance in RFC 7013 section 4: "The Information Element
    must be unique within the registry, and its description must
represent
    a substantially different meaning from that of any existing
    Information Element. An existing Information Element that can be
    reused for a given purpose should be reused."

I have some more issues I would like to discuss:
- section 3.2.5, as an example, contains a list of sourceIPv6Address. In
IPFIX, we would export those as a list [RFC 6313]
Does this RADIUS document want to only reuse the semantic of individual
IPFIX information element?
And also the semantic of multiple information elements [RFC 6313]?
And also the IPFIX encoding? For example, some IPFIX fields are "right
justified" (sourceTransportPort), while some others are not (localID)
The document should be clear.


----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

-

   1.  IP-Port-Limit-Info: This attribute may be carried in RADIUS
       Access-Accept, Access-Request, Accounting-Request or CoA-Request
       packet.  The purpose of this attribute is to limit the total
       number of TCP/UDP ports and/or ICMP identifiers allocated to a
       user, associated with one or more IPv4 addresses.

I see "may be carried". Could it be carried in other packets ... was my
first question.
It was not clear (until I read the rest of the document) that it's a non
compulsory attribute only in 
Access-Accept, Access-Request, Accounting-Request or CoA-Request
packets.
Proposal:
   1.  IP-Port-Limit-Info: The purpose of this attribute is to limit the
total
       number of TCP/UDP ports and/or ICMP identifiers allocated to a
       user, associated with one or more IPv4 addresses. This attribute
is 
       carried in RADIUS Access-Accept, Access-Request,
Accounting-Request or 
       CoA-Request packets.  

Same remark for the other two points below. 

 2.  IP-Port-Range: This attribute may be carried in RADIUS
       Accounting-Request packet.  The purpose of this attribute is to
       report by an address sharing device (e.g., a CGN) to the RADIUS
       server the range of TCP/UDP ports and/or ICMP identifiers that
       have been allocated or deallocated associated with a given IPv4
       address for a user.

   3.  IP-Port-Forwarding-Map: This attribute may be carried in RADIUS
       Access-Accept, Access-Request, Accounting-Request or CoA-Request
       packet.  The purpose of this attribute is to specify how an IPv4
       address and a TCP/ UDP port (or an ICMP identifier) is mapped to
       another IPv4 address and a TCP/UDP port (or an ICMP identifier).

-

   1.  IP-Port-Limit-Info: This attribute may be carried in RADIUS
       Access-Accept, Access-Request, Accounting-Request or CoA-Request
       packet.  The purpose of this attribute is to limit the total
       number of TCP/UDP ports and/or ICMP identifiers allocated to a
       user, associated with one or more IPv4 addresses.


Only for TCP/UDP? I believe Mirja had the same question.

- Section 2. Terminology.
Typically, you want to have that note at the beginning:

   Note that the definitions of "internal IP address", "internal port",
   "internal realm", "external IP address", "external port", "external
   realm", and "mapping" are the same as defined in Port Control
   Protocol (PCP) [RFC6887], and the Common Requirements for Carrier-
   Grade NATs (CGNs) [RFC6888].

If I know of RFC6887 and RFC6888, I don't read the definitions.

- Section 3.1.3

   This attribute is of type "TLV" as defined in the RADIUS Protocol
   Extensions [RFC6929].  It contains the following sub-attributes:

   ...

   o  either an IP-Port-Int-IPv4-Addr TLV (see Section 3.2.4) or an IP-
      Port-Local-Id TLV (see Section 3.2.11),

   o  either an IP-Port-Int-IPv6-Addr TLV (see Section 3.2.5) or an IP-
      Port-Local-Id TLV (see Section 3.2.11),

Depending on the IP-Port-Type value, it's one of the two above, not
both.

- I believe the RFC 2119 keywords usage is sometimes weak (generic
comment for the entire document)
Example1: section 3.1.3

   The attribute contains a 2-byte IP internal port number that is
   associated with an internal IPv4 or IPv6 address, or a locally
   significant identifier at the customer site, and a 2-byte IP external
   port number that is associated with an external IPv4 address.  The
   internal IPv4 or IPv6 address, or the local identifier must be
   included; the external IPv4 address may also be included.

Not RFC 2119 keywords. Shoud they be MUST and MAY?
And the next paragraph contains RFC 2119 keywords.

   The IP-Port-Forwarding-Map Attribute MAY appear in an Access-Accept
   packet.  It MAY also appear in an Access-Request packet to indicate a
   preferred port mapping by the device co-located with NAS.  However
   the server is not required to honor such a preference.

Example2:

   IP-Port-Ext-IPv4-Addr TLV MAY be included in the following
   Attributes:

   o  IP-Port-Limit-Info Attribute, identified as 241.TBD1.3 (see
      Section 3.1.1).

   o  IP-Port-Range Attribute, identified as 241.TBD2.3 (see
      Section 3.1.2).

   o  IP-Port-Forwarding-Mapping Attribute, identified as 241.TBD3.3
      (see Section 3.1.3).

Don't you need the RFC2119 keyword in the reverse situation, in section
3.1.1 (IP-Port-Limit-Info Attribute)?  
OLD:
   o  an optional IP-Port-Ext-IPv4-Addr TLV (see Section 3.2.3).

NEW:
   o The IP-Port-Limit-Info Attribute MAY contain the
IP-Port-Ext-IPv4-Addr (see Section 3.2.3).

This logic would not only be clearer, but would also avoid
inconsistencies.
See section 3.1.3
OLD:

It contains the following sub-attributes:
   ...
   o  an IP-Port-Ext-IPv4-Addr TLV (see Section 3.2.3).


NEW:

It contains the following sub-attributes:

  ...
   o  an optional IP-Port-Ext-IPv4-Addr TLV (see Section 3.2.3).

Example 3:

   natEvent

      Integer.  This field contains the data (unsigned8) of natEvent
      (230) defined in IPFIX, right justified, and unused bits must be
      set to zero.  It indicates the allocation or deallocation of a
      range of IP ports as follows:

MUST?