Re: [Dots] AD review of draft-ietf-dots-signal-channel-25 (4th Part)

[Benjamin Kaduk <[e-mail]>]

On Wed, Jan 16, 2019 at 03:29:45PM +0000, [e-mail] wrote:
> Re-,
> 
> Please see inline.
> 
> Cheers,
> Med
> 
> > -----Message d'origine-----
> > De : Benjamin Kaduk [mailto:[e-mail]]
> > Envoyé : mercredi 16 janvier 2019 01:14
> > À : [e-mail]
> > Cc : [e-mail]
> > Objet : AD review of draft-ietf-dots-signal-channel-25
> > 
> > 
> > Section 4.4.3
> > 
> > I'm not questioning the decision, but I am a bit curious how we ended up
> > with a boolean "attack-status" parameter and no quantitative
> > measurements.
> > 
> 
> [Med] This is not a Boolean. The attack-status reflects the status as observed by the client. Actually, we restricted the information to the minimum information that can be reported by the client without leaking privacy/local information that may be subject to local policies at the client domain. 
> 
> >    The DOTS server indicates the result of processing a PUT request
> >    using CoAP response codes.  The response code 2.04 (Changed) is
> >    returned if the DOTS server has accepted the mitigation efficacy
> >    update.  The error response code 5.03 (Service Unavailable) is
> >    returned if the DOTS server has erred or is incapable of performing
> >    the mitigation.
> > 
> > What (if anythin) should the client do in response?  Should the client
> > assume the server is dead and move to a different one?
> 
> [Med] 5.03 uses Max-Age option to indicate when to retry. This is discussed in RFC7252. I can update the text if you think this is helpful to remind this. 
> 
> > 
> > Section 4.4.4
> > 
> >    terminating period SHOULD be set by default to 120 seconds.  If the
> >    client requests mitigation again before the initial active-but-
> >    terminating period elapses, the DOTS server MAY exponentially
> >    increase the active-but-terminating period up to a maximum of 300
> >    seconds (5 minutes).
> > 
> > Is the base of the exponent supposed to be 2 (i.e., a doubling of the
> > delay)?  This should probably be made explicit.
> 
> [Med] Done. Thanks.
> 
>   Also, is the input to
> > the doubling the previous total delay, or the amount of delay remaining
> > when the repeated request arrives?
> 
> [Med] The server sets active-but-terminating, then if it receives a request before that timer expires, it doubles the active-but-terminating and so on. The server repeats this process at most for 5 minutes.  
> 
> > 
> >    If a mitigation is triggered due to a signal channel loss, the DOTS
> >    server relies upon normal triggers to stop that mitigation
> >    (typically, receipt of a valid DELETE request, expiry of the
> >    mitigation lifetime, or observation of traffic to the attack target).
> > 
> > I think "observation of traffic" probably needs more explanation -- as
> > written, it sounds like any packet arriving would be sufficient, which
> > is probably not the intended behavior...
> 
> [Med] We meant "inspecting the traffic", but avoided to use the word because this term is not well perceived.  
> 
> I can update the text If you are OK with "inspecting". 

I would be, but Tiru's note about "scrubbing" is better.

> > 
> > Section 4.5.1
> > 
> > Does the "max-retransmit" value exclude the initial transmission?
> 
> [Med] Yes.
> 
> > 
> > I'm not sure whether there are any particularly interesting security
> > considerations to mention about what happens when ack-random-factor is
> > insufficiently random; it seems like things would only get really bad if
> > the value was constant across peers.
> > 
> 
> [Med] A brief discussion on this change's impact can be found in Section 4.8.1 of RFC7252. 
> 
> > Section 4.5.2
> > 
> >                                                  A DOTS client MUST NOT
> >    transmit a "CoAP Ping" while waiting for the previous "CoAP Ping"
> >    response from the same DOTS server.
> > 
> > Is this intended to block retransmissions of the same CoAP Ping?
> 
> [Med] This is intended to block retransmission before the retransmission timeout expires.  
> 
> 
> > 
> >    sid:  Session Identifier is an identifier for the DOTS signal channel
> >       session configuration data represented as an integer.  This
> >       identifier MUST be generated by DOTS clients.  'sid' values MUST
> >       increase monotonically.
> > 
> > What event triggers the increase?
> 
> [Med] Send a PUT request for configuration purpose.   
> 
> > 
> > Section 4.5.3
> > 
> >                                        When a DDoS attack is active,
> >    refresh requests MUST NOT be sent by DOTS clients and the DOTS server
> >    MUST NOT terminate the (D)TLS session after the expiry of the value
> >    returned in Max-Age Option.
> > 
> > It seems like there's a bit of a race condition with attacks and refresh
> > requests coming in in parallel.
> > 
> 
> [Med] Is there any change that you see to the text? 

I don't think so -- it's only an issue if the server decides to enforce the
"refresh requests MUST NOT be sent", but we don't say anything to indicate
that it should do such enforcement.

> > Section  4.6
> > 
> >    The DOTS server can return the error response code 5.03 in response
> >    to a request from the DOTS client or convey the error response code
> >    5.03 in a unidirectional notification response from the DOTS server.
> > 
> > Just to check my understanding, this unidirectional notification would
> > only happen if there was a previous Observe to establish the channel?
> 
> [Med] Yes.
> 
> > 
> > The "alt-server-record" description here is pretty vague on the string
> > formatting used, v4 vs. v6, etc.;
> 
> [Med] "IP addresses" is used on purpose to refer to both IPv4 and IPv6 addresses.
> 
>  I guess that the actual "type
> > inet:ip-address" in the YANG module is probably specific enough, though.
> > 
> 
> [Med] Yes, indeed.
> 
> 
> > Section 4.7
> > 
> > The first two paragraphs seem to have high overlap and would probably
> > benefit from at least being joined together if not also trimmed down a
> > bit.
> 
> [Med] Joined together in the my local copy with the 2nd para which starts now with "Concretely, ".
> 
> > 
> >    In case of a massive DDoS attack that saturates the incoming link(s)
> >    to the DOTS client, all traffic from the DOTS server to the DOTS
> >    client will likely be dropped, although the DOTS server receives
> >    heartbeat requests in addition to DOTS messages sent by the DOTS
> >    client.  In this scenario, the DOTS agents MUST behave differently to
> >    handle message transmission and DOTS session liveliness during link
> >    saturation:
> > 
> > Do we need to say how the agents independently detect the
> > link-saturation situation?
> > 
> 
> [Med] When the agents are aware that an attack is ongoing, they will infer links are likely to be saturated.

Okay.

> > Section 5.1
> > 
> > It looks like the tree diagram and the module differ about the
> > "mandatory false" nature of some nodes, e.g., upper-port.  Perhaps the
> > tree diagram should be regenerated?
> > 
> 
> [Med] Will double check.
> 
> > Section 5.2
> > 
> > I mostly expect some heartburn from various folks about overriding
> > protocol number 0's interpretation as the IPv6 hop-by-hop option to mean
> > "all protocols", though it will probably be fine in practice.  I am
> > including a suggested rewording in my editorial changes that may help
> > alleviate concerns.
> 
> [Med] Good catch. We can fix this by updating the text to:
> 
> "If 'target-protocol' is not specified, then the request applies to any protocol."
> 
> Will also check your proposal. Thanks.
> 
> > 
> > Section 6
> > 
> > I have several thoughts about the CBOR encoding, some of which I
> > mentioned in some previous emails.  (I also don't have much experience
> > with CBOR encoding, so I probably have some things wrong, too...)
> > 
> > It seems that CBOR major type 0 also encodes some width information for
> > integers.  Do we want to require that the CBOR encoding always uses the
> > width that matches the types in the YANG module, or do we permit a
> > smaller encoding when the value in question fits?  (Presumably the
> > answer falls out naturally if we expect people to use off-the-shelf CBOR
> > libraries...)
> > 
> > We say that the initial set of key values is comprehension-required;
> > does that mean that any other future extensions are necessarily
> > comprehension-optional, and there can be no additional
> > comprehension-required keys?
> 
> [Med] Future extensions can be either comprehension-required or comprehension-optional. This will depend on each use case. 
> 
> > 
> > It's probably appropriate to make a statement in the initial text here
> > about the port number and/or media type indicating that this is the
> > syntax used for the top-level object.
> > 
> > I already mentioned that the encoding of the integer form of the key
> > values takes a different number of bytes depending on the integer value,
> > so I think those keys are rearranged in the -26 (I reviewed the -25).
> 
> [Med] I guess Tiru has already addressed that comment. 
> 
> > 
> > Section 7.1
> > 
> >    A key challenge to deploying DOTS is the provisioning of DOTS
> >    clients, including the distribution of keying material to DOTS
> >    clients to enable the required mutual authentication of DOTS agents.
> >    EST defines a method of certificate enrollment by which domains
> > 
> > I think we need a reference for EST.
> 
> [Med] Done. Thanks.
> 
> > 
> >    o  TLS False Start [RFC7918] which reduces round-trips by allowing
> >       the TLS second flight of messages (ChangeCipherSpec) to also
> >       contain the DOTS signal.
> > [...]
> >    o  TCP Fast Open [RFC7413] can reduce the number of round-trips to
> >       convey DOTS signal channel message.
> > 
> > These are in something of a specification limbo, as they both appear to
> > only be formally defined for use with TLS, but TLS False Start seems to
> > have a straightforward extension to DTLS.  It may be worth tweaking the
> > text a little to acknowledge the situation here.
> 
> [Med] Can you please indicate which tweaking do you have in mind? 
> 
> TFO allows to send data in the SYN without waiting for the completion of the 3WHS. Obviously, such delay is specific to TCP. 

My suggestions, which you can tweak or ignore as you like while waiting for
IETF LC review:

"TLS False Start is formally defined for use with DTLS, but the same
techniques are applicable to DTLS as well."

"Being TCP-specific, TCP Fast Open is only suitable for TLS and not DTLS."

-Ben