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

[<[e-mail]>]

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:[e-mail]]
> Envoyé : jeudi 13 juin 2019 06:03
> À : The IESG
> Cc : [e-mail]; Yong Cui; Ian Farrer;
> [e-mail]; [e-mail]; [e-mail]
> 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. 

>