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

From: Benjamin Kaduk <[email-masked]>
Date: Tue, 12 May 2020 19:12:22 -0700

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/

   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.

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

   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/

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.

   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?  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?  What would the
interpretation be if it was not provided?

   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.

   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?

   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?

   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?

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)?

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?

       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.

         [...]
       }
       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; 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.)

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

   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.

            "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 ']'?
(Have the examples been run through a JSON validator?)

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, [...]"?

   example, a PUT request shown in Figure 9.  The DOTS server installs

nit: either "the PUT request shown" or "a PUT request as shown".

                 "actions": {
                   "forwarding": "accept",
                   "rate-limit": "20.00"

20 bytes per second?  That seems rather small.

   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.

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

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.

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.

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.

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

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

Thanks,

Ben

---

From: <[email-masked]>
Date: Wed, 13 May 2020 10:47:56 +0000

Hi Ben, 

Thank you for the review. 

Please see inline.

Cheers,
Med

> -----Message d'origine-----
> De : Benjamin Kaduk [mailto:[email-masked]]
> Envoyé : mercredi 13 mai 2020 04:12
> À : [email-masked]; [email-masked]
> Cc : [email-masked]
> Objet : AD evaluation of draft-ietf-dots-signal-filter-control-03
> 
> Hi all,
> 

[Med] Fixed. Thanks.

> otherwise this DOTS client would not be authorized to modify it

[Med] After rereading, I think we can get rid of it. I deleted that part of the sentence. 

> "using"); perhaps s/the capability of/a capability for/ (though other

[Med] Fixed. 

>       reported by a DOTS server.

[Med] Fixed. Thanks.

> The referenced section is just an example; the actual YANG definition

[Med] pointing to section 4.3 is better. Fixed. 

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

> "YANG-formatted structure" or just "YANG structure" is better.

[Med] Changed to "YANG structure".

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

> situation has changed in the interim and the mitigation update is no

[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  

> Perhaps mention "including the ACL update" for clarity?

[Med] Works for me. 

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

> which "signal-control" would be ambiguous or a name conflict?

[Med] What about "dots-control"?

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

> 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
> Examples" as was previously used, and the body text here as well.)

[Med] Not sure about this one. Can you please clarify?

> field; it's not clear that we need to diverge from that style (applies

[Med] Good catch. Fixed. Thanks. 

> (vs. 123) is intentional and correct.

[Med] I confirm.

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

> belief) or the rate-limiting.  Perhaps "Installing the accpe-list can

[Med] Fixed. 

> nit: either "the PUT request shown" or "a PUT request as shown".

[Med] Fixed.  

> 20 bytes per second?  That seems rather small.

[Med] Indeed. Will increase the value used in the example.

> nit: I think s/the attack traffic/attack traffic/ is fine grammar.

[Med] Fixed. 

> prefer one or the other?

[Med] 2-byte encoding is preferred to keep the message compact. 

  Since we're allocating from an "IETF Review"
> to me how much say the DEs have in which value gets used.

[Med] Updated the table to suggest 52/53/54 values. 

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

> polling/OBSERVE notification.

[Med] Yeah.  

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

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

> either "such a compromised" or "DOTS clients").

[Med] Fixed. Thanks.

---

From: kaname nishizuka <[email-masked]>
Date: Thu, 14 May 2020 11:55:17 +0900

Hi Ben, Med,

Thank you for the detailed review.
All the changes and the proposal texts looks good to me. thanks Med for catching them all.

thanks,
Kaname

On 2020/05/13 19:47, [email-masked] wrote:
> _______________________________________________
> Dots mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/dots

---

From: <[email-masked]>
Date: Mon, 18 May 2020 07:12:15 +0000

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

> De : [email-masked]
> [mailto:[email-masked]]
> Envoyé : mercredi 13 mai 2020 12:48
> À : Benjamin Kaduk; draft-ietf-dots-signal-filter-
> [email-masked]; [email-masked]
> Cc : [email-masked]
> Objet : RE: AD evaluation of draft-ietf-dots-signal-filter-control-03
> 
> Hi Ben,
> 
> >    This specification addresses the problems discussed in Section
> 1.1
> > by
> > The referenced section is just an example; the actual YANG
> definition
> > is
> > in Section 4.3 thereof.
> 
> > Is there a way to "cancel" the signal-channel-made changes and
> revert
> > to
> >    diagnostic payload indicating a failed ACL update as a response
> to
> > an
> [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.
> 
> [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
> [Med] If an acl-list is present, then acl-name must be present.
> Nevertheless, an acl-list is not a mandatory.
> 
> >          [...]
> >        }
> >        leaf activation-type {
> > Just as a sanity-check: this "mid" of 124 is consistent with the
> note
> > in
> > belief) or the rate-limiting.  Perhaps "Installing the accpe-list
> can
> > be
> > done by sending, [...]"?
> 
> [Med] Fixed.
> 
> 
> >
> > "skew" wherein an update made in one place is ~immediately
> overridden
> > by
> > Similarly, a reminder that bad things happen when a DOTS server
> fails
> > to
> >    A compromised DOTS client can use the filtering control
> capability
> > to

---

From: "Jon Shallow" <[email-masked]>
Date: Mon, 18 May 2020 09:45:24 +0100

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: [email-masked]] On Behalf Of
[email-masked]
> Sent: 18 May 2020 08:12
> To: Benjamin Kaduk; [email-masked];
> [email-masked]
> Cc: [email-masked]
> Subject: Re: [Dots] AD evaluation of
draft-ietf-dots-signal-filter-control-03

---

From: <[email-masked]>
Date: Mon, 18 May 2020 08:59:34 +0000

Hi Jon,

Please see inline. 

Cheers,
Med

> De : Jon Shallow [mailto:[email-masked]]
> Envoyé : lundi 18 mai 2020 10:45
> À : BOUCADAIR Mohamed TGI/OLN; [email-masked]; Benjamin Kaduk; draft-
> [email-masked]; [email-masked]
> Objet : RE: [Dots] AD evaluation of draft-ietf-dots-signal-filter-
> control-03
> 
> Hi Med,
> 
> Old:

[Med] Fixed. Thanks.

> I-D.ietf-dots-signal-channel into their (new) respective RFCs, but not

[Med] Only the embedded refs in the YANG module was updated. For the others, this will be done when the RFCs will be published.  

> > https://github.com/boucadair/filter-control/blob/master/draft-ietf-
> dots-
> > signal-filter-control-04.txt
> >

---

From: Benjamin Kaduk <[email-masked]>
Date: Wed, 27 May 2020 10:14:00 -0700

Hi Med,

Please go ahead and upload the new rev; the changes at
https://github.com/boucadair/filter-control/blob/master/AD-Review.pdf look
good.

A little bit of commentary inline.

> [Med] After rereading, I think we can get rid of it. I deleted that part of the sentence. 

Good point.  The new text in the security considerations covers this point
anyway.

>    channel or delete some of these filters.  This behavior is deployment

What you added is just right; thanks.
(My original mental model was that the signal-filter-control operation was
an "override" and thus that the override would be reverted at some point;
subsequent discussion has clarified that it's just another mechanism to
update the status, and not an override at all.  Your text describes this
nicely.)

> [Med] This will be a mitigation request as defined in draft-ietf-dots-signal-channel.

I see.  Thanks for the new text about "mandatory attribute when 'acl-list'
is included".

> We can add a reminder if you think this is useful. 

I don't think we need a reminder in the document; it's pretty clear in the
main signal-channel doc but I hadn't read that recently enough.

> s/client MUST immediately/client MUST by default immediately  

I don't have a particular preference between those versions.  Your
explanation makes sense to me, so we can probably leave this as-is for now.

> [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. T…

I think we are on the same page as to how the YANG features are intended to
work.  What I'm still not sure about is whether there's anything in
ietf-dots-signal-control other than the acl-list (that is to say, whether
it makes sense to implement ieft-dots-signal-control but not support the
control-filtering feature).

> [Med] What about "dots-control"?

Sure.

> [Med] Actually, no. We do have an activation-type value for each acl-name in the list. 

It looks like I misread the scope; sorry!

> [Med] Not sure about this one. Can you please clarify?

This referred back to a previous comment:

% nit: "sample examples" feels redundant; maybe "some examples" or just
% "examples"?

> > (vs. 123) is intentional and correct.

Thanks!

> > either "such a compromised" or "DOTS clients").

Thanks for all these fixes!

-Ben