Re: [Anima] Roman Danyliw's Discuss on draft-ietf-anima-bootstrapping-keyinfra-22: (with DISCUSS and COMMENT)

["Max Pritikin (pritikin)" <[e-mail]>]

inline comments, 

> On Jul 10, 2019, at 6:34 PM, Michael Richardson <[e-mail]> wrote:
> 
> 
> Roman Danyliw via Datatracker <[e-mail]> wrote:
>> (5) Section 1.3.2.  Cite the origin of the taxonomy which defines
>> “class 2+” devices – likely RFC7228
> 
> done.
> 
>> (6) Section 1.5.  Per “Upon successfully validating a voucher artiface, a
>> status telemetry MUST be returned.”, what is a “voucher artiface”?  is
>> that just a “voucher”?
> 
> It's a typo. It should be voucher artifact.
> We used "Voucher Artifact" all over RFC8366.
> 
>> (7) Section 2.6.1.  What is a “Network Time Protocols” – specifically
>> why is protocols plural?  Is this something more than NTP?
> 
> Yes, NTPv4, but it could also be IEEE PTP, or even GSM.
> It doesn't much matter... no network ==> no network time.
> 
>> (8) Section 4.1.  Per “The communication between the pledge is over IPv6
>> Link-Local addresses”, if that is the case, why does Figure 3 show IPv4
>> and Appendix A describes an IPv5 approach?
> 
> Because some members of the WG continued to complain that IPv6 isn't
> ubiquitous, and Appendix A was the compromise.
> I would be happy to rip it out, because I think the argument is specious.
> 
>> (9) Section 5.1., Per “The pledge maintains a security paranoia
>> concerning the provisional state …”, what does it mean to “maintain a
>> security paranoia”?
> 
> I guess I don't know.
> Max?  Seems like useless words.

The intention is to warn implementers that the HTTP and other data received until the voucher content is verified could be from an attacker since the TLS connection is in a provisional mode. Perhaps this is clearer:  

-            <t>The pledge maintains a security paranoia concerning the
-            provisional state, and all data received, until a voucher is
-            received and verified as specified in <xref
-            target="CompletingAuthenticationBootstrapping"></xref></t>
-
+            <t>The pledge performs input validation of all data received
+               until a voucher is verified as specified in <xref
+               target="CompletingAuthenticationBootstrapping"></xref> and
+               the TLS connection leaves the provisional state. Until these
+               operations are complete the pledge could be communicating
+               with an attacker.</t>


>> (10) Section 5.1.  Per “A pledge that can not maintain as many
>> connections as there are eligible proxies.”, this is an unfinished
>> sentence.  What should the pledge do?
> 
> fixed.
>        A pledge that can not maintain as many connections as there are
>    -   eligible proxies.  If no connection is making progess after 5 seconds
>    -   then the pledge SHOULD drop the oldest connection and go on to a
>    -   different proxy: the proxy that has been communicated with least
>    -   recently.  If there were no other proxies discovered, the pledge MAY
>    -   continue to wait, as long as it is concurrently listening for new
>    -   proxy announcements.
>    +   eligible proxies will need to rotate among the various choices,
>    +   terminating connections that do not appear to be making progress.  If
>    +   no connection is making progess after 5 seconds then the pledge
>    +   SHOULD drop the oldest connection and go on to a different proxy: the
>    +   proxy that has been communicated with least recently.  If there were
>    +   no other proxies discovered, the pledge MAY continue to wait, as long
>    +   as it is concurrently listening for new proxy announcements.
> 
>> (11) Section 5.2.  created-on.  The value to use in this field of this
>> field is not described here.
> 
> -            RECOMMENDED to populate this field. This provides additional
> -            information to the MASA.</t>
> +            RECOMMENDED to populate this field with the current date and
> +            time in yang:date-and-time format. This provides additional
> -            information to the MASA.</t>
> 
>> (12) Section 5.2.  Per “All other fields MAY be omitted in the pledge
>> voucher-request”, should this be read as guidance that the fields
>> mentioned above this text are mandatory (i.e., created-on, nonce,
>> proximity-registrar-cert). If so, what value should pledges without
>> clocks use?
> 
> I guess:
>            Pledges that have no real-time clocks MAY omit this field.
> 
>> (13) Section 5.3.  Per “If these validations fail the registrar SHOULD
>> respond
>> with an appropriate HTTP error code”, a few questions:
> 
> Ha, I kept asking for us to be specific about error codes, but I missed this
> part.
> 
> -          If these validations fail the registrar SHOULD respond with an
> -          appropriate HTTP error code.
> +          If these validations fail the registrar SHOULD respond with the
> +          HTTP 404 error code.  If the voucher-request is in an unknown
> +          format, then an HTTP 406 error code is more appropriate.
> +          A situation that could be resolved with administrative action
> +          (such as adding a vendor to a whitelist) MAY be responded with an
> +          403 HTTP error code.
> 
> 
>> ** Is this text suggesting that silent failure?  Given the text says
>> SHOULD,
>> validation could fail and the registrar would not share this failure?
>> ** What specific codes should be used?
> 
> I believe that there may be other error codes that would make sense, so
> I'm sticking with SHOULD.
> 
>> (14) Section 5.4.  Per “The registrar initiates the connection and uses
>> the MASA URL obtained as described in Section 2.8 for [RFC6125]
>> authentication of the MASA.”, RFC6125 doesn’t have a Section 2.8.
>> Please clarify how this connection is made.
> 
> aha. It's section 2.8 of this document! Some punctuation is missing.
> 
> -          <xref target="obtainmasaurl" /> for <xref target="RFC6125" />
> -          authentication of the MASA.
> +          <xref target="obtainmasaurl" />. The mechanisms in
> +          <xref target="RFC6125" /> SHOULD be used authentication of the
> +          MASA.  Some vendors will establish explicit (or private) trust
> +          anchors for validating their MASA; this will typically done as
> +          part of a sales channel integration.  Registars SHOULD permit
> +          trust anchors to be pre-configured on a per-vendor basis.
> 
>> (15) Section 5.5.  What is the relationship between the value of the
>> created-on field passed by the pledge per Section 5.2, and described in
>> Section 5.5 as being populated by the registrar.
> 
> -        <t hangText="created-on:">Registrars are RECOMMENDED to populate
>         this field. This provides additional information to the MASA.</t>
> +        <t hangText="created-on:">
> +          The Registrars SHOULD populate this field with the current date and
> +          time when the Registrar formed this voucher request. This field
> +          provides additional information to the MASA.
> +        </t>
> 
> 
>> (16) Section 5.5. What is a “statistically unique identity” per the
>> “idevid-issuer” field?
> 
> Max?
> 
> I don't think the word statistically is useful here.

It is possibly pedantic. The voucher leaf of idevid-issuer (see RFC8366) is the Authority Key Identifier as defined in RFC5280 and I probably have it in my head that this is only statistically unique when its based on the key identifier (where the keys are only statistically unique). 

-        is included to ensure a statistically unique identity.</t>
+        is included to ensure a unique identity.</t>

> The serial-number extracted from the pledge IDevID is unique per IDevID
> issuer, so we include that here.  I realize that my implementation does
> not include this at all, and I just use the certificate from the
> prior-signed-voucher-request.
> 
>> (17) Section 5.7.  Per the JSON snippet:
> 
>> ** It isn’t labeled as a figure or example; and isn’t referenced in the
>> text.
> 
>> ** This isn’t valid JSON: no commas between items.  What does the
>> notation “/*
>> TRUE=Success, FALSE=Fail”” mean – opening with a /* and closing with a “?
> 
> fixed.
> I need to go back throuth an renumber the figures (or rather,use anchors).
> 
>> (18) Section 5.8.2.  Per “Each time the Manufacturer Authorized Signing
>> Authority (MASA) issues a voucher, it places it into the audit log for
>> that device.  The details are described in Section 5.8.”, this
>> reference to Section 5.8 on how the MASA logs doesn’t seem correct.
>> Section 5.8 describes how a
>> registrar asks a MASA about its logs.
> 
>             Each time the Manufacturer Authorized Signing Authority (MASA)
> -            issues a voucher, it places it into the audit log for that device.
> -            The details are described in <xref target="authzLogRequest" />.
> +            issues a voucher, it appends details of the assignment to
> +            an internal audit log for that device.
> +            The internal audit log is processed when responding to
> +            requests for details as described in <xref
> +            target="authzLogRequest" />.
> 
> Does this work for you?
> It is not our intend to specify how the internal audit log works, just
> what it must be able to produce.
> 
>> (19) Section 5.9.4.  What happens if the pledge doesn’t re-negotiate an
>> EST TLS session after been successful enrolled?
> 
> I'm not sure.
> Max?

If the client skips renegotiating the server does not have operational proof that the client can successfully use the certificate as a TLS client certificate. Instead it will only receive the telemetry message where the client claimed everything is ’SUCESS’. 
I didn’t use ‘MUST' because of the keyusage morass where perhaps the cert is to be used for something where TLS isn’t valid. In other enrollment protocol discussions we either make exceptions for “misuse” of a key type during enrollment itself (such as performing a signing operation when the key usage says not to) or accept that the TLS client authentication might be with a different keypair than the one distributed (such as using EST to maintain and re-enroll a cert that isn’t appropriate for TLS). Using "SHOULD” here avoids having BRSKI take a stance on any of those issues. 

- max

>> (20) Section 5.9.4.  In what way is a the SubjectKeyIdentifier included
>> in the success telemetry message?
> 
> If there is success, I don't think the SubjectKeyIdentifier needs to be
> logged.  If there is failure, then the idea was that it would go into the
> reason in some textual form.
> 
>> (21) Section 6.  What is “../voucherrequest”?
> 
> It's a typo for /requestvoucher!
> 
>> (22) Section 7.  Per “This section is considered non-normative: use
>> suggested methods MUST be detailed in specific profiles of BRSKI” --
>> the clause after the colon doesn’t parse.  What is the guidance
>> relative to profiles?
> 
> How about:
>        This section is considered non-normative: use of the suggested
>        mechanism here MUST be detailed in specific profiles of
>        BRSKI.  This is a subject for future work.
> 
> I don't understand "What is the guidance relative to profiles?"
> 
>> (23) Section 11.0.  The caution about untrusted data and HTTP libraries
>> seems warranted.  Wouldn’t this also apply to the application code too?
> 
> In what way to do you mean?
> Maybe this paragraph has been mis-understood.
> We are trying to say that despite the headers and content being untrusted,
> that mature HTTP libraries should not be particularly vulnerable.  Is
> that what you understood?
> 
>> (24) Section 11.1.  Per the list of examples where a MASA is
>> unavailable or uncooperative, wouldn’t a manufacturing going out of
>> business also be a concern?
> 
> Absolutely.
> One reason why we call it the Manufacturer *AUTHORIZED* signing authority,
> is that we believe that this is something Manufacturers will outsource,
> and buyers will insist that there is an escrow plan.
> 
>> (25) Section D.1.4.  Provide an openssl version and reference that
>> generated this output
> 
> Well, it was done with libcrypto 1.1.1c, from (open source ruby) code linked
> against it.  I can certainly say that, but I don't know what
> "reference" means here.
> 
>> (26) Per Christian Huitema’s Last Call SecDir Review
>> (https://datatracker.ietf.org/doc/review-ietf-anima-bootstrapping-keyinfra-20-secdir-lc-huitema-2019-06-04/),
>> please respond to the last two issues – random number generation and the
>> missing assertion leaf.
> 
> I had not seen this second review, thank you.
> I will read this on Thursday and post additional changes.
> 
> --
> ]               Never tell me the odds!                 | ipv6 mesh networks [
> ]   Michael Richardson, Sandelman Software Works        |    IoT architect   [
> ]     [e-mail]  http://www.sandelman.ca/        |   ruby on rails    [
> 
> 
> 
> --
> Michael Richardson <[e-mail]>, Sandelman Software Works
> -= IPv6 IoT consulting =-
> 
> 
>