To:
ietf-provreg@cafax.se
From:
axelm@nic.at
Date:
Fri, 26 Feb 2010 11:42:16 +0100
Sender:
owner-ietf-provreg@cafax.se
User-Agent:
Thunderbird 2.0.0.23 (Windows/20090812)
Subject:
[ietf-provreg] NITS review of draft-gould-rfc4310bis (pre-6 version)
FYI, i sent the following to the Author and document shepherd: --- Hello, Here's my NIT review for the draft (note that you might receive contradicting comments from other IETF reviewers, and i deliberately haven't compared the bis-Draft to the original RFC 4310): Generally, i think there are a still a few open issues in the document. Some might be nitpicking, some of them are major, and could create interopability issues: The major issues from my perspective are: ========================================= - What's the motivation behind replacing 4310? This is not explained in the document. - What's the relation between the OPTIONAL keyData in the DS interface, and pre-existing DS records in the registry? - It's unclear whether servers can "mix" the DS and the key Data interface, and what the implications are. - I would appreciate if the relation between provisioning interface and the DNS interface is described in greater detail (if that is in scope!) - What is expected to happen on the DNS interface for a certain provisioning request, for example a maxSigLifetime change? This could be part of a non-normative "examples" or "use cases" section. I think it's also worthwhile to resolve the nitpicking issues, particularly because it's a -bis document that tries to resolve issues in the original RFC, and hence should have very little "bugs" left. Details in sequential order by section: ======================================= - There's no need to extend abbreviations in the Abstract (although it's not disallowed, either). - The abstract should say "This document obsoletes RFC 4310" instead of "is intended to"... - Copyright: Since this document does very likely include Material that was published before November 2008, which means that another copyright boiler plate is likely required. I haven't checked that recently, but is suppose there's a different "ipr" tag required for that. Bernie, are you aware of what the latest "good" ipr tag is? - Section 1: In the introduction it might be unclear to the reader what "secDNS-1.1" and "secDNS-1.0" refer to. Is that a name of a protocol, or some identifier of the XML schema described? Ok, i see now it's defined in 1.1, so that is a "chicken and egg" problem - not sure how to resolve that.. (but also not really serious) I am missing in the introduction (or somewhere else, maybe?) what the motivation for changing RFC 4310 is. Adding such a section might help readers and implementors to understand the issues and reasoning behind that update. - Section 2: it might not be necessary to mention again that "secDNS-1.0" is defined in RFC 4310, and "secDNS-1.1" defined in this document. It's already in the introduction - i would remove it. "in magrating from secDNS-1.0 to secDNS-1.1" is enough. The sentence that a client SHOULD only use 1.1 in the migration is slightly misleading. In a situation where the server announces *both* versions, it makes sense. However, in a situation where a server only announces 1.1, there's an implicit MUST already from the EPP base specification. Therefore, i suggest something simpler like: "If a server announces support for both secDNS-1.0 and secDNS-1.1 in a greeting, clients supporting both versions SHOULD prefer secDNS-1.1". - bullet 1 is "self-fulfilling". Better would be "A server migrating from secDNS-1.0 SHOULD support both secDNS-1.0 and secDNS-1.1 for a reasonable migration period" (the "period of time" should be specified somehow, because 0.56 seconds is clearly not enough ;-) - i don't understand the reasoning for bullet 2 - Why is it important to seperate the handlers? If i create a single handler that complies perfectly to both the old and new version, why would that be a problem? Also, the term "protocol handler" is somehow vague... - bullet 3: I would add "element" and "command" behind the XML tags - makes it clear what they refer to. Also, maybe amend "(indicating the secDNS extension _version_)" for clarity. The sentence is also a bit longish and confusing (at least to a non-native speaker like me ;) Nit: You're having "was included" twice, and "is included" once in the list below bullet 3 - is that intentional? Section 3: - "Only those new elements are described here" would be better than "descriptions are described", i think.. Section 4: - Is it intended that a server MUST supports just *one* of those two interfaces, or *at least* one of them, but MAY support both? This is not entirely clear from the text.. Section 4.1: - "relies uses" - typo? What are "creates, adds, removes, replaces"? Are those EPP commands, or abstract operations? If they are operations, where are they defined? What's the relation of those operations to EPP commands? - Generally, it might make sense to keep a consistent style when refering to EPP commands. For example, section 2 uses "<domain:info>" for identifying a response to an EPP command, while this section uses the plain word "info". - The bullet describing the OPTIONAL keyData element is a bit confusing to me, particularly because it's not clear to which "server" the fragment "use in server validation" refers to - the DNS or the EPP server? Maybe something like "describes the key data that the EPP server uses as input for validating the DS records provided in the command"? - MAJOR issue: Is that keyData only used for validating the DS records provided in the same command only, or could there be cases where this keyData is also used for validating DS records that have been provisioned to the registry before? An example for example when performing "add" and "remove" updates, what's the relation between keyData and pre-existing DS records? Section 4.3: - The example snippets should at least use proper XML namespace definitions for "secDNS", to make clear what the schema is... Is it -1.1 or is it -1.0, for example? Section 5.1.2: - The second bullet describing the contents of the "dsData" element confuses me slighty, particularly the "that describe .... That describes" indirection (if that's not a leftover from some copy & paste operations). Maybe this sentence could be split up for clarity? Section 5.1.3: I understand that the "transfer" command mentioned here are the "transfer query" commands. However, that's not explained in that section, which makes it look as if it overlaps the following Section where the transform type transfer commands are described. It would be good to clarify that 5.1.3 is about the query sub-types of the transfer command. The relation between 5.2.4 and 5.1.3 is unclear... Section 5.2.5: - Typo "An OPTIONAL The <secDNS:chg>" ... (Note: i have not reviewed the XML schema for validity, nor have i validated the examples against the schema. I assume that this has already been done as part of previous reviews) Tia, Alex -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=- List run by majordomo software. For (Un-)subscription and similar details send "help" to ietf-provreg-request@cafax.se