Re: [Softwires] WGLC on draft-ietf-softwire-map-radius-14 - 2 of 2

[傅瑜 <[e-mail]>]

Hi Ian,

 

Please see my reply inline:

 

Thanks for your comments

 

Yu

 

From: [e-mail] [mailto:[e-mail]] On Behalf Of [e-mail]
Sent: Friday, April 06, 2018 10:59 PM
To: Yong Cui; Softwires WG
Subject: Re: [Softwires] WGLC on draft-ietf-softwire-map-radius-14 - 2 of 2

 

Hi,

 

Review part 2.

 

Cheers,

Ian

 

-----

4. Attributes 

4.a) 'sub options' is used. Conventionally, this is

hypenated as 'sub-options'. Please change throughout.

 

[fuyu] Ok, I will change it throughout.

 

4.b) It would be more accurate to say:

Different combinations of sub-options are

   required for each type of S46 Container...

 

[fuyu] Ok, I will update this sentence.

 

4.c)

"  The RADIUS attribute

   for Dual-Stack Lite [RFC6333] is defined in [RFC6519]."

 

This is the first time in the document that DSLite is referenced.

Would it be better to put a paragraph in the Introduction saying

that this document is not concerned with DSLite/RADIUS as it's 

already covered in RFC6519.

[fuyu] I will add a paragraph in the Introduction.

 

4.c)

The S46 Container Options section describes how the containers and

options are constructed. It would read better to have this overview text

before descrbing the structure of any of the attributes/options.

 

[fuyu] OK

 

4.e)

In the description text around Fig.2 a pointer to the table in 4.7

would be helpful. It may actually be better to move the table from

section 4.7 into section 4.2 so they can be compared directly by

the reader.

[fuyu] Ok, I will move the table from section 4.7 to section 4.2.

 

4.f)

Figure 2 doesn't really make it clear that the relevant (and necessary)

sub-options (numbered 1-5) is decided by which on of the three

S46 container options is being used. This makes it confusing to understand.

 

As a suggestion, would 3 diagrams (one for each type) structured like this

be more clear showing what is necessary and what is optional?

 

                       / (Mandatory)            /1.Rule-IPv6-Prefix

                      |                        |   sub-option

                      | 1.S46-Rule ------------+ 2.Rule-IPv4-Prefix

                      |    sub-option          |   sub-option

                      |                        | 3.EA Length

 S46 MAP-E Container--+                         \  sub-option

  Option              | 2.S46-BR Sub Option

                - - - - - - - - - - - - - - - - - - - - - - - - - 

                      | (Optional)               /1.PSID-offset

                      |                         |   sub-option

                      | 3.S46-PORTPARAMS -------+ 2.PSID-len

                      |     sub-option          |   sub-option

                      |                         | 3.PSID

                       \                         \  sub-option

 

The paragraph begining 'There are three types of S46 Container Options...'

would need to be moved before this for readability.

 

[fuyu] You have missed S46-DMR Sub Option.  I think that one diagram will make the overall structure of the S46 Container Option more clearly than divided it into 3 diagrams. Do you think if I add the mandatory and optional for each sub-option in the figure 2 will be better?

 

4.g)

The paragraph begining 'There are three types of S46-Rule Sub Options...'

seems to be in the wrong section. It belongs in 4.3.1.

 

[fuyu] Yes, sorry, it is a fault.

 

4.h)

s/The S46-BR Sub Option can only be encapsulated in the MAP-E Container

   Option or the Lightweight 4over6 Container Option./

   The S46-BR Sub Option can only be encapsulated in the MAP-E or 

   Lightweight 4over6 Container Options./

 

[fuyu] I will update this sentence.

 

4.i)

4.3.3.  S46-DMR Sub Option

s/set to all zero./set to all zeros./

 

[fuyu] Done

 

4.j)

4.3.4. S46-V4V6Bind Sub Option 

s/There MUST be at most one/There MUST be exactly one/

 

[fuyu] Done

 

4.k)

4.3.5.  S46-PORTPARAMS Sub Option

Suggest that the first sentance is extended to say:

The S46-PORTPARAMS Sub Option specifies optional port set information

   that MAY be provisioned to CEs to configure sharing of an IPv4 address.

 

[fuyu] Done

 

4.l)

Throughout this section, it would be a good idea to put in a pointer

to the section of RFC7598 that the option/sub-option corresponds to.

[fuyu] OK, I will add a pointer to RFC7598.

 

4.m)

s/The Softwire46-Multicast attribute conveys the IPv6 prefixes to be

   used in [RFC8114] to synthesize IPv4-embedded IPv6 addresses./

   The Softwire46-Multicast attribute conveys the IPv6 prefixes to be

   used to synthesize IPv4-embedded IPv6 addresses as per [RFC8114]./

 

[fuyu] Done.

 

4.n)

This attribute MAY be used in Access-Request packets as a hint to the

   RADIUS server.  For example, if the BNG is pre-configured with

   Softwire46-Multicast, these prefixes MAY be inserted in the

   attribute.

 

Is this saying that the attribute could be pre-configured with a

value in the BNG's AAA client meaning that it wouldn't be requested from

AAA, but would be supplied in the DHCP message? Please can you expand

the description. I also wonder if this statement is true for any of

the other attributes described in the document.

 

[fuyu]  Some attribute like Sofwire46-Multicast and other attributes described in the document

may be pre-configured in the BNG based on the implementation by the different device company.

 

4.o)

The definitions of which RADIUS messages types the multicast 

attribute can appear in should also exist for the unicast and priority

attributes.

Update - having read section 4.10, the information is there, but

is duplicated for multicast. One common format for the information

and pointers would be cleaner.

 

[fuyu] I will make a common format for the unicast and multicast.

 

4.p)

The description text and formatting between the 4.2-4.8 options is

inconsistent and should be aligned. Personally, I think the bullet

point list of what is and isn't permitted in section 4.9 is clear

and could be usefully used in 4.2-4.9.

 

[fuyu] Sorry, because the section 4.9 is merged into the document after 12 version.

          So the description between the 4.2-4.8 and 4.9 is not the same. I will try to update

         the 4.2-4.8 to be the same as 4.9

 

4.q)

4.9.1.  ASM-Prefix64 TLV

The field in the diagram is called 'Prefix-Length', but in the

description, this field is 'Length'. This also needs fixing

in 4.9.2 and 4.9.3

 

[fuyu] Ok, I will update it.

 

4.r)

4.9.2 & 4.9.3

s/This fiel is reserved./This field is reserved./

 

[fuyu]  Done.

 

4.s)

Section 4.9 uses TBD1 as a placeholder for an IANA assignment. TBD1-3

are already used earlier in the document.

 

[fuyu] I will update it.

---------

 

6. IANA Considerations

6.a)

"This document requires the assignment of two new RADIUS Attribute..."

there is then a list of 3 new attributes.

 

[fuyu] Sorry, it is a fault, I will update it.

 

6.b)

6.1.  S46 Mechanisms and Their Identifying Option Codes

It seems that there is a requirement here for the BNG AAA client/DHCPv6

server to provide a translation between the RADIUS option code in 

the newly requested registry and the RFC8026 option 111 values (which

will be sent to the DHCPv6 client) listed in the "Option Codes Permitted in

the S46 Priority Option" IANA registry.

This process isn't described at the moment, and I wonder if this is really

the right way to solve the problem. Is this something that you have

considered?

 

 

[fuyu] I think the BNG will provide a translation between the “Softwire46-Priority Attribute TBD2” and the RFC8026 option 111 values.

 

------------

7.  Security Considerations

7.a)

"  A malicious user may use MAC address spoofing on the shared password

   that has been preconfigured on the DHCPv6 server to get unauthorized

   configuration information."

 

I'm not sure I understand the nature of this attack. Is it really the 

DHCPv6 server that you mean here, or the BNG (specifically the AAA

client function)? Is there a reference to where this attack is described

in greater deatail.

 

[fuyu] Sorry, this description may make some confusing. I think delete this paragraph will be better.

 

7.b)

RFCs 7599 and 8114 should also be listed.

 

[fuyu] I will add these two RFCs

 

I-D Nits output

 

[fuyu] I will try to figure out  the I-D nits in the next version.

 

idnits 2.15.01 
 
tmp/draft-ietf-softwire-map-radius-15.txt:
   Attempted to download rfc5822 state...
   Failure fetching the file, proceeding without it.
 
  Checking boilerplate required by RFC 5378 and the IETF Trust (see
  https://trustee.ietf.org/license-info)
  ----------------------------------------------------------------------------
 
     No issues found here.
 
  Checking nits according to https://www.ietf.org/id-info/1id-guidelines.txt:
  ----------------------------------------------------------------------------
 
     No issues found here.
 
  Checking nits according to https://www.ietf.org/id-info/checklist :
  ----------------------------------------------------------------------------
 
     No issues found here.
 
  Miscellaneous warnings:
  ----------------------------------------------------------------------------
 
  -- The document date (March 21, 2018) is 16 days in the past.  Is this
     intentional?
 
 
  Checking references for intended status: Proposed Standard
  ----------------------------------------------------------------------------
 
     (See RFCs 3967 and 4897 for information about using normative references
     to lower-maturity documents in RFCs)
 
  -- Obsolete informational reference (is this intentional?): RFC 5226
     (Obsoleted by RFC 8126)
 
 
     Summary: 0 errors (**), 0 flaws (~~), 0 warnings (==), 2 comments (--).
 
     Run idnits with the --verbose option for more detailed information about
     the items above.
--------------------------------------------------------------------------------