Thank you Carsten for the review and comments. I created individual github issues for these comments and all other review comments of acme-subdomains at https://github.com/upros/acme-subdomains/issues I have committed fixes and closed the associated issues for 6 of these 10 comments. Michael is working on an update to address the security considerations comment: https://github.com/upros/acme-subdomains/issues/27 For the remaining three comments, please see inline below. I have proposed changes on github branches for these three, have raised PRs, but have not merged these onto master yet. Thank you for https://github.com/upros/acme-subdomains/pull/14, this has now been merged onto master. Regards, Owen -----Original Message----- From: Carsten Bormann via Datatracker <noreply@xxxxxxxx> Sent: Wednesday 23 November 2022 10:50 To: art@xxxxxxxx Cc: acme@xxxxxxxx; draft-ietf-acme-subdomains.all@xxxxxxxx; last-call@xxxxxxxx Subject: Artart last call review of draft-ietf-acme-subdomains-04 Reviewer: Carsten Bormann Review result: Ready with Issues Thank you for an easy-to-read document that is accessible even to readers who are not ACME experts. # Minor technical ## "default value" subdomainAuthAllowed is said to have a default value (value assumed if not included) of false, i.e., absence of the field implies the value to be false (except in metadata, which is a separate inconsistency that might surprise implementers). However, in several places, the text seems to instruct the server to specifically include subdomainAuthAllowed with a value of false in certain cases, apparently turning this into a three-valued field (true, false, absent). Which one is it? (This could easily become an interoperability problem.) [ofriel] I have attempted to remove the ambiguity. I have used the patterns and guidelines associated RFC8555 wildcard usage to influence my choice of text here. I have also removed the default subdomainAuthAllowed inconsistency between authorization object (assume false if absent) and directory metadata (changed from no default to assume false if absent). https://github.com/upros/acme-subdomains/commit/91ea76cd5fce242ea389506ba02267051386c054 # Major editorial ## terminology ### parent The term "parent" is usually reserved for the direct ancestor (single edge in the graph). What is defined here really is an "ancestor domain". (Given the definition of subdomain that explicitly includes self, each domain also is its own parent domain; I'm not clear whether this is intended here.) Unfortunately, this unusual terminology is now hard-coded in the names of fields added by this specification, so it is not a purely editorial decision to adjust the terminology to common usage. [ofriel] This is blindingly obvious now that you have pointed this out. I have changed to use your recommended "ancestor domain" rather than "parent domain" terminology. I do not think its that huge an issue having to change the parentDomain field in the newOrder identifers payload to ancestorDomain, and have already started to code this up in my acmez client and pebble server golang implementations. https://github.com/upros/acme-subdomains/pull/37/commits/96e2f14521f8356a12555507714ffa80b9cc7cfa ### subdomain The definition of subdomain (of a domain given) appears to include the domain given. This fine point might be lost on the reader; it can be surprising (subalpine definitely does not include alpine). Several pieces of text sound like setting subdomainAuthAllowed to true only allows subdomains, which actually does not make a difference due to the subtlety of the meaning of subdomain. [ofriel] Indeed, the definition of subdomain lifted from RFC8499 does appear to allow this interpretation. I think the key sentence in the definition is " A domain is a subdomain of another domain if it is contained within that domain. ". If the reader/implementor interprets *another* domain as implicitly meaning a *different* domain then a domain cannot be a subdomain of itself. As a stop gap, I have proposed a clarifying explanation in acme-subdomains that does not change the RFC8499 definition, but states that for the purposes of this document a domain cannot be a subdomain of itself. Otherwise we could end up in a paradoxical situation where a server creates an authorization object for an identifier and sets "subdomainAuthAllowed"=false, but if a domain is allowed be a subdomain of itself, does this mean that the authorization is in fact not valid for itself?? https://github.com/upros/acme-subdomains/commit/50f29bd8c617efad0ba4bd56341624d67681b9a2 Does the RFC8499 definition need to be updated? What is ISEG guidance here? -- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call
