Early Review of draft-ietf-netconf-crypto-types-10
review-ietf-netconf-crypto-types-10-secdir-early-shekh-yusef-2019-07-22-00

Request Review of draft-ietf-netconf-crypto-types-10
Requested rev. 10 (document currently at 11)
Type Early Review
Team Security Area Directorate (secdir)
Deadline 2019-07-22
Requested 2019-07-09
Requested by Mahesh Jethanandani
Draft last updated 2019-07-22
Completed reviews Secdir Early review of -10 by Rifaat Shekh-Yusef (diff)
Comments
If possible, schedule a call with the NETCONF chairs prior to the IETF 105 meeting.  This meeting would primarily to have the SecDir reviewer up to speed on the draft in question, in hopes that they could attend the NETCONF WG session on Monday morning.
Assignment Reviewer Rifaat Shekh-Yusef
State Partially Completed
Review review-ietf-netconf-crypto-types-10-secdir-early-shekh-yusef-2019-07-22
Posted at https://mailarchive.ietf.org/arch/msg/secdir/nnmvjYbOTrA_gTua7Bs7k081QXs
Reviewed rev. 10 (document currently at 11)
Review result Not Ready
Review completed: 2019-07-22

Review
review-ietf-netconf-crypto-types-10-secdir-early-shekh-yusef-2019-07-22

There is the open issue of the proper structure of this YANG model, which was 
discussed with the security ADs and IESG, and still to be discussed with IANA.


Meanwhile, I have the following comments:

Page 6, hash-algorithm_t
Why would you include SHA1 and indicate that it is obsolete? why not just drop it?

Page 8, hash-algorithm-t
Why would the default be 0, i.e. NONE?
I think you should select a minimum algorithm that would be considered acceptable as the default.

page 17, encryption-algorithm-t
Why would you include RC4 algorithms?

page 19, signature-algorithm-t
Why would you include dsa-sha1?

page 40, grouping symmetric-key-grouping, leaf hidden-key { nacm:default-deny-write
If I understand hidden-key, it is a key that is not accessible through this model. 
So, what is this meant to describe?

page 45, grouping symmetric-key-pair-with-cert-grouping, input { leaf subject...
The user of Subject field is discouraged, and the SAN field should be used instead.
Take a look at the following:
https://tools.ietf.org/html/rfc6125#section-4