[lamps] Genart early review of draft-ietf-lamps-rfc7030-csrattrs-08
Lars Eggert via Datatracker <noreply@ietf.org> Thu, 04 April 2024 14:07 UTC
Return-Path: <noreply@ietf.org>
X-Original-To: spasm@ietf.org
Delivered-To: spasm@ietfa.amsl.com
Received: from ietfa.amsl.com (localhost [IPv6:::1]) by ietfa.amsl.com (Postfix) with ESMTP id E17BEC15152C; Thu, 4 Apr 2024 07:07:04 -0700 (PDT)
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
From: Lars Eggert via Datatracker <noreply@ietf.org>
To: gen-art@ietf.org
Cc: draft-ietf-lamps-rfc7030-csrattrs.all@ietf.org, spasm@ietf.org
X-Test-IDTracker: no
X-IETF-IDTracker: 12.9.0
Auto-Submitted: auto-generated
Precedence: bulk
Message-ID: <171223962490.52812.1499746415411816194@ietfa.amsl.com>
Reply-To: Lars Eggert <lars@eggert.org>
Date: Thu, 04 Apr 2024 07:07:04 -0700
Archived-At: <https://mailarchive.ietf.org/arch/msg/spasm/RObjp1IplT-LDvBXdBpJl0YrH9E>
Subject: [lamps] Genart early review of draft-ietf-lamps-rfc7030-csrattrs-08
X-BeenThere: spasm@ietf.org
X-Mailman-Version: 2.1.39
List-Id: This is the mail list for the LAMPS Working Group <spasm.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/spasm>, <mailto:spasm-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/spasm/>
List-Post: <mailto:spasm@ietf.org>
List-Help: <mailto:spasm-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/spasm>, <mailto:spasm-request@ietf.org?subject=subscribe>
X-List-Received-Date: Thu, 04 Apr 2024 14:07:05 -0000
Reviewer: Lars Eggert Review result: Ready with Nits # Gen-ART review of draft-ietf-lamps-rfc7030-csrattrs-08 CC @larseggert ## Comments ### "Abstract", paragraph 2 ``` This document updates RFC7030 (EST) and clarifies how the CSR ``` Document status does not indicate the update. ### Section 1, paragraph 5 ``` Section 2.6 says that the CSR attributes "can provide additional descriptive information that the EST server cannot access itself". This is extended to describe how the EST server can provide values that it demands to use. After significant discussion, it has been determined that Section 4.5 of [RFC7030] specification is sufficiently difficult to read and ambiguous to interpret that clarification is needed. ``` This document is a "patch RFC" for RFC7030, which might be easy to produce but puts the burden on the reader/implementer when it comes to figuring out what the new updates specification actually is. I'd much prefer an actual 7030bis, also given that there already is a verified erratum for RFC7030. Yes, it's more work for the authors and the WG. ### Section 3.2, paragraph 5 ``` The ASN.1 syntax for CSR Attributes as defined in EST section 4.5.2 is as follows: CsrAttrs ::= SEQUENCE SIZE (0..MAX) OF AttrOrOID AttrOrOID ::= CHOICE (oid OBJECT IDENTIFIER, attribute Attribute } Attribute { ATTRIBUTE:IOSet } ::= SEQUENCE { type ATTRIBUTE.&id({IOSet}), values SET SIZE(1..MAX) OF ATTRIBUTE.&Type({IOSet}{@type}) } This remains unchanged, such that bits-on-the-wire compatibility is maintained. ``` In a "patch RFC", it's not useful to include the bit's that don't change, because doing so creates a copy of the text that can get out of sync if 7030 is actually changed down the road. Omit? ### Section 3.3, paragraph 0 ``` 3.3. Alternative: Use of CSR templates ``` Why is this alternative included here? If there is any value to doing so (e.g., for posterity) move it to an appendix so it's clear it's not part of the normative content of this doc. ### Section 5.5.1, paragraph 1 ``` <CODE BEGINS> MD0GCSqGSIb3DQEJBzASBgcqhkjOPQIBMQcGBSuBBAAiMBIGCSqGSIb3DQEJDjEFBgNVBAUGCCqGSM49BAMD <CODE ENDS> ``` Line too long. ## Nits All comments below are about very minor potential issues that you may choose to address in some way - or ignore - as you see fit. Some were flagged by automated tools (via https://github.com/larseggert/ietf-reviewtool), so there will likely be some false positives. There is no need to let me know what you did with these suggestions. ### Typos #### Section 3.2, paragraph 11 ``` - Extension, MUST NOT include more than one element with a particiular - - ``` ### Duplicate references Duplicate normative references to: `https://www.itu.int/rec/T-REC-X.680`. (The URL for [X.690] is incorrect.) ### Uncited references Uncited references: `[RFC5652]`. ### Grammar/style #### Section 3.2, paragraph 13 ``` faked with an empty big string. The avoid this drawback, this specification ^^^^^^^^^ ``` After "The", the verb "avoid" doesn't fit. Is "avoid" spelled correctly? If "avoid" is the first word in a compound adjective, use a hyphen between the two words. Using the verb "avoid" as a noun may be non-standard. ## Notes This review is in the ["IETF Comments" Markdown format][ICMF], You can use the [`ietf-comments` tool][ICT] to automatically convert this review into individual GitHub issues. Review generated by the [`ietf-reviewtool`][IRT]. [ICMF]: https://github.com/mnot/ietf-comments/blob/main/format.md [ICT]: https://github.com/mnot/ietf-comments [IRT]: https://github.com/larseggert/ietf-reviewtool
- [lamps] Genart early review of draft-ietf-lamps-r… Lars Eggert via Datatracker
- Re: [lamps] Genart early review of draft-ietf-lam… Michael Richardson