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

[Michael Richardson <[e-mail]>]

Hi, this got quite long.
I've split this up into nits and substantive comments.
This email is about the nits... it should all be boring and uncontroversial,
requiring no answers from you, I hope.
Please see a diff at: https://tinyurl.com/y5l4xz3z

Benjamin Kaduk via Datatracker <[e-mail]> wrote:
    > ----------------------------------------------------------------------
    > COMMENT:
    > ----------------------------------------------------------------------

    > [All new comments for the -28]

    > Please respond to the secdir re-review.

    > Abstract

    > nit: hyphenate "manufacturer-installed"

done.

    >    or on limited/disconnected networks.  Support for lower security
    > models, including devices with minimal identity, is described for

    > nit: maybe "deployment models with less-stringent security
    > requirements"?

I had actually edited the abstract into leaving that sentence incomplete, now fixed.

    > Section 1

    >    [I-D.ietf-anima-autonomic-control-plane].  This bootstrap process
    > satisfies the [RFC7575] section 3.3 of making all operations secure by
    > default.  Other users of the BRSKI protocol will need to provide

    > nit: missing "requirement"

already fixed it seems

    >    between manufacturer and owner: in it's default modes it provides a
    > cryptographic transfer of control to the initial owner.  In it's
    > strongest modes, it leverages sales channel information to identify

    > nit: s/it's/its/

already fixed.

    > Section 1.2

    >    domainID: The domain IDentity is a unique hash based upon the
    > Registrar CA's certificate.  Section 5.8.2 specifies how it is
    > calculated.

    > nit: the grammar here is weird; I'd suggest s/hash based upon/value
    > derived from/

done.

    >    MASA Audit-Log: A list of previous owners maintained by the MASA on
    > a per device (per pledge) basis.  Described in Section 5.8.1.

    > nit: maybe "anonymized list" since the MASA doesn't really track owners
    > directly?

done.

    >    Ownership Tracker: An Ownership Tracker service on the global
    > Internet.  The Ownership Tracker uses business processes to accurately
    > track ownership of all devices shipped against domains that have
    > purchased them.  Although optional, this component allows vendors to
    > provide additional value in cases where their sales and distribution
    > channels allow for accurately tracking of such ownership.  Ownership
    > tracking information is indicated in

    > nit: either "accurate tracking of" or "accurately tracking"

fixed.

    >    ANI: The Autonomic Network Infrastructure as defined by
    > [I-D.ietf-anima-reference-model].  This document details specific
    > requirements for pledges, proxies and registrars when they are part of
    > an ANI.

    > Maybe refer to section 9 specifically?

okay.

    > Section 2.3.1

    >    The serialNumber fields is defined in [RFC5280], and is a SHOULD
    > field in [IDevID].  IDevID certificates for use with this protocol

    > nit: s/fields/field/

fixed a different way for Christian.

    > Section 2.5.3

    >    The registrar uses an Implicit Trust Anchor database for
    > authenticating the BRSKI-MASA TLS connection MASA certificate.  The
    > registrar uses a different Implicit Trust Anchor database for
    > authenticating the BRSKI-EST TLS connection pledge client certificate.
    > Configuration or distribution of these trust anchor databases is
    > out-of-scope of this specification.

    >    Configuration or distribution of this trust anchor databases is out-
    > of-scope of this specification.  Note that the trust anchors in/
    > excluded from the database will affect which manufacturers' devices are
    > acceptable to the registrar as pledges, and can also be used to limit
    > the set of MASAs that are trusted for enrollment.

    > We seem to duplicate the "out-of-scope" text at the end/start of the
    > two paragraphs (and the second paragraph uses the definite article
    > "the" as if it was only talking about one of the two trust anchor
    > databases).

okay. keeping the second version, changing this->these.

    > Section 2.6.1

    >    A pledge with a real time clock in which it has confidence in, MUST
    > check the above time fields in all certificates and signatures that ir
    > processes.

    > nits: s/ir/it/, and s/in// from "in which it has confidence in" (your
    > choice which one).

done.

    > Section 2.6.2

    >    year certificate lifetimes.  Registrars SHOULD be configurable on a
    > per-manufacturer basis to ignore pledge lifetimes when they did not
    > follow the RFC5280 recommendations.

    > nit: we want "they" to be the manufacturer, not the registrar, so we
    > can't use this pronoun.

Should be pledge, fixed.

    > Section 2.7

    >    be more important in the future.  In the ANIMA ACP scope, new
    > devices will be quarantined behind a Join Proxy.

    > I can't decide whether the reader would benefit from being hit with a
    > hammer of "and as such will only have link-local connectivity, to the
    > Join Proxy".  The use of "quarantined" makes me lean towards "not"...

agreed.

    > Section 3.3

    doc>    Example (2) The following example illustrates a registrar voucher-
    doc> request.  The 'prior-signed-voucher-request' leaf is populated with the
    doc> pledge's voucher-request (such as the prior example).  The pledge's
    doc> voucher-request is a binary CMS signed object.  In the JSON encoding
    doc> used here it must be base64 encoded.  The nonce and assertion MAY be
    doc> carried forward from the pledge request to the registrar request.  The
    doc> serial-number is extracted from the pledge's Client Certificate from
    doc> the TLS connection.  See Section 5.5.

    > Since this is an example, the "MAY be carried forward" feels out of
    > place -- while it's true in the general case, this is not the place to
    > say it; we can just describe what the example does, here.

Agreed, I will change that to "have been carried forward"

    > Section 4

    >    This section applies is normative for uses with an ANIMA ACP.  The

    > nit: pick one of "applies to" or "is normative for".

The text now says:
   doc> This section is normative for uses with an ANIMA ACP.

    >    use of GRASP mechanism part of the ACP.  Other users of BRSKI will

    > nit: missing "is"

already fixed.

    > Section 4

    >    Registrar itself.  In this scenario the pledge is unaware that there
    > is no proxing occurring.  This is useful for Registrars which are

    > nit: s/proxing/proxying/

fixed, ty.

    > Section 4.3

    >    The M_FLOOD message MUST be sent periodically.  The default SHOULD
    > be 60 seconds, the value SHOULD be operator configurable but SHOULD be
    > not smaller than 60 seconds.  The frequency of sending MUST be such

    > nits: "default period" (or similar); s/be not/NOT be/

fixed.

    > Section 5

    >    While EST section 3.2 does not insist upon use of HTTP persistent
    > connections, ([RFC7230] section 6.3) BRSKI-EST connections SHOULD use

    > nit: comma after parenthetical, not before.

fixed.

    doc>    A self-signed certificate for the Registrar is acceptable as the
    doc> voucher will validate it.

    > nit: "will" applies only when everything works, so maybe "can" or "will
    > validate it in the case of successful enrollment".

okay.

    >    A pledge that can connect to multiple registries concurrently SHOULD

    > nit: s/registries/Regitstrars/

fixed.

    > Section 5.3

    >    on the authenticated identity presented.  For different networks,
    > examples of Automated acceptance may include:

    > nit: s/A/a/

fixed.

    > 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.


    >    client certificate.  Registrars SHOULD also permit an HTTP Basic and
    > Digest authentication to be configured.

    > nit: s/an//

fixed.

    > Section 5.4.1

    >    be uniquely identified.  This can be done by any stateless method
    > that HTTPS supports: such as with HTTP Basic or Digest authentication

    > nit: this colon is not appropriate usage.

ok.

    >    Stateful methods involving API tokens, or HTTP Cookies are not
    > recommended.

    > nit: zero commas or two, around "or HTTP Cookies", but one is right
    > out.

ok.

    >    This document does not make a specific recommendation as there is
    > likely different tradeoffs in different environments and product

    > nit: s/is/are/

ok.

    > Section 5.5

    >    idevid-issuer: The Issuer value from the pledge IDevID certificate
    > is included to ensure a uniqueness of the serial-number.  In the case
    > of nonceless (offline) voucher-request, then an appropriate value needs
    > to be configured from the same out-of-band source as the serial-number.

    > nit: I suggest s/a uniqueness of/a unique interpretation of/ (but if
    > you don't take that, the "a" is superfluous in the current
    > formulation).

ok.

    doc>    The MASA verifies that the registrar voucher-request is internally
    doc> consistent but does not necessarily authenticate the registrar
    doc> certificate since the registrar MAY not be known to the MASA in
    doc> advance.  The MASA performs the actions and validation checks

    > I suggest s/MAY not be known/MAY be unknown/.

ok.

    > Section 5.5.5

    doc>    voucher-request MUST include a 'proximity-registrar-cert' that is
    doc> consistent with to the certificate used to sign the registrar

    > nit: s/to// (first one)

ok

    > Section 5.6.1

    doc>    The pledge MUST verify that the voucher nonce field is accurate and
    doc> matches the nonce the pledge submitted to this registrar, or that the
    doc> voucher is nonceless (see Section 7.2).

    > In my previous Discuss position I had asked: % Is the pledge supposed
    > to accept a nonceless voucher in response to a % nonce-ful voucher
    > request?  and we had some discussion that helped clarify the intent.  I
    > just have a suggested rewording, that I *think* is entirely editorial
    > and might reduce the potential for confusion: NEW The pledge MUST
    > verify the nonce information in the voucher.  If present, the nonce in
    > the voucher must match the nonce the pledge submitted to the registrar;
    > vouchers with no nonce can also be accepted (according to local
    > policy).

okay.

    > Section 5.7

    doc>    The Status field indicates if the voucher was acceptable.  Boolean
    doc> values are acceptable.

    > nit: I suggest """acceptable, as a boolean, where "true" indicates the
    > voucher was acceptable""".

ok

    doc>    The version, and status fields MUST be present.  The Reason field
    doc> SHOULD be present whenever the status field is negative.  The Reason-
    doc> Context field is optional.

    > nit: no comma after "version".  nit: s/negative/false/

ok

    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 5.8.1

    doc> It's hard to call Figure 17 an "example" when it doesn't conform to the
    doc> CDDL schema...

fixed.

    doc>    The domainID is a binary value calculated SubjectKeyIdentifier
    doc> according to Section 5.8.2.  It is encoded once in base64 in order to
    doc> be transported in this JSON container.

    > nit: I suggest "binary SubjectKeyIdentifier value calculated"

ok.

    > Section 5.8.2

    doc>    used as the domainID.  If not, then it is the SPKI Fingerprint as
    doc> described in [RFC7469] section 2.4 is to be used.  This value needs

    > nit: drop "is to be used" or "then it is".

ok

    > Section 5.8.3

    doc>    A relatively simple policy is to white list known (internal or
    doc> external) domainIDs.  To require all vouchers to have a nonce.

    > nit: sentence fragment.

ok.

    doc>    Alternatively to require that all nonceless vouchers be from a
    doc> subset

    > nit: comma after "alternatively".  (Hmm, this is also a sentence
    > fragment.)

ok.

    > Section 5.9.4

    doc>    In order to communicate this indicator, the client HTTP POSTs the
    doc> following to the server at the new EST endpoint at "/.well-known/est/
    doc> enrollstatus".

    > "The following" is just more text, not a formal description of a
    > protocol element.

    >      "reason-context": "Additional information"

    > Is this supposed to be a string or a JSON object similar to
    > /voucher_status?

Yes, fixed this to be consistent.

    > Section 6

    doc>    When used within BRSKI, the original RFC7030 EST endpoints remain
    doc> Base64 encoded, but the new BRSKI end points which send and receive
    doc> binary artifacts (specifically, /requestvoucher) are binary.  That

    > The other two occurrences spell this "/.well-known/est/requestvoucher".

ok.

    > Section 7.2

    doc>    The following are a list of alternatives behaviours that the pledge
    doc> can be programmed to implement.  These behaviours are not mutually
    doc> exclusive, nor are they dependant upon other.  Some of these methods

    > nits: singular/plural mismatch "are"/"list"; "upon each other"

fixed.

    > Section 7.4.1

    >    A MASA has the option of not including a nonce is in the voucher,

    > nit: s/is//

already fixed.

    doc>    This is useful to support use cases where registrars might not be
    doc> online during actual device deployment.

    > nit: there's enough intervening text that we should probably replace
    > "this is" with "nonceless vouchers are".

ok.

    doc>    authorized to provide this functionality to this customer.  The MASA
    doc> is RECOMMENDED to use this functionality only in concert with an
    doc> enhanced level of ownership tracking (out-of-scope.)

    > I suggest s/out-of-scope/the details of which are out of scope for this
    > document/.

ok.

    > Section 7.4.2

    doc>    functionality.  This provides a proof-of-proximity check that
    doc> reduces the need for ownership verification.

    > I suggest reiterating the assumption that pledge and JP are on a
    > link-local connection; whether or not to reiterate that JP and
    > registrar have a trust relationship (with respect to not falsifying
    > proximity information) is less clear to me.

ok.

    > Section 7.4.3

    > We might benefit from some introductory text here to suggest that
    > updating/extending trust anchors would be desirable in the case of a
    > vanished or uncooperative manufacturer.

ok.

    doc>    This weaker factor reset might leave valuable credentials on the
    doc> device and this may be unacceptable to some owners.

    > nit: s/factor/factory/

ok.

    > Section 9

    doc>    Provider organizations.  Equivalent enterprises that has significant
    doc> layer-3 router connectivity also will find significant benefit,

    > nit: s/has/have/

ok.

    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

    doc>    asssertion is satisfied.  Other uses of BRSKI will need make similar
    doc> analysis if they use proximity.

    > nit: maybe "proximity information" or "proximity assertions".

ok.

    > Section 10.3

    doc>    While the contents of the signed part of the pledge voucher request
    doc> can not be changed, they are not encrypted at the registrar.  The
    doc> ability to audit the messages by the owner of the network a mechanism
    doc> to defend against exfiltration of data by a nefarious pledge.  Both

    > nit: missing verb ("is a mechanism", "provides a mechanism", etc.)

fixed.

    doc>    The manufacturer knows the IP address of the Registrar, but it can
    doc> not see the IP address of the device itself.  The manufacturer can not
    doc> track the device to a detailed physical or network location, only to
    doc> the the location of the Registrar itself.  That is likely to be at

    > nit: we probably don't need the last "itself".

okay.

    doc>    is likely on the same network as the device.  A manufacturer that
    doc> sells switching/routing products to enterprises should hardly be
    doc> surprised if additional purchases switching/routing products are
    doc> purchased.  Deviations from a historical trend or an establish

    > nit: we probably only need one of "purchases" and "purchased".

agreed.

    > Section 10.6

    doc>    Section Section 7.4.3 describes some ways in which a manufacturer

    > nit: duplicate "Section".

fixed.

    > Section 10.7

    doc>    existing products.  Said products might be previous deployed and
    doc> need to be re-initialized, purchased used, or just kept in a warehouse
    doc> as long-term spares.

    > nit: s/previous deployed and need/previously deployed that need/

fixed.

    > Section 11.2

    doc>    In particular implementations should pay attention to the advance in
    doc> [RFC4086] section 3, particulary section 3.4.  Devices which are reset
    doc> to factory default in order to perform a second bootstrap with a new
    doc> owner MUST NOT seed their random number generators in the same way.

    > nit: s/same way/same way across bootstraps/

fixed.

    > Section 11.5

    >    This next section examines the risk due to a compromised MASA key.
    > This is followed by examination of the risk of a compromised
    > manufacturer IDevID signing key.  The third section sections below

    > nit: I think these appear in the other order.

done.

    > Section 13.1

    > [cabforumaudit] and [dnssecroot] probably would be fine as just
    > informative references.

okay.


--
Michael Richardson <[e-mail]>, Sandelman Software Works
 -= IPv6 IoT consulting =-