Re: [Anima] Benjamin Kaduk's Discuss on draft-ietf-anima-bootstrapping-keyinfra-28: (with DISCUSS and COMMENT) [COMMENTS]

[Benjamin Kaduk <[e-mail]>]

On Tue, Oct 22, 2019 at 04:23:57PM -0400, Michael Richardson wrote:
> 
> Hi, this got quite long.
> I've split this up into nits and substantive comments.
> This email is about the substantive comments.
> Please see a diff at: https://tinyurl.com/y5l4xz3z

Thanks; I'll put some editorial nits at the end of this message.

> Benjamin Kaduk via Datatracker <[e-mail]> wrote:
>     > Thanks for the time and attention to detail put into addressing my
>     > previous Discuss and Comment points.  I have one new Discuss-level
>     > point and one that I think may have been missed (probably because I
>     > inadvertently numbered two points with the same number).
> 
>     > (16) The audit log response (Section 5.8.1) describes the nonce as a
>     > JSON string, but the RFC 8366 nonce is a binary value.  I think we need
>     > to describe some mapping procedure (such as always base64-encoding as
>     > is done for domainID, even if the original nonce is itself
>     > base64-encoded random data) in order to be fully functional.
> 
> In the JSON mapping of the YANG, binary values are always base64 encoded, as
> JSON does not support binary values.

I apparently forgot that we were going through a YANG->JSON encoding when I
wrote the above, sorry for that.  But thank you for the extra reminders
about base64-encoding that are in the latest copy.

> This would be an issue in the constrained-voucher CBOR mapping through.
> 
>     > % (1) The text of the document suffers from lack of clarity throughout
>     > about % whether the nonce-ful operation is mandatory or not, with
>     > several % figures and discussions making declarative statements about
>     > nonce usage % and others saying that nonce usage is optional. (See
>     > COMMENT.)
> 
> 
> 
>     > [keep in mind when reading]
> 
>     > % (5.1) the new /enrollstatus EST endpoint seems under-specified.
> 
>     > Shame on me, I reused (5) for two points and have renumbered this to
>     > (5.1) so we don't miss it again.  We don't anywhere give a formal
>     > description of the contents of the POST body; there's just an example
>     > JSON object.
> 
> Agreed, and there is a comment about the subjectKeyInfo.
> 
> I had opened issue: https://github.com/anima-wg/anima-bootstrap/issues/141
> last week when I started writing this.
> 
> We have resolved this ticket as follows:
> 
>    The original purpose of the subjectKeyIdentifier was to indicate which
>    keypair the client (pledge) had attempted to enroll. Upon successful
>    enrollment, a new TLS connection is started (usually), which proves that the
>    enrollment is success, and so no KeyIdentifier is needed.
>    In the case of a failure, the existing TLS connection is maintained, so
>    the server (Registrar) can associate the failure with the previous attempt
>    to enroll.
>    This change (removing the subjectKeyIdentifier) was made in -05, but the
>    prose was not updated. The use of a subjectKeyIdentifier, which might no
>    longer be a SHA-1 hash of a public key, is problematic, as the CSR does
>    not include an appropriate KeyIdentifier, so the server would be left
>    trying to calculate it, using the default (and now obsolete) SHA-1
>    mechanism. Including the KeyIdentifier has proved more trouble than
>    benefit.

The removal of SKI seems okay, but I'm still looking for something like
"the body of the POST is a JSON object with the following [whoops, I forget
the right terminology for name/value pairs]"

> Esko has complained that we have 14 open issues; that's probably negligence
> on my part about not closing them as we solved the issues.
> So we also closed all but three "auth48" tickets that had been opened.
> 
>     > Section 2.5.1
> 
>     >    The pledge is the device that is attempting to join.  The pledge can
>     > talk to the Join Proxy using link-local network connectivity.  In most
>     > cases, the pledge has no other connectivity until the pledge completes
>     > the enrollment process and receives some kind of network credential.
> 
>     > I'd consider s/can talk to/is assumed to talk to/.
> 
> okay.  some have complained this isn't strong enough.
> Maybe MUST talk to?

I don't have a strong opinion on this, but it feels like something that
will probably be different for ACP vs. other deployments -- MUST feels
right for ACP but is probably not universal.

>     > Section 3
> 
>     > In my previous comment, I said:
> 
>     > % The "proximity-registrar-cert" leaf is used in the pledge voucher- %
>     > requests.  This provides a method for the pledge to assert the %
>     > registrar's proximity.
>     > %
>     > % "proximity" in what sense?  How much verification/confidence needs to
>     > be % done?  (Also, I would have expected the assertion to go the other
>     > way; % that the registrar asserts the pledge's proximity -- how does
>     > the pledge % have a way to know that the registrar is nearby when the
>     > proxy is % transparent?)
> 
>     > We talked about this some, but I think we're still making an unstated
>     > assumption that there is a link-local or pre-ACP connection involved.
>     > Making that an explicit assumption would be helpful.
> 
> I have added the following text to make this assumption explicit:
> 
> +      <t>
> +        This network proximity results from the following properties in the
> +        ACP context:  the pledge is connected to the Join Proxy
> +        (<xref target="proxydetails" />) using a Link-Local IPv6 connection.
> +        While the Join Proxy does not participate in any meaningful sense in
> +        the cryptography of the TLS connection (such as via a Channel
> +        Binding), the Registrar can observe that the connection is via the
> +        private ACP (ULA) address of the join proxy, and could not come from
> +        outside the ACP.  The Pledge must therefore be at most one IPv6
> +        Link-Local hop away from an existing node on the ACP.
> +      </t>
> +      <t>
> +        Other users of BRSKI will need to define other kinds of assertions if
> +        the network proximity described above does not match their needs.
> +      </t>

Thank you!

> ====
> 
>     > Also, we should probably give a reference for base64 (not necessarily
>     > here), such as Section 4 of RFC 4648.
> 
> And to the lead in, I've referenced RFC4648, which this is the first
> reference to base64.
> 
>          <section title="Examples" anchor="voucher-request-examples">
>              <t>This section provides voucher-request examples for illustration
> -                purposes.  The contents of the certificate have been elided
> +                purposes.  JSON encoding rules specify that any binary
> +                content by base64 encoded (<xref target="RFC4648" />).
> +                The contents of the certificate have been elided
> 
> 
>     > Section 5.1
> 
>     >    Use of TLS 1.3 (or newer) is encouraged.  TLS 1.2 or newer is
>     > REQUIRED.
> 
>     > How painful would it be to require 1.3 at this point?  RFC 8446 has
>     > been out for more than a year, and the TLS WG is leaning on me to
>     > tighten up on this...
> 
> Yes, but how many FIPS-140 validated TLS 1.3 implementations are there that
> are drop-in available to router platforms with 3-5 year R&D cycles?
> OpenSSL is FIPS for 1.0.1 and 1.0.2 which do not include TLS 1.3, for
> instance (which is 1.1.x).

I take it that FIPS-140 validation is required for those $100k BFRs, then?

> WolfSSL has FIPS-140 for TLS 1.3 (to read the press releases), and some
> entities could drop it in, but others might have license issues.
> 
> How about if I change the text to say:
> 
>               Use of TLS 1.3 (or newer) is encouraged.
>               TLS 1.2 or newer is REQUIRED on the Pledge side.
>               TLS 1.3 (or newer) SHOULD be available on the Registrar server interface,
>               and the Registrar client interface, but TLS 1.2 MAY be used.
>               TLS 1.3 (or newer) SHOULD be available on the MASA server interface, but TLS
>               1.2 MAY be used.

That's an okay compromise on the 1.3-encouragement front, but I don't think
that everyone will read "Foo server interface" and "Foo client interface"
in the same way.

> Getting TLS 1.3 on a web framework platform is not that hard if it does not
> need FIPS-140.  Mandating that 1.3 be available reduces the domino effect
> that keeps systems from organically upgrading.
> 
>     doc>    IDevID certificate is out-of-scope of this specification.  Note that
>     doc> the trust anchors in/excluded from the database will affect which
>     doc> manufacturers' devices are acceptable to the registrar as pledges, and
>     doc> can also be used to limit the set of MASAs that are trusted for
>     doc> enrollment.
> 
>     > I can't tell if we want to bring the division into two distinct trust
>     > anchor databases into this discussion as well.
> 
> I think that there are a bunch of considerations which go rather far into how
> the domain owner wants to operate things.   I have started a document:
>     Operational Considerations for BRSKI Registrar
> 
> but, it's just a shell so far.

Okay.

>     > Section 5.4
> 
>     doc>    Section 2.8.  The mechanisms in [RFC6125] SHOULD be used
>     doc> authentication of the MASA.  Some vendors will establish explicit (or
> 
>     > nit: s/used/used for/ Also, we tend to require that people using 6125
>     > specify what name type in the cert to verify (e.g., DNS-ID) against
>     > what expected name.  It would be pretty easy to convince me that we
>     > don't need to do that here, though.
> 
> diff --git a/dtbootstrap-anima-keyinfra.xml b/dtbootstrap-anima-keyinfra.xml
> index 8f82c58..4394881 100644
> --- a/dtbootstrap-anima-keyinfra.xml
> +++ b/dtbootstrap-anima-keyinfra.xml
> @@ -1912,9 +1912,14 @@ locator3  = [O_IPv6_LOCATOR, fe80::1234, 41, nil]]]></artwork>
>            connection and uses the MASA URL obtained as described in
>            <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.
> +          MASA using a DNS-ID that matches that which is found in the IDevID.
> +          Registrars MAY include a mechanism to override the MASA URL on a
> +          manufacturer-by-manufacturer basis, and within that override it is
> +          appropriate to provide alternate anchors.
> +          This will typically used by some vendors to establish explicit
> +          (or private) trust
> +          anchors for validating their MASA that is part of a sales channel
> +          integration.
>          </t>
> 
>     > Section 5.4
> 
>     >    Use of TLS 1.3 (or newer) is encouraged.  TLS 1.2 or newer is
>     > REQUIRED.
> 
>     > As above, how painful would requiring 1.3 be?
> 
> Consistently with the text above:
> 
>           Use of TLS 1.3 (or newer) is encouraged. TLS 1.2 or newer is
>           REQUIRED.  TLS 1.3 (or newer) SHOULD be available.
> 
> 
>     > Section 5.5
> 
>     >    prior-signed-voucher-request: The signed pledge voucher-request
>     > SHOULD be included in the registrar voucher-request.  The entire CMS
>     > signed structure is to be included, base64 encoded for transport in the
>     > JSON structure.
> 
>     > I think we need a ref for (which) base64.
> 
> I looked through.
> RFC8366 does not reference RFC7951 (it should), which explains JSON
> serialized YANG.  RFC7951 and RFC7950 specify original base64.
> {I am annoyed that we aren't using base64url in a Postel-rule compatibility
> mode everywhere...}
> 
> Section 3.3 already referenced 7951, btw
> 
> I have added the text to section 5:
>    BRSKI uses existing CMS message formats for existing EST operations.
>    BRSKI uses JSON [RFC8259] for all new operations defined here, and
>    voucher formats.  In all places where a binary value must be carried
>    in a JSON string, the use of base64 format ([RFC4648] section 4) is
>    to be used, as per [RFC7951] section 6.6.
> 
>     > Section 5.5.2
> 
>     doc>    The registrar's certificate chain is extracted from the signature
>     doc> method.  The entire registrar certificate chain was included in the CMS
>     doc> structure, as specified in Section 5.5.  This CA certificate will be
>     doc> used to populate the "pinned-domain-cert" of the voucher being issued.
> 
>     > By saying "this CA certificate", are we excluding use cases where the
>     > pinned-domain-cert is not the global root CA of the organization (or is
>     > the implication just that you don't send the rest of the chain)?
> 
> Yes.
> 1) There may not be a global root CA for the organization.  It could be self-signed.
> 2) The registrar gets to decide which thing it wants pinned.
> 3) The pledge is expected to have enough certificate chaining to deal with
>    either situation.  (This has changed: at one point memcmp() was considered enough)
> 4) the EST /cacerts request lets the Pledge "upgrade" to an organizational anchor.
> 
>     > Section 5.5.6
> 
>     doc>    The MASA populates the audit-log with the nonce that was verified.
>     doc> If a nonceless voucher is issued, then the audit-log is to be populated
>     doc> with the JSON value "null".
> 
>     > The quotes around "null" are perhaps anti-helpful.
> 
> _null_ is a JSON literal.   Are you complaining that some people might write:
>        nonce: "null"
> rather than
>        nonce: null
> ?

I am indeed.  Perhaps "the JSON primitive null", but I'm starting to think
I should stop trying to wordsmith this...

>     doc>    The keys to this JSON hash are case-insensitive.  Figure 15 shows an
>     doc> example JSON.
> 
>     > This (case-insensitivity) seems to be a drastic divergence from the RFC
>     > 8259 standard behavior and as such would require some justification.
>     > nit: s/hash/object/
> 
> okay, changed this to lower case.
> 
>     > Section 7.3
> 
>     doc>    A registrar can choose to accept devices using less secure methods.
>     doc> These methods are acceptable when low security models are needed, as
>     doc> the security decisions are being made by the local administrator, but
>     doc> they MUST NOT be the default behavior:
> 
>     > I'm having a hard time parsing "low security models"; the best I can
>     > come up with is "threat models where low security is adequate".
> 
> Does this work for you:
> 
> 7.2.  Pledge security reductions
> 
> -   The following are a list of alternatives behaviours that the pledge
> -   can be programmed to implement.  These behaviours are not mutually
> -   exclusive, nor are they dependent upon other.  Some of these methods
> -   enable offline and emergency (touch based) deployment use cases.
> -   Normative language is used as these behaviours are referenced in
> -   later sections in a normative fashion.
> +   The following is a list of alternative behaviours that the pledge can
> +   be programmed to implement.  These behaviours are not mutually
> +   exclusive, nor are they dependent upon each other.  Some of these
> +   methods enable offline and emergency (touch based) deployment use
> +   cases.  Normative language is used as these behaviours are referenced
> +   in later sections in a normative fashion.

(That diff is fine with me, though it's the wrong one.  What's in the
linked diff for Section 7.3 is also fine.)

> 
>     > Section 7.4.1
> 
>     doc>    and/or not requiring one to be present in the voucher-request.  This
>     doc> results in distribution of a voucher that never expires and in effect
> 
>     > "expires-on" is distinct from the nonce-ful freshness check, so I think
>     > some additional wordsmithing is in order.
> 
> A Pledge will not, in general, have a real time clock it can rely on.
> A nonced voucher is fresh, while a nonceless one is not.
> I have changed this to:
> 
>             This
>             results in distribution of a voucher that may never expire and in

Thanks.

>     doc>    In the ACP, the Join Proxy is found to be proximal because
>     doc> communication between the pledge and the join proxy is exclusively on
> 
>     > nit(?) My dictionary doesn't list anything for "proximal" that is a
>     > good match for "with proximity"; might be worth double-checking.
> 
> I find it:
>   https://www.dictionary.com/browse/proximal?r=75&src=ref&ch=dic

I mean, my dictionary finds it *as a word*, just not with a definition that
looks great for how you're using it here.

>     > Section 11.3
> 
>     > We had some discussion around my previous comment:
> 
>     > % Additionally, in order to successfully use the resulting voucher the
>     > % Rm would have to attack the pledge and return it to a bootstrapping %
>     > enabled state.  This would require wiping the pledge of current
>     > %
>     > % ... and I think there is a different attack if the Rm is in a
>     > position % to delay or drop network traffic between the pledge and the
>     > intended % registrar, to cause Rm's voucher to be delivered first even
>     > though it is % generated after the intended registrar's authorization
>     > process.  The % intended registrar would need to require reports on
>     > voucher processing % status (or investigate their absence) in order to
>     > detect such a case.
> 
>     > but I can't remember if we decided that we didn't need to discuss the
>     > risk in the document.
> 
> I think that the explanation if pretty complete.
> 
>     > Appendix D.2
> 
>     > An RFC Editor note about the RFC 8366 assignment of OID
>     > 1.2.840.113549.1.9.16.1.40 was removed from -23 to -28; do the examples
>     > properly use that assigned OID now?
> 
> We got a MASA URL Extension OID for:
>    1.3.6.1.5.5.7.1.32
> 
> the examples date from before that, and do not use it yet.

I will update my Discuss position to help us remember.


editorial nits on the linked diff:

Section 2.5.3 talks about the set of MASA that are trusted being limited
by the MASA TA database, but in the paragraph about the BRSKI-EST client
TA database.

s/described Appendix B/described in Appendix B/ (sorry, section number
not visible from diff; maybe 40% through the diff)

Section 5.4 is missing an "for" in "SHOULD be used for authentication of
the MASA".

s/accesptable/acceptable/

Figure 17 is introduced as an "abstract example", though it seems more
of a concrete one after this diff.

-Ben