Subject: [Anima] GEN-ART review of constrqained-voucher

From: Michael Richardson <[email-masked]>
Date: Wed, 23 Jun 2021 23:55:45 -0400

Russ provided a welcome early review of
draft-ietf-anima-constrained-voucher-10 on May 13.
We have crunched through this, and created a pull request:
   https://github.com/anima-wg/constrained-voucher/pull/120

    rh> Major Concerns:

    rh> Section 4 says: "... sign using its IDevID X.509 certificate, or if
    rh> an IDevID is not available its manufacturer-installed raw public key
    rh> (RPK)."

    rh> This is not accurate.  Signatures are created with a private key, and
    rh> then they are validated with the public key in a certificate.  A sign
    rh> operation cannot use an RPK; a signnature validation operation might.

We have changed the text to say:
   It signs using the private key associated with its IDevID X.509
   certificate, or if an IDevID is not available, then the private key
   associated with its manufacturer-installed raw public key (RPK).

Does this rewording work better for you?

    rh> Section 6.2 says: "... and MUST NOT distinguish between them."  There
    rh> are many different contexts that one might "distinguish" that are
    rh> fine.  I think you mean that the implementation MUST respond to the
    rh> two in the same manner.

Our text now says:
    Registrars that have implemented shorter URLs MUST also respond in
    equivalent ways to the corresponding "/.well-known/brski/\<short-name\>"
    URLs, and MUST NOT distinguish between them.

    rh> Section 6.4.1 is confusing to me.  The section says that a
    rh> "constrained Pledge MAY use the following optimized EST-coaps
    rh> procedure", however, there are MUST statements in the numbered
    rh> paragraphs that follow.

This is section 6.3.1 ("Pledge Extension") in version 11.
Point 4 has a MUST.
Points 1,2,3 were the optimization, if the Pledge opted not to use the MAY
process, then would have jumped to point 4, which is also RFC8995 section
5.9.1 (EST Distribution of CA Certificates).

I wonder how can write this better.

    rh> Minor Concerns:

    rh> Section 1 says: "...  As the tooling to convert YANG documents into a
    rh> list of SID keys is still in its infancy, the table of SID values
    rh> presented here **should** be considered normative rather than the output
    rh> of the pyang tool."

    rh> It is unclear to me what this means to an implementer.  When does
    rh> this situation change?  This seems to be begging for a MUST or
    rh> SHOULD.

I've changed the **should** to MUST.
The situation does not change until this document is replaced, I think.

If someone creates an implementation of this document using a SID-aware,
YANG-driven code generator, then they had better use the values in the
document, not the the values derived through the yang-sid process.
(Or they'd better check!)

We expected that draft-ietf-core-yang-cbor and draft-ietf-core-sid to
have been published already, and maybe at that point, since we are stuck with
a normative reference to them, we would be able to see that IANA isn't going
to renumber of our keys on us, and we could remove this language sometime
at or before AUTH48... but...

    rh> Section 5 says: "Saving header space is important...".  Okay.  It is
    rh> not just the headers.  Maybe something like: "To keep the protocol
    rh> messages small..."

okay.

    rh> Section 5 ends with: "For example, the following more complete
    rh> response is possible."  Nothing follows.  Not sure what is meant.

This paragraph has been rewritten enough that nothing is left.

    rh> Section 6 and Section 6.1 say the same thing.  Drop one.

Yes, that was a goof, already fixed.

    rh> The last paragraph of Section 6.4.1 and the only paragraph in Section
    rh> 6.4.2 say the same thing.  Drop one.

also a goof.

    rh> Section 8.1 ends with:

    }    When the Registrar is using a COSE-signed constrained format
    } voucher request towards MASA, instead of a regular CMS-signed voucher
    } request, the COSE_Sign1 object contains a protected and an
    } unprotected header, and according to [I-D.ietf-cose-x509], would
    } carry all the certificates of the chain in an "x5bag" attribute
    } placed in the unprotected header.

    rh> I think there should be a MUST statement in this paragraph.  Maybe:

    ]    When the Registrar is using a COSE-signed voucher instead of a
    ] CMS-signed voucher, the COSE_Sign1 object contains a protected header
    ] and an unprotected header.  The Registrar MUST place all all the
    ] certificates for the chain in an "x5bag" attribute in the unprotected
    ] header [I-D.ietf-cose-x509].

Adopted.

    rh> Section 8.2 says: "... reduce on average the duration ...".  I cannot
    rh> figure out what is actually being reduced.  Please add more
    rh> explanation or drop the bulk of the sentence.  Rationale that comes
    rh> later in the section does not say anything about duration being
    rh> reduced, but it does talk about avoiding retransmission of the same
    rh> certificates.

{now section 7.2}
I'll think about how to reword.  I don't know right now.

The network is slow.  Each enrollment could take a few seconds given the
number of bytes that need to be exchanged.
The certificates aren't retransmitted (in the sense of TCP), but are
redundantly sent.  Not sending them means less bandwidth used, which reduces
how long each onboarding event takes.

    rh> Nits:

    rh> Abstract: Please remove the references.  The RFC Editor Guidlines for
    rh> Abstracts include: "An Abstract should be complete in itself; it
    rh> should not contain citations unless they are completely defined
    rh> within the Abstract."

How do we say that are extending protocol FOO?

I won't comment on the rest of the nits unless I don't understand.

    rh> Section 8.3, Figure 2: Please find fome other way to represent
    rh> [RPK3].  This looks like a reference, and that is not the intent.

okay.

    rh> Section 8.3, first paragraph after Figure 2: s/certificate-less
    rh> enrollment/enrollment without certificates/

done.

--
Michael Richardson <[email-masked]>   . o O ( IPv6 IøT consulting )
           Sandelman Software Works Inc, Ottawa and Worldwide

---

From: Carsten Bormann <[email-masked]>
Date: Thu, 24 Jun 2021 06:11:13 +0200

> (Or they'd better check!)

How would that be different if yang-cbor/sid were already published?

> at or before AUTH48... but…

I would expect specifications to include a SID file together with the YANG spec.

Grüße, Carsten

---

From: Michael Richardson <[email-masked]>
Date: Thu, 24 Jun 2021 12:01:01 -0400

Carsten Bormann <[email-masked]> wrote:
    >> If someone creates an implementation of this document using a
    >> SID-aware, YANG-driven code generator, then they had better use the
    >> values in the document, not the the values derived through the
    >> yang-sid process.  (Or they'd better check!)

    > How would that be different if yang-cbor/sid were already published?

In that case, we would submit our sid file, IANA would process it, and we'd
get SID definitions, and we could be sure that it would be stable.
(Or rather, that's at least, making it stable is not our problem)

    >> have been published already, and maybe at that point, since we are
    >> stuck with a normative reference to them, we would be able to see that
    >> IANA isn't going to renumber of our keys on us, and we could remove
    >> this language sometime at or before AUTH48... but…

    > I would expect specifications to include a SID file together with the

Would you?  I supposed, but we haven't done that as yet.
I can easily add our SID file.

I don't know.  It's a question that I've asked multiple times, and the
answers have been very hand-wavey.   Does ietf-core-sid say we are going to
do that?  I don't see that anywhere.

Section 3 says:

   Registration of the ".sid" file associated to a YANG module is
   optional but recommended to promote interoperability between devices
   and to avoid duplicate allocation of SIDs to a single YANG module.
   Different registries might have different requirements for the
   registration and publication of the ".sid" files.  For a diagram of
   one of the possibilities, please refer to the activity diagram on
   Figure 1 in Appendix C.

The diagram doesn't explicitly say that I include the sid file.
It was stated IANA would be creating and maintaining SID files for anything
previously published.

--
]               Never tell me the odds!                 | ipv6 mesh networks [
]   Michael Richardson, Sandelman Software Works        |    IoT architect   [
]     [email-masked]  http://www.sandelman.ca/        |   ruby on rails    [