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

[Kent Watsen <[e-mail]>]

[adding Dave, author of the draft-ietf-dnsop-attrleaf]


Hi Ben,

  I have trimmed the below response down to just the remaining open items.


Hi Dave,

  Could you please search for "draft-ietf-dnsop-attrleaf" in this thread,
  which regards this Section 4.2 of the zerotouch draft [1], and provide
  your opinion?  In particular, since _sztp is under _tcp, does that mean
  that it is not a globally scoped entry?
 
  [1] https://tools.ietf.org/html/draft-ietf-netconf-zerotouch-27#section-4.2),


Thanks,
Kent


>> >> > ----------------------------------------------------------------------
>> >> > DISCUSS:
>> >> > ----------------------------------------------------------------------
>>
>> >> > (3) Nonce length
>> >> >
>> >> > Section 7.3 describes the nonce leaf:
>> >> >
>> >> >         leaf nonce {
>> >> >           type binary {
>> >> >             length "8..32";
>> >> >
>> >> > There is probably some discussion to be had about the minimum nonce
>> >> > length (not necessarily in the document itself).  Do you have a 
>> >> > pointer handy to previous disucsions or do we need to have it now?
>> >> > (I do see that this is just following RFC 8366, so hopefully this
>> >> > is an easy question.)
>> >> 
>> >> 
>> >> I sent email to my RFC 8366 co-authors, as they were behind setting
>> >> this min nonce length.  I have yet to hear back from them, but will
>> >> let you know when I do.
>> >
>> > [covered in separate thread]
>> 
>> [Bringing back into this thread]
>> 
>> I emailed the RFC 8366 authors (CC you) regarding your concern with the
>> minimum-allowed nonce length.
>> 
>> As for this draft, I feel that the easiest solution is to change the YANG
>> as follows:
>> 
>>        leaf nonce {
>>          type binary {
>> -          length "8..32";
>> +          length "16..32";
>>          }
>> 
>> It is within the range allowed by RFC 8366 (i.e., no compatibility violation)
>> while eliminated the low-end that you objected to.  I can't imagine there
>> being an issue in asking a low-end device (even a measly IoT thing) to generate
>> an extra 8 bytes of random data.
>> 
>> I've made this change in -27.
>
> Sounds good.  This is certainly the easiest way forward, and if there are
> no objections there's not much reason to not just go with it.  I may have
> some generic desire in the abstract to fully understand whether it's
> needed, but I am pretty sure I can suppress that desire if needed :)

Okay, let's just go with it.  You may want to engage Michael Richardson, from
that fork in this thread, to consider if there is a true need.



>> >> > ----------------------------------------------------------------------
>> >> > COMMENT:
>> >> > ----------------------------------------------------------------------
>> 
>> >> > I a little bit wonder if we want references for TLS and/or HTTP client
>> >> > authentication.  Section 2.5 of RFC 8040 might be enough (though it is
>> >> > of course not citing TLS 1.3).
>> >> 
>> >> This draft's use of TLS and HTTP authentication is exclusively for
>> >> RESTCONF (RFC 8040).   I'm generally hoping to just reference that
>> >> RFC and let it speak for itself.
>> >> 
>> >> Correct, RFC 8040 does not cite TLS 1.3 explicitly, though TLS 1.3 is
>> >> allowed, as Section 2.1 says:
>> >> 
>> >>    RESTCONF does not require a specific version of HTTP.  However, it 
>> >>    is RECOMMENDED that at least HTTP/1.1 [RFC7230] be supported by all
>> >>    implementations.
>> >> 
>> >> BTW, does this comment regard Section 9.6?  
>> >
>> > Section 9.6 is about making sure the client does not send sensitive data to
>> > an unauthenticated server, which is not limited to the data in the
>> > certificate; my comment here is more about the mechanics of the client
>> > authenticating itself to a server for the server to make authorization
>> > decisions (admittedly, I did not mention "authorization" prior to now, so
>> > my apologies for being unclear).  Client authentication is mentioned
>> > directly or in passing in at least sections 5.1 (which does mention Section
>> > 2.5 of RFC 8040 already) and 5.3 (ditto), as well as 9.6.
>> 
>> As you say, authentication is mentioned in several places already 
>> and, if I understand you correctly, this text is sufficient.
>> 
>> While the word "authorization" does not appear in this document, 
>> Section 9.13 discusses "access", which relates to statements in 
>> RFC 8040, such as:
>> 
>>    The RESTCONF server MUST authenticate client access to any protected
>>    resource.
>> 
>>    The server MUST NOT allow any RESTCONF operation for any resources
>>    that the client is not authorized to access.
>> 
>> To provide a more complete picture, the bootstrap server exposes just
>> two RPCs.  Each RPC *requires* an authenticated client-credential to
>> function.  That is, "get-bootstrapping-data" can only return the 
>> bootstrapping data for the authenticated client credential; there is
>> no other parameter passed for the server to determine which data to 
>> return. Likewise, "report-progress" can only report progress for the
>> authenticated client credential; there is no other parameter passed
>> for the server to determine for which client is reporting the data.
>> 
>> This COMMENT began by asking if we might want references for TLS 
>> and/or HTTP client authentication, and now maybe authorization.
>> Given the above discussion, what is your recommendation?
>
> I think that the document will be good with no additional change for this
> matter; the contents of RFC 8040 and the way we use RESTCONF makes things
> clear enough.

Excellent.  This item is closed.



>> >> > Section 3.4
>> >> >
>> >> > Thank you for including the motivating text about sign-then-encrypt.
>> >> > I do wonder if it's worth saying anything about why the well-publicized
>> >> > security risks of mac-then-encrypt do not apply.  (The authors of
>> >> > draft-campbell-sip-messaging-smime probably already have some text
>>> >> > that could be used, but it doesn't seem to be in the public view yet.)
>> >> 
>> >> Are you referring to the padding oracle attack?  As mentioned above, the
>> >
>> > Right.
>> >
>> >> solution presented in this document doesn't lend the device to being a
>> >> very good decryption oracle.  I suppose the fix would be to add to this
>> >> section (or the Security Considerations section?) something like:
>> >> 
>> >>   This document specifies the encryption of signed objects, as opposed
>> >>   to the signing of encrypted objects, as might be expected given well-
>> >>   publicized oracle attacks (e.g., the padding oracle attack).  This
>> >>   document does not view such attacks as being feasible in the context
>> >>   of the solution because i) the decrypted text never leaves the device
>> >>   and ii) the solution does not differentiate between a "bootstrap-error"
>> >>   cause by a decryption failure versus a failure occurring when parsing
>> >>   the decrypted text.
>> >
>> > That works for me, thanks.  (But feel free to leave it out if you don't
>> > think it's adding value, too.)
>> 
>> I added a slightly reduced version of the above text (taking out clause "ii"):
>> https://github.com/netconf-wg/zero-touch/commit/388d0355b3c7c6ce4aa48028a4ef89dc64147304.
>> 
>> Still good?
>
> Yes.

Zipping along (item closed)


  

>> >> > Section 4.2
>> >> >
>> >> > I agree with Adam about registering "zerotouch" (and the name is
>> >> > perhaps overly generic?).
>> >> >
>> >> > I'm also not sure I properly understand the "zt-info"/zt-* TXT
>> >> > records' usage; would they need to be registered akin to
>> >> > draft-moonesamy-dnsop-special-use-label-registry?
>> >> 
>> >> First, regarding the term "zerotouch" being perhaps overly generic,
>> >> I have somewhat felt this way for a while.  One thing that could be
>> >> done fairly easily is to more the bulk of the "zerotouch" references
>> >> to "sztp", the acronym given throughout.  Admittedly, what SZTP
>> >> stands for isn't tremendously better, but I think that it is
>> >> generally better than just "zerotouch".  Thoughts?
>> >
>> > SZTP does seem better than just "zerotouch" to me, all things considered.
>> 
>> 
>> Okay, I did the following:
>> 
>>   When referring to the draft/solution:
>>     Zero Touch --> SZTP
>> 
>>   When referring to the bootstrapping artifact:
>>     zero touch information --> conveyed information
>>     zerotouch-information  --> conveyed-information
>> 
>>   For the CMS content types:
>>     id-ct-zerotouchInformationXML  --> id-ct-sztpConveyedInfoXML
>>     id-ct-zerotouchInformationJSON --> id-ct-sztpConveyedInfoJSON
>> 
>>   For the YANG modules:
>>     ietf-zerotouch-information.yang      --> ietf-sztp-conveyed-info.yang
>>     ietf-zerotouch-bootstrap-server.yang --> ietf-sztp-bootstrap-server.yang
>> 
>>   For the DNS/service name:
>>     _zerotouch --> _sztp
>> 
>> A big change, though I scripted most of it. Here's the diff:
>> https://github.com/netconf-wg/zero-touch/commit/394b863d1850019fd451554a9f86c3c10d280d08
>
> Thank you for your willingness to make these disruptive changes "for the
> good of the team"; I know it's pretty thankless work.

Your welcome, and thanks for the acknowledgement.  (item closed)




>> >> Second, I am not a DNS expert, do you know who we can discuss
>> >> such things with?  That said, I guess our idea was to use TXT
>> >> records like RFC 1464, where the TXT value itself has the form
>> >> "<attribute name>=<attribute value>", in which case it doesn't
>> >> seem to need IANA registration?
>> >
>> > Please correct me if I'm wrong, but I think this issue was
>> > already covered in a different AD's ballot thread.
>> 
>> Correct, Section 4.2 was updated (posted in -26) per Alexey's DISCUSS.
>> Per your original comment (and his, and Adam's), Section 10.6 now
>> requests IANA to register the service name "sztp" (was "zerotouch"). 
>> 
>> > That said, the addition of <serial number>._zerotouch.fqdn in the
>> > -26 seems to indicate that mention of draft-ietf-dnsop-attrleaf
>> > is appropriate, if I remember correctly how that works.
>> 
>> I've just now read draft-ietf-dnsop-attrleaf.  I see the applicability,
>> but I don't understand your proposal.  Looking at DataTracker, I see
>> that it is already in RFC Ed Queue, so I think you're suggesting me
>> treat it as a fait accompli, and add an IANA Consideration section
>> to register "_sztp", yes?  Assuming that is the case, then what should
>
> From memory, yes.
>
>> be done with the service name registration in Section 10.6, added per
>> comments from Alexey and Adam?
>
> I think we'll need to get some further input from Alexey and/or Adam, but
> my understanding is that we would need both registrations -- the service
> name registration covers our _sztp._tcp.fqdn SRV records, but we are also
> using <serial number>._sztp._tcp.fqdn TXT records, and so (IIUC) we'd need
> to add a reference to this document for the TXT _tcp entry that RFC 6763
> (DNS-SD) is currently the reference for.

This item remains open.  Hopefully Dave can provide guidance.




>> >> >   The device MUST parse the provided onboarding information document,
>> >> >   to extract values used in subsequent steps.  Whether using a stream-
>> >> >   based parser or not, if there is an error when parsing the onboarding
>> >> >
>> >> > This line makes me consider the scenario where a stream-based parser is
>> >> > used with a trusted bootstrap server and no CMS-layer signature.  At the
>> >> > TLS layer, a truncation attack by the network is possible, and if
>> >> > truncation is not detectable at the application layer, the device could end
>> >> > up misconfigured with neither party aware (unless there's an additional
>> >> > response or something that I'm forgetting about).  I think that for the XML
>> >> > and JSON formats we know and love, truncation would make for a malformed
>> >> > stream due to the outermost scope container, but please correct me if I'm
>> >> > wrong.  There are probably some security considerations to mention w.r.t.
>> >> > any future new encodings of this data model, though.
>> >> 
>> >> The steps are roughly: 
>> >>   a) HTTPS GET an XML/JSON document (the get-bootstrapping-data" response).
>> >>   b) extract the CMS-based artifacts from the XML/JSON document.
>> >>   c) if encrypted, decrypt.
>> >>   d) if signed, authenticate.
>> >>   e) extract the onboarding-information (another XML/JSON doc) from the CMS artifact.
>> >>   f) process the onboarding-information XML/JSON doc
>> >> 
>> >> So here we're at step (f), where the text mentions the possible use of a
>> >> stream parser.  This is rather long after step (a), where TLS truncation
>> >> may occur and, presumably caught in step (b).  This is by way of saying
>> >> that I don't think this is an issue, but interested to hear your response.
>> >> 
>> >> FWIW, the point of this "stream-based parser" comment is to highlight
>> >> that, unlike most all the other progress-types, which seem to reflect
>> >> a serial processing of the steps, the parsing may either be a distinct
>> >> step (e.g., a DOM-based parser) or something that is splayed across all
>> >> the other steps (a stream-based parsed).  In the first case, the all
>> >> the "parsing-*" progress reports (including any parsing-error report)
>> >> are expected to be transmitted before any, e.g., boot-image-* reports
>> >> are sent; whereas, in the second case, the parsing-* reports can be
>> >> intermixed.
>> >> 
>> >> I don't think there is a TLS concern, but perhaps the paragraph needs
>> >> to be reworded?
>> >
>> > I think your understanding basically matches mine; I tried to include in my
>> > remark that it would only possibly apply to the case where the predicates
>> > for (c) and (d) are false.  I just plain don't know whether (a)/(b) will
>> > choke if the GET response is an incomplete XML/JSON document.  If it
>> > properly errors out, then there's no concern here, and we should just move
>> > on.  In any case, even if there was an issue, this paragraph would not
>> > really be the place to talk about it -- my comment is only located here
>> > because this is where we start talking about stream-based parsers.  The
>> > errors in question are not necessarily those generated by the stream-based
>> > parser, but can also include those generated at earlier steps.
>> 
>> Okay, if we step back from the stream-parsing angle, and instead just focus
>> on the TLS truncation concern, my first thought is that said error will be
>> detected in (a) and (b) will not be entered.  In case (b) is entered, then
>> it seems that (b) would detect a malformed response from the server, as it
>> would normally need to do.
>
> Okay, defense in depth is a good thing.  It sounds like we don't need to
> say anything about this in the document, IIUC.

Thanks for letting this slide.  (item closed)


  
>> >> > Section 6.2
>> >> >
>> >> > "base64encodedvalue==" is pretty cute, though maybe we could add some
>> >> > trailing numbers to provide different values for the different fields?
>> >> 
>> >> The issue here is that the example documents must be valid (fwiw, they
>> >> are tested each time `xml2rfc` is run).  Previous versions of this 
>> >> document included compete base64 encoding of the real objects, but 
>> >> people complained that it greatly distracted from readability.  To
>> >> address this, the WG agreed to use "base64encodedvalue==" in examples
>> >> to represent YANG "binary" data.
>> >> 
>> >> Is it okay to leave this as is?
>> >
>> > This is a non-blocking comment, so by definition my answer is "yes" :)
>> > I mostly just wanted to point out that the examples use the same literal
>> > string to fill in for many different data types -- I agree with not using
>> > "real" examples since they're bulky and not readable in encoded form, but
>> > was just wondering if we could use different short strings to represent
>> > semantically different objects.
>> 
>> Okay, let's close this with no update.  That said, you may be pleased (or
>> irked) to see that the nonce min-length change forced one example to have
>> to change to "<nonce>extralongbase64encodedvalue=</nonce>" in order to pass
>> build-time validation tests ;)
>
> Cool, I wish my CI was that good :)

Really, it's the only way to go.  The time spent setting it up is easily
recovered in the course of the project.  I wish all draft authors extra
time in their lives to do more rewarding stuff than chase down issues
that otherwise would've been easily caught.



 
>> >> > Section 7.2
>> >> >
>> >> > If we're going to say "and receives signed data in the response", maybe we
>> >> > could actually give an example that shows the (base64'd) CMS structure that
>> >> > corresponds to the signature?  Not necessarily the whole payload, but
>> >> > enough to see the outer structure at least...
>> >> 
>> >> See previous response regarding "base64encodedvalue==".  It's tricky
>> >> business.  That said, in a separate "expert review" response from Russ
>> >> Housley, we were thinking to add an appendix section containing all
>> >> the possible ASN.1 structures.  For instance:
>> >> 
>> >>   X. ASN.1 for Various Artifacts
>> >>   X.1. Zero Touch Information
>> >>   X.2. Signed Zero Touch Information
>> >>   X.3. Encrypted and Signed Zero Touch Information
>> >>   X.4. Owner Certificate
>> >>   X.5. Encrypted Owner Certificate
>> >>   X.6. Ownership Voucher
>> >>   X.7. Encrypted Ownership Voucher
>> >> 
>> >> Would this bridge the gap for you?
>> >
>> > That would help a lot, thanks!
>> 
>> Is it okay for me to back out of this one?  I had thought previously
>> that Russ would provide the ASN.1, but he said he didn't have time and,
>> well, I'd rather not venture into this if it can be avoided...
>
> It will be okay if nothing happens on this front.  (You've already put
> in a huge amount of effort anyway!)

Okay (item closed).




> Huge thanks for all your work on this -- hopefully the end is in sight!
>
> -Benjamin

You bet, it was my pleasure.  I hope to thank you in person sometime.


Cheers,
Kent