Re: [Softwires] Benjamin Kaduk's Discuss on draft-ietf-softwire-yang-14: (with DISCUSS and COMMENT)

[<[e-mail]>]

Hi Ben, 

Great!

I added this NEW text to cover your DDoS comment:

   o  Instructing the BR to install entries which in turn will induce a
      DDoS attack by means of the notifications generated by the BR.
      This 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.

Thank you for the review. 

Cheers,
Med

> -----Message d'origine-----
> De : Benjamin Kaduk [mailto:[e-mail]]
> Envoyé : jeudi 10 janvier 2019 20:07
> À : BOUCADAIR Mohamed TGI/OLN
> Cc : The IESG; [e-mail]; Sheng Jiang; softwire-
> [e-mail]; [e-mail]
> Objet : Re: Benjamin Kaduk's Discuss on draft-ietf-softwire-yang-14: (with
> DISCUSS and COMMENT)
> 
> Hi Med,
> 
> On Thu, Jan 10, 2019 at 02:08:02PM +0000, [e-mail] wrote:
> > Hi Benjamin,
> >
> > Thank you for the review.
> >
> > Please see inline.
> >
> > Cheers,
> > Med
> >
> > > -----Message d'origine-----
> > > De : Benjamin Kaduk [mailto:[e-mail]]
> > > Envoyé : mardi 8 janvier 2019 20:38
> > > À : The IESG
> > > Cc : [e-mail]; Sheng Jiang; softwire-
> > > [e-mail]; [e-mail]; [e-mail]
> > > Objet : Benjamin Kaduk's Discuss on draft-ietf-softwire-yang-14: (with
> > > DISCUSS and COMMENT)
> > >
> > > Benjamin Kaduk has entered the following ballot position for
> > > draft-ietf-softwire-yang-14: Discuss
> > >
> > > 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-yang/
> > >
> > >
> > >
> > > ----------------------------------------------------------------------
> > > DISCUSS:
> > > ----------------------------------------------------------------------
> > >
> > > 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.
> > >
> >
> > [Med] Will leave this one to our AD.
> 
> And he has done a fine job with it!
> 
> >
> > > 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.
> > >
> >
> > [Med] Thank you for catching this one.
> >
> > There is a copy/paste bug:
> >
> > OLD:
> >
> >             container binding-table-versioning {
> >               description
> >                 "binding table's version";
> >               leaf version {
> >                 type uint64;
> >                 description
> >                   "Timestamp when the binding table was activated.
> >
> >                    A binding instance may be provided with binding
> >                    entries that may change in time (e.g., increase
> >                    the size of the port set). When an abuse party
> >                    presents an external IP address/port, the version
> >                    of the binding table is important because, depending
> >                    on the version, a distinct customer may be
> >                    identified.
> >
> >                    The timestamp is used as a key to find the
> >                    appropriate binding table that was put into effect
> >                    when an abuse occurred. ";
> >               }
> >               leaf date {
> >                 type yang:date-and-time;
> >                 description
> >                   "Timestamp of the binding table";
> >                 reference
> >                   "RFC7422: Deterministic Address Mapping to Reduce
> >                             Logging in Carrier-Grade NAT Deployments";
> >               }
> >             }
> >
> >
> > NEW:
> >
> >             container binding-table-versioning {
> >               description
> >                 "binding table's version";
> >               leaf version {
> >                 type uint64;
> >                 description
> >                   "A version number for the binding table.";
> >               }
> >               leaf date {
> >                 type yang:date-and-time;
> >                 description
> >                   "Timestamp when the binding table was activated.
> >
> >                    A binding instance may be provided with binding
> >                    entries that may change in time (e.g., increase
> >                    the size of the port set). When an abuse party
> >                    presents an external IP address/port, the version
> >                    of the binding table is important because, depending
> >                    on the version, a distinct customer may be
> >                    identified.
> >
> >                    The timestamp is used as a key to find the
> >                    appropriate binding table that was put into effect
> >                    when an abuse occurred. ";
> >                 reference
> >                   "RFC7422: Deterministic Address Mapping to Reduce
> >                             Logging in Carrier-Grade NAT Deployments";
> >               }
> >             }
> 
> Ah, a very easy resolution -- sorry for missing that it was just a
> copy/paste issue.
> 
> > >
> > > ----------------------------------------------------------------------
> > > COMMENT:
> > > ----------------------------------------------------------------------
> > >
> > > Please expand CE on first usage.
> > >
> > > 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?
> >
> > [Med] Some of these counters may be applicable to some other tunneling
> techniques, but not all of them. As such, these counters cannot be considered
> as generic.
> >
> > If in the future, a YANG module is to be defined for some tunneling
> technique and similar counters are also applicable fro that technique, that
> module can use the traffic-stat grouping defined in draft-ietf-softwire-yang.
> 
> Ok.
> 
> > >
> > > 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.
> >
> > [Med] The text is clear, IMO. Furthermore, the range of allowed values is
> called out explicitly in the module:
> >
> >             leaf softwire-num-max {
> >               type uint32 {
> >                 range "1..max";
> >               }
> 
> My apologies, I must have found the wrong place in the module -- I thought
> there was not a range specified.
> 
> >
> > >
> > > 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?
> > >
> >
> > [Med] 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.
> 
> Okay, thanks for the explanation (and no need to cover it in the document
> itself).
> 
> > > 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)
> >
> > [Med] We used "abuse" in reference to what is discussed in RFC6269 :
> https://tools.ietf.org/html/rfc6269#section-13.1. We may add a pointer to
> that section if you think this is useful.
> 
> I think "abuse" is fine, it's just the combination "abuse party" that is
> unexpected.  If we want to indicate "the party responsible for abuse", it
> may be easiest to just use that descriptive phrase rather than trying to
> coin a compound noun.
> 
> > >
> > > 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)?
> > >
> >
> > [Med] 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.
> 
> Probably worth a mention, then.
> 
> Thanks (and I'll go clear my Discuss now),
> 
> Benjamin