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

ianfarrer@gmx.com Fri, 06 April 2018 14:59 UTC

Return-Path: <ianfarrer@gmx.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 423D612711E for <softwires@ietfa.amsl.com>; Fri, 6 Apr 2018 07:59:18 -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, FREEMAIL_FROM=0.001, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_LOW=-0.7, SPF_PASS=-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 7f95Zz4esfXy for <softwires@ietfa.amsl.com>; Fri, 6 Apr 2018 07:59:15 -0700 (PDT)
Received: from mout.gmx.net (mout.gmx.net [212.227.17.20]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 977E91205D3 for <softwires@ietf.org>; Fri, 6 Apr 2018 07:59:14 -0700 (PDT)
Received: from ians-mbp.lan ([80.159.240.8]) by mail.gmx.com (mrgmx102 [212.227.17.174]) with ESMTPSA (Nemesis) id 0LrqNe-1eMyc33l9S-013iaD; Fri, 06 Apr 2018 16:59:05 +0200
From: ianfarrer@gmx.com
Content-Type: multipart/alternative; boundary="Apple-Mail=_0085C139-93F8-47D0-9CF9-AD7C60C8AADD"
Mime-Version: 1.0 (Mac OS X Mail 11.2 \(3445.5.20\))
Date: Fri, 06 Apr 2018 16:59:05 +0200
References: <B0A79189-C9E5-4CB8-945E-C2DBB16D71F7@tsinghua.edu.cn>
To: Yong Cui <cuiyong@tsinghua.edu.cn>, Softwires WG <softwires@ietf.org>
In-Reply-To: <B0A79189-C9E5-4CB8-945E-C2DBB16D71F7@tsinghua.edu.cn>
Message-Id: <960FCC2B-2F3D-4F9B-9835-2C81768E3512@gmx.com>
X-Mailer: Apple Mail (2.3445.5.20)
X-Provags-ID: V03:K1:GGk4guFPYXgA1ZlLoyVQ8be7SHVtvg+mqoj9gr6PyweCO4zZD9H FyyNc8FAxlHFsBX6KNPnYmxDxGN2TCeU1dCEnqLBsb+hgNgkBDifusedBAgzFKUogduh0AN FGss4AdymPVOU7fX+w6YbkjAxAOHUeNHHql9iehU4jYgTpJ7ZIKNwjh1Oe2ClOVXxIpYu8d nQz1JZZY5C4zRKc2/Qczw==
X-UI-Out-Filterresults: notjunk:1;V01:K0:0UsZb5lLsLo=:VJpy+MOm6TtAt8aS1g2MwH jP3Lu0ZBMAJy4LlbxFiuht1PiVY95TqbHRJHVv8/o4EHB/UvsyVoPXhNn71q59seyJGBAIBbA u4uXaYZbP5lL6biY3T05r5h0NWwuzPhKGhDjA/Vb9/eERZoPhpKKy/5RsT6nGeJqyV0v2BR33 9dkv5MusMlyxdVjhnHeGE/M4IhH7fFq1Ev2J7JY8q/uFyc1MbzYmS7gfUTxxzX/fOHl1G1spB mRoDIBEXV3FHxynuv3BSSX8KkhTO1XvXdmMyGlNv0ce4jGHkQ9Dr+jXsEhvzM8gM2hCVZjR8G MEd7sbyxWZmPxnRJ37KBstGkr9HceWJ6CNMGTTlgb9FySxCij9U4rZTYfozg5OiithOkfhCry P+JCC0WSg8CjJlIZYI5OExuVc5lVpCRIRw8uLdb6iG4xHcMxdt3hAgFTgDTBeNPNsWvhUa1VT 0W8Q9p96ItzjM9t8ltbrgxpQ7Fqlq6D+8EUUHLRkVAqhmBXy9VVjnBlkehwbRi52v18GWr2AR nbg5F86yKHD4VaWgc1G+MsptnbbcROML3mfxxjpgvw9Djf3KW5C3P5X0hKeYNjYmJqY+3rZZT PhG+HVkh2XYm2Q8pDxKdX3WlGv/cTAgBqrLByofRArqtJFqe8TEMMkbk6aBCEawAh4ZwLtDko NHtEcsxT72+DgQOKc4s5q7m6fCVgkDmJYFQ8cFT5v2tcVYjT9Fo9fG1ZtN551DTPLopiQi3Ax 3gLqWWiWYXFYIuzi9FyQ5uhBTEiQyWk4FvwQtDp2vXL4FIM18QEbmUyjs7ib5yZAsejAbOuB/ CUt8+2NmZ/+skjktH6cp5v6XVwtZR+AxTzvbCHtVcuubKI1ogA=
Archived-At: <https://mailarchive.ietf.org/arch/msg/softwires/YeTYZt4lu5wM56o83VBU1JAr_-0>
Subject: Re: [Softwires] WGLC on draft-ietf-softwire-map-radius-14 - 2 of 2
X-BeenThere: softwires@ietf.org
X-Mailman-Version: 2.1.22
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: Fri, 06 Apr 2018 14:59:18 -0000

Hi,

Review part 2.

Cheers,
Ian

-----
4. Attributes 
4.a) 'sub options' is used. Conventionally, this is
hypenated as 'sub-options'. Please change throughout.

4.b) It would be more accurate to say:
Different combinations of sub-options are
   required for each type of S46 Container...

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.

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.

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.

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.

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.

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

4.i)
4.3.3.  S46-DMR Sub Option
s/set to all zero./set to all zeros./

4.j)
4.3.4. S46-V4V6Bind Sub Option 
s/There MUST be at most one/There MUST be exactly one/

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.

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.

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

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.

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.

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.

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

4.r)
4.9.2 & 4.9.3
s/This fiel is reserved./This field is reserved./

4.s)
Section 4.9 uses TBD1 as a placeholder for an IANA assignment. TBD1-3
are already used earlier in the document.

---------

6. IANA Considerations
6.a)
"This document requires the assignment of two new RADIUS Attribute..."
there is then a list of 3 new attributes.

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?


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


7.b)
RFCs 7599 and 8114 should also be listed.

I-D Nits output

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



> On 26. Mar 2018, at 15:43, Yong Cui <cuiyong@tsinghua.edu.cn> wrote:
> 
> Hi folks,
> 
> The authors of draft-ietf-softwire-map-radius-14 believe this document is ready for advancement.
> We would like to issue a working group last call starting from today to April 9th.
> 
> Please send your substantial comments to the list during the last call. You are also welcome to send your editorial comments directly to the authors.
> 
> Thanks for reviewing the draft.
> 
> 
> Yong Cui and Ian Farrer
> 
> 
> _______________________________________________
> Softwires mailing list
> Softwires@ietf.org
> https://www.ietf.org/mailman/listinfo/softwires