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

From: Barry Leiba <[email-masked]>
Date: Fri, 13 Mar 2020 01:05:00 -0400

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