Re: [Dots] AD evaluation of draft-ietf-dots-signal-filter-control-03

[Jon Shallow <[e-mail]>]

Hi Med,

Old:
Installing the accpe-list
New:
Installing the accept-list

There have been some changes for I-D.ietf-dots-data-channel and
I-D.ietf-dots-signal-channel into their (new) respective RFCs, but not all
of them.

Regards

Jon

> -----Original Message-----
> From: Dots [mailto: [e-mail]] On Behalf Of
[e-mail]
> Sent: 18 May 2020 08:12
> To: Benjamin Kaduk; [e-mail];
> [e-mail]
> Cc: [e-mail]
> Subject: Re: [Dots] AD evaluation of
draft-ietf-dots-signal-filter-control-03
> 
> Hi Ben, all,
> 
> FWIW, a candidate version is available at:
> https://github.com/boucadair/filter-control/blob/master/draft-ietf-dots-
> signal-filter-control-04.txt
> 
> Diff: https://github.com/boucadair/filter-control/blob/master/AD-
> Review.pdf
> 
> Cheer,
> Med
> 
> > -----Message d'origine-----
> > De : [e-mail]
> > [mailto:[e-mail]]
> > Envoyé : mercredi 13 mai 2020 12:48
> > À : Benjamin Kaduk; draft-ietf-dots-signal-filter-
> > [e-mail]; [e-mail]
> > Cc : [e-mail]
> > Objet : RE: AD evaluation of draft-ietf-dots-signal-filter-control-03
> >
> > Hi Ben,
> >
> > Thank you for the review.
> >
> > Please see inline.
> >
> > Cheers,
> > Med
> >
> > > -----Message d'origine-----
> > > De : Benjamin Kaduk [mailto:[e-mail]]
> > > Envoyé : mercredi 13 mai 2020 04:12
> > > À : [e-mail];
> > [e-mail]
> > > Cc : [e-mail]
> > > Objet : AD evaluation of draft-ietf-dots-signal-filter-control-03
> > >
> > > Hi all,
> > >
> > > This one is also generally in good shape, with mostly nit-level
> > > comments.
> > > (I didn't find it on github, so those are inline.)
> > >
> > > I have a nagging suspicion that I'm repeating some comments I made
> > on
> > > the core protocol documents (having forgotten the response and thus
> > > why
> > > the change in question should not be made; my apologies in advance
> > for
> > > my poor memory.
> > >
> > > Section 1.1
> > >
> > >    The DOTS data channel protocol [I-D.ietf-dots-data-channel] is
> > used
> > >    for bulk data exchange between DOTS agents to improve the
> > >    coordination of parties involved in the response to the
> > Distributed
> > >
> > > nit: s/the/a/
> > >
> >
> > [Med] Fixed. Thanks.
> >
> > >    the inbound link to the DOTS client domain.  In other words, the
> > > DOTS
> > >    client cannot use the DOTS data channel protocol to withdraw the
> > >    accept-list filters when a DDoS attack is in progress.  This
> > > assumes
> > >    that this DOTS client is the owner of the filtering rule.
> > >
> > > nit: the last sentence feels a little disconnected from the previous
> > > discussion at the moment; I'd suggest adding at the end ", since
> > > otherwise this DOTS client would not be authorized to modify it
> > > anyway"
> > > or similar.
> >
> > [Med] After rereading, I think we can get rid of it. I deleted that
> > part of the sentence.
> >
> > >
> > > Section 1.2
> > >
> > >    This specification addresses the problems discussed in Section
> > 1.1
> > > by
> > >    adding the capability of managing filtering rules using the DOTS
> > >    signal channel protocol, which enables a DOTS client to request
> > the
> > >    activation (or deactivation) of filtering rules during a DDoS
> > > attack.
> > >
> > > nit: the grammar here does not quite match up ("capability of" vs.
> > > "using"); perhaps s/the capability of/a capability for/ (though
> > other
> > > fixes are possible).
> >
> > [Med] Fixed.
> >
> > >
> > >    Sample examples are provided in Section 4, in particular:
> > >
> > > nit: "sample examples" feels redundant; maybe "some examples" or
> > just
> > > "examples"?
> > >
> > >    o  Section 4.1 illustrates how the filter control extension is
> > used
> > >       when conflicts with Access Control List (ACLs) are detected
> > and
> > >       reported by a DOTS server.
> > >
> > > nit: s/List/Lists/
> >
> > [Med] Fixed. Thanks.
> >
> > >
> > > Section 3.1
> > >
> > >    A filtering rule controlled by the DOTS signal channel is
> > > identified
> > >    by its ACL name (Section 7.2 of [I-D.ietf-dots-data-channel]).
> > > Note
> > >
> > > The referenced section is just an example; the actual YANG
> > definition
> > > is
> > > in Section 4.3 thereof.
> >
> > [Med] pointing to section 4.3 is better. Fixed.
> >
> > >
> > >    The activation or deactivation of an ACL by the DOTS signal
> > channel
> > >    overrides the 'activation-type' (defined in Section 7.2 of
> > >    [I-D.ietf-dots-data-channel]) a priori conveyed with the
> > filtering
> > >    rules using the DOTS data channel protocol.
> > >
> > > Is there a way to "cancel" the signal-channel-made changes and
> > revert
> > > to
> > > the previous activation-type?
> >
> > [Med] This is up to the DOTS client to tweak these filtering rules as
> > a function of the context. For example,
> > - if a rule was deactivated because of a conflict, the client may
> > delete that rule
> > - if the rule was a rate-limit, the client will deactivate the rule.
> >
> > I added this NEW text:
> >
> >    Once the attack is mitigated, the DOTS client may use the data
> >    channel to control the 'activation-type' (e.g., revert to a default
> >    value) of some of the filtering rules controlled by the DOTS signal
> >    channel or delete some of these filters.  This behavior is
> > deployment
> >    specific.
> >
> > Do we need to say more?
> >
> >   Or do we just make the client remember
> > > the previous status and manually set that status in such a case?
> > >
> > > Section 3.2.1
> > >
> > > Why is the acl-name an optional attribute?
> >
> > [Med] Because:
> >
> >    This specification extends the mitigation request defined in
> >    Section 4.4.1 of [I-D.ietf-dots-signal-channel] to convey the
> >    intended control of configured filtering rules.
> >
> > A mitigation request does not need to include an acl name to be valid.
> >
> >  What would the
> > > interpretation be if it was not provided?
> >
> > [Med] This will be a mitigation request as defined in draft-ietf-dots-
> > signal-channel.
> >
> > >
> > >    intended control of configured filtering rules.  Concretely, the
> > > DOTS
> > >    client conveys 'acl-list' attribute with the following sub-
> > > attributes
> > >    in the CBOR body of a mitigation request (see the YANG-encoded
> > >    structure in Section 3.2.2.1):
> > >
> > > nit: YANG is a data description language, not an encoding; perhaps
> > > "YANG-formatted structure" or just "YANG structure" is better.
> >
> > [Med] Changed to "YANG structure".
> >
> > >
> > >    As the attack evolves, DOTS clients can adjust the 'activation-
> > > type'
> > >    of an ACL conveyed in a mitigation request or control other
> > filters
> > >    as necessary.  This can be achieved by sending a PUT request with
> > a
> > >    new 'mid' value.
> > >
> > > This potentially involves a lot of new 'mid' values if many change
> > are
> > > made using signal-channel filtering control over the course of a
> > given
> > > mitigation.  Is this going to cause a noticeable increase in the
> > > amount
> > > of state required at the DOTS server?
> >
> > [Med] No. Only the request with the highest 'mid' value will be
> > maintained.
> >
> > We can add a reminder if you think this is useful.
> >
> > >
> > >    If the DOTS client receives a 5.03 (Service Unavailable) with a
> > >    diagnostic payload indicating a failed ACL update as a response
> > to
> > > an
> > >    initial mitigation or a mitigation with adjusted scope, the DOTS
> > >    client MUST immediately send a new request which repeats all the
> > >    parameters as sent in the failed mitigation request but without
> > >    including the ACL attributes.  After the expiry of Max-Age
> > returned
> > >
> > > Why does this need to be a MUST-level requirement?  What if the
> > > situation has changed in the interim and the mitigation update is no
> > > longer needed?
> >
> > [Med] These requests are supposed to happen in a short interval and
> > clients have to immediately place a mitigation request to avoid the
> > inconvenience of including the ACLs.
> >
> > What about the following:
> >
> > s/client MUST immediately/client MUST by default immediately
> >
> > >
> > >    in the 5.03 (Service Unavailable) response, the DOTS client
> > retries
> > >    with a new mitigation request (i.e., a new 'mid') that repeats
> > all
> > >    the parameters as sent in the failed mitigation request.
> > >
> > > Perhaps mention "including the ACL update" for clarity?
> > >
> >
> > [Med] Works for me.
> >
> > > Section 3.2.2.1
> > >
> > >        +--rw acl-list* [acl-name] {control-filtering}?
> > >
> > > A question for the YANG doctor, no doubt, but is there a need for a
> > > feature indicator that gates the entire content of a given module
> > (as
> > > opposed to just using module-level granularity to indicate support)?
> >
> > [Med] I'm not a YANG doctor, but a feature needs to be defined within
> > the module itself to mark parts that are conditional. I don't quite
> > see why a module need to be explicitly taged as optional. This is
> > actually why it is recommended that if you have a module with a large
> > set of nodes with an if-feature, to define these nodes in a separate
> > module.
> >
> > >
> > > Section 3.2.2.2
> > >
> > >      namespace
> > >         "urn:ietf:params:xml:ns:yang:ietf-dots-signal-control";
> > >      prefix signal-control;
> > >
> > > Can we think of anything else that might be in scope in the future
> > for
> > > which "signal-control" would be ambiguous or a name conflict?
> >
> > [Med] What about "dots-control"?
> >
> > >
> > >        list acl-list {
> > >          key "acl-name";
> > >
> > > Seeing this reminds me of a shadow of a memory that list keys are
> > > implicitly mandatory, which seems relevant for an earlier comment.
> > >
> >
> > [Med] If an acl-list is present, then acl-name must be present.
> > Nevertheless, an acl-list is not a mandatory.
> >
> > >          [...]
> > >        }
> > >        leaf activation-type {
> > >          type ietf-data:activation-type;
> > >          default "activate-when-mitigating";
> > >          description
> > >            "Sets the activation type of an ACL.";
> > >
> > > This structure seems to bind us to having a single activation-type
> > > that
> > > applies lto all of the ACLs in the acl-list;
> >
> > [Med] Actually, no. We do have an activation-type value for each acl-
> > name in the list.
> >
> >
> >  furthermore, the global
> > > structure of where we augment means that we can only have one (acl-
> > > list,
> > > activation-type) pair within a given mitigation request.  I
> > *believe*
> > > that is consistent with the core signal protocol in terms of what's
> > > expected to be in a single message, but please confirm.
> > >
> > > Section 4
> > >
> > > (The section heading should probably get the same treatment of
> > "Sample
> > > Examples" as was previously used, and the body text here as well.)
> >
> > [Med] Not sure about this one. Can you please clarify?
> >
> > >
> > > Section 4.1
> > >
> > >    Host: {host}:{port}
> > >
> > > nit(?): the data-channel doc just uses "example.com" for the host
> > > header
> > > field; it's not clear that we need to diverge from that style
> > (applies
> > > throughout).
> >
> > [Med] Good catch. Fixed. Thanks.
> >
> > >
> > >    The DOTS client can also decide to send a PUT request to
> > deactivate
> > >    the "an-accept-list" ACL, if suspect traffic is received from an
> > >    accept-listed source (2001:db8:1234::/48).  The structure of that
> > > PUT
> > >    is the same as the one shown in Figure 5.
> > >
> > >    [...]
> > >    Uri-Path: "mid=124"
> > >
> > > Just as a sanity-check: this "mid" of 124 is consistent with the
> > note
> > > in
> > > Section 3.2.1 that adjusting the 'activation-type' "[...] can be
> > > achieved by sending a PUT request with a new 'mid' value", so the
> > 124
> > > (vs. 123) is intentional and correct.
> >
> > [Med] I confirm.
> >
> > >
> > >             "ietf-dots-signal-control:acl-list": [
> > >               {
> > >                 "ietf-dots-signal-control:acl-name": "an-accept-
> > list",
> > >                 "ietf-dots-signal-control:activation-type":
> > > "deactivate"
> > >               }
> > >             ]
> > >            "lifetime": 3600
> > >
> > > Shouldn't there be a comma after the ']'?
> >
> > [Med] Yes. Thanks for catching this.
> >
> > > (Have the examples been run through a JSON validator?)
> >
> > [Med] will double check.
> >
> > >
> > > Section 4.3
> > >
> > >    Consider a DOTS client that contacts its DOTS server during
> > 'idle'
> > >    time to install an accept-list that rate-limits all (or a part
> > >    thereof) traffic to be forwarded to 2001:db8:123::/48 as a last
> > >    resort countermeasure whenever required.  It does so by sending,
> > > for
> > >
> > > nit: I could imagine a little confusion as to whether the action
> > > referenced by "it does so" is the installation of the accept-list
> > (my
> > > belief) or the rate-limiting.  Perhaps "Installing the accpe-list
> > can
> > > be
> > > done by sending, [...]"?
> >
> > [Med] Fixed.
> >
> >
> > >
> > >    example, a PUT request shown in Figure 9.  The DOTS server
> > installs
> > >
> > > nit: either "the PUT request shown" or "a PUT request as shown".
> >
> > [Med] Fixed.
> >
> > >
> > >                  "actions": {
> > >                    "forwarding": "accept",
> > >                    "rate-limit": "20.00"
> > >
> > > 20 bytes per second?  That seems rather small.
> >
> > [Med] Indeed. Will increase the value used in the example.
> >
> > >
> > >    For some reason (e.g., the DOTS server, or the mitigator, is
> > > lacking
> > >    a capability or capacity), the DOTS client is still receiving the
> > >    attack traffic which saturates available links.  To soften the
> > >
> > > nit: I think s/the attack traffic/attack traffic/ is fine grammar.
> > >
> >
> > [Med] Fixed.
> >
> > > Section 5.1
> > >
> > >    o  Note to the RFC Editor: Please delete (TBA1-TBA2-TBA3) once
> > the
> > >       CBOR key is assigned from the 1-16383 range.  Please update
> > >       Table 1 accordingly.
> > >
> > > IIRC the unallocated values in this range include both values with a
> > > 2-byte encoding and with a 3-byte encoding; do we have a reason to
> > > prefer one or the other?
> >
> > [Med] 2-byte encoding is preferred to keep the message compact.
> >
> >   Since we're allocating from an "IETF Review"
> > > range, we get to suggest a value to IANA and it's not immediately
> > > clear
> > > to me how much say the DEs have in which value gets used.
> >
> > [Med] Updated the table to suggest 52/53/54 values.
> >
> > >
> > > Section 6
> > >
> > > We may get someone asking "this document uses YANG; why aren't you
> > > using
> > > the YANG security considerations template?"  I guess the signal-
> > > channel
> > > doc made it through without any particular note in that regard, so
> > > maybe
> > > we'll be safe for this one, too.
> >
> > [Med] Agree. We do say explicitly the following in the signal channel:
> >
> > =
> > This YANG module is not intended to be used via NETCONF/
> >    RESTCONF for DOTS server management purposes; such module is out of
> >    the scope of this document.  It serves only to provide a data model
> >    and encoding, but not a management data model.
> > ==
> >
> > >
> > > I'd consider mentioning that with both signal and data channels
> > > available for updating ACL activation status, there is potential for
> > > "skew" wherein an update made in one place is ~immediately
> > overridden
> > > by
> > > another update.  Fortunately, there was already some degree of
> > > asynchronicity/external-changes possible, so clients should already
> > be
> > > prepared for this sort of situation and will cope properly by
> > > polling/OBSERVE notification.
> >
> > [Med] Yeah.
> >
> > >
> > > We could also consider saying something about how the restriction to
> > > only changing activation status over the signal channel (but not
> > > creating new ACLS entirely) means that if a client hasn't prepared
> > > adequately prior to an attack, it can get stuck in a bad place.
> > This,
> > > of course, is also not really new with this document, but is a
> > pretty
> > > relevant consideration for its use.
> >
> > [Med] Good point. Added this NEW text:
> >
> >    This specification does not allow to create new filtering rules,
> >    which is the responsibility of the DOTS data channel.  DOTS client
> >    domains should be adequately prepared prior to an attack, e.g., by
> >    creating filters that will be activated on demand when an attack is
> >    detected.
> >
> > >
> > > Similarly, a reminder that bad things happen when a DOTS server
> > fails
> > > to
> > > maintain namespace separation across clients for ACL names could be
> > in
> > > order (but is not required).
> >
> > [Med] Added this NEW text:
> >
> > As a reminder,
> >    DOTS servers must associate filtering rules with the DOTS client
> > that
> >    created these resources.  Failure to ensure such association by a
> >    DOTS server will have severe impact on DOTS client domains.
> >
> > >
> > >    A compromised DOTS client can use the filtering control
> > capability
> > > to
> > >    exacerbate an ongoing attack.  Likewise, such compromised DOTS
> > > client
> > >    may abstain from reacting to an ACL conflict notification
> > received
> > >
> > > nit: singular/plural mismatch "DOTS client"/"such compromised" (so
> > > either "such a compromised" or "DOTS clients").
> >
> > [Med] Fixed. Thanks.
> >
> > >
> > > Thanks,
> > >
> > > Ben
> 
> _______________________________________________
> Dots mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/dots