Hi Ian, Many thanks for providing such a thorough review of the document. We really appreciate your comments. Please see inline(TX>).
There are some issues in the draft which need editing, we will update a new version soon, with these bits cleaned up. Thank you and best regards, Tianxiang 2016-11-15 17:23 GMT+08:00 Ian Farrer <[email protected]>: > Hi, > > Here’s a review of the updated version of the draft. > > Thanks, > Ian > > > General Comments: > Please do language review throughout. > > Throughout the document, there are multiple places where the client is > described as sending a ORO containing the ’S46 container options’, or similar > (e.g. Figure 1, description text under figure 1). This should be 'S46 > container option code’. Please revise throughout. > > TX>There are indeed a number of language problems, we will do a thorough language review of the document. > > Figure 1 > 4. The DHCPv6 advertise message will have an S46 container option as well. > > General comment on figure 1 text > There is a lot of requirements language here that I think is unnecessary. The > purpose of the text is to describe how the message flow works and this can be > done without the need for RFC2119 language. The use of SHOULD, particularly > in point 2 leaves questions like: under what conditions is it acceptable for > the DHCPv6 server NOT to fill in the shared password, and what are the > expected effects of this? > > TX>Agree, we will remove some of the requirements language, and make it more concise. I think some wordings which are necessary should remain, e.g. the MUST in point 3. > Figure 1 Text > 4. Suggest "After receiving the Access-Accept message with the corresponding > Attribute, the BNG SHOULD respond the user an Advertisement message.” is > replaced with: "After receiving the Access-Accept message with the > corresponding Attribute, the BNG SHOULD respond to the DHCPv6 client with an > Advertisement message.” > > 6. The inclusion of DHCPv4o6 as an option here complicates things as it has > it’s own set of message flows which are separate to this process. I’m not > sure if this is also the case for PCP based provisioning. It may be best to > limit the scope of this to the S46 options described in RFC7598 for clarity. > > TX>It's true, PCP also uses different message flows. We will limit the scope of this to the S46 options described in RFC7598, and remove descriptions of DHCP4o6 and PCP. > > Section 4.2 > As draft-ietf-softwire-unified-cpe-08 is just about to be published, and the > draft allows for multiple S46 attributes, it would make sense to also have an > attribute for OPTION_S46_PRIORITY so that operator prioritisation for > multiple mechanisms can be supplied. This may need an IANA registry for the > allowed values for this new RADIUS attribute with the same contents as Option > Codes Permitted in the S46 Priority Option. > A pointer and some text for RFC6519 (DSLite attribute) could be included will > also be needed. > > TX>I think the solution here would be to consider OPTION_S46_PRIORITY as a type of S46 Container Option in section 4.2, and define the s46-option-code field as a Sub Option in section 4.3. > > Across all of the following attribute definitions, it might be helpful to > ‘borrow’ the definitions in RFC7598 for consistency (e.g. which parts are > variable, where 0 padding is used). > 4.3.2 > The Sublen is defined as being 18. Shouldn’t this be variable (but a multiple > of 16) if it is a list of addresses? > > BR-ipv6-address - This describes a single address, but the diagram shows a > list of addresses. > > 4.3.3 > The Sublen is given as 8. Shouldn’t this be variable (1 for prefix len, > variable for the prefix). > > TX>Agree, this should be varable length. > dmr-ipv6-prefix-len > (Note on this one, the RFC7598 text is wrong. It should allow values of 0-96 > as described in RFC7599 Section 5.1. I have raised an errata on this) > > TX>Thanks for pointing that out, we will use 0-96 here. > dmr-ipv6-prefix > This states it is a 32 bit filed, but it’s meant to contain an IPv6 prefix. > > TX> Agree, this should be a variable field, which could be an IPv6 prefix or address of the BR. We will do a thorough edit of section 4 to keep the description unified with RFC7598. > 4.3.4 > There should only be a single > > TX>There is a typo here, it was meant to say "There MUST be at most one S46-V4V6Bind Sub Option included in each Lighweight4over6 Container Option". This correlates to the phrase "There MUST be at most one OPTION_S46_V4V6BIND option" in section 5.3 of RFC 7598. > > The Sublen should be variable. > > TX>Agree. > 4.3.5 > Is 6 the correct SubLen here (I don’t know if the SubType and SubLen fields > should be counted in the SubLen)? > > TX>Yes, the SubType and SubLen fields should be counted. We will also make sure it's correct for the other options defined. > > Section 7. > The word ‘proofing’ should probably be replaced with ‘spoofing’ > > Can the dictionary attack on the shared password be mitigated? As this > communication takes place between the DHCPv6 server and the RADIUS server, > wouldn’t the use of ACLs preventing users being able to send requests > directly to the RADIUS server prevent this type of attack? > > TX> I think the dictionary attack here is used by the attacker on the BNG server, to get the password which is pre-configured on the BNG. The attacker could then masquerading as a legitimate user to access configuration information by requesting the BNG server, the BNG server would then communicate with the AAA server. Would ACL prevent this?
_______________________________________________ Softwires mailing list [email protected] https://www.ietf.org/mailman/listinfo/softwires
