Re: [CDNi] AD review of draft-ietf-cdni-uri-signing-19

[Barry Leiba <[e-mail]>]

What's the status of this document?  It's been about four months since
my AD review, in "revised I-D needed" substate.

Barry

On Fri, Mar 13, 2020 at 1:05 AM Barry Leiba <[e-mail]> wrote:
>
> Here is my AD review of draft-ietf-cdni-uri-signing-19.  My apologies for taking a couple of weeks to get to this: the IESG has, as you can imagine, been swamped with both heavy telechat schedules because of the impending AD changeover and unexpected craziness with IETF 107 planning and decisions related to the meeting cancellation.
>
> Kevin, thanks so much for a very thorough and excellent shepherd writeup!
>
> A couple of these comments are substantive, so I would like to discuss and resolve them before last call.  I’m going to set the draft’s substate to “revised I-D needed”.
>
> Please expand “CDNI” in the abstract... and possibly in the title as well, though you could wait for the RPC to do that if you prefer.  Please also fully expand “CDN” on its first use in the introduction.
>
> Please replace “UA” in the abstract with “user agent”.
>
>    This document uses the terminology defined in the CDNI Problem
>    Statement [RFC6707].
>
> I believe that terminology definitions are normative references, so I believe this makes 6707 normative.  Happily, normative downrefs such as this are accepted now, so please make the reference normative and I will make sure it’s called out in the last call notice.
>
> — Section 1.3 —
>
>    With symmetric keys, the same key is
>    used by both the signing entity for signing the URI as well as by the
>    verifying entity for verifying the Signed URI.
>
> Nit: Think of “both” as an open parenthesis (and the end of he sentence as the closing one here).  The first “by” is outside the parens (before “both”), but the second is inside, and that’s not parallel.  Make it either “by both the signing entity [...] and the verifying entity” or “both by the signing entity [...] and by the verifying entity”.
>
>    distributed openly (because they cannot be used to sign URIs) and
>    private keys are kept secret by the URI signer.
>
> I would say “and the corresponding private keys”.
>
> — Section 1.4 —
>
>    While the URI signing method defined in this document was primarily
>    created for the purpose of allowing URI Signing in CDNI scenarios,
>    i.e., between a uCDN and a dCDN, there is nothing in the defined URI
>    Signing method that precludes it from being used in a non-CDNI
>
> As you see here, you capitalize “URI Signing” inconsistently,even in this one paragraph.  Please check the document for consistency in the capitalization: pick one and use it throughout.
>
> — Section 2 —
>
>    HTTP or HTTPS URI [RFC7230] (Section 2.7).
>
> This is a quirk of the tools and not a fault in the draft, but this results in the html version having a clickable link to RFC 7230, and then another clickable link to Section 2.7 of this document (which, of course, isn’t there).  I think if you change this to “HTTP or HTTPS URI (see [RFC7230] Section 2.7)” the generated link will work better.
>
>    o  a reserved character (as defined in [RFC3986] Section 2.2),
>
> Any reserved character?  And the three reserved characters in bullets 1, 3, and 5 can be any three different ones?  I think I know what you mean, but it seems that an implementer who doesn’t could go quite wrong.  Maybe some more words along with these bullets would be better?
>
> [For example, in the URI < http://example.com/content?sign=jwtdatahere&other >, I think that if I strictly implement what you have here I would interpret “example.com/” as the attribute name (because “/“ is a reserved character), “content” as the JWT package, and “?” as the terminating special character.  But you’re looking for something more like “sign” as the attribute name and “jwtdatahere” as the JWT package, yes?]
>
> Also, I don’t see any examples in the draft of a Signed URI, and I think an example or two here of a couple of variations would also help.
>
> — Section 2.1 —
> Several of the subsections that deal with time values repeat this text exactly:
>
>    Note: The time
>    on the entities that generate and verify the signed URI SHOULD be in
>    sync.  In the CDNI case, this means that CSP, uCDN, and dCDN servers
>    need to be time-synchronized.  It is RECOMMENDED to use NTP [RFC5905]
>    for time synchronization.
>
> I suggest cleaning this up by moving that note up here to the end of Section 2.1, saying, “Note: When claims related to time are used, the time on the entities...”
>
> — Section 2.1.7 —
>
>    previously seen the nonce used in a request for the same content
>
> This is the only spot where you don’t capitalize “Nonce”.
>
> — Section 2.1.9 —
>
>    This claim MUST be understood and processed by
>    all implementations.
>
> Section 2.1 already said this about all claims, so why say it here about this one in particular?
>
> — Section 2.1.10 —
>
>    Client IP (cdniip) [optional] - The Client IP (cdniip) claim hold an
>
> Nit: “holds”
>
> — Section 2.1.15 —
>
>    The URI Container (cdniuc) claim takes one of the following forms
>
> This is the only claim definition that doesn’t say “[optional]”.  Should it?
>
> — Section 3.2 —
>
>    the following section defines a mechanism using HTTP Cookies
>
> Please use a specific section xref instead.
>
> — Section 6.7 —
> Thanks for including advice to the experts.
>
>    Expert Reviewers should be empowered to pass judgements
>    as they see fit
>
> This is very vague and, to me, implies more capriciousness than I think you really intend.  Can you come up with another way to word this that gives a bit more guidance?  What sorts of judgments are they likely to make?
>
> — Section 7 —
>
>       seconds to prevent a sudden network issues from denying legitimate
>
> Nit: either remove “a” or make it “issue”.
>
> —
> Barry
>