Hi We’ve just posted -15 of this draft incorporating the IESG evaluation DISCUSSes/comments and the Secdir/Genart review comments. Many thanks for those.
Thanks, Ian Below is the log of DISCUSS/Comments received during the review and the changes that have been incorporated to address them. ------- DISCUSS Points: Alissa Cooper (Supported by Adam Roach, Ben Campbell, Warren Kumari and Alexey Melnikov) The security considerations do not seem to follow the YANG security guidelines https://trac.ietf.org/trac/ops/wiki/yang-security-guidelines <https://trac.ietf.org/trac/ops/wiki/yang-security-guidelines>. They do not list the specific writeable and readable subtrees/nodes and why they are sensitive. The fact that all the writeable nodes could "negatively affect network operations" seems trivially true for most writeable YANG module nodes. In the case of these modules, there seem to be multiple different threats relevant to different nodes, including exposure of data about individual users/customers, potential for disruption of the operations of the BR or CE, etc. The Security Considerations section has been rewritten expressly describing the threats associated to the relevant nodes. -- Benjamin Kaduk (supported by Adam Roach, Benjamin Campbell and Alexey Melnikov) Discuss 1 This document has 7 listed authors/editors. Since, per RFC 7322, documents listing more than five authors are unusaul, and seven is greater than five, let's talk about the author count. The document follow’s Adam Roach’s suggestion (in support of Benjamin’s DISCUSS) above, and now lists 2 editors and a Contributors section. — Benjamin Kaduk (Supported by Warren Kumari) Discuss 2 The binding-table-versioning container's "version" leaf is of type uint64 but the in-module description indicates that it is a timestamp. If it is actually supposed to be a timestamp, then the units and zero time need to be specified, but it seems more likely that this should just be described as an abstract version number, if I understand the prose text about this container correctly. Description text changed to match the leaf’s type: Version number for this binding table. ------------------------- Comments: Alissa Cooper Comment 1 I think "external party" would make more sense than "abuse party.” Sentence reworded (in both instances) to: When a party who is the victim of abuse presents an external IP address/port, —------------- Benjamin Kaduk Comment 1 Please expand CE on first usage. The abstract gives the full name and the abbreviation is explained in the Introduction. Comment 2 Section 4.1 It feels a little strange to put something as generic as /if:interfaces/if:interface/if:statistics:sent-ipv4-packets in the ietf-softwire-ce module. Are these counters likely to be useful for other (non-softwire?) tunneling techniques? The counters can be re-used bu other modules. No change is needed. --- Comment 3 Section 5.2 o softwire-num-max: used to set the maximum number of softwire binding rules that can be created on the lw4o6 element simultaneously. This paramter must not be set to zero because this is equivalent to disabling the BR instance. This seems to leave it ambiguous whether a server should reject an attempt to set it to zero, or accept it but diable the BR instance. The text is clear. range "1..max". No change is needed. --- Comment 4 Section 7 leaf enable-hairpinning { type boolean; default "true"; description "Enables/disables support for locally forwarding (hairpinning) traffic between two CEs."; reference "Section 6.2 of RFC7596"; Is a global toggle sufficient or would there be cases where more fine-grained control would be needed? A+P is designed to reduce as much as possible the per-subscriber state at the network/BR. Requiring fine-grained control would require some extra state to be maintained, which is not desired. Having the general parameter is sufficient. --- Comment 5 Section 8 container algo-versioning { [...] leaf date { type yang:date-and-time; description "Timestamp when the algorithm instance was activated. An algorithm instance may be provided with mapping rules that may change in time (for example, increase the size of the port set). When an abuse party presents an external IP address/port, the version of the algorithm is important because depending on the version, a distinct customer may be identified. nit: "abuse party" is probably not a term that everyone is familiar with. (similarly in br-instances) Sentence reworded to: When a party who is the victim of abuse presents an external IP address/port, --- Comment 6 Section 9 Is there any possibility of a situation where the invalid-/added/modified-entry notifications cause a substantial amount of notification traffic (i.e., a DoS level of traffic)? Added some text among those lines: This is in theory possible if the BR is under the control of a non-authorized/misbehaving entity. The DDoS can be softened by defining a notification interval, but given that this interval parameter can be disabled or set to a low value by the misbehaving entity, the same problem will be observed. ——————————— Ignas Bagdonas Comment 1 Is the derived-from() complexity for statistics counters really required given that the module itself is for softwires, and any tunnel using it would be covered? reply: Because these counters are applicable to some interface/tunnel schemes, the augment must be anyway subject to a "when derived-from()" clause (as a function of the tunnel type). The counters are designed so that other modules can reuse them, if needed. This can be done by using this statement: uses softwire-common:traffic-stat; ——————————————— Suresh Krishnan Comment 1 I would have thought putting in a prioritization mechanism (like RFC8026 does) for ordering the different mechanisms would have been useful in this YANG module in order to configure the CE. Was this something that was considered? responses: 1) We didn't consider 8026-like prioritization at the CE because that feature is not specific to A+P but applies to a bunch of IPv4 service continuity mechanisms including DS-Lite and 464xlat. That feature has to be defined, if needed, in a separate module. 2) Mirja Kühlewind Comment 1 Sec 4.2: "softwire-path-mru: optionally used to set the maximum IPv6 softwire packet size that can be received, including the encapsulation/translation overhead. Needed if the softwire implementation is unable to correctly calculate the correct IPv4 Maximum Receive Unit (MRU) size automatically [RFC4213]." I guess this should both be IPv6…? IPv4 is correct here. Changed to the wording below to clarify: Needed if the softwire implementation is unable to correctly calculate the correct IPv4 payload Maximum Receive Unit (MRU) size automatically (see Section 3.2 of [RFC4213)]. --- Comment 2 Why does the description of "rcvd-ipv4-bytes" say "IPv4 traffic received for processing, in bytes"..? Does the "for processing" have any special meaning or why is it only phrased like this for that one entry? No special meaning is intended. Reworded with: IPv4 traffic received, in bytes. Comment 3 Also the description for "sent-ipv4-bytes" and "sent-ipv4-packets" could be unified. Reworded with: Number of IPv4 packets received. —————— Philip Hallam-Baker (Secdir Review) Comment 1 The document describes a schema and has appropriately identified the read/write security concerns arising from it. One issue that I thing could be usefully spelled out is that the use of automated tools to decode structures of this type is not merely a programming convenience. Attempts to parse length delimited objects nested in length delimited structures using handwritten code is error prone and has led to introduction of numerous buffer overrun vulnerabilities. Added the following text to the Security considerations given in [RFC7950] are also applicable here. — Roni Even (Genart review) Comment 1 In section 2 " The adopts the Network Management Datastore Architecture (NMDA)" there is a word missing? Reworded to: The YANG modules in this document adopt the Network Management Datastore Architecture (NMDA) [RFC8342]. The meanings of the symbols used in tree diagrams are defined in [RFC8340].
_______________________________________________ Softwires mailing list Softwires@ietf.org https://www.ietf.org/mailman/listinfo/softwires
