Early Review of draft-ietf-rtgwg-yang-key-chain-13
review-ietf-rtgwg-yang-key-chain-13-yangdoctors-early-lhotka-2017-02-20-00
| Request | Review of | draft-ietf-rtgwg-yang-key-chain-13 |
|---|---|---|
| Requested revision | 13 (document currently at 24) | |
| Type | Early Review | |
| Team | YANG Doctors (yangdoctors) | |
| Deadline | 2017-02-17 | |
| Requested | 2017-02-07 | |
| Requested by | Mehmet Ersue | |
| Authors | Acee Lindem , Yingzhen Qu , Derek M. Yeung , Ing-Wher Chen , Zhaohui (Jeffrey) Zhang | |
| Draft last updated | 2017-02-20 | |
| Completed reviews |
Rtgdir Early review of -02
by
Ines Robles
(diff)
Yangdoctors Early review of -13 by Ladislav Lhotka (diff) Genart Last Call review of -17 by Matthew A. Miller (diff) Secdir Last Call review of -17 by Vincent Roca (diff) Genart Telechat review of -20 by Matthew A. Miller (diff) |
|
| Assignment | Reviewer | Ladislav Lhotka |
| State | Completed | |
| Review |
review-ietf-rtgwg-yang-key-chain-13-yangdoctors-early-lhotka-2017-02-20
|
|
| Reviewed revision | 13 (document currently at 24) | |
| Result | Almost Ready | |
| Completed | 2017-02-20 |
review-ietf-rtgwg-yang-key-chain-13-yangdoctors-early-lhotka-2017-02-20-00
# General Comments
## Cryptographic algorithm types
What is the reason for representing these as a YANG choice with empty leaves? I
think it would be more natural to use a single leaf, either an enumeration or
(if extensibility is important) identityref.
## Reusability
The module defines key-chain as a grouping with the aim of making it reusable
in other modules. However, this approach has known problems that are discussed
in draft-ietf-netmod-schema-mount. I am not sure how relevant they are in this
case but, for one, the "key-chain-ref" type is not applicable if the
"key-chain" grouping is used in another module. An alternative is not to use
the grouping and rely on schema mount.
## Key string style
The difference between ASCII and hexadecimal formats of key strings should be
explained. I understand that the latter is a hash of the key and, if so, I'd
suggest to include "hexadecimal-string" also in state data.
Also, I believe that storing clear-text key in configuration is insecure and
Security Considerations should warn against it.
## Example
It might be useful to include an appendix with example instance data.
# Specific comments
## Sec. 2
- paragraph 2: s/where ever/wherever/
## Sec. 3
- paragraph 1: replace both Key-Id a Key-ID with Key ID (the latter is used
in other places of the
text).
- paragraph 2: the suggested way of supporting asymmetric keys looks like a
hack, I would suggest
a more explicit representation, e.g. using a choice.
## Sec. 4
- The module has inconsistent indentation: up to "grouping
crypto-algorithm-types", top-level
statements are indented with four spaces, the subsequent ones with five
spaces.
## Sec. 6
- The statement "Given that the key chains themselves are sensitive data, it
is RECOMMENDED
that the NETCONF communication channel be encrypted." is misleading because
RFC 6241 requires that transport protocols for NETCONF guarantee
confidentiality (and RFC 8040 does the same for RESTCONF).