Re: [Softwires] Benjamin Kaduk's No Objection on draft-ietf-softwire-map-radius-24: (with COMMENT)

<mohamed.boucadair@orange.com> Thu, 13 June 2019 07:21 UTC

Return-Path: <mohamed.boucadair@orange.com>
X-Original-To: softwires@ietfa.amsl.com
Delivered-To: softwires@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 5A95312023F; Thu, 13 Jun 2019 00:21:44 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.599
X-Spam-Level:
X-Spam-Status: No, score=-2.599 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, RCVD_IN_DNSWL_LOW=-0.7, SPF_PASS=-0.001, UNPARSEABLE_RELAY=0.001, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id gJoatbJcSguR; Thu, 13 Jun 2019 00:21:41 -0700 (PDT)
Received: from orange.com (mta136.mail.business.static.orange.com [80.12.70.36]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id F373A120259; Thu, 13 Jun 2019 00:21:40 -0700 (PDT)
Received: from opfednr03.francetelecom.fr (unknown [xx.xx.xx.67]) by opfednr24.francetelecom.fr (ESMTP service) with ESMTP id 45PZtC1PqQz21Df; Thu, 13 Jun 2019 09:21:39 +0200 (CEST)
Received: from Exchangemail-eme6.itn.ftgroup (unknown [xx.xx.13.23]) by opfednr03.francetelecom.fr (ESMTP service) with ESMTP id 45PZtC0rf3zDq7F; Thu, 13 Jun 2019 09:21:39 +0200 (CEST)
Received: from OPEXCAUBMA2.corporate.adroot.infra.ftgroup ([fe80::e878:bd0:c89e:5b42]) by OPEXCAUBM41.corporate.adroot.infra.ftgroup ([fe80::857d:4f67:b0a7:10d7%21]) with mapi id 14.03.0439.000; Thu, 13 Jun 2019 09:21:38 +0200
From: <mohamed.boucadair@orange.com>
To: Benjamin Kaduk <kaduk@mit.edu>, The IESG <iesg@ietf.org>
CC: "draft-ietf-softwire-map-radius@ietf.org" <draft-ietf-softwire-map-radius@ietf.org>, Yong Cui <cuiyong@tsinghua.edu.cn>, Ian Farrer <ianfarrer@gmx.com>, "softwire-chairs@ietf.org" <softwire-chairs@ietf.org>, "softwires@ietf.org" <softwires@ietf.org>
Thread-Topic: Benjamin Kaduk's No Objection on draft-ietf-softwire-map-radius-24: (with COMMENT)
Thread-Index: AQHVIZzgCO+SBDQcJ02bJqJL3ohKiaaZF1sg
Date: Thu, 13 Jun 2019 07:21:38 +0000
Message-ID: <787AE7BB302AE849A7480A190F8B93302EAA602B@OPEXCAUBMA2.corporate.adroot.infra.ftgroup>
References: <156039857091.27144.11177820030867633181.idtracker@ietfa.amsl.com>
In-Reply-To: <156039857091.27144.11177820030867633181.idtracker@ietfa.amsl.com>
Accept-Language: fr-FR, en-US
Content-Language: fr-FR
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
x-originating-ip: [10.114.13.245]
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: base64
MIME-Version: 1.0
Archived-At: <https://mailarchive.ietf.org/arch/msg/softwires/GHzQsGMTYqXQaYCc24mVE2g5m6Y>
Subject: Re: [Softwires] Benjamin Kaduk's No Objection on draft-ietf-softwire-map-radius-24: (with COMMENT)
X-BeenThere: softwires@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: softwires wg discussion list <softwires.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/softwires>, <mailto:softwires-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/softwires/>
List-Post: <mailto:softwires@ietf.org>
List-Help: <mailto:softwires-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/softwires>, <mailto:softwires-request@ietf.org?subject=subscribe>
X-List-Received-Date: Thu, 13 Jun 2019 07:21:45 -0000

Re-,

Thank you for the detailed review. Much appreciated, as usual!

Please see inline.

Cheers,
Med

> -----Message d'origine-----
> De : Benjamin Kaduk via Datatracker [mailto:noreply@ietf.org]
> Envoyé : jeudi 13 juin 2019 06:03
> À : The IESG
> Cc : draft-ietf-softwire-map-radius@ietf.org; Yong Cui; Ian Farrer;
> softwire-chairs@ietf.org; ianfarrer@gmx.com; softwires@ietf.org
> Objet : Benjamin Kaduk's No Objection on draft-ietf-softwire-map-radius-
> 24: (with COMMENT)
> 
> Benjamin Kaduk has entered the following ballot position for
> draft-ietf-softwire-map-radius-24: No Objection
> 
> 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-softwire-map-radius/
> 
> 
> 
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
> 
> It might be worth a brief note somewhere about why attributes of this
> sort can be included in Accounting-Request packets

[Med] This is already covered by this text: 

   In some deployments, the DHCP server may use the Accounting-Request
   to report to a AAA server the softwire configuration returned to a
   requesting host.  It is the responsibility of the DHCP server to
   ensure the consistency of the configuration provided to requesting
   hosts.  Reported data to a AAA server may be required for various
   operational purposes (e.g., regulatory).


 (and, to a lesser
> extent, CoA-Request packets).
> 
> Section 1
> 
>    Since IPv4-in-IPv6 softwire configuration information is stored in an
>    AAA server, and user configuration information is mainly transmitted
>    through DHCPv6 between the BNGs and Customer Premises Equipment (CEs,
>    a.k.a., CPE), new RADIUS attributes are needed to propagate the
>    information from the AAA servers to BNGs.
> 
> nit: maybe "from the AAA servers to BNGs so that they can be propagated
> to CPE over the existing DHCPv6 options."?
> 

[Med] Updated as follows:

"from the AAA servers to BNGs so that they can be provided to CEs using the existing DHCPv6 options."

> Section 2
> 
> Please use the boilerplate text from RFC 8174 (including BCP 14
> mention).
> 

[Med] Added to BCP 14 mention. 

> Section 3.1
> 
>       The Softwire46-Configuration Attribute conveys the configuration
>       information for MAP-E, MAP-T, or Lightweight 4over6.  The BNG
>       SHALL use the configuration information returned in the RADIUS
>       attribute to populate the DHCPv6 Softwire46 Container Option
>       defined in Section 5 of [RFC7598].
> 
> nit: "Option(s)" seems more appropriate, since that section defines
> separate options for MAP-E and MAP-T.

[Med] OK.

> 
> Section 3.1.1.1
> 
> Just to double-check my understanding: since this is an "attribute",

[Med] This is a sub-attribute that appears under Softwire46-Configuration. This is discussed in the introduction text of Section 3.1.1.

> it's a top-level container with the 'type' value bearing universal
> semantics for all RADIUS packets, as opposed to a "tlv" that appears
> within a given attribute and whose 'type' values only have meaning in
> the context of that attribute?  But this interpretation doesn't hold up
> for (e.g.) Section 3.3.3, which defines an "attribute" but uses a
> "TLV-Type" that is in the range of values scoped only to the structures
> defined in this document.
> 
> Section 3.1.3.2
> 
>    There MUST be at least one Softwire46-BR included in each
>    Softwire46-MAP-E or Softwire46-Lightweight-4over6.
> 
> This seems to be in conflict with Table 2, which says that exactly one
> Softwire-BR is included in each MAP-E or Lightweight-4over6 attribute.

[Med] Good catch. Updated the table: s/1/1+.

> 
> Section 3.1.6.1
> 
>                          This field that specifies the
>          numeric value for the Softwire46 algorithm's excluded
>          port range/offset bits (a bits), as per Section 5.1
>          of [RFC7597]. Allowed values are between 0 and 15.
> 
> nit: "This field that specifies" is redundant; just "This field
> specifies" would be  fine.

[Med] OK.

> 
> I'm also not sure I understand the range being between 0 and 15, when we
> previously talk about this being an 8-bit value ("right justified").

[Med] We used to have a 8-bit field that we then updated to 32-bit to be aligned with radext reco. Fixed.

> 
> Section 3.1.6.3
> 
> This format seems needlessly confusing.  We encode as an integer (32-bit
> unsigned), but then state that this 32-bit integer contains a 16-bit
> value, right justified.  And then we only interpret the f irst 'k' bits
> on the left of the 16-bit value.  Isn't there a simpler way to encode a
> 'k' bit value in a 32-bit field?

[Med] We used to have a 16-bit field but changed it to 32-bit to be aligned with RFC6158. The proposed approach is straightforward for extracting the psid bits.  

> 
> Section 3.2
> 
>           Softwire46 mechanisms are prioritized in the appearance order
>           of the in the Softwire46-Priority Attribute.
> 
> Do we want to say explicitly that the first-appearing mechanism is most
> preferred?

[Med] We can. 

> 
> Section 3.3.1
> 
>     TLV-Length
>        16 octets. The length of asm-prefix64 must be to 96 [RFC8115].
> 
> nit(?): I can't parse the grammar here -- what does "must be to" mean?
> (Same question for following section.)

[Med] Updated to: "The length of asm-prefix64 must be /96 [RFC8115]."

> 
> Please expand "ASM" on first use.

[Med] Done.

> 
> Section 4
> 
>    1.  The CE creates a DHCPv6 Solicit message.  For unicast softwire
>        configuration, the message includes an OPTION_REQUEST_OPTION (6)
>        with the Softwire46 Container option codes as defined in
>        [RFC7598].  [...]
> 
> nit: with all of them (the Softwire46 Container option codes)?

[Med] Not all of them because some mechanisms may not be supported. I updated the text to:

" Softwire46 Container option code(s)"

> 
>    2.  On receipt of the Solicit message, the BNG constructs a RADIUS
>        Access-Request message containing a User-Name Attribute (1)
>        (containing either a CE MAC address, interface-id or both), a
>        User-Password Attribute (2) (with a pre-configured shared
>        password as defined in [RFC2865].  The Softwire46-Configuration
>        Attribute and/or Softwire46-Multicast Attribute are also included
>        (as requested by the client).  The resulting message is sent to
>        the AAA server.
> 
> Perhaps clarify whether the shared password is shared between BNG/AAA
> server, or CE/AAA server?

[Med] Updated with an explicit mention of CE-AAA server.

> 
>    6.  The BNG sends a Reply message to the client containing the
>        softwire container options enumerated in the ORO.
> 
> nit: maybe "DHCPv6 Reply" to match the "DHCPv6 Request" in step (5)?

[Med] Fixed. 

> 
>    The authorization operation could also be done independently, after
>    the authentication process.  In this case, steps 1-5 are completed as
>    above, then the following steps are performed:
> 
> nit: "authorization" or "authorize" do not appear in the previous
> procedure anywhere; it would be rhetorically more clear what this
> statement refers to if there was explicit mention in the prior
> procedure.

[Med] Will double check this one. 

> 
>    o  In both the configuration message flows described above the
>       Message-authenticator (type 80) [RFC2869] SHOULD be used to
>       protect both Access-Request and Access-Accept messages.
> 
> Why is this SHOULD and not MUST?  Maybe an example of when it would not
> be needed would be helpful?

[Med] The set of requirements are deployment-oriented. I already changed my local copy to avoid the use of normative language. This applies also for the following comments. 

> nit: Message-Authenticator has two capital letters.
[Med] Fixed. 

> 
>    o  As specified in [RFC8415], Section 18.2.5, "Creation and
>       Transmission of Rebind Messages", if the DHCPv6 server to which
>       the DHCPv6 Renew message was sent at time T1 has not responded by
>       time T2, the CE (DHCPv6 client) SHOULD enter the Rebind state and
>       attempt to contact any available server.  In this situation, a
> 
> This seems to just be restating the requiremnets from RFC 8415, in which
> case the normative language is not needed...
> 
>       secondary BNG receiving the DHCPv6 message MUST initiate a new
>       Access-Request message towards the AAA server.  The secondary BNG
>       includes the Softwire46-Configuration Attribute in this Access-
>       Request message.
> 
> ... but this MUST does need to use normative language (as it currently
> does).
> 
>    o  For Lightweight 4over6, the subscriber's binding state needs to be
>       synchronized between the clients and the Lightweight AFTR
>       (lwAFTR)/BR.  This can be achieved in two ways: static pre-
> 
> We have not previously talked about the "subscriber"; is there some
> realignment of terminology or earlier definition that would help clarify
> how the subscriber relates to the other entities in play?
> 

[Med] Changed to "CE".

>       configuration of the bindings on both the AAA server and lwAFTR,
>       or on-demand whereby the AAA server updates the lwAFTR with the
>       subscriber's binding state as it is created or deleted.
> 
> (I assume this is done "out of band" from the perspective of this
> document.)

[Med] Yes. 

> 
> Section 6
> 
> As the secdir reviewer notes, there are a lot of references here.
> That's usually good, avoiding duplication of content/etc., but this text
> seems to claim that there is discussion of known security
> vulnerabilities in RADIUS discussed in RFCs 2607, 2865, and 2869, and a
> quick review of those documents does not find extensive discussion of
> such vulnerabilities (and not much in the Security Considerations
> section where one might expect to find it).  I think that just those
> references may not provide a comprehensive picture of the state of
> RADIUS (in)security, and so some additional exposition to tie together
> what those documents are saying (including Section references as
> appropriate), as well as potential other information, would be in order.

[Med] Will update the text with the exact section references. 

> I note that there was some fairly recent discussion of the general state
> of RADIUS security in the IESG processing of
> draft-ietf-radext-coa-proxy, along the lines of "it basically boils down
> to you have to trust your neighbor/contractual arrangement".  I do *not*
> think that the three RFCs from 2000 convey that sentiment well, and so
> the following paragraph's statement about targetting deployments with a
> trust relationship is quite important.  Perhaps we can tweak the writing
> a bit so that these two points tie together more clearly.

[Med] Any suggestion to better convey this message? 

> 
> It may be worth mentioning earlier (in the Introduction?) that this
> document only targets deployments with a trusted relationship between
> RADIUS client/server (that can use IPsec/TLS as appropriate).

[Med] We can but IMHO this will be redundant. 

> 
> I am also not entirely sure about the "does not introduce any security
> issue other than the ones already identified in RADIUS documents",
> though admittedly the additional exposure seems quite minimal.
> Specifcally, without these RADIUS attributes, DHCPv6 negotiation of
> softwires includes in-band protocol flows conveying softwire
> configuration information between DHCP client and BNG, but now that
> information flows additionally to the AAA server.  The additional
> exposure seems minimal, though, since the necessary softwire
> configuration information inherently needs to be in *some* central
> location, so we're just exchanging one way of configuring the BNG ("out
> of band") for another (RADIUS).  The most notable difference would seem
> to be that now we have to trust the AAA server to not leak the softwire
> configuration details (as opposed to whatever central configuration
> server would otherwise be used), but since both are rather inherently
> trusted roles, there's not in the general case more attack surface to
> worry about.

[Med] How is this specific to the attributes defined in the document? 

> 
> Section 7.3
> 
> If the registry is to be "Option Codes Permitted in the
> Softwire46-Priority Attribute", that seems to imply that there are some
> option codes *not* permitted.  Where are those option codes enumerated?
> (I would have guessed at
> https://www.iana.org/assignments/dhcpv6-parameters/dhcpv6-
> parameters.xhtml#dhcpv6-parameters-2
> , but the values don't seem to match up.)  Do we need to add a note
> somewhere about future allocations of such option codes needing to
> decide whether or not they are permitted in the Softwire46-Priority
> Attritube?

[Med] New (attribute) codes that can be listed will naturally include a IANA request to be added to the registry. I'm not sure a note is needed so that each new attribute has to declare whether it can be listed or not. 

>