Subject: [ietf-provreg] NITS review of draft-gould-rfc4310bis (pre-6 version)

From: [email-masked]
Date: Fri, 26 Feb 2010 11:42:16 +0100

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 [email-masked]

---

From: Bernie Hoeneisen <[email-masked]>
Date: Fri, 26 Feb 2010 12:05:27 +0100 (CET)

Hi Alex

Great review!

> Bernie, are you aware of what the latest "good" ipr tag is?

Hmmmm...this has been a moving target for a long time...

According to

  http://greenbytes.de/tech/webdav/rfc2629xslt/rfc2629xslt.html#attribute-ipr

it should be "pre5378Trust200902".

cheers,
  Bernie