Re: [Netconf] Alexey Melnikov's Discuss on draft-ietf-netconf-zerotouch-25: (with DISCUSS and COMMENT)

[Alexey Melnikov <[e-mail]>]

Hi Kent,

On Mon, Dec 10, 2018, at 11:03 PM, Kent Watsen wrote:
> Hi Alexey,
> 
> Thanks for your review!
> See below for responses.
> 
> 
> Kent // coauthor
> 
> 
> 
> > ----------------------------------------------------------------------
> > DISCUSS:
> > ----------------------------------------------------------------------
> >
> > Thank you for a well written document, it was a pleasure to read.
> > I have a small list of issues that I would like to discuss before
> > recommending approval of this document:
> >
> >
> > In Section 5.3:
> >
> >   If the zero touch information artifact contains redirect information,
> >   the device MUST, within limits of how many recursive loops the device
> >   allows, process the redirect information as described in Section 5.5.
> >   This is the recursion step, it will cause the device to reenter this
> >   algorithm, but this time the data source will definitely be a
> >   bootstrap server, as that is all redirect information is able to
> >   redirect a device to.
> >
> > I think you need to specify a "max redirect" value in order to
> > prevent intentional or unintentional misconfigurations. Without
> > such limit it is trivial to introduce denial-of-service attack
> > on naive device implementations.
> 
> There is a Security Consideration on this very point, where is says:
> 
>    Implementations SHOULD limit the maximum number of recursive
>    redirects allowed; no more than a half dozen seems reasonable.

I think this is weaker than I would like. I also didn't read the second part (half dozen) as normative. So my suggestion is below.

> Here's direct link:
>   https://tools.ietf.org/html/draft-ietf-netconf-zerotouch-25#section-9.9
> 
> Is this now acceptable as is?  If not, would it be suffice to 
> add a reference the Security Consideration?  If not, then, yes,
> a max-redirect value could be defined, but it would have to be 
> somewhat high (25?) so that the standard doesn't prematurely
> limit some unforeseen use.  Please advise.

I think I prefer something like this:

    Implementations MUST limit the maximum number of recursive
    redirects allowed; the maximum number of recursive
    redirects allowed SHOULD be 25.

And adding a reference to the Security Considerations.

I.e., I think imposing a limit should be a MUST with the value 25 recommended.

> > 2)
> > 10.3.  The SMI Security for S/MIME CMS Content Type Registry
> >
> >   IANA is kindly requested to add two entries in the "SMI Security for
> >   S/MIME CMS Content Type" registry (1.2.840.113549.1.9.16.1), with
> >   values as follows:
> >
> >   Decimal  Description                             References
> >   -------  --------------------------------------  ----------
> >   TBD1      id-ct-zerotouchInformationXML          [RFCXXXX]
> >   TBD2      id-ct-zerotouchInformationJSON         [RFCXXXX]
> >
> >   id-ct-zerotouchInformationXML indicates that the "zerotouch-
> >   information" is encoded using XML.  id-ct-zerotouchInformationJSON
> >   indicates that the "zerotouch-information" is encoded using JSON.
> >
> > You define these values, but they are not used anywhere in the document.
> 
> Aye, you're right!  At least, they're not referenced by name, which
> complicates searching for them...  (see my next response for more)
> 
> 
> 
> > It looks like you intended for this to be used in several places,
> > for example:
> >
> > 3.1.  Zero Touch Information
> >
> >   When the zero touch information artifact is unsigned, as it might be
> >   when communicated over trusted channels, the CMS structure's top-most
> >   content type MUST be one of the OIDs described in Section 10.3, or
> >   the OID id-data (1.2.840.113549.1.7.1), in which case the encoding
> >   (JSON, XML, etc.)  SHOULD be communicated externally.  In either
> >   case, the associated content is an octet string containing
> >   "zerotouch-information" data in the expected encoding.
> >
> > Did you intend to use the above OIDs here?
> 
> Yes, exactly, and also in two other places in Section 3.1.  In
> all three instances, we could either:
> 
>   OLD:
>     ...one of the OIDs described in Section 10.3,
> 
>   NEW1:
>     ...one of the OIDs described in Section 10.3 (i.e., 
>     id-ct-zerotouchInformationXML or id-ct-zerotouchInformationJSON),
> 
> or
> 
>   NEW2:
>     ...one of the OIDs described in Section 10.3 (i.e., 
>     1.2.840.113549.1.9.16.1.TBD1 or 1.2.840.113549.1.9.16.1.TBD2),
> 
> Do you have a preference?

I think NEW1 is clearer.

> > ----------------------------------------------------------------------
> > COMMENT:
> > ----------------------------------------------------------------------
> >
> > 4.2.  DNS Server
> >
> >   To use a DNS server as a source of bootstrapping data, a device MAY
> >   perform a multicast DNS [RFC6762] query searching for the service
> >   "_zerotouch._tcp.local.".  Alternatively the device MAY perform DNS-
> >   SD [RFC6763] via normal DNS operation, using the domain returned to
> >   it from the DHCP server; for example, searching for the service
> >   "_zerotouch._tcp.example.com".
> >
> > I agree with Adam that "zerotouch" much be a registered as service 
> > name, see https://www.iana.org/assignments/service-names-port-numbers\ /service-names-port-numbers.xhtml.
> 
> Agreed, and I responded to Adam previously as follows:
> 
>   ===start===
>   You are quite right.  Here is the Github commit to add an IANA 
> registration
>   for the "zerotouch" service:  
> https://github.com/netconf-wg/zero-touch/commit/6280763339a3e91b9efb21703b26a6bdbf05349b.

This looks reasonable. (Note, saying that this is an extension of HTTPS in a note doesn't help. I think it would be better to say "this protocol uses HTTPS as a substrate". The current text would make people wonder why you need to register a new service name.)
 
>   Please let me know if you suggest any further changes.  
>   ===stop===
> 
> And likewise, Alexey, please also let me know if you suggest any 
> further changes.  
> 
> 
> 
> 
> >   Artifact to Resource Record Mapping:
> >
> >      Zero Touch Information:  Mapped to a TXT record called "zt-info"
> >        containing the base64-encoding of the binary artifact described
> >         in Section 3.1.
> >
> >      Owner Certificate:  Mapped to a TXT record called "zt-cert"
> >         containing the base64-encoding of the binary artifact described
> >         in Section 3.2.
> >
> >      Ownership Voucher:  Mapped to a TXT record called "zt-voucher"
> >         containing the base64-encoding of the binary artifact described
> >         in Section 3.3.
> >
> > So just to double check, these would be zt-info._zerotouch._tcp.example.com,
> > etc?
> 
> I'm not a DNS expert, but my understanding is that lookup would be on, 
> e.g., _zerotouch._tcp.example.com and that the TXT records would be 
> returned as follows:
> 
> _zerotouch._tcp.example.com.  3600  IN  TXT  "zt_info=base64encodedvalue=="
> _zerotouch._tcp.example.com.  3600  IN  TXT  "zt_cert=base64encodedvalue=="
> _zerotouch._tcp.example.com.  3600  IN  TXT  "zt_voucher=base64encodedvalue=="

This is not what your text is saying. I also don't think that your description is correct. "TXT record called X" is not something defined. You need to define TXT record which can contain attribute=value syntax and allowed attributes are one of 3 you define.

> For instance ['\' added for readability]:
> 
>   # dig txt gab.com
>   <snip/>
>   ;; ANSWER SECTION:
>   gab.com.		300	IN	TXT	"v=spf1 include:_spf.protonmail.ch mx ~all"
>   gab.com.		300	IN	TXT	"ca3-56e3739f7f644cdaaa0e7edc903cf648"
>   gab.com.		300	IN	TXT	"protonmail-verification=\
>                                   96ca5019e36fc3a6dc5c4399efabe68792e55cc7"
>   <snip/>
> 
> Do you feel that an example in the document is warranted?

Definitely you should add some examples. 

> > 8.1.  DHCPv4 Zero Touch Option
> >
> >      o bootstrap-server-list: A list of servers for the
> >         client to attempt contacting, in order to obtain
> >         further bootstrapping data, in the format shown
> >         in [common-field-encoding].
> >
> > [common-field-encoding] in this and subsequent sections looks like a
> > reference to Section 8.3. Please fix before publication, as this looks
> > like an undefined reference.
> 
> Good grief, fixed (in both locations)!
> 
>   s/[common-field-encoding]/Section 8.3/
> 
> 
> 
> > 8.3.  Common Field Encoding
> >
> >   Both of the DHCPv4 and DHCPv6 options defined in this section encode
> >   a list of bootstrap server URIs.  The "URI" structure is an option
> >   that can contain multiple URIs (see [RFC7227], Section 5.7).
> >
> >     bootstrap-server-list:
> >
> > This is confusing, because I believe the following is a single entry
> > in the list, not the whole list syntax:
> >
> >     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-...-+-+-+-+-+-+-+
> >     |       uri-length              |          URI                  |
> >     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-...-+-+-+-+-+-+-+
> >
> >     o uri-length: 2 octets long, specifies the length of the URI data.
> >
> >     o URI: URI of zerotouch bootstrap server, using the HTTPS URI
> >       scheme defined in Section 2.7.2 of RFC7230.  URI MUST be in
> >       form "https://<ip-address-or-hostname>[:<port>]".
> >
> > So if I am correct above, please clarify this by changing
> > "bootstrap-server-list:" to "bootstrap-server-list is a list of 1 or more
> > items, each with the following syntax:" (Or something like this.)
> >
> > Also, "URI" deserve to be a Normative Reference, as it defines the
> > generic syntax you are referring to.
> 
> 
> I have ask my coauthor, Ian, to respond to this comment.

Ok.

Best Regards,
Alexey