Re: [Gen-art] genART review of draft-ietf-cdni-logging-15

[Martin Thomson <[e-mail]>]

And because I hit send too soon, here's more...

I think that this is ready.  I have a couple of major concerns:

Major General 1.

However, I would like to see more consideration given to forward
compatibility.  There are a lot of normative statements regarding what
MUST be included in the file, but no actions described for a receiver.
That means that it is hard to establish expectations regarding file
format evolution.  If the file is rejected when the version isn't
"CDNI/1.0", then that makes changes to that field impossible (and the
specification of ABNF for it somewhat pointless).

Major Specific 2.

   An implementation of the CDNI Logging interface MUST support the
   TLS_DHE_RSA_WITH_AES_128_GCM_SHA256 cipher suite ( [RFC5288]).

Please don't do that.  There are mandatory to implement cipher suites
in all the specs you cite and you don't need anything special here.  I
have no problem with recommending PFS suites, but you might be better
served there by citing the uta TLS usage draft:
https://tools.ietf.org/html/draft-ietf-uta-tls-bcp

S 3.4.1
   o  cs(<HTTP-header-name>):
HTTP header fields, esp. in HTTP/2 can contain HTAB, even if that
isn't permitted by the grammar.  You probably want to describe how
that is handled.

Bottom of P33 (won't adding the established-origin directive
invalidate the MD5?  What do you expect to happen there?  (I see two
choices, it would help if you could even *hint* at what you would
prefer implementations to do).

Nit: HTTP/2 not HTTP/2.0

Question: There is no mention made of range requests, for which this
might be well suited, particularly if files get large.

I note the privacy considerations are pretty reasonable.  I'd be much
happier if IP anonymization were not optional though.



On 12 February 2015 at 12:02, Martin Thomson <[e-mail]> wrote:
> I am the assigned Gen-ART reviewer for this draft. For background on
> Gen-ART, please see the FAQ at
>
> <http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.
>
> Please resolve these comments along with any other Last Call comments
> you may receive.
>
> Document: draft-ietf-cdni-logging-15
> Reviewer: Martin Thomson
> Review Date: 2015-02-12
> IETF LC End Date: 2015-02-18
> IESG Telechat date: (if known)
>
> Summary:
>
> Major issues:
>
> Minor issues:
>
> S3.1 needs to define the basic atom that is used for the logging file.
> It appears as though the atom is an octet.
>
> S3.2 uses the wildcard ABNF rule (i.e., <text....>) too aggressively.
> RECLINE (a potentially confusing name) could be more strictly defined.
>
>     <CDNI Logging File> = 1*<DIRGROUP RECGROUP>
>
> ... is not valid and could be:
>
>     CDNI-LOG-FILE = 1*(DIRGROUP / RECGROUP)
>
> That implies that an empty file is not valid.  I'd have thought that *
> rather than 1* would be better.
>
> S3.3 again uses wildcard rules too aggressively.  DIRNAME (not the
> unix command) should be defined more explicitly, even if it is just to
> specify what characters are allowed.  Use text to further constrain
> it.  As it stands, a generic processor cannot know that the name
> doesn't permit the inclusion of (for instance) ":" or CRLF.  A common
> method is to define something like this as:
>
>     DIRNAME = Version-directive / UUID-directive / ... / extension-directive
>
> More commonly, go up a level and define directive using the choice.
> That way, each directive can define a name and value together using
> ABNF.  e.g.
>
>     Version-directive = "Version" ":" HTAB "CDNI" "/" 1*DIGIT "." 1*DIGIT
>
> I wouldn't be so prescriptive about the version format.  Unless there
> are specific semantics you want to extract from each digit.
>
> What is a receiver expected to do if the version *isn't* CDNI/1.0 ?
> Discard the entire file?  That doesn't seem like a good way to ensure
> forward compatibility.
>
> UUID: this a Universally Unique IDentifier (UUID)
>          from the UUID Uniform Resource Name (URN) namespace specified
>          in [RFC4122]) for the CDNI Logging File.
>
> I don't know how to apply this.  Is it the UUID portion of the URN or a URN?
>
> Please don't use MUST here: If the
>          two values are equal, then the received CDNI Logging File MUST
>          be considered non-corrupted.
>
> MD5 collisions are easy to manufacture.  I'd also drop the MUST from
> the following sentence.
>
> S3.4: more wildcards
>
> S3.4.1: I wouldn't worry about HTTP/2 being special here.  All that
> speculation is painful, especially since HTTP/2 will likely be
> published before this document.  Better to just cite it and avoid
> getting caught up in the details.
>
>
>
>
> Nits/editorial comments:
>
> There are lots of uses of lowercase "may".
>
> Page 6 has a few instances of missing hyphens in dCDN3/dCDN2 etc...
> S6.2.1: chunck
>
> S3.1 the DATE/TIME rules can/should reference RFC 3339
>
> S3.3 using ":" HTAB as a separator makes it hard to construct these
> files manually.
>
> S3.4 use of "c:" in combination with the unordered list is confusing
> when the prefix is actually "c-".  Maybe use the string "c-" quotes
> and all to denote the prefix.
>
> S3.4.1 citing RFC 7230 is sufficient for HTTP/1.1; and probably for
> HTTP in general at this moment.