Re: [Dots] AD review of draft-ietf-dots-data-channel-25

["Konda, Tirumaleswar Reddy" <[e-mail]>]

Please see inline 

> -----Original Message-----
> From: Dots <[e-mail]> On Behalf Of Benjamin Kaduk
> Sent: Friday, February 15, 2019 12:47 AM
> To: [e-mail]
> Cc: [e-mail]; [e-mail]
> Subject: Re: [Dots] AD review of draft-ietf-dots-data-channel-25
> 
> This email originated from outside of the organization. Do not click links or
> open attachments unless you recognize the sender and know the content is safe.
> 
> Hi Med,
> 
> On Thu, Feb 14, 2019 at 02:31:26PM +0000, [e-mail]
> wrote:
> > Hi Ben,
> >
> > Thank you for the review.
> >
> > Please see inline.
> >
> > Cheers,
> > Med
> >
> > > -----Message d'origine-----
> > > De : Benjamin Kaduk [mailto:[e-mail]] Envoyé : mercredi 13
> > > février 2019 17:46 À : [e-mail]
> > > Cc : [e-mail]
> > > Objet : AD review of draft-ietf-dots-data-channel-25
> > >
> > > This is my AD review of the -25
> > >
> > > I see that Med made a commit on github that preemptively addressed
> > > at least one of these comments, but will trust the authors to do to
> > > the right thing with anything in here that's stale.
> > >
> > > A handful of generic and/or high-level comments before the
> > > section-by-section nitty-gritty stuff.
> > >
> > > The author count is large (7); RFC 7322 notes that the stream
> > > approver (i.e., the IESG) will ask questions if the count is more
> > > than five.  What should I tell people when they ask?  (The authors
> > > are listed also in the YANG module itself, if changes are made.)
> >
> > [Med] Actually, the set of co-authors of the YANG module is not exactly the
> same.
> 
> Whoops, my bad for not checking.
> 
> > Anyway, in order to comply with the rules and avoid spending extra cycles on
> this, we added a new section called "Contributing Authors". Nevertheless, the
> set of authors is not modified in the YANG module.
> 
> Thanks.  (To be clear, I'm happy to go to bat for everyone with the IESG for this
> sort of thing, I just need an argument to present that's not me making things
> up.)
> 
> I don't know of a specific policy for YANG module authorship, so that part
> should be okay as far as I know.
> 
> > >
> > > Can someone (the shepherd?) confirm that an automated syntax checker
> > > has run over the JSON in examples?
> > >
> > > The concept of "DOTS client domain" is being used for actual
> > > protocol purposes here (most notably as a bound on the prefix(es)
> > > that a client can request mitigation for, but I don't remember
> > > seeing a formal definition for how any DOTS actor would know the specific
> bounds of such a client domain.
> > > Is there text somewhere that I missed that we can point to, or will
> > > we need to add some?
> >
> > [Med] Both "DOTS client domain" and "DOTS server domain" are used in the
> architecture draft. "DOTS client's domain" and "DOTS server's domain" are also
> used in the architecture and requirements I-D.
> >
> > If a formal definition is needed, I prefer this to be done in the architecture or
> the requirements I-D.
> 
> I think it would be a somewhat better fit in the architecture document, just to
> note that the "domain" is something that can concretely be demarcated by IP
> addresses/prefixes (as opposed to a more nebulous concept to aid conceptual
> understanding).

DOTS client domain is already explained in Section 2.2.2 of the architecture draft as follows:

<snip>
   The DOTS server enforces authorization of DOTS clients' signals for
   mitigation.  The mechanism of enforcement is not in scope for this
   document, but is expected to restrict requested mitigation scope to
   addresses, prefixes, and/or services owned by the DOTS client's
   administrative domain, such that a DOTS client from one domain is not
   able to influence the network path to another domain.
</snip>

We can add the following line to explain how the DOTS server identifies the DOTS client domain at a protocol level:

DOTS server can identify the DOTS client domain either using the 'cdid'
parameter or the client's DNS name specified in the Subject 
Alternative Name extension's dNSName type or SRV-ID in the client
certificate.

Cheers,
-Tiru

> 
> > >
> > > We don't say much about what validation the server does on input
> > > data, and we probably should.  For example, does the server need to
> validate 'cuid'
> >
> > [Med] We avoided to be redundant here. This is covered by this text:
> >
> > "This attribute has the same
> >       meaning, syntax, and processing rules as the 'cuid' attribute
> >       defined in [I-D.ietf-dots-signal-channel]."
> >
> > > and/or 'cdid' in dots-client-creation requests?
> >
> > [Med] This is covered by this text:
> >
> > "This attribute has the same meaning, syntax, and processing
> >       rules as the 'cdid' attribute defined in
> >       [I-D.ietf-dots-signal-channel]"
> 
> I guess I'm mostly just concerned about if there's anything that doesn't
> translate directly, since the CoAP and HTTP constructs would have to describe
> the formal validation in somewhat different terms (i.e., for consistency
> between URL paths and message bodies).  But in general I'm happy to avoid
> redundancy as you desire.
> 
> > And other parts in the text such as:
> >
> >    If the request is received via a server-domain DOTS gateway, but the
> >    DOTS server does not maintain a 'cdid' for this 'cuid' while a 'cdid'
> >    is expected to be supplied, the DOTS server MUST reply with "403
> >    Forbidden" status-line and the error-tag "access-denied".  Upon
> >    receipt of this message, the DOTS client MUST register (Section 5).
> >
> >
> >   What about validating that
> > > the (e.g.) ACL name in the PUT URL matching the name in the body of
> > > the request?
> >
> > [Med] Those are supposed to be covered following RESTCONF base spec.
> 
> Is this supposed to be RFC 8040 Section 3.5.3?  I am not sure that I'm reading
> that as covering the property I want (and will double-check if you confirm that's
> what you have in mind).
> 
> >   There are probably others as well.
> > >
> > > The examples all use "Host: {host}:{port}" which is not really an
> > > example but rather a template.  Since they are supposed to be
> > > examples, we should pick a concrete hostname (and port) to use.
> >
> > [Med] I will change some figures to "example.com" but will leave it for
> schema-like figures.
> 
> Of course.
> 
> > >
> > > There is, shall we say, a "high degree of overlap" between Sections
> > > 5/6/7 and the YANG model field descriptions.  I mostly assume that
> > > the WG considered letting the YANG model (and the core RESTCONF
> > > spec) stand alone without the additional examples/specification of
> > > the use of RESTCONF to manage clients, aliases, and filter rules, so
> > > I won't suggest that we revisit the question.  But I do think that
> > > it would be good to have some explicit text acknowledging the
> > > overlap and stating which one is authoritative.
> >
> > [Med] Fixed.
> >
> > >
> > > There seems to be some mismatch between whether the IPv6 ACL subtree
> > > uses "ttl" or "hoplimit" -- Figure 7 has "ttl" but the rest of the
> > > document seems to (properly) use "hoplimit".  Someone else should
> > > double-check the relevant places, though, as I'm not sure I was
> > > looking at all of them with this issue in mind.
> >
> > [Med] Both are correct.
> >
> > Figure 7 reuses draft-ietf-netmod-acl-model which defines TTL as :
> >
> >     leaf ttl {
> >       type uint8;
> >       description
> >         "This field indicates the maximum time the datagram is allowed
> >          to remain in the internet system.  If this field contains the
> >          value zero, then the datagram must be dropped.
> >
> >          In IPv6, this field is known as the Hop Limit.";
> >       reference
> >         "RFC 791: Internet Protocol,
> >          RFC 8200: Internet Protocol, Version 6 (IPv6) Specification.";
> >     }
> >
> > For the other figures, the fields are defined in the data-channel draft.
> 
> Ah, thanks for the pointer.
> 
> > >
> > > I'm also a bit curious about the use of an explicit "capabilities"
> > > tree for fine-grained feature detection, as opposed to native YANG
> "feature"s.
> >
> > [Med] These are not serving the same purpose. Features are about parts of a
> module which are conditional, if you will. Our "capabilities" branch is meant to
> retrieve the filtering match capabilities are supported/enabled by a DOTS
> server. That information is used by a client to tweak its requests.
> >
> >   The
> > > latter would allow the relevant nodes to just not exist when
> > > unsupported,
> >
> > [Med] A filtering capability may be supported but not enabled. The server is
> free to include an explicit field with the appropriate status or not. This is
> implementation-specific.
> 
> Thanks for the explanation.
> 
> > > as opposed to the explicit-capabilities formulation where they exist
> > > but are (presumably) ignored.  (I don't remember us explictily
> > > saying that they're ignored in this case, either; might be worth
> > > adding some text.)
> > >
> > > In a similar vein, we include 'capabilities' nodes for a few
> > > matching features that are listed as "mandatory fields" for ACL
> > > matching in Table 1, and thus whose value would always be "true".
> > > Why do we need the capability leaves in such a case?
> >
> > [Med] I guess you are referring to Figure 23 which says the following:
> >
> >    Figure 23 shows an example of a response received from a DOTS server
> >    which only supports the mandatory filtering criteria listed in
> >
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >    Section 4.1.
> 
> Correct.
> 
> > The module allows to return other capabilities than those listed in table 1.
> 
> Yes.  But the figure should not claim to be an example that *only* supports the
> mandatory parts, when it shows an example that supports additional
> capabilities than the mandatory ones.
> 
> > >
> > > idnits notes a few references that can be updated along with the
> > > other changes; it also flags a "reference in abstract" for the RFC
> > > Editor note which we could probably ignore (but could probably also
> > > just remove the brackets around the text in question).
> >
> > [Med] idnits confuses the note to the RFC editor with the abstract. Fixed,
> anyway.
> >
> > >
> > > I also looked at the references (especially the
> > > normative/informative
> > > split) and have a few suggestions:
> > >
> > > - the IEEE.754.1985 reference is not needed;
> >
> > [Med] Agree, but it is not harmful to cite it either :-)
> 
> My personal opinion is that it *is* harmful, since we are not using a floating-
> point wire encoding; we're using a string encoding.
> 
> > The reference is quoted to justify why we went with decimal64, not uint64 for
> example + why that unit is chosen. This allows to ease grafting DOTS with BGP
> Flowspec for instance, which cites IEEE.754.1985 too.
> 
> (RFC 5575 seems to claim to be using the actual binary floating-point
> encoding.)
> 
> >  we don't use the binary
> > >   floating-point encoding but rather a string one for YANG decimal64
> > > - I think that RFC 8499 (dnsop-terminology-bis) can wholly supersede our
> > >   usage of RFC 1983, so the 1983 cite can be dropped as well
> >
> > [Med] Done.
> >
> > > - draft-ietf-dots-requirements (for terminology),
> >
> > [Med] For this one, we are following the same approach as for other
> terminology documents (e.g., RFC8340) that are listed as informative. I prefer
> to leave it as informative for now.
> 
> Okay, we can see if anyone else complains.
> 
> >  RFC 7950, and RFC 8259
> >
> > [Med] OK for these two.
> >
> > >   should probably all move to the normative section
> > >
> > > Section 1
> > >
> > > The sub-bullet point about "If a network resource ... informs its
> > > servicing DOTS gateway of all suspect IP addresses that need to be
> > > drop- or accept-listed ..." made me wonder if that was more of a
> > > signal-channel activity or a data-channel one.  Perhaps this is just
> > > a matter of the text not being as clear as it could be.
> >
> > [Med] The point is that a client is not sure that an attack is active and
> targeting the client domain but it wants to enforce some preventive actions
> during an investigation period. For example, this can be triggered by an
> administrator who is informed that an attack is currently targeting other
> networks, but its network is likely to be subject to that attack too. Other
> preventive actions which require further investigation may be considered.
> >
> > I changed the text as follows:
> >
> > OLD:
> >   detects a potential DDoS attack from
> >
> > NEW
> >    is informed about a potential DDoS attack from
> >
> >  (I also wonder if we should say
> > > "further investigation" since we don't really have an automated way
> > > to indicate that yet.)
> 
> On further reflection, would "further manual investigation" be appropriate?
> 
> > > Section 2
> > >
> > > When we talk about tree diagrams, should we also say something like
> > > "Note that the full module's tree has been split across several
> > > figures to aid the exposition of the various sub-trees"?
> >
> > [Med] Done. Thanks.
> >
> > >
> > > Section 3.1
> > >
> > > When we talk about using GET to retrieve running configuration, do
> > > we want to note that since the data channel can fail during attack
> > > time, it's expected to be common to perform such a GET before
> > > attempting to make modifications to configuration?
> >
> > [Med] The data channel is supposed to be invoked when no attack is ongoing.
> Normal RESTCONF operations are therefore followed. I don't see the need to
> add a note.
> 
> I always have to consider edge cases in my IESG reviews -- what happens if an
> attack starts after the request is sent but before the response is received?
> 
> > >
> > >    A DOTS client registers itself to its DOTS server(s) in order to set
> > >    up DOTS data channel-related configuration data and receive state
> > >    data (i.e., non-configuration data) from the DOTS server(s)
> > >    (Section 5).  Mutual authentication and coupling of signal and data
> > >    channels are specified in [I-D.ietf-dots-signal-channel].
> > >
> > > I think we should split the "mutual authentication" and "coupling of
> > > signal and data channels" into their own sentence, and flesh them
> > > out slightly more.  So, section references to Sections 8 and 4.4.1,
> > > respectively, the usage of TLS client certificates, the coupling of
> > > channels via the client's identity ('cuid' and 'cdid'), etc.
> >
> > [Med] Done.
> >
> > >
> > >                                   A TLS heartbeat [RFC6520] verifies
> > >    that the DOTS server still has TLS state by returning a TLS message.
> > >
> > > I expect this will get some pointed comments during IETF LC, given
> > > the recent-ish IETF-wide discussions about heartbeats/keepalives in general.
> > > Is there perhaps a RESTCONF-level probe message that could be used
> > > to check liveliness; a capabilities query perhaps?
> >
> > [Med] There is no such mechanism. The approach in the data channel draft is
> aligned with RFC8071 for keepalives.
> 
> I guess we'll just have to wait to see what kind of comments we get, then.
> (Thanks for the pointer to 8071.)
> 
> > >
> > >    A DOTS server may detect conflicting filtering requests from distinct
> > >    DOTS clients which belong to the same domain.  For example, a DOTS
> > >    client could request to drop-list a prefix by specifying the source
> > >    prefix, while another DOTS client could request to accept-list that
> > >    same source prefix, but both having the same destination prefix.  It
> > >    is out of scope of this specification to recommend the behavior to
> > >    follow for handling conflicting requests (e.g., reject all, reject
> > >    the new request, notify an administrator for validation).  DOTS
> > >    servers SHOULD support a configuration parameter to indicate the
> > >    behavior to follow when a conflict is detected.  Section 7.2
> > >    specifies the behavior when no instruction is supplied to a DOTS
> > >    server.
> > >
> > > I'm a bit confused by the "out of scope of this specification to
> > > recommend the behavior to follow for handling conflicting requests",
> > > since not only does the last sentence of the paragraph give a
> > > pointer to a specified behavior in case of conflicts, but we also
> > > mention it in a couple other places (e.g., the bottom of page 43).
> >
> > [Med] The specified behavior is a default behavior, not a recommended one.
> 
> In effect a default is a recommendation, though -- a recommendation for what
> to do if you don't know any better.  Perhaps "it is out of scope of this
> specification to provide guidance on how conflicting requests may be safely
> handled, with the default behavior being to reject such requests"?
> 
> > I update this text:
> >
> >    If the request is conflicting with an existing filtering installed by
> >    another DOTS client of the domain, and absent any local policy, the
> >                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> That helps (maybe the "and" is not even needed?).  Do you want to do a sweep
> for other places where the text talks about 409 responses?
> 
> >    DOTS server returns "409 Conflict" status-line to the requesting DOTS
> >    client.  The error-tag "resource-denied" is used in this case.
> >
> > >
> > > Also in that paragraph, it's unclear that a 2119 SHOULD is
> > > appropriate for "support a configuration parameter"; there's no
> > > interoperability considerations for having or not having such a configuration
> knob.
> >
> > [Med] This is important for interoperability (or at least for ensuring a
> deterministic behavior). E.g., during service subscription a technical clause may
> be negotiated out of band how to deal with conflicts from clients of the same
> domain.
> 
> It seems that the default behavior of disallowing conflicts would always be
> interoperable, but I'm happy to leave this as-is for now.
> 
> > >
> > > Section 3.3 NAT Considerations have "high overlap" with the text at
> > > the end of the signal-channel's "Design Overview".  At a minimum we
> > > should diff them and enforce convergence, but do we want to consider
> > > just having one refer to the other?
> >
> > [Med] Makes sense. Section 3.3 is now deleted and replaced with this NEW
> text:
> >
> >    NAT considerations for the DOTS data channel are similar to those
> >    discussed in Section 3 of [I-D.ietf-dots-signal-channel].
> 
> Cool, thanks.
> 
> > >
> > > Section 3.5
> > >
> > > I had to read up on RESTCONF and NETCONF while reviewing this, but
> > > from what I've seen so far, the "error-severity" field is only
> > > present in NETCONF and not RESTCONF.  If so, then we shouldn't need
> > > to talk about it here, since we use RESTCONF exclusively.  I also
> > > couldn't find whether there's a registry that we should add the "loop-
> detected" error-tag to.
> > > Can anyone here help me out?
> > >
> >
> > [Med] We used the template in https://tools.ietf.org/html/rfc6241#appendix-
> A because the errors in 8040 are the ones imported from there.
> 
> Thanks for the pointer.   It sounds like I'll need to ask around about this
> one.
> 
> > There is no registry for the errors; only a YANG module which is not
> maintained by IANA. This is why no action is included in the IANA section.
> >
> > > Section 4.2
> > >
> > > Is there any plan/expectation for filtering/match rules for QUIC?
> > > It is probably premature to put anything in this document
> > > specifically, but still worth thinking about.
> >
> > [Med] Some of the existing fields can be already reused for QUIC (UDP, port
> number). There are few fields in the QUIC public header that are available
> (public flags, CID, version). A match on the first N bytes of the UDP payload can
> be considered but I do think this is not mature enough.
> >
> > The key point is that our module is extensible.
> 
> Indeed.
> 
> > >
> > > The order in which the leaves appear in the "capabilities/ipv6" and
> > > "capabilities/tcp" subtrees do not match the order they appear in
> > > the ACL subtree itself; it would be good to keep the order consistent.
> >
> > [Med] Fixed.
> >
> > FWIW, the order in capabilities/* follows the order of the fields as they
> appear in the header. We don't have a control on the ACL order because we
> are reusing an external module.
> 
> Ah, good to know.
> 
> > >
> > > We don't really say much about what the semantis of the "port-range"
> > > capabilities are; is that supposed to indicate any port-matching
> > > ability at all, or specifically the low-to-high range matching, or also the
> "operator"
> > > options?
> >
> > [Med] Updated as follows:
> >
> > OLD:
> >               "Support of filtering based on a port range.";
> >
> > NEW:
> >
> >              "Support of filtering based on a port range.
> >
> >               This includes filtering based on a source port range,
> >               destination port range, or both. All operators
> >               (i.e, less than or equal, greater than or equal, equal to,
> >               and not equal to) are supported.";
> 
> Thanks!
> 
> > >
> > > Section 4.3
> > >
> > > We define an "operator" typedef that is rather different from that
> > > from netmod-acl-model.  Do we want to use a different name to aid
> > > disambiguation?  ("bitmask-operator" comes to mind as an option.)
> >
> > [Med] It is not recommended in yang to use prefixes to disambiguate nodes..
> The disambiguation is ensured by the context/position in the tree. For example,
> this is why we are using name and not acl-name or ace-name, etc. I will leave
> the text as it is.
> 
> For names this makes natural sense to me, but this question is about a typedef.
> I have less clear of a picture for how typedefs are or are not namespaced (is it
> just per-module?).
> 
> > >
> > >     typedef fragment-type {
> > >       type bits {
> > >         bit df {
> > >           position 0;
> > >           description
> > >             "Don't fragment bit for IPv4.
> > >              This bit must be set to 0 for IPv6.";
> > >
> > > Set to zero by whom?  What should the receiver do if it is set otherwise?
> > >
> >
> > [Med] In IPv6, fragmentation is only done by the source. Hence, this value for
> IPv6. A fragment-type with the first bit set will be discarded by the server.
> 
> I was looking for a few more words in the text, like "must be set to 0 when it
> appears in an IPv6 filter".
> 
> > > What are the semantics if (either or both of target-fqdn and
> > > target-uri) and target-prefix are specified?  (I suppose maybe this
> > > could be covered in Section 6.1 when we say that at least one is
> > > required in ACL-creation
> > > requests.)
> >
> > [Med] The resulting IP prefixes/addresses will be bound to the alias. Added
> the following in Section 6.1:
> >
> >    If more target-* clauses (e.g., 'target-prefix' and 'target-fqdn')
> >    are included in a POST or PUT request, the DOTS server binds all
> >    resulting IP addresses/prefixes to the same resource.
> >
> > >
> > > The units for the "rate-limit" node should be specified in the YANG
> > > module and not in the description of how to install filtering rules.
> >
> > [Med] Added "units" to the YANG module.
> >
> > >
> > >       list dots-client {
> > >         key "cuid";
> > >         description
> > >           "List of DOTS clients.";
> > >         leaf cuid {
> > >           type string;
> > >           description
> > >             "A unique identifier that is randomly generated by
> > >              a DOTS client to prevent request collisions.";
> > >
> > > I don't think "randomly generated" is really correct here.
> >
> > [Med] Changed to:
> >
> >          "A unique identifier that is generated by a DOTS client
> >           to prevent request collisions.";
> >
> > >
> > > The "capabilities/icmp/rest-of-header" description should be more
> > > clear that (per draft-ietf-netmod-acl-model) it is supposed to match
> > > both the ICMP four-byte field and the ICMPv6 message body.
> >
> > [Med] OK.
> >
> > >
> > > Section 5.1
> > >
> > > It may be worth reiterating that (per the signal-channel doc)
> > > gateways may rewrite the 'cuid'.
> >
> > [Med] OK:
> >
> >    As a reminder, DOTS gateways may rewrite the 'cuid' used by peer DOTS
> >    clients (Section 4.4.1 of [I-D.ietf-dots-signal-channel]).
> >
> > >
> > > When POST is used to create a dots-client resource, how does the
> > > client know the path of the created resource (to be used for
> > > subsequent RESTCONF requests)?  (I assume it is supposed to just use
> > > the submitted 'cuid', but we need to explicitly say this.  This also
> > > seems to render much of the distinction between POST and PUT moot,
> > > for our usage, though I do not propose any change to the text.)
> >
> > [Med] The procedure to determine the root path is recalled in Section 3.1,
> then normal RESTCONF operation is followed.
> >
> > >
> > >    The DOTS gateway, that inserted a 'cdid' in a PUT request, MUST strip
> > >    the 'cdid' parameter in the corresponding response before forwarding
> > >    the response to the DOTS client.
> > >
> > > Why does this apply only to PUT and not POST?
> >
> > [Med] Because RFC8040 says the following:
> >
> >    If the POST method succeeds, a "201 Created" status-line is returned
> >    and there is no response message-body.
> >    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> Whoops, I clearly missed that part.  Thanks for calling it out.
> 
> > >
> > > Section 6.1
> > >
> > >    DOTS clients within the same domain can create different aliases for
> > >    the same resource.
> > >
> > > My reading of this text is that client A can create alias "foo" for
> > > IP prefix 128.0.1.5/31 and clinet B can create alias "bar" for the
> > > same IP prefix, and that DOTS supports that process.  (Just to
> > > confirm that the text is saying what it's intended to.)
> >
> > [Med] Yes.
> >
> >   I do wonder if we want to say
> > > something about whether alias names need only be unique per 'cuid'
> > > or in some more global fashion.  (Having a global uniqueness
> > > property is perhaps convenient in order to interface with non-DOTS
> > > mechanisms for querying aliases, or for the DOTS server to interact
> > > with network filtering
> > > appliances.)
> >
> > [Med] The specification does require uniqueness per cuid:
> >
> >         |  +--rw aliases
> >           |  |  +--rw alias* [name]
> >           |  |     +--rw name                 string
> >
> > We don't have a requirement for uniqueness per domain or globally.
> >
> > If such requirement arise, the semantic/logic for creating those aliases can
> be handled out of band.
> 
> Ok.
> 
> > >
> > > Figure 16 puts double-quotes around "string" in the pseudo-schema,
> > > which feels weird to me.
> >
> > [Med] This is also what we have done for other figures , e.g., 11. The intent is
> to use a scheme-like message body while trying to preserve JSON compliance.
> >
> > >
> > >    target-fqdn:   A list of Fully Qualified Domain Names (FQDNs).  An
> > >       FQDN is the full name of a resource, rather than just its
> > >       hostname.  For example, "venera" is a hostname, and
> > >       "venera.isi.edu" is an FQDN [RFC1983].  Refer to
> > >       [I-D.ietf-dnsop-terminology-bis] for more details.
> > >
> > > I don't think this text is particularly well-aligned with RFC 8499
> > > and would suggest trimming it substantially and just pointing to that RFC.
> >
> > [Med] Done.
> >
> > >
> > >    If the request is missing a mandatory attribute or its contains an
> > >    invalid or unknown parameter, "400 Bad Request" status-line MUST be
> > >    returned by the DOTS server.  The error-tag is set to "missing-
> > >    attribute", "invalid-value", or "unknown-element" as a function of
> > >    the encountered error.
> > >
> > > This text seems to preclude any future extension that adds new error
> > > tags; is this intended?
> >
> > [Med] Those errors are only about the tree failure cases called in the first
> sentence.
> >
> > >
> > > Section 7.1
> > >
> > >    A DOTS client which issued a GET request to retrieve the filtering
> > >    capabilities supported by its DOTS server, SHOULD NOT request for
> > >    filtering actions that are not supported by that DOTS server.
> > >
> > > What is the server behavior if the client ignores this SHOULD NOT?
> >
> > [Med] This is covered by this text:
> >
> >    If the request is missing a mandatory attribute or
> >    contains an invalid or unknown parameter (e.g., a match field not
> >    supported by the DOTS server), "400 Bad Request" status-line MUST be
> >    returned by the DOTS server in the response.
> 
> sounds good.
> 
> > >
> > >    Figure 23 shows an example of a response received from a DOTS server
> > >    which only supports the mandatory filtering criteria listed in
> > >    Section 4.1.
> > >
> > > This seems inaccurate, as the mandatory filtering criteria exlude
> > > the rate-limit among others.
> >
> > [Med] rate-limit is an action, not a filtering criteria.
> >
> > >
> > > Section 7.2
> > >
> > >    activation-type:  Indicates whether an ACL has to be activated
> > >       (immediately or during mitigation time) or instantiated without
> > >       being activated (deactivated).  Deactivated ACLs can be activated
> > >       using a variety of means such as manual configuration on a DOTS
> > >       server or using the DOTS data channel.
> > >
> > > Is this done by the data channel or the signal channel?
> >
> > [Med] Yes, but this is not supported in the base signal-channel spec. This is
> the done using the filtering control feature (draft-nishizuka-dots-signal-control-
> filtering). This is why signal channel is not listed after "such as".
> 
> Okay.  (I was literally just wondering if this was a typo.)
> 
> > >
> > >       If this attribute is not provided, the DOTS server MUST use
> > >       'activate-when-mitigating' as default value.
> > >
> > > Why do we specify default values here instead of in the YANG module?
> >
> > [Med] There is no default statement for enumeration. To solve this, a new
> type is defined in the module (activation-type). This type is then used for the
> activation-type leaf with a default value set to activate-when-mitigating.
> 
> That's a clever workaround!
> 
> > The change in tree diagram is (no need to insert the code here):
> >
> > OLD:
> >        |        +--rw activation-type?    Enumeration
> >
> > NEW:
> >      |        +--rw activation-type?    activation-type
> >
> >
> > >
> > >       Filters that are activated only when a mitigation is in progress
> > >       MUST be bound to the DOTS client which created the filtering rule.
> > >
> > > Other filters do not need to be bound to the DOTS client that created them?
> >
> > [Med] They are...but those filters are already enforced because they are
> immediate.
> 
> The wording here is still weird, though -- by calling out the delayed-activation
> filters it implies that immediate-activation filters are excluded from the
> statement, but we know that immediate-activation filters
> also have the same property.   So I'm not entirely sure exactly what
> information this sentence is intended to call out.
> 
> > > Wouldn't we still want to remove them when the dots-client resource
> > > in question is deleted?
> >
> > [Med] This is supposed to be done by the client itself. For amnesiac clients,
> we do have the following:
> 
> Oh.  I think I was remembering Section 5.2's "resources bound to this DOTS
> client will be deleted by the DOTS server" and assuming it applies to the filters
> associated with that client.
> 
> >    In order to avoid stale entries, a lifetime is associated with alias
> >    and filtering entries created by DOTS clients.  Also, DOTS servers
> >    may track the inactivity timeout of DOTS clients to detect stale
> >    entries.
> 
> This is probably enough to ensure safe operation without the above, though,
> hmm...
> 
> > >
> > >    destination-ipv4-network:  The destination IPv4 prefix.  DOTS servers
> > >       [...]
> > >       This is a mandatory attribute in requests with an 'activation-
> > >       type' set to 'immediate'.
> > >
> > > I somehow thought there were YANG attributes that could indicate
> > > this conditional requirement in the module itself, though I am
> > > hardly a YANG expert.
> >
> > [Med] We are reusing fields from ietf-netmod-acl, it is not easy to
> manipulate the fields as we would want.
> 
> Okay.
> 
> > >
> > >                                                  The error-tag is set to
> > >    "missing-attribute", "invalid-value", or "unknown-element" as a
> > >    function of the encountered error.
> > >
> > > Same comment as above about (non-)extensibility.
> >
> > [Med] Idem as above.
> >
> > >
> > > Section 7.3
> > >
> > > I see that the "test-acl-*" and "test-ace-*" acl and ace objects in
> > > these examples do in fact use different names for the semantically
> > > different objects, but perhaps we could help the reader's eye and
> > > use strings with a larger Hamming distance?  (I thought they were
> > > all the same on my first
> > > read.)
> >
> > [Med] Fixed.
> >
> > >
> > > (I also am a little confused at why the "ace" "name" field is
> > > considered a non-config field, in Figure 31, but this seems more
> > > likely to be my confusion than an error in the document.)
> >
> > [Med] This is because the name is the key of the ace list.
> >
> > RFC8040 says the following:
> >
> >    To retrieve only the non-configuration child resources, the "content"
> >    parameter is set to "nonconfig".  Note that configuration ancestors
> >    (if any) and list key leafs (if any) are also returned.
> 
> Thanks for the pointer.
> 
> > >
> > > Section 8
> > >
> > >    o  DOTS server MUST NOT enable both DOTS data channel and direct
> > >       configuration to avoid race conditions and inconsistent
> > >       configurations arising from simultaneous updates from multiple
> > >       sources.
> > >
> > >    o  DOTS agents SHOULD enable DOTS data channel to configure aliases
> > >       and ACLs, and only use direct configuration as a stop-gap
> > >       mechanism to test DOTS signal channel with aliases and ACLs.
> > >       Further, direct configuration SHOULD only be used when the on-path
> > >       DOTS agents are within the same domain.
> > >
> > > Doesn't all this discussion of "direct configuration" conflict with
> > > the "MUST NOT" in the first bullet point?
> >
> > [Med] The second bullet is about controlled testing of the *signal channel*
> with aliases/acls communicated by the data channel.
> 
> Whoops, my misread.
> 
> > >
> > > Section 10
> > >
> > > Generally with the security considerations template for YANG
> > > modules, we need to list out all the nodes considered sensitive and
> > > the consequences of
> > > writing(/reading) each one in turn.
> > >
> >
> > [Med] The text does already call out those that are specific to this document.
> For other nodes, we do have this text:
> >
> >    "YANG ACL-specific security
> >    considerations are discussed in [I-D.ietf-netmod-acl-model]."
> >
> > I think we are OK.
> 
> I would suggest adding a new sentence in the "All data nodes defined"
> paragraph about "This module reuses YANG structures from [I-D.ietf-netmod-
> acl-model], and the security considerations for those nodes continue to apply
> for this usage.", since that text is at the other end of the section and it's
> otherwise hard to make a linkage between them.
> 
> Thanks,
> 
> Ben
> 
> _______________________________________________
> Dots mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/dots