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

From: Kathleen Moriarty <[email-masked]>
Date: Wed, 2 Nov 2016 22:59:47 -0400

Benoit,

Can you take a look at the response below to see if you agree your discuss
and comments have been addressed?

Thank you,
Kathleen

On Wed, Oct 5, 2016 at 2:15 AM, <[email-masked]> wrote:

> Hi Benoit,
>
> Please see inline how the updated version takes into account your review.
>
> Cheers,
> Med
>
> > -----Message d'origine-----
> > De : Benoit Claise [mailto:[email-masked]]
> > Envoyé : jeudi 15 septembre 2016 14:26
> > À : The IESG
> > Cc : [email-masked]; [email-masked];
> > [email-masked]; MORAND Lionel IMT/OLN; [email-masked];
> > [email-masked]
> > Objet : Benoit Claise's Discuss on draft-ietf-radext-ip-port-radius-ext-
> > 11: (with DISCUSS and COMMENT)
> >
> > 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).
> >
> [Med] Thank you.
>
> > 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".)
> >
>
> [Med] The description of the attribute was updated. Basically:
> * Clarify the text
> * We don't have any more sub-registries because we are relying on the IANA
> Protocol Numbers
>
>
> >     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.
>
> [Med] The attribute is renamed to "sourceTransportPortsLimit" + text
> updated to clarify the use of the attribute.
>
> > Is
> >     the field IPv4 specific, or is IPv6 supported as well? (If not, why
> >     not?)
> >
>
> [Med] It is valid for both IPv4 and IPv6. The text was updated to clarify
> that point.
>
> >     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."
> >
>
> [Med] We made the following changes:
> * We changed the Value field to be still a "string" but per definition of
> ietf-draft-radext-datatype and not to propose as a new IPFIX IE.
> * We added a new text to clarify the intent of the attribute:
>
>    The primary issue addressed by this TLV is that there are CGN
>    deployments that do not distinguish internal hosts by their internal
>    IP address alone, but use further identifiers for unique subscriber
>    identification.  For example, this is the case if a CGN supports
>    overlapping private or shared IP address spaces (refer to [RFC1918]
>    and [RFC6598]) for internal hosts of different subscribers.  In such
>    cases, different internal hosts are identified and mapped at the CGN
>    by their IP address and/or another identifier, for example, the
>    identifier of a tunnel between the CGN and the subscriber.  In these
>    scenarios (and similar ones), the internal IP address is not
>    sufficient to demultiplex connections from internal hosts.  An
>    additional identifier needs to be present in the IP-Port-Range
>    Attribute and IP-Port-Forwarding-Mapping Attribute in order to
>    uniquely identify an internal host.  The IP-Port-Local-Id TLV is used
>    to carry this identifier.
>
> > 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]?
>
> [Med] The intent is to reuse the semantic of individual IEs. The rationale
> basically was that IPFIX offers a rich IE registry that can be reused in
> our context.
>
> > 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.
>
> [Med] localId is not defined as an IPFIX attribute.
>
> >
> >
> > ----------------------------------------------------------------------
> > 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.
>
> [Med] The initial wording is correct. It is aligned with the text in
> Section 3.1.1.
>
> >
> > 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.
>
> [Med] We updated the text so that any transport protocol can be used. The
> new text says the following:
>
>    The format of IP-Port-Type TLV is shown in Figure 4.  This attribute
>    carries the IP transport protocol number defined by IANA (refer to
>    [ProtocolNumbers])
>
> >
> > - 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.
>
> [Med] OK.
>
> >
> > - 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.
>
> [Med] Fair. Changed to:
>
>    o  If the internal realm is with IPv4 address family, the IP-Port-
>       Forwarding-Map Attribute MUST contain the IP-Port-Int-IPv4-Addr
>       TLV (see Section 3.2.4); if the internal realm is with IPv6
>       address family, the IP-Port-Forwarding-Map Attribute MUST contain
>       the IP-Port-Int-IPv6-Addr TLV (see Section 3.2.5).
>
> >
> > - 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?
>
> [Med] Fixed. The NEW text is provided below:
>
> NEW:
>
>    The attribute contains a 2-byte IP internal port number and a 2-byte
>    IP external port number.  The internal port number is associated with
>    an internal IPv4 or IPv6 address that MUST always be included.  The
>    external port number is associated with a specific external IPv4
>    address if included, but otherwise with all external IPv4 addresses
>    for the end user.
>
> > 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).
>
> [Med] Fixed.
>
> >
> > 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?
>
> [Med] Yes. Fixed in the new version.
>
> _______________________________________________
> radext mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/radext
>

-- 

Best regards,
Kathleen