From nobody Tue Jan 12 04:13:33 2021 Return-Path: X-Original-To: yang-doctors@ietf.org Delivered-To: yang-doctors@ietfa.amsl.com Received: from ietfa.amsl.com (localhost [IPv6:::1]) by ietfa.amsl.com (Postfix) with ESMTP id 8D6DC3A11DB; Tue, 12 Jan 2021 04:13:22 -0800 (PST) MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit From: =?utf-8?q?J=C3=BCrgen_Sch=C3=B6nw=C3=A4lder_via_Datatracker?= To: Cc: draft-ietf-netconf-crypto-types.all@ietf.org, last-call@ietf.org, netconf@ietf.org X-Test-IDTracker: no X-IETF-IDTracker: 7.24.0 Auto-Submitted: auto-generated Precedence: bulk Message-ID: <161045360252.14510.16755799701987783827@ietfa.amsl.com> Reply-To: =?utf-8?b?SsO8cmdlbiBTY2jDtm53w6RsZGVy?= Date: Tue, 12 Jan 2021 04:13:22 -0800 Archived-At: Subject: [yang-doctors] Yangdoctors last call review of draft-ietf-netconf-crypto-types-18 X-BeenThere: yang-doctors@ietf.org X-Mailman-Version: 2.1.29 List-Id: Email list of the yang-doctors directorate List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 12 Jan 2021 12:13:23 -0000 Reviewer: Jürgen Schönwälder Review result: Ready with Issues The crypto modules aim at providing a flexible reusable infrastructure of groupings for modeling cryptographic keys and related concepts. The flexibility of the definitions provided of course comes with a certain amount of complexity. >From a YANG perspective, draft-ietf-netconf-crypto-types-18.txt is in a good and close to publish state (a couple of minor issues left). I also tried to understand what is being modeled here and hence I also have some questions concerning the concepts modeled and I hope these are easy to answer/resolve as well. - I have compiled the YANG modules using yanglint 0.16.105. - s/ietf-crypt-types/ietf-crypto-types/ - Does it make sense to have a new notation like | The diagram above uses syntax that is similar to but not | defined in [RFC8340]. or shall this simply become regular text? - "Identities use to specify uncommon formats are enabled [...]" is this correct or should it be "used to specify"? - Second bullet 2.1.4.1: Perhaps change [...] specified by the "format" identity Section 2.1.2 associated [...] to [...] specified by the "format" identity (see Section 2.1.2) associated [...] What is 'encoded in the format specified by the "format" identity' I assume this refers to the key value that is encrypted and stored in encrypted-value, no? - Same bullet in 2.1.4.1: The "encrypted-value" node is the key, encrypted by the key referenced by the "encrypted-by" node, encoded in the format specified by the "format" identity Section 2.1.2 associated with the ancestor node using this grouping. Do I understand correctly that the encrypted-value always holds an encrypted key? If so, a better name for the leaf would perhaps have been encrypted-key and for the grouping encrypted-key-grouping. I assume you did not pick this name since you wanted to use encrypted-key for other purposes? This makes sense and may have led to encrypted-key-value-grouping, in the sense of an encrypted 'key-value'? For the definition of the grouping, does it actually matter that the value encrypted is a key? Or would it make sense to rename the grouping to encrypted-value-grouping and then the second bullet becomes: The "encrypted-value" node holds the value, encrypted by the key referenced by the "encrypted-by" node, encoded in the format specified by the "format" identity (see Section 2.1.2) associated with the ancestor node using this grouping. Looking at the definition of encrypted-key-value-grouping, it seems that the name encrypted-value-grouping would actually be more appropriate, there is nothing in the definition that says that the value must be a key for something. And is it "the" ancestor node or "an" ancestor node? Perhaps the possible confusion here is that ancestor node may mean different things, schema tree versus data tree. What about this? The "encrypted-value" node holds the value, encrypted by the key referenced by the "encrypted-by" node, encoded in the format specified by the "format" identity (see Section 2.1.2) associated with the ancestor data node of the data node using this grouping. Hm. Did I reverse engineer this correctly? - In 2.1.4.2 last item, where do I see that the encrypted-password node is encoded using the CMS EnvelopedData structure? There is no "format" identity here, so this is hard coded to always have the CMS EnvelopedData structure (or is it CMS EncryptedData structure, see below)? OK, if I lookup the definition, it seems to use a hardcoded format. - Always use cleartext instead plain-text? I understand that some people make a distinction between plain text, plaintext, and cleartext. Does this document do that? I do not think so, and if so, we may reduce confusion by picking just one term. On the other hand, if there is a distinction made, then the document should perhaps be explicit about it somewhere and define how these terms are used. - The example in section 2.2 is helpful and much appreciated. - s/confugration/configuration/ - The encrypted-password description says that it uses a" CMS EnvelopedData structure, per Section 8 in RFC 5652, encoded using ASN.1 distinguished encoding rules (DER)". However, section 8 of RFC 5652 defined EncryptedData and not EnvelopedData. So what is the intention here? - Following up on the previous point, it seems that the EncryptedData format does not introduce a salt, i.e., if the same password is used multiple times and they are all protected by the same key, then this is visible since the encrypted formats will all be the same as well. Is this something to be concerned about? Well, this is not really a yang-doctor question... - The encrypted-one-symmetric-key-format again refers to the EnvelopedData per Section 8 in RFC 5652 but section 8 only defines EncryptedData. So either the type name is wrong or the reference is wrong. - The encrypted-password container description refers to the EnvelopedData and either the reference to section 8 is wrong or it should be EncryptedData (see above). Is it OK to have a fixed encrypted password format? Perhaps it is, I am just checking. (The password leaf in RFC 7317 uses the ianach:crypt-hash format, i.e., it is somewhat extensible but then this module may target different use cases as it deals with encrypted passwords instead of hashed passwords.) - I wonder why this is a SHOULD and not a MUST: "Identifies the symmetric key's format. Implementations SHOULD ensure that the incoming symmetric key value is encoded in the specified format."; This statement shows up several times when the key-format identities are used. I wonder what this means to an implementer. If I receive a key format (say one-symmetric-key-format) and a corresponding blob of data, do I have to decode this to see whether the format works out? If later the key-format is changed to something else (lets say octet-string-key-format), do I reject such a change or is it OK as long as the binary data I have would be a valid value given the new format? Perhaps this is implementation specific? Well, since we deal with keys, ... - The definitions of the features symmetric-key-encryption and private-key-encryption have copy and paste text from the definition of the feature password-encryption. I think they need to be changed to: feature symmetric-key-encryption { description "Indicates that the server supports encryption of symmetric keys."; } feature private-key-encryption { description "Indicates that the server supports encryption of private keys."; } - If I have a 'octet-string-key-format' key and I use it to create an encrypted key. How do I know which crypto algorithm is used? I have not digged into it, but do the various CMS structures provide this information somewhere? In the case of asymmetric keys, the format gives a hint whether I am dealing with RSA keys or EC keys. For symmetric keys, I am not sure in call cases how the algorithm is determined. - The asymmetric-key-pair-with-certs-grouping descriptions talk about 'IDevID' and 'LDevID' and 'TPM-protected asymmetric keys'. This seems to call for additional references to explain what these are. - s/is was unclear/it was unclear/ - RFC 8407 suggests that the XML registry Registrant Contact is: Registrant Contact: The IESG. This does not seem to be handled in a consistent manner if I look at recent RFCs but I think the idea was to give the responsibility to the IESG, assuming the IESG is a more stable entity than WGs. From nobody Tue Jan 12 10:41:29 2021 Return-Path: X-Original-To: yang-doctors@ietf.org Delivered-To: yang-doctors@ietfa.amsl.com Received: from ietfa.amsl.com (localhost [IPv6:::1]) by ietfa.amsl.com (Postfix) with ESMTP id 851163A11BD; Tue, 12 Jan 2021 10:41:12 -0800 (PST) MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit From: =?utf-8?q?J=C3=BCrgen_Sch=C3=B6nw=C3=A4lder_via_Datatracker?= To: Cc: draft-ietf-netconf-trust-anchors.all@ietf.org, last-call@ietf.org, netconf@ietf.org X-Test-IDTracker: no X-IETF-IDTracker: 7.24.0 Auto-Submitted: auto-generated Precedence: bulk Message-ID: <161047687248.13931.17900123352005904827@ietfa.amsl.com> Reply-To: =?utf-8?b?SsO8cmdlbiBTY2jDtm53w6RsZGVy?= Date: Tue, 12 Jan 2021 10:41:12 -0800 Archived-At: Subject: [yang-doctors] Yangdoctors last call review of draft-ietf-netconf-trust-anchors-13 X-BeenThere: yang-doctors@ietf.org X-Mailman-Version: 2.1.29 List-Id: Email list of the yang-doctors directorate List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 12 Jan 2021 18:41:19 -0000 Reviewer: Jürgen Schönwälder Review result: Ready with Issues The crypto modules aim at providing a flexible reusable infrastructure of groupings for modeling cryptographic keys and related concepts. The flexibility of the definitions provided of course comes with a certain amount of complexity. >From a YANG perspective, draft-ietf-netconf-trust-anchors-13.txt is in a good and close to publish state (a couple of minor issues left). I also tried to understand what is being modeled here and hence I also have some questions concerning the concepts modeled and I hope these are easy to answer/resolve as well. - I have compiled the YANG modules using yanglint 0.16.105. - The prefix 'ts' is rather short, the set of two letter prefixes is rather small limited. This comment also applies to the other documents, crypto-types uses 'ct. Perhaps this is not a problem since collisions can be handled but if we go for rather short prefixes, we will have to exercise the collision resolution. (I see that 'ct' has already been used by ietf-complex-types, RFC 6095.) A possible alternative could be to use sec-ct, sec-ts, sec-ks, ...). RFC 8407 provides the following guidelines (Section 4.2): Prefix values SHOULD be short but are also likely to be unique. Prefix values SHOULD NOT conflict with known modules that have been previously published. Well, having short and terse prefixes is actually nice and enhancing programmer readability. - s/is defines a truststore/defines a truststore/ - s/to be grouped references/to be grouped and referenced/ - In 2.2.1, I was not sure what CA certificates are and what EE certificates are. I then tried to guess EE = end entity cert, but this does not explain CA since the term used in crypto types is trust anchor cert. The description in the XML clarified that my guess was kind of correct. Perhaps explain upfront what these acronyms mean? Or perhaps the acronyms can be avoided by simply spelling things out? They do not appear to be used frequently. - s/