Subject: [yang-doctors] Review of draft-ietf-rtgwg-yang-key-chain-13

From: "Ladislav Lhotka" <[email-masked]>
Date: Mon, 20 Feb 2017 03:49:56 -0800

Reviewer: Ladislav Lhotka
Review result: Almost Ready

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

---

From: Ladislav Lhotka <[email-masked]>
Date: Mon, 20 Feb 2017 12:56:37 +0100

Hi,

this is my old review, on Mehmet's request I entered it into the datatracker, so no action is needed. I just removed my objection that turned out to be nonsense.

Lada

> On 20 Feb 2017, at 12:49, Ladislav Lhotka <[email-masked]> wrote:
> 
> _______________________________________________
> yang-doctors mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/yang-doctors

--
Ladislav Lhotka, CZ.NIC Labs
PGP Key ID: 0xB8F92B08A9F76C67

---

From: "Acee Lindem (acee)" <[email-masked]>
Date: Mon, 20 Feb 2017 12:53:04 +0000

Hi Lada, 
I believe we¹ve addressed all of these other than adding an explicit leaf
for key direction (which was discussed on the RTGWG list). The current
version is https://www.ietf.org/id/draft-ietf-rtgwg-yang-key-chain-15.txt

Thanks,
Acee

---

From: Ladislav Lhotka <[email-masked]>
Date: Mon, 20 Feb 2017 13:58:09 +0100

> On 20 Feb 2017, at 13:53, Acee Lindem (acee) <[email-masked]> wrote:
> 
> Hi Lada, 

Yes, I saw it, looks good.

Thanks, Lada