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

From: Benjamin Kaduk via Datatracker <[email-masked]>
Date: Wed, 12 Jun 2019 21:02:50 -0700

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 (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."?

Section 2

Please use the boilerplate text from RFC 8174 (including 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.

Section 3.1.1.1

Just to double-check my understanding: since this is an "attribute",
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.

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.

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

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?

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?

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

Please expand "ASM" on first use.

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

   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?

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

   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.

   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?
nit: Message-Authenticator has two capital letters.

   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?

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

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

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

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.

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?

---

From: <[email-masked]>
Date: Thu, 13 Jun 2019 07:21:38 +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:[email-masked]]
> Envoyé : jeudi 13 juin 2019 06:03
> À : The IESG
> Cc : [email-masked]; Yong Cui; Ian Farrer;
> [email-masked]; [email-masked]; [email-masked]
> Objet : Benjamin Kaduk's No Objection on draft-ietf-softwire-map-radius-
> 24: (with COMMENT)
> 
> 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
> 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."

> Please use the boilerplate text from RFC 8174 (including BCP 14

[Med] Added to BCP 14 mention. 

> separate options for MAP-E and MAP-T.

[Med] OK.

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

> Softwire-BR is included in each MAP-E or Lightweight-4over6 attribute.

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

> specifies" would be  fine.

[Med] OK.

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

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

> Do we want to say explicitly that the first-appearing mechanism is most

[Med] We can. 

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

> 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)"

> server, or CE/AAA server?

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

> nit: maybe "DHCPv6 Reply" to match the "DHCPv6 Request" in step (5)?

[Med] Fixed. 

> statement refers to if there was explicit mention in the prior

[Med] Will double check this one. 

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

> how the subscriber relates to the other entities in play?

[Med] Changed to "CE".

> (I assume this is done "out of band" from the perspective of this

[Med] Yes. 

> appropriate), as well as potential other information, would be in order.

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

> a bit so that these two points tie together more clearly.

[Med] Any suggestion to better convey this message? 

> RADIUS client/server (that can use IPsec/TLS as appropriate).

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

> trusted roles, there's not in the general case more attack surface to

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

> decide whether or not they are permitted in the Softwire46-Priority

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

---

From: Benjamin Kaduk <[email-masked]>
Date: Thu, 13 Jun 2019 18:58:06 -0500

>    operational purposes (e.g., regulatory).

So it is; thanks for pointing it out.  (I guess I would have expected to
see something earlier in the document, but this is up to your discretion.)

> > extent, CoA-Request packets).

What about CoA-Request, though?

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

Thanks

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

Thanks!

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

Okay

> [Med] Any suggestion to better convey this message? 

I'd just add another sentence at the end of the second paragraph: "These
well-established properties of the RADIUS protocol place some limitations
on how it can safely be used, since there is some inherent requirement to
trust the counterparty to not misbehave." and start the third paragraph
with "Accordingly, [...]".

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

It's your call.

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

It's only specific to the attributes in this document in that it's the
information content of these attributes getting a slightly different
exposure.  I think I recently suggested on a different document a rewording
like "does not introduce any security issues inherently different from
those already identified in [other documents]" for a different document, if
you were interested in something like that.

> [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…

I think my point here is that I'm confused whether this is a registry of
freshly allocated values or a subset of values allocated elsewhere (and
thus reusing the numerical value from the other registry).  It sounds like
it's the former, though, so it's just the "Permitted in" in the title that
confuses me (vs. "for" or similar) ... but looking at the IANA page this
wording seems to be pretty widespread already, so I don't see any grounds
to change this one just on my account.

Thanks,

Ben

---

From: <[email-masked]>
Date: Fri, 14 Jun 2019 12:15:56 +0000

Hi Ben, 

Please see inline.

Cheers,
Med

> De : Benjamin Kaduk [mailto:[email-masked]]
> Envoyé : vendredi 14 juin 2019 01:58
> À : BOUCADAIR Mohamed TGI/OLN
> Cc : The IESG; [email-masked]; Yong Cui; Ian
> Farrer; [email-masked]; [email-masked]
> Objet : Re: Benjamin Kaduk's No Objection on draft-ietf-softwire-map-
> radius-24: (with COMMENT)
> 

[Med] Added this NEW text: 

 A configuration change (e.g., BR address) may result in an exchange
 of CoA-Requests between the BNG and the AAA server as shown in
 Figure 3.  Concretely, when the BNG receives a CoA-Request message
 containing Softwire46 attributes, it sends a DHCPv6 Reconfigure
 message to the appropriate CE to inform that CE that an updated
 configuration is available.  Upon receipt of such message, the CE
 sends a DHCPv6 Renew or Information-Request in order to receive the
 updated Softwire46 configuration.  In deployments where the BNG
 embeds a DHCPv6 relay, CoA-Requests can be used following the
 procedure specified in [RFC6977].

> > [Med] This is a sub-attribute that appears under Softwire46-
> Configuration. This is discussed in the introduction text of Section
> 3.1.1.
> >
> > [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
> > >
> > [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.
> 
> Okay
> 

[Med] Works for me. Updated the text accordingly. 

> you were interested in something like that.

[Med] Ok, thanks.

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

[Med] Thank you for clarifying. I maintained the text as it is.

---

From: Benjamin Kaduk <[email-masked]>
Date: Fri, 14 Jun 2019 18:54:22 -0500

Thanks!

-Ben