Last Call Review of draft-ietf-netconf-crypto-types-20
review-ietf-netconf-crypto-types-20-secdir-lc-smyslov-2021-08-24-00

I have reviewed this document as part of the security directorate's ongoing
effort to review all IETF documents being processed by the IESG.  These
comments were written primarily for the benefit of the security area directors.
 Document editors and WG chairs should treat these comments just like any other
last call comments.

When I was re-assigned to review this draft the indicated deadline was already
missed by 8 months, so I don't know how relevant the review is. I reviewed
the latest -20 version of the draft instead of -18, that was requested.

The draft defines common YANG data types and groupings useful for cryptography.
I din't try to check the YANG module itself. 

Issues:

Shouldn't a Privacy Considerations section be added to the draft?
The draft defines quite a lot of privacy-sensitive information (like certificates)
with no restriction on read access (as far as I understand).

Section 3.5.
While I understand and support the idea, expressed in this section, I think that
the way it is expressed makes it difficult to follow in practice. In general, it's
not always obvious how to estimate the "strength" of the underlying secure transport.
For this reason it's not clear for me how it is supposed to "compare" the 
"strength" of the transport with the "strength" of the keys being transported.

In addition, the requirement, that "Implementations SHOULD fail the write-request if ever
the strength of the private key is greater then the strength of the
underlying transport" looks wrong to me. You don't need to have
1024 bits transport protocol strength to transfer 1024 bit key, since
even for say 256 bits it's infeasible to break.

I think that the better approach would be to advise using strong
ciphersuites for transport protocols defined in corresponding RFCs.
For example, for TLS 1.3 there are ciphersuites marked as "recommended",
that were evaluated by IETF crypto community.

Section 3.6:
   For instance, AES
   using "EBC" SHOULD NOT be used to encrypt passwords, whereas "CBC"
   mode is okay since it a unique initialization vector (IV) should be
   used for each run.

Not only IV for CBC should be unique, it should be unpredictable (random or pseudo-random).
I also think that lowercase "should" here must be uppercase, or even MUST.

Typos, nits:

Section 3.6: 
   In order to thwart rainbow attacks, algorithms that result in a
   unique output for the same input SHOULD be used. 

s/SHOULD/SHOULD NOT

   For instance, AES
   using "EBC" SHOULD NOT be used to encrypt passwords, whereas "CBC"
   mode is okay since it a unique initialization vector (IV) should be
   used for each run.

s/EBC/ECB
s/it//
I believe "okay' is a bit of slang, isn't it?

Section 3.8:
   Since the module in this document only define groupings, these
   considerations are primarily for the designers of other modules that
   use these groupings.

s/define/defines