Re: [CDNi] Working Group Last Call on draft-ietf-cdni-uri-signing

["Brandenburg, R. (Ray) van" <[e-mail]>]

Hi Ben,

Thanks for the thorough review! See my comments below.

Ray

On 02/07/16 13:35, "CDNi on behalf of Ben Niven-Jenkins" <[e-mail] on behalf of [e-mail]> wrote:

Kevin, Colleagues,

I reviewed draft-ietf-cdni-uri-signing-09 & below are my comments.

General:

The document is not clear on whether implementers are expected to implement both shared secret & digital signature algorithms for signed URI validation. This could lead to interop problems where different implementation implement one but not the other.

Maybe specify a minimum baseline of MUST support shared secret (as AFAIK that is what is used in the field) with SHOULD implement digital signatures for the 1 corner case for which digital signatures provide an advantage.

I’m somewhat against specifying must implement both shared secret & digital signatures because I’ve never seen a digital signature based signed URI in the field and I’m familiar with quite a few signed URI algorithms/implementations of different CDNs. That says to me folks are not likely to implement more than the shared secret algorithm unless they really need to. I know Cisco have a digital signature implementation maybe they could help with how much use it sees in the field?

[Ray] Agreed that we shouldn’t make digital signatures mandatory. As for making support for shared secrets mandatory: I guess that makes sense. 

The document could do with some more examples of {URI, attributes, URISigningPackage, Signed URI} covering e.g. IPv4 & IPv6, one including a URI Path Pattern etc.

Section 2.1:
URI Pattern Container (UPC) definition - how does one represent a URI pattern that contains a literal ‘;’ because the URI/URI pattern contains in path parameters to match against? Does presence of a literal ‘;’ need to be escaped with ‘$' too to distinguish it from the use of ‘;’ to separate URI patterns within the URI Pattern Container?

[Ray] Good point. Literal semi-colons do indeed need to be escaped. I’ll add this in the next revision

Section 2.2:
Why is there a KID (string) & KID_NUM (integer) for shared secrets/keys and only CKI (integer) for Client IP keys? To be consistent should it be CKI_NUM?

The document also says
   In cases
   where numerical KEY IDs are used, it is RECOMMENDED to use KID_NUM
   instead of KID.
Which makes me think, why? What the KID means/represents is communicated out of band (and how to do that is out of scope of the document). It seems straightforward to say KID is always a string (which can represent an integer) and get rid of KID_NUM entirely. That would mean we need one less attribute defining (or two less if we need string & integer attributes for CKI) and avoids folks having to work out what to do & implement corner cases like what to do if KID & KID_NUM are both present.

Why not just have KID and CKI both as strings and remove KID_NUM?

[Ray] This was the consensus result of a lengthy discussion between the authors both on and off list ☺ The main reason we included a string-based KID was so that we can have URLs to public keys. However, as Leif pointed out, for interop and performance reasons it helps to have maximum length for the KID field. With public keys this is hard to do. So in cases where performance is essential, using a fixed-length int field is faster. The reason we didn’t extend this to the CKI field is that for CKI we only support symmetric encryption, and hence a integer key index should be sufficient.

Along similar lines. Why have both HF & DSA. Is it implicit from the value of the attribute (e.g. 'SHA-256' vs ‘ECDSA’) whether a shared secret or digital signature algorithm is being used? Or is the purpose to enable URI to be signed (& subsequently) verified with either (if both are specified) or with both?

[Ray] We explicitly rule out having both HF and DSA information elements in a single Signed URI. The reason we went for separate IEs is that it makes describing the algorithm slightly less complex. And given that we base64-encode the full URI Signing Package anyway it doesn’t really matter in terms of namespace collisions.

Section 3.1:
8)a) says
           Note: the Original URI
           Container information element MUST be the last information
           element in the buffer before the signature information
           element.
8)b) does not have a similar statement. Is the URI Pattern Container also required (MUST) to be the last information element in the buffer?

[Ray] The Original URI Container is a special case in the sense that it isn’t communicated along with the Signed URI but is removed from the package after having calculated the signature. 

Section 5.4:
I don’t see the point of key-id, hash-function, digital-signature-algorithm & version.

The absence of the VER attribute is specified by the document to mean Version 1. The absence of the HF & DSA attributes is specified by the version of the signing method. Version 1 (the document) specifies them as SHA-256 & ECDSA accordingly. Why re-specify this in the Metadata object?

[Ray] I’m not sure I understand. What the metadata ‘version’ field is for is to allow CDNs to mandate other versions of URI Signing in the future (e.g. by setting the version property to 2). The same goes for the hash-function and digital-signature-algorithm properties: a CDN could mandate that URI Signing version 1 is used, but still only allow SHA-512 instead of SHA-256 as used by default.

key-id is for when there is only a single key id as an optimisation if someone wants to have a single key id & not transmit it in the signed URI? Does;t seem worth it to me for a narrow corner case. If it is worth it just specify if KID/KID_NUM is missing from the signed URI and key-id-set contains a single value then use that? Or if KID/KID_NUM is missing the surrogate checks all keys in key-id-set and if one matches the URI is authorised?

[Ray] See earlier comment

(Personally I think KID/KID_NUM in the Signed URi is pointless, their meaning is distributed out of band so why not also distribute the values out of band/via metadata and have the behaviour be any valid key/secret in key-id-set passes and remove KID/KID_NUM [and maybe CKI] from the Signed URI entirely)

[Ray] I’m not sure I understand. How would the dCDN know which Key is used to calculate a given signature? Or are you saying it should just check all possible Keys that are in the allowed set?

Section 5.4:
This confused me:
   Note that the Key ID information element is not needed if only one
   key is provided by the CSP or the Upstream CDN for the content item
   or set of content items covered by the CDNI Metadata object. 
until I realised that “Key ID information element” was referring to the element of the Signed URI and not the key-id property of the metadata object specified immediately above the sentence. Maybe phrase as “Note that the Key ID information element does not need to be included in the Signed URI if only one key…"

[Ray] Good point. Will update. 

Section 9:
   In the case where asymmetric keys are used, the KID information
   element might contain the URL to the public key.  To prevent
   malicious clients from signing their own URIs and inserting the
   associated public key URL in the KID field, thereby passing URI
   validation, it is important that CDNs check whether the URI conveyed
   in the KID field is in the allowable set of KIDs as listed in the
   CDNI metadata or set via configuration.
Or specify things as I suggest above and remove the need for KID/KID_NUM being present in the Signed URI and therefore this scenario/vulnerability being able to occur in the first place?

[Ray] See my earlier comment. I don’t see how the dCDN would ever be able to know which key was used. But maybe I’m missing something.

nits:
Section 1:
   The primary goal of URI Signing is to make
   sure that only authorized User Agents (UAs) are able to access the
   content, with a CSP being able to authorize every individual request.
Is it really to authorise the User Agent or the IP address the User Agent is behind? There is nothing in the signed URI that identifies the particular User Agent. URIs can still potentially be shared/exchanged between User Agents behind the same IP address/subnet, however the impact of such sharing is limited/acceptable for the use case.

I would s/User Agents (UAs)/IP address(es)/

Section 1:
I do not know what is meant by distribution policy in this sentence:
   Splitting the role of performing per-request
   authorization by CSP and the role of validation of this authorization
   by the CDN allows any arbitrary distribution policy to be enforced
   across CDNs without the need of CDNs to have any awareness of the
   actual CSP distribution policy.

Section 2.2:
   The Key ID Information Element is used to retrieved the key which is
   needed as input to the algorithm for validating the Signed URI.
s/retrieved/retrieve/

Section 2.4:
   However, a client or CDN may append other
   query parameters unrelated to URI Signing to the Signed URI.  Such
   additional query parameters SHOULD NOT use the same name as the URI
   Signing Package Attribute to avoid namespace collision and potential
   failure of the URI Signing validation.
s/client/User Agent/ is probably more aligned with the rest of the document. There are some other places where you use client. Probably worth being consistent and picking one of client or User Agent to use throughout.

Section 3:
   In case the
   URI Pattern Container information element is used, the CSP has full
   flexibility to specify which elements of the URI (including the
   scheme part) are protected by the URI.
I think you mean “protected by the Signed URI”?

Section 3:
   The process of generating a Signed URI can be divided into four sets
   of steps: 1) Compose URI Signing IEs with original URI / URI pattern,
   2) Compute the URI Signature, 3) Encode the URI Signing Package, and
   4) Assemble the parts to create the Signed URI.
This would be better as a list of numbered bullets IMO.


[Ray] Thanks again!

HTH
Ben

> On 29 Jun 2016, at 14:43, Kevin Ma J <[e-mail]> wrote:
> 
> Hi All,
> 
>  As per the discussion in BA, we have received the updated URI Signing draft and would like to move it to WGLC.  (Leif and Phil, you're up!)
> 
>  Today will begin the two-week WGLC on: https://tools.ietf.org/html/draft-ietf-cdni-uri-signing-09
> 
>  Please send any comments, questions, concerns to the CDNI mailing list: [e-mail]
> 
>  The WGLC will end on Wednesday July 13, 2016.
> 
> thanx!
> 
> --  Kevin and Francois
> 
> _______________________________________________
> CDNi mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/cdni

_______________________________________________
CDNi mailing list
[e-mail]
https://www.ietf.org/mailman/listinfo/cdni


This message may contain information that is not intended for you. If you are not the addressee or if this message was sent to you by mistake, you are requested to inform the sender and delete the message. TNO accepts no liability for the content of this e-mail, for the manner in which you use it and for damage of any kind resulting from the risks inherent to the electronic transmission of messages.