Subject: [netconf] Benjamin Kaduk's No Objection on draft-ietf-netconf-sztp-csr-12: (with COMMENT)

From: Benjamin Kaduk via Datatracker <[email-masked]>
Date: Tue, 14 Dec 2021 16:09:52 -0800

Benjamin Kaduk has entered the following ballot position for
draft-ietf-netconf-sztp-csr-12: No Objection

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)

Please refer to https://www.ietf.org/blog/handling-iesg-ballot-positions/
for more information about how to handle DISCUSS and COMMENT positions.

The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-netconf-sztp-csr/

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

Thanks for this document; it seems useful to have the flexibility to
specify a CSR and get an LDevID during enrollment.

I had a similar concern as Zahed's Discuss point (though I'm not sure
I can really express my concerns very well yet), and will attempt to follow
the discussion thereof.

Section 2.1

Why do we need the "sztp-csr" namespace prefix on the new nodes in the
get-bootstrapping-data example, but not need the "ietf-sztp-csr" prefix
on the nodes in the ietf-restconf/errors example?

Section 2.3

           description
             "Provides the CSR generated by the SZTP-client.

              When present, the SZTP-server SHOULD respond with
              an SZTP onboarding information message containing
              a signed certificate for the conveyed CSR.  The
              SZTP-server MAY alternatively respond with another
              HTTP error containing another 'csr-request', in
              which case the SZTP-client MUST invalidate the
              previously generated CSR.";

I'm curious what is intended by "invalidate the [...] CSR".  My
background on CSR usage hasn't really encountered a sense of "validity"
for them that would need cancellation, since it's not really a important
piece of information in its on right (just a way to ask a CA to do
something).  The interesting bit is whether the CA actually issued
anything, but I'm not sure how much harm a stale CSR would actually
cause.

Section 3.2

     identity cmp-csr {
       base certificate-request-format;
       description
         "Indicates that the ZTP-client supports generating
          requests using a constrained version of the PKIMessage
          containing a p10cr structure defined in RFC 4210.";

I'd probably spend a few more words to indicate the nature of the
constraints that cause us to say "constrained version".
(Likewise for the cmc-csr.)

            Enables the ZTP-server to provide a fully-populated
            CertificationRequestInfo structure that the ZTP-client
            only needs to sign in order to generate the complete
            'CertificationRequest' structure to send to ZTP-server
            in its next 'get-bootstrapping-data' request message.

            When provided, the ZTP-client SHOULD use this structure
            to generate its CSR; failure to do so MAY result in a
            400 Bad Request response containing another 'csr-request'
            structure.

This guidance seems to risk running afoul of the first rule of PKIs:
know what you sign.  While I understand that the context for this
protocol exchange is a relatively "dumb" device getting onboarded into
the owner's deployment, and that the owner may in fact require some
things in the LDevID that were not coded into the device by its
implementors, I don't think we should stand mute on the security
considerations of asking for a certificate that contains
attributes/restrictions/etc. that are not understood.

           leaf cmc-csr {
             type binary;
             description
               "A constrained version of the 'Full PKI Request'
                message defined in RFC 5272, encoded using ASN.1

(Same comment about "constrained version" as above.)

                For asymmetric key-based origin authentication based on
                the initial device identity certificate's private key
                that signs the encapsulated CSR signed by the local
                device identity certificate's private key, the PKIData
                contains one cmsSequence element and no
                otherMsgSequence element.  The cmsSequence is the

This part says nothing about whether there is a reqSequence element in
the toplevel PKIData...

                TaggedContentInfo and it includes a bodyPartID element
                and a contentInfo.  The contentInfo is a SignedData
                encapsulating a PKIData with one reqSequence element
                and no cmsSequence or otherMsgSequence elements. The
                reqSequence is the TaggedRequest and it is the tcr
                CHOICE. The tcr is the TaggedCertificationRequest and
                it a bodyPartId and the certificateRequest elements.
                [...]

... since this reqSequence seems to refer to the PKIData inside the
SignedData in the contentInfo in the cmsSequence.  Should we say
anything about the presence/absence of reqSequence in the toplevel
PKIData (since we do in the other two cases)?

         case cmp-csr {
           leaf cmp-csr {
             type binary;
             description
               [...]
                For asymmetric key-based origin authentication of a
                CSR based on the initial device identity certificate's
                private key for the associated initial device identity
                certificate's public key, PKIMessages contains one
                PKIMessage with the header and body elements, no
                protection element, and should contain the extraCerts
                element. [...]

Is this a "should" or a "SHOULD"?

                For asymmetric key-based origin authentication based on
                the initial device identity certificate's private key
                that signs the encapsulated CSR signed by the local
                device identity certificate's private key, PKIMessages
                contains one PKIMessage with the header, body, and
                protection elements, and should contain the extraCerts
                element. [...]

(ditto)

Section 4

I'd consider also mentioning the CMC, CMP, and PKCS#10 security
considerations as being applicable to the relevant CSR choices.

Section 4.1.1

   The security of this private key is essential in order to ensure the
   associated identity certificate can be used as a root of trust.

"root of trust" is something of a loaded term (see the ongoing
discussion in the RATS WG for differing interpretations of what it
means).  I'd suggest using a different phrasing, like "can be used to
authenticate the device it is issued to".

Section 4.1.3

Should we mention the "nonce" parameter to get-bootstrapping-data
here?

   When a public/private key pair associated with the manufacturer-
   generated identity certificate (e.g., IDevID) is used for the
   request, there may not be confirmation to the SZTP-client that the
   response has not been replayed; however, the worst case result is a
   lost certificate that is associated to the private key known only to
   the SZTP-client.

That's only the worse-case result if we assume that the private key is
not compromised.  We might want to reiterate that assumption and/or
mention the scope of consequences for the case where the private key is
compromised.

Section 4.2.2

   When a new asymmetric key is used, with the CMP or CMC formats, the
   parent ASN.1 structure of the CSR provides origin authentication
   using either the manufacturer-generated private key or a shared
   secret.  In this way the proof-of-possession of the CSR is directly
   linked to the proof-or-origin provided by the parent ASN.1 structure.

I think this section would be a good place to talk about how in the "raw
PKCS#10" case, there needs to be a level of trust in the bootstrapping
server roughly analogous to how an RA would be trusted, in that they are
the only entity that has verified the client identity and the
bootstrapping server's assertion of the client identity is a key step in
the certificate issuance process.

Such discussion might also contrast use of the IDevID at the TLS layer
with HTTP-layer authentication of the client.

Section 4.4

Though our "identity" and "grouping" statements (as noted) to not appear
in any protocol-accessible nodes as-is, we might still feel empowered to
discuss any considerations that would apply when they actually are
instantiated in other modules.  However, it seems that there would not
be anything new to say other than what's in RFCs 2986, 4210, and 5272,
and if we take my suggestion to reference them from the toplevel
section 4, there would be no need to do so again here.

NITS

Abstract, Introduction

I'd consider clarifying that when we "extend" the get-bootstrapping-data
RPC to include a CSR, that's as the input to the RPC, not the output.
(I initially misread it when reading the abstract.)

Section 2.2

You may want to use more-recent dates than 2015 for the examples.

I suggest using different "BASE64VALUE="s for the different public
keys in the two-asymmetric-key example.

Section 3.2

             leaf-list algorithm-identifier {
               type binary;
               min-elements 1;
               description
                 "An AlgorithmIdentifier, as defined in RFC 2986,
                  encoded using ASN.1 distinguished encoding rules
                  (DER), as specified in ITU-T X.690.";

At risk of excessive pedanticism, I see RFC 2986 discuss the
parametrized AlgorithmIdentifier{}, which compares it to the
unparameterized AlgorithmIdentifier in the ensuing discussion.
AlgorithmIdentifier is (also) defined in, e.g., RFC 5280 if we didn't
want to get into the parameterization here.  (Any change made here
should be made throughout, of course.)

     grouping csr-grouping {
       description
         "Enables a ZTP-client to convey a certificate signing
          request, using the encoding format selected by a
          ZTP-server's 'csr-request' response to the ZTP-client's
          previously sent 'get-bootstrapping-data' request
          containing the 'csr-support' node.";

(nit) Since we're (now) in just the ztp-types module, we can't
necessarily assume there will be a "get-bootstrapping-data" request
involved.

                For asymmetric key-based origin authentication of a
                CSR based on the initial device identity certificate's
                private key for the associated identity certificate's
                public key, the PKIData contains one reqSequence

(nit) I think the "associated identity certificate's public key"
phrasing here is a bit confusing.  I think that we're trying to cover
the case where the key from the IDevID is reused for the LDevID, so
we're using the LDevID private key to authenticate the CSR and it's
sigining "it's own" public key", but in the context of this YANG
grouping we've mostly lost the context of the "associated identity
certificate".
(A similar comment applies to the CMP case.)

                elements. The reqSequence is the TaggedRequest and it
                is the tcr CHOICE. The tcr is the

(nit) should we have another word after "CHOICE", like "arm" or
"branch"?

                TaggedCertificationRequest and it a bodyPartId and the

(nit) also, I'd maybe s/the/a/ for these two "the <foo> is the <bar>"
constructions since they seem to just be describing the type of a given
ASN.1 element.
Also, s/it/it is/.

Section 4.1.1

   private keys.  For instance, an NMS controller/orchestrator
   application could periodically prompt the SZTP-client to generate a
   new private key and provide a certificate signing request (CSR) or,
   alternatively, push both the key and an identity certificate to the
   SZTP-client using, e.g., a PKCS #12 [RFC7292].  [...]

maybe "a PKCS #12 message"?

Section 4.1.5

   The CMP and CMC certificate request formats defined in this document
   support origin authentication.  A raw PKCS#10 does not support origin
   authentication.

"PKCS#10 CSR"

---

From: Kent Watsen <[email-masked]>
Date: Tue, 21 Dec 2021 17:55:13 +0000

Hi Ben,

Thank you for your thoughtful review.  Responses to your comments are below.

Also, diffs are here: https://github.com/netconf-wg/sztp-csr/commit/ee5b28d35ec26c895ce4691f85affd1487d51f3e <https://github.com/netconf-wg/sztp-csr/commit/ee5b28d35ec26c895ce4691f85affd1487d51f3e>.

Best regards,
Kent (and Russ and Sean)

> On Dec 14, 2021, at 7:09 PM, Benjamin Kaduk via Datatracker <[email-masked]> wrote:
> 

Zahed’s discuss is ongoing, but I think we’re just an edit away now.

> on the nodes in the ietf-restconf/errors example?

You write “example” but I think you mean “tree diagrams”.  The tree-diagram at the top of page 7 is stitched together via some scripts.  Technically, the parent node, "error-info”, is of type “yang:anydata”, which is a YANG for “anything, no data model needed, can go here”, which means what is there now isn’t wrong.  That said, your observation is valid, the dichotomy is stark.  I have updated the scripts to prefix all those nodes the same as in the earlier tree diagram.  Since you cannot see what it looks like from the GitHub diff, the new rendered view follows:

   module: ietf-restconf
     +--ro errors
        +--ro error* []
           +--ro error-type       enumeration
           +--ro error-tag        string
           +--ro error-app-tag?   string
           +--ro error-path?      instance-identifier
           +--ro error-message?   string
           +--ro error-info
              +--ro sztp-csr:csr-request
                 +--ro sztp-csr:key-generation!
                 |  +--ro sztp-csr:selected-algorithm
                 |     +--ro sztp-csr:algorithm-identifier    binary
                 +--ro sztp-csr:csr-generation
                 |  +--ro sztp-csr:selected-format
                 |     +--ro sztp-csr:format-identifier    identityref
                 +--ro sztp-csr:cert-req-info?    ct:csr-info

> anything, but I'm not sure how much harm a stale CSR would actually

The intention of “invalidate” was to, e.g., delete the private key that may have been generated specifically for the previous CSR.  Thinking about this now, the authors are of the opinion that deleting the private key is likely all that any SZTP-client could ever do.  That being the case, we suggest:

OLD: the SZTP-client MUST invalidate the previously generated CSR.
NEW: the SZTP-client MUST delete any key generated for the previously generated CSR.

Makes sense?

> (Likewise for the cmc-csr.)

The authors think that “profiled” is a better word than “constrained”, as so suggest:

   description
     "Indicates that the ZTP-client supports generating
      requests using a profiled version of the PKIMessage
      that MUST contain a PKIHeader followed by a PKIBody
      containing only the p10cr structure defined in RFC 4210.";

Thoughts?

> attributes/restrictions/etc. that are not understood.

After a little discussion, the authors are unable to understand a scenario where it makes sense that a ZTP-client would disregard and/or decorate/extend the CertificationRequestInfo structure from the ZTP-server.  That being the case, we recommend s/SHOULD/MUST/ and s/MAY/will/ as follows:

           When provided, the ZTP-client MUST use this structure
           to generate its CSR; failure to do so will result in a
           400 Bad Request response containing another 'csr-request'
           structure.

Better?

>                distinguished encoding rules (DER), as specified
>                in ITU-T X.690.
> 

The authors again think that “profiled” is a better word than “constrained”, as so suggest:

    description
            "A profiled version of the 'Full PKI Request'
             message defined in RFC 5272, encoded using ASN.1
             distinguished encoding rules (DER), as specified
             in ITU-T X.690.

Thoughts?

>                otherMsgSequence element.  The cmsSequence is the ...
> 

The authors propose:

OLD: and no otherMsgSequence element. 
NEW: and  no controlSequence, reqSequence, or otherMsgSequence elements. 

Good?

> PKIData (since we do in the other two cases)?

Yes, the authors propose:

OLD: […]

NEW:   PKIData contains one cmsSequence element and no
            controlSequence, reqSequence, or otherMsgSequence
            elements. […]

Good?

> Is this a "should" or a "SHOULD"?

Yes.  Fixed.

>                protection elements, and should contain the extraCerts

Also now a "SHOULD” here.

> considerations as being applicable to the relevant CSR choices.

Silly authors, we forgot the apple pie  ;)

We added:

   For the various CSR formats, when using PKCS#10, the security
   considerations in [RFC2986] apply, when using CMP, the security
   considerations in [RFC4210] apply and, when using CMC, the security
   considerations in [RFC5272] apply.

> authenticate the device it is issued to".

Fix applied - thanks!

> Should we mention the "nonce" parameter to get-bootstrapping-data

Hmmm, if we were to mention the “nonce” parameter at all, it would only be to say that it has *nothing* to do with the CSR mechanism presented in this document…or, at least it doesn’t have any relation currently.  To be clear, the “nonce” parameter is defined in RFC 8572 (SZTP) as follows:

      leaf nonce {
        type binary {
          length "16..32";
        }
        description
          "This optional input parameter enables a device to
           communicate to the bootstrap server a nonce value.
           This may be especially useful for devices lacking
           an accurate clock, as then the bootstrap server
           can dynamically obtain from the manufacturer a
           voucher with the nonce value in it, as described
           in RFC 8366.";
        reference
          "RFC 8366:
             A Voucher Artifact for Bootstrapping Protocols";
      }

Thoughts?

> mention the scope of consequences for the case where the private key is

Good point.  We appended the following to the end of the paragraph:

	Protection of the private-key information is vital to public-key
	cryptography.  Disclosure of the private-key material to another
	entity can lead to masquerades.

Better?

> the certificate issuance process.

The authors propose replacing the paragraph above with the following:

     When the bootstrapping device's manufacturer-generated private key
     (e.g., the IDevID key) is reused for the CSR, proof-of-origin is
     verified by validating the IDevID certification path and ensuring that
     the CSR uses the same key pair.

     When a fresh asymmetric key is used with the CMP or CMC formats, the
     authentication is part of the protocols, which could employ either
     the manufacturer-generated private key or a shared secret.  In addition,
     CMP and CMC support processing by a RA before the request is passed
     to the CA, which allows for more robust handling of errors.

Thoughts?

> with HTTP-layer authentication of the client.

The authors believe this is best addressed with more “apple pie” in the top-level of Section 4 (Security Considerations):

   For the various authentication mechanisms, when using TLS-level
   authentication, the security considerations in [RFC8446] apply and,
   when using HTTP-level authentication, the security considerations in
   [RFC7235] apply.

Would this satisfy your discuss?

> section 4, there would be no need to do so again here.

We put the “apple pie” into the top-level of Section 4.

> (I initially misread it when reading the abstract.)

Done.

> You may want to use more-recent dates than 2015 for the examples.

Done.

> keys in the two-asymmetric-key example.

Not fixed, given:

1.4.  Conventions

   Various examples used in this document use a placeholder value for
   binary data that has been base64 encoded (e.g., "BASE64VALUE=").
   This placeholder value is used as real base64 encoded structures are
   often many lines long and hence distracting to the example being
   presented.

> should be made throughout, of course.)

Not fixed, since the document refs RFC 2986 for the CSR structure, so having a forward ref to RFC 5280 doesn’t seem right to us.  Thoughts?

> necessarily assume there will be a "get-bootstrapping-data" request

Good catch.  Fixed.

> (A similar comment applies to the CMP case.)

Not fixed, due to complexity and that this is just a “nit”.  More detail, one author wrote:

"We have gone back and forth on the text a couple of times. I am not sure I won’t just revert to what we had before.  I will note that if we make a change here we have to change it 6 different times, 3X for CMC and 3X for CMP. The phrasing was specifically done this way and amended along the way to make it clear which key is doing what.  We got three cases two are asymmetric-based and one is symmetric-based. We needed to differentiate between the 1st two, and that convoluted text is how we did it."

> (nit) should we have another word after "CHOICE", like "arm" or

Done. used “branch”.

> constructions since they seem to just be describing the type of a given

All done.

> maybe "a PKCS #12 message"?

Fix applied.

>   support origin authentication.  A raw PKCS#10 does not support origin

Done.

Thanks again!
Kent (and Russ and Sean)

---

From: Benjamin Kaduk <[email-masked]>
Date: Wed, 22 Dec 2021 14:32:14 -0800

Hi Kent (and Russ and Sean),

Comments inline.

> You write “example” but I think you mean “tree diagrams”.  The tree-diagram at the top of page 7 is stitched together via some scripts.  Technically, the parent node, "error-info”, is of type “yang…

You're right, I must have meant "tree diagrams"; sorry about that.
Thanks for the explanation and updates!

> NEW: the SZTP-client MUST delete any key generated for the previously generated CSR.

Yes, that makes sense.  [comment about "overkill" removed since we use the
single-use property for replay detection]

>       containing only the p10cr structure defined in RFC 4210.";

That seems to flow better to me and not leave me looking for more content
somewhere else.  So, looks like a clear improvement.

>            400 Bad Request response containing another 'csr-request'

Those changes are okay in terms of the protocol operations.
I think there are probably still some security considerations to document,
in that the ZTP-client is trusting the ZTP-server to tell it what to sign
(in the CSR).  The ZTP client trusts the ZTP server for equally sensitive
things already, so this seems to  mostly be a documentation issue: "the ZTP
server has full control over the contents of the CSR that the ZTP client
generates (except for the actual private key), which can result in the
client receiving a certificate with permissions or restrictions that it
does not recognize.  The ZTP server is already a trusted part of the
ecosystem and it is natural to extend that trust by relying on it to give
the ZTP client a CSR template that will be appropriate for its
implementation and role in the network."

>              distinguished encoding rules (DER), as specified

Sounds good.  It looks like the "constrained" phrasing is also present in
the identity cmc-csr; presumably we should make an analogous change there
and anywhere else relevant.

> NEW: and  no controlSequence, reqSequence, or otherMsgSequence elements. 

Yup.

>             controlSequence, reqSequence, or otherMsgSequence

I don't think we mention controlSequence anywhere else at present (though
it is mentioned in the "NEW" text above).  I don't think the omission
seemed noteworthy when I was doing my original review, so my primary
consideration here is just that we give it consistent treatment everywhere
(in terms of whether or not we mention it).

>              A Voucher Artifact for Bootstrapping Protocols";

I did see (after I originally wrote the remark) that the "nonce" is not new
in this document...but since nonces are a common way to prevent replay
attacks, I figured I would still ask the question.
The 8572 discussion covers the nonce going into the voucher to
detect/prevent replay.  I suppose in principle that one could also have a
setup that puts the nonce into the certificate as well, but I don't expect
that to be common.  So my thinking is that we should consider putting in
some text along the lines of "though the get-bootstrapping-data RPC does
contain a 'nonce' parameter, it is defined by RFC 8572 for use in
preventing replay of a voucher; that mechanism is unrelated to detecting
replay of the certificate that can be returned by the
get-bootstrapping-data RPC".

> 	entity can lead to masquerades.

Yes

>      to the CA, which allows for more robust handling of errors.

That's good text to have.  I think it leaves unaddressed my desire to
document that the PKCS#10 format does not provide this property, and the
implication that if PKCS#10 is used with a new asymmetric key, there is an
additional requirement on the SZTP server to be trusted infrastructure in
the PKI (as well as the existing requirement to be trusted in the
deployment/operations sense).  Unless we propose to forbid the use of
PKCS#10 with new asymmetric keys?

> Would this satisfy your discuss?

This is fine "apple pie" text to have, though not quite the direction I was
thinking.  That said, looking at it again, I'm no longer sure that the
direction I was thinking has much to say -- basically it's just that using
the IDevID for TLS provides proof of origin as being from the device while
HTTP-level authentication does not ... but it's only proof to the SZTP
server, which (per the previous bit) would need to be trusted to properly
relay that information into the PKI.  So it's not terribly useful
information to add.

> Not fixed, since the document refs RFC 2986 for the CSR structure, so having a forward ref to RFC 5280 doesn’t seem right to us.  Thoughts?

I am not going to insist on excessive pedanticism -- leaving it alone is
probably fine.

> "We have gone back and forth on the text a couple of times. I am not sure I won’t just revert to what we had before.  I will note that if we make a change here we have to change it 6 different time…

I think my comment can be distilled to: "there is no context in this
paragraph to clearly tie down what the 'associated identity certificate'
is."  A practitioner skilled in the art that has read the entire document
will figure it out, but in terms of quality of writing it's not great.
(There are plenty of examples of worse writing in the RFC Series, though,
so don't stress out about it.)

> >   support origin authentication.  A raw PKCS#10 does not support origin

Thank you, too!

-Ben

---

From: Russ Housley <[email-masked]>
Date: Thu, 23 Dec 2021 13:12:18 -0500

Ben:

Responding to just one comment in this message ...

> (in terms of whether or not we mention it).

When looking at the text to address your comment, I could see not a role for controlSequence in this situation; however, I think there might possibly be a role in some of the other places.  For that reason, I thik it should be allowable in other places.  That is, the text should not exclude it.  If for consistency, you would rather not exclude controlSequence anywhere, I am fine with that too.

Russ

---

From: Benjamin Kaduk <[email-masked]>
Date: Fri, 24 Dec 2021 19:22:23 -0800

Hi Russ,

> When looking at the text to address your comment, I could see not a role for controlSequence in this situation; however, I think there might possibly be a role in some of the other places.  For tha…

I have no strong preference here.  If you have looked at all the cases and
think this one is "different enough" that controlSequence is not needed, I
do not object to having it mentioned here.

Thanks,

Ben