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

Benjamin Kaduk via Datatracker <noreply@ietf.org> Wed, 15 December 2021 00:09 UTC

Return-Path: <noreply@ietf.org>
X-Original-To: netconf@ietf.org
Delivered-To: netconf@ietfa.amsl.com
Received: from ietfa.amsl.com (localhost [IPv6:::1]) by ietfa.amsl.com (Postfix) with ESMTP id C9A623A0C39; Tue, 14 Dec 2021 16:09:52 -0800 (PST)
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
From: Benjamin Kaduk via Datatracker <noreply@ietf.org>
To: The IESG <iesg@ietf.org>
Cc: draft-ietf-netconf-sztp-csr@ietf.org, netconf-chairs@ietf.org, netconf@ietf.org, mjethanandani@gmail.com, mjethanandani@gmail.com
X-Test-IDTracker: no
X-IETF-IDTracker: 7.41.0
Auto-Submitted: auto-generated
Precedence: bulk
Reply-To: Benjamin Kaduk <kaduk@mit.edu>
Message-ID: <163952699208.6437.8936066509149136808@ietfa.amsl.com>
Date: Tue, 14 Dec 2021 16:09:52 -0800
Archived-At: <https://mailarchive.ietf.org/arch/msg/netconf/ra_KfLp2HPUZajLIYQ_MBLf-sfw>
Subject: [netconf] Benjamin Kaduk's No Objection on draft-ietf-netconf-sztp-csr-12: (with COMMENT)
X-BeenThere: netconf@ietf.org
X-Mailman-Version: 2.1.29
List-Id: NETCONF WG list <netconf.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/netconf>, <mailto:netconf-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/netconf/>
List-Post: <mailto:netconf@ietf.org>
List-Help: <mailto:netconf-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/netconf>, <mailto:netconf-request@ietf.org?subject=subscribe>
X-List-Received-Date: Wed, 15 Dec 2021 00:09:58 -0000

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"