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

From: "Benoit Claise" <[email-masked]>
Date: Thu, 15 Sep 2016 05:26:23 -0700

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?

---

From: Dean cheng <[email-masked]>
Date: Mon, 19 Sep 2016 10:11:14 +0000

Hi Benoit,

Many thanks for your comments. We'll incorporate

your proposals and fixes for other comments we

have received together into a new revision of

this draft at a later time when this round of

discussion concludes. We'll need some time

requiring consensus among authors but also the WG.

Please see below as well.

Regards

Dean

> -----Original Message-----

> From: radext [mailto:[email-masked]] On Behalf Of Benoit

> Claise

> Sent: Thursday, September 15, 2016 5:26 AM

> To: The IESG

> Cc: [email-masked]; [email-masked];

> [email-masked]; [email-masked]; [email-masked]

> Subject: [radext] Benoit Claise's Discuss on draft-ietf-radext-ip-port-

> radius-ext-11: (with DISCUSS and COMMENT)

> Please refer to https://www.ietf.org/iesg/statement/discuss-<https://www.ietf.org/iesg/statement/discuss-criteria.html>

> criteria.html<https://www.ietf.org/iesg/statement/discuss-criteria.html>

> First of, I applause the attempt to reuse/map/combine data models

> (IPFIX information element and RADIUS attribute).

> The first set of issues has been sent via the IPFIX experts, copied

> here for tracking purposes.

>     destinationTransportPort, but the description must say this if it

>     The Information Element localID has an inappropriate name; it is

> far

>     strings in IPFIX, see RFC 7013 section 4.2). It is underspecified

> as

>     Without a complete enumeration of possible types (n.b. 'etc.' in

> the

>     description), it is not a useful Information Element. Its purpose

>     reused for a given purpose should be reused."

In my email on 9/11, we're planning to replace "transportType"

(as a proposed new IE) by just a port type and make IP-Port-Type

TLV optional. Also, we're planning to remove IP-Port-Local-Id TLV

so that "localID" (as a proposed new IE) will not be needed.

For the remaining proposed IE, i.e., "natTransportLimit", we'll

change the name to "transportLimit" (or something similar) so that

this IE can be used for other scenario in addition to

NAT case, but also in both IPv4 and IPv6 networking. Text

will be added for the explanation.

> - 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?

In Section 3.2.5, the IP-Port-Int-IPv6-Addr TLV contains

a single sourceIPv6Address (not a list), and we intend to

reuse the semantics of the original IPFIX definition. This

is true also for IP-Port-Int-IPv4-Addr TLV defined in

Section 3.2.4, where sourceIPv4Address is also an IPFIX IE.

> 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.

>    1.  IP-Port-Limit-Info: The purpose of this attribute is to limit

> the total

>        carried in RADIUS Access-Accept, Access-Request, Accounting-

> Request or

>        CoA-Request packets.

Thank you very much for your proposal, and we'll incorporate this

into the next revision.

> Same remark for the other two points below.

Thank you and we will update the text for the following

two in the same fashion as you proposed above.

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

We're planning to make change to IP-Port-Type TLV as described

in my email on 9/11, summarized below:

   - Make this TLV optionally included in

     IP-Port-Limit-Info Attribute and

     IP-Port-Forwarding-Map Attribute

   - If not included, the port type/number is a "don't care".

   - The value of this TLV contains an single entry from       http://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.xhtml.

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

OK, we'll remove this paragraph.

> both.

This is a good catch - thanks! We will update the text so that

only one is included dependindg on IPv4 or IPv6 scenario.

>    significant identifier at the customer site, and a 2-byte IP

> external

>    packet.  It MAY also appear in an Access-Request packet to indicate

> a

>    o The IP-Port-Limit-Info Attribute MAY contain the IP-Port-Ext-IPv4-

> Addr (see Section 3.2.3).

> MUST?

Thanks! We'll take your proposal but also apply it to

others in the text.

> _______________________________________________

> radext mailing list

> [email-masked]<mailto:[email-masked]>

> https://www.ietf.org/mailman/listinfo/radext

---

From: Kathleen Moriarty <[email-masked]>
Date: Wed, 21 Sep 2016 16:17:10 -0400

Hello,

Since I haven't seen much movement on this draft and the comment below
indicates there will be another pass at this draft by the WG, is it
safe to pull this off of the next telechat and wait for the WG?  If
another WGLC is needed to address the expert review, I think that is
the right course of action.  If not and comments and discusses will be
addressed by this Friday, please let me know.

Thank you,
Kathleen

On Mon, Sep 19, 2016 at 6:11 AM, Dean cheng <[email-masked]> wrote:
>> Please refer to https://www.ietf.org/iesg/statement/discuss-
>
>> criteria.html
>
>    - If not included, the port type/number is a "don’t care".
>
>
>
>    - The value of this TLV contains an single entry from
> http://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.xhtml.
>
>
>
>>
>
>> [email-masked]

-- 

Best regards,
Kathleen

---

From: Dean cheng <[email-masked]>
Date: Thu, 22 Sep 2016 06:22:30 +0000

Hi Kathleen,

Yes we authors are in the process to
update the draft based on the comments.
Personally I think it is better to
wait for the new revision for the
final review and discussion.

Regards
Dean

> From: radext [mailto:[email-masked]] On Behalf Of Kathleen
> Moriarty
> Sent: Wednesday, September 21, 2016 1:17 PM
> To: Dean cheng; [email-masked]; [email-masked]
> Cc: Benoit Claise; [email-masked]; [email-masked]; The
> IESG; [email-masked]
> Subject: Re: [radext] Benoit Claise's Discuss on draft-ietf-radext-ip-
> port-radius-ext-11: (with DISCUSS and COMMENT)
> 
> Hello,
> 
> >> Cc: [email-masked];
> >> [email-masked];
> >
> >> https://datatracker.ietf.org/doc/draft-ietf-radext-ip-port-radius-
> ext
> >> /
> >
> >>
> >
> >>
> >
> >>
> >
> >> --------------------------------------------------------------------
> -
> >> -
> >
> >> DISCUSS:
> >
> >> --------------------------------------------------------------------
> -
> >> -
> >
> >>
> >
> >> --------------------------------------------------------------------
> -
> >> -
> >
> >> COMMENT:
> >
> >> --------------------------------------------------------------------
> -
> >> -
> >
> >>
> >
> >> -
> >
> >>
> >
> >> IP-Port-Ext-IPv4-