Re: [yang-doctors] Yangdoctors last call review of draft-ietf-netconf-ssh-client-server-03

"Gary Wu (garywu)" <garywu@cisco.com> Tue, 01 August 2017 19:44 UTC

Return-Path: <garywu@cisco.com>
X-Original-To: yang-doctors@ietfa.amsl.com
Delivered-To: yang-doctors@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 893F812EC48; Tue, 1 Aug 2017 12:44:25 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -14.523
X-Spam-Level:
X-Spam-Status: No, score=-14.523 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, RCVD_IN_DNSWL_HI=-5, RCVD_IN_MSPIKE_H3=-0.01, RCVD_IN_MSPIKE_WL=-0.01, RP_MATCHES_RCVD=-0.001, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001, USER_IN_DEF_DKIM_WL=-7.5] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (1024-bit key) header.d=cisco.com
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id VC5znih3t_rQ; Tue, 1 Aug 2017 12:44:14 -0700 (PDT)
Received: from alln-iport-2.cisco.com (alln-iport-2.cisco.com [173.37.142.89]) (using TLSv1.2 with cipher DHE-RSA-SEED-SHA (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id A93DD1322F7; Tue, 1 Aug 2017 12:44:14 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=cisco.com; i=@cisco.com; l=5326; q=dns/txt; s=iport; t=1501616654; x=1502826254; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-id:content-transfer-encoding: mime-version; bh=7IlZUFryl83dG2VxVGMi8rlTjkE4Su1tyq2+UC06M4A=; b=BKg4iscevEOt3A0ycqOfIQZw851/3acCVQ4dFjmmjWwMW5muFS8HynIX a63PV1zvxNp5pXNTV8LnLFSoMNWbxqwayyfFGfL3TjCpFiqDMu7mOMVoa 4wB8P48ybsx9bFfPhZQ6CS3vF3+2MdXCuX1xiBFIGdlGWhnbcUSZClTSJ 0=;
X-IronPort-Anti-Spam-Filtered: true
X-IronPort-Anti-Spam-Result: A0DDAgCP2YBZ/51dJa1eGgEBAQECAQEBAQgBAQEBg1pkODUnB44HkACXeQ6CBCyFGwIahAs/GAECAQEBAQEBAWsohRkGIxFFEAIBCA4MAiYCAgIwFRACBAENBYovEK8mgiaLUgEBAQEBAQEBAQEBAQEBAQEBAQEBARgFgQuCHYICgUyBYSuCe4Q6BgESAQlQglkwghIfBZ93AodOhxyFPYImkBuVeAEfOH8LdxVbAYUEHIFndodxDRcHgQWBDgEBAQ
X-IronPort-AV: E=Sophos;i="5.41,306,1498521600"; d="scan'208";a="460020395"
Received: from rcdn-core-6.cisco.com ([173.37.93.157]) by alln-iport-2.cisco.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 01 Aug 2017 19:44:13 +0000
Received: from XCH-ALN-010.cisco.com (xch-aln-010.cisco.com [173.36.7.20]) by rcdn-core-6.cisco.com (8.14.5/8.14.5) with ESMTP id v71JiDgJ023434 (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=FAIL); Tue, 1 Aug 2017 19:44:13 GMT
Received: from xch-rcd-006.cisco.com (173.37.102.16) by XCH-ALN-010.cisco.com (173.36.7.20) with Microsoft SMTP Server (TLS) id 15.0.1210.3; Tue, 1 Aug 2017 14:44:13 -0500
Received: from xch-rcd-006.cisco.com ([173.37.102.16]) by XCH-RCD-006.cisco.com ([173.37.102.16]) with mapi id 15.00.1210.000; Tue, 1 Aug 2017 14:44:12 -0500
From: "Gary Wu (garywu)" <garywu@cisco.com>
To: Kent Watsen <kwatsen@juniper.net>, Andy Bierman <andy@yumaworks.com>, "yang-doctors@ietf.org" <yang-doctors@ietf.org>
CC: "draft-ietf-netconf-ssh-client-server.all@ietf.org" <draft-ietf-netconf-ssh-client-server.all@ietf.org>, "netconf@ietf.org" <netconf@ietf.org>
Thread-Topic: Yangdoctors last call review of draft-ietf-netconf-ssh-client-server-03
Thread-Index: AQHTB+7XG4uCskNmz0G4+8XG9F6WsqJuuaaAgAESEYA=
Date: Tue, 01 Aug 2017 19:44:12 +0000
Message-ID: <F7570952-C769-4B30-9F2E-A30C09C917AF@cisco.com>
References: <150128005005.20715.8994557350330588925@ietfa.amsl.com> <C7ABD900-ACB9-4BFC-B30D-3A7274DDB657@juniper.net>
In-Reply-To: <C7ABD900-ACB9-4BFC-B30D-3A7274DDB657@juniper.net>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
x-ms-exchange-messagesentrepresentingtype: 1
x-ms-exchange-transport-fromentityheader: Hosted
x-originating-ip: [10.32.199.45]
Content-Type: text/plain; charset="utf-8"
Content-ID: <6AC5F7B131F98F4188469765838A86BA@emea.cisco.com>
Content-Transfer-Encoding: base64
MIME-Version: 1.0
Archived-At: <https://mailarchive.ietf.org/arch/msg/yang-doctors/wGcjCRK2Gf3ebAwX9kivMPrtIa8>
Subject: Re: [yang-doctors] Yangdoctors last call review of draft-ietf-netconf-ssh-client-server-03
X-BeenThere: yang-doctors@ietf.org
X-Mailman-Version: 2.1.22
Precedence: list
List-Id: Email list of the yang-doctors directorate <yang-doctors.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/yang-doctors>, <mailto:yang-doctors-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/yang-doctors/>
List-Post: <mailto:yang-doctors@ietf.org>
List-Help: <mailto:yang-doctors-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/yang-doctors>, <mailto:yang-doctors-request@ietf.org?subject=subscribe>
X-List-Received-Date: Tue, 01 Aug 2017 19:44:26 -0000

    C1:
    
     (M3) transport-params leafrefs
    
     Example of <transport-params> in sec. 4.2 shows ietf-ssh-common
     as the module namespace but this will never happen in real usage.
    
     People will copy the various identityref examples in this section
     and not encode the identityref prefix correctly.
    
    
    <KENT> I added prefixes to the example.  
    
    <KENT> This would've been caught earlier (via validation) had the
    transport-params grouping contained some mandatory values, as then
    the identities would have to show up in other examples (e.g., 
    ex-netconf-server.xml), which would then have to encode the 
    identities from another namespace.  I sent email to Gary asking
    him about if any of these values are mandatory.

<GARY> This is something I had debated as well for SSH and TLS transport
parameters.  I settled on modeling what was implemented in popular vendors and
open source, with the ability for the identities to be extended as needed.
    
    <KENT> Also related, I notice that there are a ton of algorithm
    identities here that I'd forgotten about.  Going back to Juergen's
    comment about factoring these out into another module, now it
    seems more prudent than ever! I just filed an issue to track
    this:  https://github.com/netconf-wg/keystore/issues/8

<GARY> IANA has registries for SSH and TLS, if we want to go down that route.
There's no protocol-independent registries for public key algorithms for
keystore that I know of unfortunately.

Thanks,
Gary

On 7/31/17, 1:23 PM, "Kent Watsen" <kwatsen@juniper.net> wrote:

    Hi Andy, 
    
    Thanks for your review.  Below are my responses to your comments.
    
    Thanks,
    Kent
    
    
    --
    
    Reviewer: Andy Bierman
    Review result: Ready with Nits
    
    Review: draft-ietf-netconf-ssh-client-server-03
    
    Modules:
     (M1) ietf-ssh-client@2017-06-13.yang
     (M2) ietf-ssh-common@2017-06-13.yang
     (M3) ietf-ssh-server@2017-06-13.yang
    
    
    YANG Usage:
    
     I did not find anything wrong in either module.
     pyang and yangdump-pro do not report any errors or warnings.
    
    Comments:
    
    C1:
    
     (M3) transport-params leafrefs
    
     Example of <transport-params> in sec. 4.2 shows ietf-ssh-common
     as the module namespace but this will never happen in real usage.
    
     People will copy the various identityref examples in this section
     and not encode the identityref prefix correctly.
    
    
    <KENT> I added prefixes to the example.  
    
    <KENT> This would've been caught earlier (via validation) had the
    transport-params grouping contained some mandatory values, as then
    the identities would have to show up in other examples (e.g., 
    ex-netconf-server.xml), which would then have to encode the 
    identities from another namespace.  I sent email to Gary asking
    him about if any of these values are mandatory.
    
    <KENT> Also related, I notice that there are a ton of algorithm
    identities here that I'd forgotten about.  Going back to Juergen's
    comment about factoring these out into another module, now it
    seems more prudent than ever! I just filed an issue to track
    this:  https://github.com/netconf-wg/keystore/issues/8
    
    
    C2:
    
    Sec 1.2 Tree Diagrams
    
    Old text should be replaced with reference to
    draft-ietf-netmod-yang-tree-diagrams-01
    
    <KENT> Is this safe?  I've resisted doing this so far because I
    wasn't sure if the tree-diagrams draft might break syntax...
    
    
    
    C3:
    
     (M1) IETF copyright says 2014; change to 2017
     (M3) IETF copyright says 2014; change to 2017
    
    
    <KENT> done!