Re: [Dots] AD review of draft-ietf-dots-rfc8782-bis-04

[Benjamin Kaduk <[e-mail]>]

Hi Med,

On Tue, Mar 02, 2021 at 02:41:38PM +0000, [e-mail] wrote:
> Hi Ben, 
> 
> Thank you for the review. 
> 
> A diff is available at https://www.ietf.org/rfcdiff?url1=draft-ietf-dots-rfc8782-bis&url2=https://raw.githubusercontent.com/boucadair/rfc8782-yang-update/master/draft-ietf-dots-rfc8782-bis.txt 
> 
> Also updated 7049 to 8949 (and sections).

Thanks!
It is okay (but not required) to submit a -05 with the pending changes once
draft submission reopens.

> Please see inline. 
> 
> Cheers,
> Med
> 
> > -----Message d'origine-----
> > De : Dots [mailto:[e-mail]] De la part de Benjamin Kaduk
> > Envoyé : lundi 1 mars 2021 23:08
> > À : [e-mail]
> > Cc : [e-mail]
> > Objet : [Dots] AD review of draft-ietf-dots-rfc8782-bis-04
> > 
> > Hi all,
> > 
> > Sorry to have taken so long on this one -- I opened it up a couple
> > times thinking "I'll just look at the diff from 8782; it'll be easy",
> > but then halfway in I realized I did actually want to give some
> > careful attention to the YANG changes and ran out of time.
> > 
> > The good news is that it's generally in good shape -- I'll request
> > the IETF LC (it will be extended due to the IETF 110 overlap) and the
> > following comments can be considered along with the last-call
> > comments.
> > 
> > Thanks for the good work!
> > 
> > -Ben
> > 
> > 
> > We'll have to bump the copyright year in the YANG because I was too
> > slow at processing this :(
> > 
> > Should we say anything about what an implementation should do if it
> > receives cuid/cdid/mid/sid in the payload body (e.g., ignore it or
> > check for consistency with the received URI-Path)?
> 
> [Med] We do already have the following: 
> 
>    'cuid' and 'mid' MUST NOT appear in the PUT request message body
>    (Figure 6).
> 
> and 
> 
>    If the request is missing a mandatory attribute, does not include
>    'cuid' or 'mid' Uri-Path options, includes multiple 'scope'
>    parameters, or contains invalid or unknown parameters, the DOTS
>                   ^^^^^^^^^^^^^^^^
>    server MUST reply with 4.00 (Bad Request).

Ah, I see now.  Should this be mentioned in the backwards-compatibility
considerations section as something that would need to be relaxed for an
8782bis server to accept requests from an 8782 client?

> > 
> > Section 3.1
> > 
> >    same CBOR key value and CBOR major type.  The only upgrade that is
> >    required to [RFC8782] implementations is to handle the CBOR key
> > value
> >    range "128-255" as comprehension-optional instead of
> > comprehension-
> >    required.  Note that this range is anticipated to be used by the
> > DOTS
> >    telemetry [I-D.ietf-dots-telemetry] in which the following means
> > are
> >    supported to avoid that a DOTS signal channel message enriched
> > with
> >    telemetry data will exacerbate message failure: (1) DOTS agents
> > use
> > 
> > nit: I suggest s/are supported to avoid that a DOTS signal channel
> > message enriched with telemetry data will exacerbate message
> > failure/are used to prevent message processing failure of a DOTS
> > signal channel message enriched with telemetry data/ -- the "avoid"
> > and "failure" are quite far apart in the original, which can be hard
> > to process.
> > ^
> 
> [Med] Fixed. 
> 
> > Section 5.3
> > 
> >            container conflict-information {
> >              description
> >                "Indicates that a conflict is detected.
> >                 Must only be used for responses.";
> > 
> > The "must only be used for responses" is mechanically redundant with
> > the "direction" choice, now, so it should probably be removed.
> > 
> 
> [Med] Agree. 
> 
> >                list acl-list {
> >                  when "../../conflict-cause ="
> >                     + " 'conflict-with-acceptlist'";
> >                  key "acl-name";
> >                  description
> >                    "List of conflicting ACLs as defined in the DOTS
> > data
> >                     channel.  These ACLs are uniquely defined by
> >                     cuid and acl-name.";
> > 
> > Now that cuid is moved out of the structure and into the Uri-Path
> > option, we have a new opportunity for separability of payload and
> > metadata. The security considerations should discuss what prevents a
> > "slice and dice" attack that uses a given payload on a different
> > connection or in a different context such that it is interprted in
> > the context of a different cuid.  (DTLS ought to be enough, assuming
> > a request or response does not get split across datagrams.)
> 
> [Med] Not sure if there is much to add because this falls under this text: 
> 
>    If the 'cuid' is guessable, a misbehaving DOTS client from within the
>    client's domain can use the 'cuid' of another DOTS client of the
>    domain to delete or alter active mitigations.  For this attack vector
>    to happen, the misbehaving client needs to pass the security
>    validation checks by the DOTS server, and eventually the checks of a
>    client-domain DOTS gateway.
> 
>    A similar attack can be achieved by a compromised DOTS client that
>    can sniff the TLS 1.2 handshake, use the client certificate to
>    identify the 'cuid' used by another DOTS client.  This attack is not
>    possible if algorithms such as version 4 Universally Unique
>    IDentifiers (UUIDs) in Section 4.4 of [RFC4122] are used to generate
>    the 'cuid' because such UUIDs are not a deterministic function of the
>    client certificate.  Likewise, this attack is not possible with TLS
>    1.3 because most of the TLS handshake is encrypted and the client
>    certificate is not visible to eavesdroppers.

I am not sure that this quite covers the class of scenario I had in mind.
Consider an on-path attacker that wants to rewrite a legitimate request
from a real client.  The 'cuid' in the request would be the valid one
corresponding to the real client, but a request payload from a different
connection would be substituted in.

My understanding is that this doesn't work because we use channel security,
so the attacker cannot modify just part of the data payload or cause (part
of) a message from one connection to be interpreted as part of the other
connection.  I was wondering if we should say that we rely on the transport
security to keep the Uri-Path metadata and the encoded YANG structure
tightly bound together.  Though, now that I say that ... it might be a
little silly to call that out explicitly.

> > 
> > (Likewise for 'sid' no longer in the signal-config messages.)
> > 
> > 
> > redirected-signal/alt-server is now an inet:domain-name but 8782 has
> > it as "string".  AFAICT that does not result in an encoding change
> > but please confirm.
> 
> [Med] I confirm. 

Thanks!

> > 
> > 
> >    sx:structure dots-signal {
> >      description
> >        "Main structure for DOTS signal message.
> > 
> >         A DOTS signal message can be a mitigation, a configuration,
> >         or a redirected signal message.";
> > 
> > Should this description list "heartbeat" as well?
> 
> [Med] Yes. This will echo 5.1:
> 
>    "A DOTS signal message can be
>    a mitigation, a configuration, a redirect, or a heartbeat message."
> 
> > 
> > Section 7.1
> > 
> > Will whichever author noticed please file an errata report against
> > RFC
> > 8782 for the random "i" in the middle of "The Server Name Indication
> > (SNI) extension [RFC6066] i defines a mechanism"?  I'd be happy to
> > mark it verified.
> > 
> 
> [Med] Done. 

And verified.

Thanks again,

Ben

> > Section 9
> > 
> >    The following list of common CoAP errors that are implemented by
> > DOTS
> >    servers.  This list is not exhaustive, other errors defined by
> > CoAP
> >    and associated RFCs may be applicable.
> > 
> > nit: comma splice
> > 
> 
> [Med] Changed to "... exhaustive; other ..."
> 
> >    o  4.15 (Unsupported Content-Format) is returned by the DOTS
> > server
> >       when the Content-Format used in the request is not formatted as
> >       "application/dots+cbor" (Section 4).
> > 
> > nit: I think either the Content-Format used "is not"
> > application/dots+cbor, or "the request is not formatted as"
> > application/dots+cbor.  The current formulation seems to say that the
> > Content-Format itself is formatted, which is weird.
> > 
> 
> [Med] Fixed.
> 
> > Section 10.6.1.1
> > 
> > We recently had draft-ietf-mpls-lsp-ping-registries-update come
> > through to, among other things, change some ranges from "private use"
> > to "first come, first served", since in the type of deployment they
> > have the "private use" could not really remain private.  That is
> > probably less of a concern for DOTS, but I mention it just in case it
> > is relevant.
> > 
> > _______________________________________________
> > Dots mailing list
> > [e-mail]
> > https://www.ietf.org/mailman/listinfo/dots
> 
> _________________________________________________________________________________________________________________________
> 
> Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
> pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
> a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
> Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.
> 
> This message and its attachments may contain confidential or privileged information that may be protected by law;
> they should not be distributed, used or copied without authorisation.
> If you have received this email in error, please notify the sender and delete this message and its attachments.
> As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
> Thank you.
>