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

["Francois Le Faucheur (flefauch)" <[e-mail]>]

Hi,

Thank you Martin. 
We are working on this.

Francois

> On 12 Feb 2015, at 02:24, Martin Thomson <[e-mail]> wrote:
> 
> 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.