Subject: Re: [Dots] New Version Notification for draft-ietf-dots-telemetry-05.txt

From: <[email-masked]>
Date: Fri, 3 Apr 2020 05:53:41 +0000

Hi Jon, 

Moving the discussion to the WG list as this is an important design change we need to make. 

Please see inline. 

Cheers,
Med

> -----Message d'origine-----
> De : Jon Shallow [mailto:[email-masked]]
> Envoyé : jeudi 2 avril 2020 21:49
> À : BOUCADAIR Mohamed TGI/OLN; Konda, Tirumaleswar Reddy; kaname
> nishizuka
> Objet : RE: [Dots] New Version Notification for draft-ietf-dots-
> telemetry-05.txt
> 
> Stepping back to look at the bigger picture, I think that I would be
> more interested in the detail of a potentially multi-vectored mutating
> attack, and so question whether having target-protocol as an overall
> restrictive view of all the detail makes sense.  It is easier to
> filter out data that I do not want / care about after receiving it.
> The remote peer may not have the same view of what to send v what I am
> interested in.
> 
> [I am concerned about over-filling a packet and having to go into CoAP
> Block2 mode though which will fail on a saturated link.  I have not
> done any sizing of that yet with genuine attack information]
> 
> So, for the /tm and /tm-setup stuff do we really need target-protocol
> and could just have protocol as an optional, but array, entity?

[Med] It is important to provide details per protocol as this will help identifying abnormal patterns. 

We could associate 0 to mean "all" protocols but I know this will hurt many. We can consider the following change:

OLD

    |           +--rw baseline* [id]
    |              +--rw id                               uint32
    |              +--rw target-prefix*                   inet:ip-prefix
    |              +--rw target-port-range* [lower-port]
    |              |  +--rw lower-port    inet:port-number
    |              |  +--rw upper-port?   inet:port-number
    |              +--rw target-protocol*                 uint8
    |              +--rw target-fqdn*                     inet:domain-name
    |              +--rw target-uri*                      inet:uri
    |              +--rw total-traffic-normal-baseline* [unit protocol]
    |              |  +--rw unit                 unit
    |              |  +--rw protocol             uint8
    |              |  +--rw low-percentile-g?    yang:gauge64
    |              |  +--rw mid-percentile-g?    yang:gauge64
    |              |  +--rw high-percentile-g?   yang:gauge64
    |              |  +--rw peak-g?              yang:gauge64

NEW:

    |        +--:(baseline)
    |           +--rw baseline* [id]
    |              +--rw id                                   uint32
    |              +--rw target-prefix*                       inet:ip-prefix
    |              +--rw target-port-range* [lower-port]
    |              |  +--rw lower-port    inet:port-number
    |              |  +--rw upper-port?   inet:port-number
    |              +--rw target-protocol*                     uint8
    |              +--rw target-fqdn*                         inet:domain-name
    |              +--rw target-uri*                          inet:uri
    |              +--rw alias-name*                          string
    |              +--rw total-traffic-normal* [unit]
    |              |  +--rw unit                 unit
    |              |  +--rw low-percentile-g?    yang:gauge64
    |              |  +--rw mid-percentile-g?    yang:gauge64
    |              |  +--rw high-percentile-g?   yang:gauge64
    |              |  +--rw peak-g?              yang:gauge64
    |              +--rw total-traffic-normal-per-protocol* [unit protocol]
    |              |  +--rw unit                 unit
    |              |  +--rw protocol             uint8
    |              |  +--rw low-percentile-g?    yang:gauge64
    |              |  +--rw mid-percentile-g?    yang:gauge64
    |              |  +--rw high-percentile-g?   yang:gauge64
    |              |  +--rw peak-g?              yang:gauge64

Of course, the protocol must be listed in target-protocol (if present).

> 
> Then for /mitigate, target-protocol is ignored in deciding what to
> send back, but the use of an optional protocol array can reduce some
> of the packet size.
> 
> total-attack-connection  / total-connection-capacity will only be for
> connection based connections - primarily TCP I suspect. 

[Med] Agree.

 However
> (target) ports may be relevant here - a server may be able to support
> X HTTPS connections, but only Y DNS (TCP) connections - or a load-
> balancer in the path is only  capable of handling Z connections - no
> matter what the port is..  So, do we want to introduce optional ports?

[Med] These ports have to be listed in target-port-range (if present). If we include port to the existing list, we can't maintain two entries for the same protocol but with distinct port. It may be more simple to make this change:

OLD:
    |              +--rw total-connection-capacity* [protocol]
    |                 +--rw protocol                     uint8
    |                 +--rw connection?                  uint64
    |                 +--rw connection-client?           uint64
    |                 +--rw embryonic?                   uint64
    |                 +--rw embryonic-client?            uint64
    |                 +--rw connection-ps?               uint64
    |                 +--rw connection-client-ps?        uint64
    |                 +--rw request-ps?                  uint64
    |                 +--rw request-client-ps?           uint64
    |                 +--rw partial-request-ps?          uint64
    |                 +--rw partial-request-client-ps?   uint64

NEW:

    |              +--rw total-connection-capacity* [protocol]
    |              |  +--rw protocol                     uint8
    |              |  +--rw connection?                  uint64
    |              |  +--rw connection-client?           uint64
    |              |  +--rw embryonic?                   uint64
    |              |  +--rw embryonic-client?            uint64
    |              |  +--rw connection-ps?               uint64
    |              |  +--rw connection-client-ps?        uint64
    |              |  +--rw request-ps?                  uint64
    |              |  +--rw request-client-ps?           uint64
    |              |  +--rw partial-request-ps?          uint64
    |              |  +--rw partial-request-client-ps?   uint64
    |              +--rw total-connection-capacity-per-port* [protocol lower-port]
    |                 +--rw protocol                     uint8
    |                 +--rw lower-port                   inet:port-number
    |                 +--rw connection?                  uint64
    |                 +--rw connection-client?           uint64
    |                 +--rw embryonic?                   uint64
    |                 +--rw embryonic-client?            uint64
    |                 +--rw connection-ps?               uint64
    |                 +--rw connection-client-ps?        uint64
    |                 +--rw request-ps?                  uint64
    |                 +--rw request-client-ps?           uint64
    |                 +--rw partial-request-ps?          uint64
    |                 +--rw partial-request-client-ps?   uint64

> 
> > > NEW:
> > > "Sending a DELETE with no 'tmid' indicates that all 'tmids' must
> be
> > > deactivated."
> > >
> 
> I found this (but I prefer your addition for consistency)
> 
>    A DOTS client that lost the state of its active 'tmids' or has to
> set
>    'tmid' back to zero (e.g., crash or restart) MUST send a GET
> request
>    to the DOTS server to retrieve the list of active 'tmid'.  The DOTS
>    client may then delete 'tmids' that should not be active anymore.
> 

[Med] Yes. The NEW text will be right after this one. 

> Other minor nits
> 
> OLD
>    attack-severity:  Attack severity.  These values are supported:
>       Emergency (1), critical (2), and alert (3).
> NEW (case of emergency)
>    attack-severity:  Attack severity.  These values are supported:
>       emergency (1), critical (2), and alert (3).
> 
> OLD
>    tmid:  Telemetry Identifier is an identifier for the DOTS pre-or-
>         ongoing-mitigation telemetry data represented as an integer.
>         This identifier MUST be generated by DOTS clients. 'tsid'
> values
> NEW tsid -> tmid
>    tmid:  Telemetry Identifier is an identifier for the DOTS pre-or-
>         ongoing-mitigation telemetry data represented as an integer.
>         This identifier MUST be generated by DOTS clients. 'tmid'
> values
> 

[Med] Fixed. Thanks.  

> Regards
> 
> Jon
> 
> 
> > -----Original Message-----
> > From: [email-masked]
> [mailto:[email-masked]]
> > Sent: 02 April 2020 19:42
> > To: Jon Shallow; Konda, Tirumaleswar Reddy; kaname nishizuka
> > Subject: RE: [Dots] New Version Notification for draft-ietf-dots-
> telemetry-05.txt
> >
> > Re-,
> >
> > The same issue applies as well for the baseline component:
> >
> > If we convey multiple protocols in target-prefix and want to signal
> all total-
> > traffic-normal-baseline + total-traffic-normal-baseline of each
> protocol listed in
> > target-prefix, we will need to have to use an index in addition to
> the unit.
> > Protocol can then become optional. The same apply for total-
> connection-
> > capacity.
> >
> > I need to think about this further.
> >
> > Cheers,
> > Med
> >
> > > -----Message d'origine-----
> > > De : BOUCADAIR Mohamed TGI/OLN
> > > Envoyé : jeudi 2 avril 2020 19:29
> > > À : 'Jon Shallow'; Konda, Tirumaleswar Reddy; kaname nishizuka
> > > Objet : RE: [Dots] New Version Notification for draft-ietf-dots-
> > > telemetry-05.txt
> > >
> > > Hi Jon,
> > >
> > > Please see inline.
> > >
> > > Cheers,
> > > Med
> > >
> > > > -----Message d'origine-----
> > > > De : Jon Shallow [mailto:[email-masked]]
> > > > Envoyé : mercredi 1 avril 2020 23:01
> > > > À : BOUCADAIR Mohamed TGI/OLN; Konda, Tirumaleswar Reddy; kaname
> > > > nishizuka
> > > > Objet : RE: [Dots] New Version Notification for draft-ietf-dots-
> > > > telemetry-05.txt
> > > >
> > > > Hi Med et al,
> > > >
> > > > Other nits (as well as 2 following sections)
> > > >
> > > > OLD:
> > > >
> > > >    ongoing-mitigation telemetry data from the DOTS server.  The
> GET
> > > >    request specify a 'tmid' (Figure 31) or not (Figure 32).
> > > >
> > > > NEW:
> > > >
> > > >    ongoing-mitigation telemetry data from the DOTS server.  The
> GET
> > > >    request specifies a 'tmid' (Figure 31) or not (Figure 32).
> > > >
> > >
> > > [Med] Fixed.
> > >
> > >
> > > > DELETE /tm
> > > >
> > > > Does no tmid= delete all tmids ?
> > > > - In particular after a restart of a client?
> > > >
> > >
> > > [Med] Yes, they should be "deactivated".
> > >
> > > NEW:
> > > "Sending a DELETE with no 'tmid' indicates that all 'tmids' must
> be
> > > deactivated."
> > >
> > > > >
> > > > > .. The below is coming from what the server sends to the
> client
> > > > perspective.  I
> > > > > accept that in the client sending to the server having target-
> > > > protocol and
> > > > > protocol may give rise to some potential confusion...
> > > > >
> > >
> > > [Med] Let's see how to avoid the inconsistency. The YANG module is
> > > currently reflecting the text we have I draft-ietf-dots-telemetry-
> > > 04#section-7.1. We need to check that section to see where the
> per-
> > > transport is mentioned and make sure we don't break things.
> > >
> > > For example, if protocol is present in the target, we to avoid
> > > conflicting (and redundant) information to be supplied in the
> > > telemetry data.
> > >
> > > > > >
> > > > > > In terms of protocol, there are still some consistencies
> even if
> > > > you accept the
> > > > > > argument that protocol is already defined in the mitigation
> > > scope
> > > > (which I am
> > > > > > not happy with).
> > > > > >
> > > > > > 1) target-protocol is optionally set and it is an array
> whereas
> > > > protocol is
> > > > > singular
> > > > > >
> > > > > > 2) protocol is defined mitigation scope augment as per
> > > > > > module: ietf-dots-telemetry
> > > > > >   augment /ietf-signal:dots-signal/ietf-signal:message-
> > > type/ietf-
> > > > > signal:mitigation-
> > > > > > scope/ietf-signal:scope:
> > > > > >     +--ro total-traffic* [unit protocol] {dots-telemetry}?
> > > > > >     |  +--ro unit                 unit
> > > > > >     |  +--ro protocol             uint8
> > > > > >     |  +--ro low-percentile-g?    yang:gauge64
> > > > > >     |  +--ro mid-percentile-g?    yang:gauge64
> > > > > >     |  +--ro high-percentile-g?   yang:gauge64
> > > > > >     |  +--ro peak-g?              yang:gauge64
> > > > > >     +--rw total-attack-traffic* [unit] {dots-telemetry}?
> > > > > >     |  +--rw unit                 unit
> > > > > >     |  +--rw low-percentile-g?    yang:gauge64
> > > > > > And so there is no consistency between total-traffic and
> total-
> > > > attack-traffic
> > >
> > > [Med] The idea was to have a view of total traffic bound to a
> target
> > > prefix/domain. We should clarify or remove the protocol for this
> one.
> > > Fair enough.
> > >
> > > > > >
> > > > > > 3) For /tm you have
> > > > > >     +--:(telemetry) {dots-telemetry}?
> > > > > >        +--rw pre-or-ongoing-mitigation* [cuid tmid]
> > > > > > ..
> > > > > >           |  +--rw target-protocol*     uint8
> > > > > > ..
> > > > > >           +--rw total-traffic* [unit protocol]
> > > > > >           |  +--rw unit                 unit
> > > > > >           |  +--rw protocol             uint8
> > > > > > ..
> > > > > >           +--rw total-attack-traffic* [unit protocol]
> > > > > >           |  +--rw unit                 unit
> > > > > >           |  +--rw protocol             uint8
> > > > > >
> > > > > > And so could argue here that protocol is not needed
> > >
> > > [Med] but we may have many protocols listed under target-protocol.
> > >
> > > > > >
> > > > > > 4) If target-protocol is not specified, then it refers to
> any
> > > > protocol.  If target-
> > > > > > protocol is not defined in /mitigate, then I have no way of
> > > > indicating which is a
> > > > > > problematic protocol if protocol is not supported.
> > >
> > > [Med] Good point. This one (and the following one) justifies that
> we
> > > need to revisit the design.
> > >
> > > We ca group the data by target-protocol and send many tmids if
> many
> > > protocols need to be returned. Protocol can be then optional.
> > >
> > > Would that be better?
> > >
> > >
>

---

From: <[email-masked]>
Date: Fri, 3 Apr 2020 08:24:02 +0000

Jon, 

FWIW, the proposed change on the tree diagram can be seen here: 

https://github.com/boucadair/draft-dots-telemetry/commit/ea7c2101c4d3f748808c25fa85900fca7bca273b#diff-37c9ec7a9a8498c6dc3829deef79b5e7 

Of course, this is a proposal for discussion. 

Cheers,
Med

> De : Dots [mailto:[email-masked]] De la part de
> [email-masked]
> Envoyé : vendredi 3 avril 2020 07:54
> À : Jon Shallow; Konda, Tirumaleswar Reddy; kaname nishizuka
> Cc : [email-masked]
> Objet : Re: [Dots] New Version Notification for draft-ietf-dots-
> telemetry-05.txt
> 
> Hi Jon,
> 
> We could associate 0 to mean "all" protocols but I know this will hurt
> many. We can consider the following change:
> 
> OLD
> 
>     |           +--rw baseline* [id]
>     |              +--rw id                               uint32
> [Med] These ports have to be listed in target-port-range (if present).
> If we include port to the existing list, we can't maintain two entries
> for the same protocol but with distinct port. It may be more simple to
> make this change:
> 
> OLD:
> _______________________________________________
> Dots mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/dots

---

From: "Jon Shallow" <[email-masked]>
Date: Fri, 3 Apr 2020 12:51:00 +0100

Hi Med,

Thanks for this.  It looks as if it might be going in the right direction - I need to check if it can be sensibly coded up.

Hi all,

Some further thoughts.

/tm
===

For /tm we are trying to use the same YANG model for 2 purposes.
1-  Client indicating to Server attack telemetry information
2- Server indicating to Client attack telemetry information.

With Server -> Client, this is initiated by GET.  There is no reason as to why Uri-Query: cannot be used for prefix=, port=, protocol=, fqdn=, uri=, and alias-name= where each query can be repeated multiple times and port= can be a range.  Obviously the queries need to be validated that they are valid for the client in the same way that PUT gets validated.

The GET response (and any Observed response) can then populate target as appropriate and fill out the remaining information with non-zero values (or certainly values of consequence).
1-  For single port and/or single protocol in the query, do we just use total-traffic, total-traffic-protocol or (missing) total-traffic-port etc. in the response?
2-  Same true for the other variants (e.g. total-attack-traffic) as well.
It may be that only total-traffic makes sense and total-traffic-protocol can be dropped (same with other variants).  There is nothing stopping the Client doing different GET queries and observing all the different query combinations (other than volumes of traffic).

Then I believe that attack-detail should be an array [id] as multiple attack vectors may be concurrent

With Client -> Server, the information can easily be reduced by use of target-* information in the PUT, so I again question whether total-traffic-protocol etc. is really needed.  Multiple PUTs can be done with different target definitions which will not get caught by the target overlapping rule.

I am making a basic assumption that any traffic based information portrayed in the PUT is kept by the DOTS server for its internal use, but any GET responses reflect the actual situation, not a regurgitation of what the Client sent the Server.

It unclear to me what the difference is between embryonic and partial connections.  Embryonic is well defined, but partial is not.  Can the text be tidied up here?

There is no notion of /tm refresh (as in /mitigate or /config refresh) - is this correct?

/mitigate
========

The use of Queries would work here for any GET /mitigate (telemetry extension) so that the responses can be controlled despite the target definitions used for the PUT mitigation request.  Again, (because of packet size limitations when under attack) do we need total-traffic-protocol etc.?

The mitigate augment needs to be the same as for /tm from total-traffic onwards (with the CBOR mapping including ietf-dots-telemetry:).

If server-originated-telemetry is set and there is an active tmid, then responses include telemetry information
1- Do the responses get sent out at telemetry-notify-interval intervals (assuming Observe active)?
2- Why does this have to be with a tmid active - surely the tmid response and telemetry response will be containing the same data? 

/tm-setup
========

When doing a GET with cuid=, but not tsid= (to get the max/min etc. configuration information) what should be returned in the tsid field in the response given that it is a key?

While the baseline information is certainly worthwhile, things can wildly fluctuate during, say, Black Friday sales, or sales for some major festival.  The baseline then really needs to be updated for when these flash crowds take place to prevent unnecessary mitigation.  The text should be updated to reflect that there may be changing baselines. - e.g. extra servers spun up to handle the additional expected loads.

Peak Embryonic connection counts are a function of the listen queue (backlog) sizes on the target server and are likely to be different per port.  I am not sure that over my time in the last 2 decades working with DDoS that I have come across that many people who are able to sensibly answer the embryonic max question - especially when SYN Cookies and other techniques can muddy the water.  There is benefit in reporting it (e.g. SYN flooding attack) but I think it is difficult to baseline.  Thoughts?

Regards

Jon

> From: Dots [mailto: [email-masked]] On Behalf Of [email-masked]
> Sent: 03 April 2020 09:24
> Cc: [email-masked]
> Subject: Re: [Dots] New Version Notification for draft-ietf-dots-telemetry-05.txt
> 
> Jon,
> 
> https://github.com/boucadair/draft-dots-
> telemetry/commit/ea7c2101c4d3f748808c25fa85900fca7bca273b#diff-
> 37c9ec7a9a8498c6dc3829deef79b5e7
>