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

From: Martin Thomson <[email-masked]>
Date: Thu, 12 Feb 2015 12:02:12 +1100

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.

---

From: Martin Thomson <[email-masked]>
Date: Thu, 12 Feb 2015 12:24:27 +1100

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.

---

From: "Francois Le Faucheur (flefauch)" <[email-masked]>
Date: Mon, 16 Feb 2015 15:16:15 +0000

Hi,

Thank you Martin. 
We are working on this.

Francois

> On 12 Feb 2015, at 02:24, Martin Thomson <[email-masked]> wrote:
>