Subject: [Anima] Iotdir telechat review of draft-ietf-anima-bootstrapping-keyinfra-17

From: Russ Housley <[email-masked]>
Date: Mon, 03 Dec 2018 15:49:07 -0800

Reviewer: Russ Housley
Review result: Not Ready

I reviewed this document as part of the IoT Directorate's effort to
IoT-related IETF documents being processed by the IESG.  These comments
were written primarily for the benefit of the Internet Area Directors.
Document authors, document editors, and WG chairs should treat these
comments just like any other IETF Last Call comments.

Document: draft-ietf-anima-bootstrapping-keyinfra-17
Reviewer: Russ Housley
Review Date: 2018-11-28
IETF LC End Date: unknown
IESG Telechat date: unknown

Summary: Has Issues

(Note: I did not review the Appendices.)

Major Concerns:

Section 2.2 says:

   ...  The registrar
   maintains control over the transport and policy decisions allowing
   the local security policy of the domain network to be enforced.

I have no idea what this means.  Please clarify.

In Section 2.3, it says:

   5.  (Optional) Signing of voucher-request by the pledges IDevID to
       enable MASA to generate voucher only to a registrar that has a
       connection to the pledge.

This is an important section to understand BRSKI, but I cannot parse
this sentence.  Please reword.

Section 2.3.1 talks about pledges being uniquely identified by a
"serial-number" in voucher and voucher-requests.  Pledges are also
uniquely identified by their serial number in certificates.

Section 2.3.1 refers to HardwareModuleName, which is defined in
RFC 4108.  It says that the HardwareModuleName hwSerialNum is base64
encoded.  RFC 4108 does not require base64 encoding.  Where does that
requirement come from?

Section 3.1 shows the YANG tree model of the Voucher-Request.  I am far
from a YANG expert, but I expected a subsequent section to describe the
semantics of each field.  The examples in Section 3.2 are useful, but
they are not a replacement.  Some fields (like voucher/expires-on) are
not described in Section 3.3.  I assume that this is building on another
module because this one contains "import ietf-voucher", but this does
not say what RFC contains the imported module to learn the rest of the
semantics.

I think that the CDDL in Sections 4.1.1 and 4.3 are supposed to be
structures.  If that is correct, the structure should look something
like the following, which includes type information:

   basic-header = [
     field1: int,
     field2: text,
   ]

   advanced-header = [
     ~basic-header,
     field3: bytes,
     field4: ~time,
   ]

I have no idea what the boxes in Figure 10 represent.

Section 7.2 does not contain enough information to make the needed
object identifier assignments.

In Section 7.4, the IESG (not the IETF Chair) should be the contact
for standards-track registrations.

I think the security considerations ought to describe the consequences
of compromise of the various private keys in the ecosystem.  Some only
impact one device, but others have much greater impact.

I think the security considerations ought to say something about the
nonce.  First, it should point to RFC 4086.  Second, it should say
something about the consequences of a poor random source.  It does not
need to be a comprehensive as the section dealing with setting time.

Minor Concerns:

The Introduction says:

   BRSKI provides a solution for secure zero-touch (automated) bootstrap
   of virgin (untouched) devices that are called pledges in this
   document.  These pledges need to discover (or be discovered by) an
   element of the network domain to which the pledge belongs to perform
   the bootstrap.  This element (device) is called the registrar. ...

I find the two uses of device to be a bit confusing.  I suggest:

   Bootstrapping Remote Secure Key Infrastructures (BRSKI) provides a
   solution for secure zero-touch (automated) bootstrap of virgin
   (untouched) devices, called pledges.  Pledges need to discover (or be
   discovered by) an element of the network domain to which the pledge
   belongs to perform the bootstrap, called the registrar. ...

In the Introduction, in the paragraph that follows the description of
the four points of trust, I think there is a significant typo.  An
IEEE 802.1AR LDevID certificate addresses points 1 and 2, and then a
"voucher" addresses points 3 and 2.  I suspect that the second "2"
should be "4".  If not, some additional text is needed to cover point 4.

Section 1.2: Please update the first paragraph to reference RFC 8174
in addition to RFC 2119, as follows: 

   The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT",
   "SHOULD", "SHOULD NOT", "RECOMMENDED", "NOT RECOMMENDED", "MAY", and
   "OPTIONAL" in this document are to be interpreted as described in
   BCP 14 [RFC2119] [RFC8174] when, and only when, they appear in all
   capitals, as shown here.

I think that "/ next steps" should be dropped from the title of
Section 1.4.

I think that Section 1.5 would be well served with bullets that are full
sentences.  For example, the document says:

   o  BRSKI: Request Voucher

This terse phrase, I think, covers the requesting of a voucher by the
pledge from the MASA, and then receiving the voucher.  Alternatively,
you could provide forward pointers that describe each of these terse
phrases.

Section 2 uses "Manufacturer Service" and "Vendor Service".  I think
they are the same thing.  Neither of these is defined in Section 1.2,
so I am not totally sure.  Please clarify.

Section 2 says:

   ...  It
   is unlikely that an integrator could provide Ownership Tracking
   services for multiple manufacturers due to the required sales channel
   integrations necessary to track ownership. ...

However, it seems possible for the integrator to provide a referral to
the appropriate Ownership Tracking service or provide a proxy.

Section 2 talks about a "PKI Registration Authority".  No context is
given.  At a minimum, a reference to RFC 5280 seems appropriate.  If
there is a more specific role that an RA might play in BRSKI, then
that should be explained or referenced.

Section 2.5.3 talks briefly about the "Implicit Trust Anchor database".
Please add a reference to RFC 7030 here; it is needed to understand the
concept.

In the security considerations, it would be good to cover how the owner
of a device might cope the a manufactured going out of business or
stopping support for a device.  This is really not a problem if the
device is already on a network and it does not ever need to move to
another network.

Section 11.1: Please add a normative reference to RFC 8174.

Nits:

The abstract says:

   ... Bootstrapping a new device can
   occur using a routable address and a cloud service, or using only
   link-local connectivity, or on limited/disconnected networks. ...

This is a complex sentence.  Can it be up-leveled for the Abstract?
If not, can it be reworded to be more straightforward? 

Please spell out "BRSKI" in the first sentence of the Introduction.
I did this in my suggested text in the previous portion of this review.

The quotes do not balance in the second to last paragraph of the
Introduction.

Please put the terms that are defined in Section 1.2 in alphabetical
order.

In section 1.2, the definition of "drop ship" talks about "drop-ship".
Please pick one spelling.

In Section 1.2, you define "Join Registrar (and Coordinator)", but the
Introduction seems to talk about this role simply as the "Registrar".
I think it would be helpful to include a "Registrar" entry here to
help the reader.  Similarly for "Join Proxy" and "Proxy".

The Introduction talks about IEEE 802.1AR LDevID; please add LDevID to
the definitions in Section 1.2.

In Figure 1: s/PKI Certificate Authority/PKI Certification Authority/

In Section 2.3: s/fake pledged/fake pledges/

In Section 2.3: s/pledges MASA/pledge's MASA/

In Section 2.3: s/(see Appendix C./(see Appendix C)./

In Section 2.3: s/pledges IDevID/pledge's IDevID/

In Section 2.6.1, the list of five time sources should be turned
into a real sentence.

In Section 2.6.1, please turn "See / (Section 5.2)" into a sentence.

In many places: s/global internet/global Internet/

In many places: s/internet at large/Internet at large/

Lists of bullets come in many flavors.  Some bullets end with a comma,
some end with a period, and others end with no punctuation at all.
Please pick one style and use it everywhere.

FYI:

I compiled the MASAURLExtnModule-2016 ASN.1 module.  It compiles fine,
but I find the indent and white space style used awkward.

---

From: Michael Richardson <[email-masked]>
Date: Tue, 04 Dec 2018 11:49:00 -0000

Thank you Russ!
I will turn your comments into issues, attempting to de-duplicate against
what we already have.

--
Michael Richardson <[email-masked]>, Sandelman Software Works
 -= IPv6 IoT consulting =-

---

From: Michael Richardson <[email-masked]>
Date: Thu, 13 Dec 2018 11:27:07 -0500

> I have no idea what this means.  Please clarify.

duplicates issue raised by Jari:
https://github.com/anima-wg/anima-bootstrap/issues/92

> this sentence.  Please reword.

https://github.com/anima-wg/anima-bootstrap/issues/93

> encoded.  RFC 4108 does not require base64 encoding.  Where does that

duplicates:
https://github.com/anima-wg/anima-bootstrap/issues/95

> not say what RFC contains the imported module to learn the rest of the

I think that the sentence:
  The notation used in this diagram is described in [RFC8366].

should be changed to say:
   The voucher-request builds upon
   the voucher artifact described in <xref target="RFC8366" />.
   The tree diagram is described in <xref target="RFC8340" />.

(we described tree-diagrams in 8366 at one point, because we didn't know if
8340 would get published in time)

> like the following, which includes type information:

We are filling in the gaps for the definition in GRASP M_FLOOD
mechanism.  We aren't defining a new structure.
I'm not sure if we can do this any other way.

> I have no idea what the boxes in Figure 10 represent.

Hmm. I guess we chopped the boxes off of the flow from section 2.4.
Would a reference back to section 2.4 help?
Maybe we should not repeat the boxes.

> object identifier assignments.

Right we had a note to fix that. It's: SMI Security for PKIX Certificate Extension
  https://www.iana.org/assignments/smi-numbers/smi-numbers.xml#smi-numbers-1.3.6.1.5.5.7.1

> for standards-track registrations.

Done.

> impact one device, but others have much greater impact.

okay.
I'm expanding issue #80 to include describing this.
    https://github.com/anima-wg/anima-bootstrap/issues/80

> need to be a comprehensive as the section dealing with setting time.

I've expanded issue #91:
     https://github.com/anima-wg/anima-bootstrap/issues/91

> Minor Concerns:

next email.

---

From: Russ Housley <[email-masked]>
Date: Thu, 13 Dec 2018 12:16:20 -0500

> 8340 would get published in time)

Okay.  That works for me.

> I'm not sure if we can do this any other way.

Something needs to be done to set the context.  Clearly, I misunderstood the intent.

> Maybe we should not repeat the boxes.

I reference to Section 2.4 with no boxes would be more helpful than the current figure.

>  https://www.iana.org/assignments/smi-numbers/smi-numbers.xml#smi-numbers-1.3.6.1.5.5.7.1

I guessed that, but no guessing should be needed for IANA registry assignment.

Russ

---

From: Brian E Carpenter <[email-masked]>
Date: Fri, 14 Dec 2018 09:40:19 +1300

On one point:

On 2018-12-14 06:16, Russ Housley wrote:
...
> Something needs to be done to set the context.  Clearly, I misunderstood the intent.

The CDDL which is being extended is defined in https://tools.ietf.org/html/draft-ietf-anima-grasp-15#section-5,
and that draft is fully approved but waiting in MISSREF limbo.

I don't have any concerns about the BRSKI CDDL because I did a toy implementation (Pledji.py, Procksy.py and Reggie.py at https://github.com/becarpenter/graspy). The CDDL in the draft has changed very slightly since then; when I have some free time I can update my Python code, but the differences are negligible.

[Side note: I don't think anybody would claim that they can automatically generate running code from CDDL yet. If that becomes realistic, it would be the acid test for CDDL used in protocol specs. But it's quite a way in the future, IMHO.]

Regards
    Brian