Re: [netconf] Paul Wouters' Discuss on draft-ietf-netconf-crypto-types-29: (with DISCUSS and COMMENT)

[Kent Watsen <[e-mail]>]

Hi Paul,

Thank you for your valuable comments.
Please find responses below.

PS, to all IESG members: if my response ever suggests that an update has been made, i.e., using words like “fixed” or “updated”, please know that I always mean that the update has been made in my local copy.  It is my intention to publish a bulk-update (to all nine drafts) once the churn dies down on these first three drafts.

Kent


> On Jan 31, 2024, at 9:19 PM, Paul Wouters via Datatracker <[e-mail]> wrote:
> 
> Paul Wouters has entered the following ballot position for
> draft-ietf-netconf-crypto-types-29: Discuss
> 
> 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/about/groups/iesg/statements/handling-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-crypto-types/
> 
> 
> 
> ----------------------------------------------------------------------
> DISCUSS:
> ----------------------------------------------------------------------
> 
> Thanks to Valery and Rifaat for their secdir reviews. I have some items I would
> like some clarifications on and some non-blocking comments.
> 
> 2.1.4.12 contains:
> 
>  +-- certificates
>    |  +-- certificate* [name]
>    |     +-- name?                       string
> 
> Certificate identity is either done by entire DN, The Common Name (CN) RDN,
> or by a list of subjectAltName (SAN) entries. Can the latter be expressed
> here? Should a type be introduced? ("CN", "DN", "SAN") ? Should the type be
> a list as 1 certificate can have multiple identities via multiple SAN entries.

Two comments.


1) Regarding “name”, the intention here is for it to not necessarily reproduce any of the fields mentioned (CN, SAN, etc.), though that would certainly be possible and likely make sense in most cases.  That said, conjoining  this comment with my response to Éric yesterday (who was hoping to surface "validity period" information, to support key/cert rollover), the name may be mangled as follows:

      <certificate>
        <name>My certificate (notBefore: AAA, notAfter BBB)</name>
        <cert-data>BASE64VALUE=</cert-data>
      </certificate>

Please note that I don’t think name-mangling a wonderful idea, but notable in that there is possibly no end to how much information could go into the “name” field.   

To make things even more complicated, please note that (in Section 2.1.4.12), the actually payload of this YANG node is a CMS that may encode a chain.  Obviously the focus would be on the leaf-cert of the chain, but it shows that picking just one CN/SAN/etc wouldn’t tell the whole story either.


2) Regarding “type” (the last two sentences in your comment), I don’t follow.  Can you provide an example so I can better understand.


My current thought is to focus on improving the “description” statement for the “name” node.  For instance, something like this:

OLD:
        leaf name {
          type string;
          description
            "An arbitrary name for the certificate.";
        }

NEW:
        leaf name {
          type string;
          description
            "An arbitrary name for the certificate.  The name should 
             provide enough information to uniquely identify the certificate.
             The certificate’s inherent identifiers (e.g., CommonName, 
             SubjectAltName, etc.) may be used or, alternatively, the
             name may encode other characteristics (e.g., notBefore,
             notAfter, etc.).";
        }


Thoughts?


> 2.2.1 talks about "asymmetric key" without specifying if it is referring to
> a public key or a private key. I think "private" is mean here on all occasions.
> Should that be clarified?

I made a couple changes, but I think that you want to see more.

Here are the changes I made:

             <t>Notably, this example module and associated configuration data illustrates that
-              a hidden asymmetric key (ex-hidden-asymmetric-key)
+              a hidden private key (ex-hidden-asymmetric-key)
               has been used to encrypt a symmetric key (ex-encrypted-one-symmetric-based-symmetric-key)
-              that has been used to encrypt another asymmetric key (ex-encrypted-rsa-based-asymmetric-key).
+              that has been used to encrypt another private key (ex-encrypted-rsa-based-asymmetric-key).
               Additionally, the symmetric key is also used to encrypt a password (ex-encrypted-password).</t>

The change that I didn’t make, which you may be looking for also, is in the artwork found in that section.  There, a YANG node call “asymmetric-key” exists, which is an instance of the “asymmetric-key-pair-grouping” that, in all but its “hidden” form, encodes both the private and public halves.  As such, it truly seems more correct to call the node “asymmetric-key”.

As a preview of things to come, the “keystore” draft also defines nodes called “asymmetric-key” for the same reason.  I see your review of the keystore draft in my Inbox, so this comment is a tad stale, but I thought worth mentioning for others who might be here.



> 2.2.1.2 uses "cleartext-key" and "cleartext-private-key". Would it not be less
> confusing to rename "cleartext-key" to "cleartext-sym-key" ?

I see your point, and the same follows for the “hidden” and “encrypted “ forms as well.

I made the following change (affects all nine drafts):

	s/cleartext-key/cleartext-symmetric-key/
	s/hidden-key/hidden-symmetric-key/
	s/encrypted-key/encrypted-symmetric-key/




> Section 3.5
> 
>        That said, expert Security opinion suggests that already it is
>        infeasible to break a 128-bit symmetric key using a classical
>        computer, and thus the concern for conveying higher-strength
>        keys begins to lose its allure.
> 
> I don't think this document should make this statement.

Statement removed - thanks.



> Section 3.6 Use of Recommended Ciphersuites
> 
> Why is this document containing this single recommendation ?  I would
> rename this section to "Use of Secure Transport Protocols" and refer
> to RFC 9325 or BCP195 and maybe mention using IKEv2/IPsec RFC 7296 with
> RFC 8247/8221 as another secure transport that can be used. (I think
> IKEv2/IPsec might not be used with netconf/restconf so perhaps ignore
> that part)

Or delete the section entirely?  
To your point, this seem better placed in protocol-specific documents.  
For instance, both the SSH and TLS RFCs make similar statements.
I’ve deleted this section, per your first sentence...
But please let me know if I misunderstood your comment.



> Section 3.8 states:
> 
>         Passwords and keys may be encrypted via a symmetric key
> 
> Doesn't this move the problem? Isn't _that_ symmetric key then not
> still stored in the clear?

But not if that symmetric key is hidden or encrypted - right?

Separately, I note that the text is a bit wrong, in that a password or a key could be encrypted by *either* a symmetric or asymmetric key.  So I made this change:


OLD:
-          <t>The module contained within this document enables passwords and keys to be
-            encrypted.  Passwords and keys may be encrypted via a symmetric key using
-            the "cms-encrypted-data-format" format.  This format uses the CMS
-            EncryptedData structure, which allows any encryption algorithm
-            to be used.</t>

NEW:
+          <t>The module contained within this document enables cleartext
+            passwords and keys to be encrypted via another key, either
+            symmetric or asymmetric.  Both formats use a CMS structure
+            (EncryptedData and EnvelopedData respectively), which allows
+            any encryption algorithm to be used.</t>

Okay?




> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
> 
> 
> 
> I see no mention of CRLs or OCSP? Is this not commonly used with NETCONF
> or RESTCONF ?

They should be commonly used, yes, but what text-edit are you looking for?

Specifically, as CRL/OCSP responses are never *configured* (right?), what might a YANG module have in it to support them.

It seems that it’s up to the implementation of the TLS-stack to fetch fresh CRL/OCSP responses, using information extracted from the certs themselves…

Please advise.


> 2.1.2 What about OpenPGP key types? Would it be expected some openpgp
> module would update this module with a new type?

It is possible that a PGP key/cert could be added by future work.  This is why this draft defined base-identities (symmetric-key-format, public-key-format, private-key-format).  YANG “identify” statements (as opposed to YANG enumerations) are extensible.

This work did not define support for PGP because PGP is never used in the NETCONF ecosystem and, well, we needed to draw the line somewhere ;)


> 2.1.4 Why not a pkix based grouping for all pkix operations?

I think that you are saying something very specific here.

Let me start with that X.509 certs do use PKI infra (e.g., CAs, CRL/OCSP distribution points, etc.).

That said, you may be asking why this work doesn’t codify output from the PKIX WG, for which I guess I’ll point to my previous comment about needing to draw the line somewhere...

Is there something specific you have in mind?  Maybe a Security Consideration?


> 2.1.4.2 What about hashed passwords (eg crypt(), PBKDF2, ARGON2, SCRYPT))
> and their parameters? I guess passwords here focus on the devices and their
> storage? not on password types received from a human or the network for validation?
> Which I would assume is the same reason we do not list PINs anywhere in the model?


Éric had a similar question.  The “password-grouping” enables configuration of authentication TO a peer, not BY a peer.   To address Éric’s comment, I made this change:

OLD:
   grouping password-grouping {
     description
-      "A password that may be encrypted.";

NEW:
   grouping password-grouping {
     description
+      "A password used for authenticating to a remote system.
+       
+       The 'ianach:crypt-hash' typedef from RFC 7317 should be
+       used instead when needing a password to authencate a
+       local account.";


And I just now added similar text to the body of the document (in Section 2.1.4.2):

                <li>The "password-grouping" enables configuration of credentials needed to
                   authenticate to a remote system.  The 'ianach:crypt-hash' typedef from
                   <xref target="RFC 7317"/> should be used instead when needing to
                   configure a password to authencate a local account.</li>

Does this address your comment?


> Section 3.8 states:
> 
>        In order to thwart rainbow attacks, algorithms that result in
>        a unique output for the same input SHOULD NOT be used. For
>        instance, AES using "ECB" SHOULD NOT be used to encrypt values,
>        whereas "CBC" mode is permissible since an unpredictable
>        initialization vector (IV) MUST be used for each use.
> 
> I didn't find this clear. How about something like:
> 
>        To securely encrypt a password or key with a symmetric key, a
>        proper block cipher mode such as an AEAD or CBC MUST be used. This
>        ensures that a random IV is part of the input, which guarantees
>        that the output for encrypting the same password or key still
>        produces a different unpredictable ciphertext. This avoids leaking
>        that some encrypted keys or passwords are the same and makes it
>        much harder to pre-generate rainbow tables to brute force attack
>        weak passwords. The ECB block cipher mode MUST NOT be used.

I swapped in your paragraph - thanks!


>        However, comparing key strengths can be complicated
> 
> Add "of different algorithms" ? Because comparing AES128 to AES256 is not complicated :)

Good point.  I added "of different algorithms"


Thanks again!
Kent