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

[Paul Wouters <[e-mail]>]

On Thu, Feb 1, 2024 at 2:06 PM Kent Watsen <[e-mail]> wrote:

>
> 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.
>

Note that in general, it is impossible for us to track other repositories
or emails with promises so we always have to check the actual draft in the
datatracker :)
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:

Understood, also from the keystore ballot discussion.


>       <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.
>

Not relevant anymore.


>
>
> 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.).";
>         }
>

Looks good.


>
> 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”.
>

Okay.


>
> 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/
>

Thanks!


> 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.
>

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.
>

That's fine too. Although I wouldn't mind a reference to 9325 just to give
implementers a nudge the right way.

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?
>

It's fine. it was more a comment than a DISCUSS. I think in the other
document you made some clarification that this
for instance could be a hidden key.



> ----------------------------------------------------------------------
> 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?
>

I was wondering if you needed configuration options for this to be
representable in the model. But I guess if you assume
these are all encoded on the CA/certs with like CRLdistributionPoints etc,
then I guess nothing is needed.

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

One config option I am aware of is a "strict mode", so if your CRL is
expired but you can't fetch a new one, you might let clients
connect or not.


> 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.
>

Yes, feel free to ignore if you dont think this comes up in your world.


>
>
> 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 ;)
>

Totally fair. I tend to read these models as generic use ones where other
people use it for more than netconf/restconf, but it is also fine that
those people
do the hard work.


> 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?
>

No I just thought that pkix related elements might make logical sense to be
within its own substructure. But it was a comment, not a discuss. I don't
feel strongly.


> 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.";
>

Ahhh ye sthanks that answers my question.


> 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?
>

Yes :)


> 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!
>

Awesome :)


>        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"
>

Ok.

I've cleared my DISCUSS,

Thanks!

Paul