Subject: [stir] WGLC Review of draft-ietf-stir-passport-rcd-10

From: Ben Campbell <[email-masked]>
Date: Sat, 27 Mar 2021 16:25:35 -0500

Hi,

Here is my (2nd) WGLC review of draft-ietf-stir-passport-rcd. I think this is on the right track, but I do have some comments :-)

Thanks!

Ben.

** Substantive Comments **:

§5.1.2: "If included, this key MUST only be included once in the "rcd” JSON object and SHOULD NOT be included if there is a "jcl" key included. The "jcd" and "jcl" keys should be mutually exclusive.”

Why not MUST NOT?  (and consider saying that ’The “jcd” and “jcl” keys _are_ mutually exclusive.”)

§5.2, 2nd to last paragraph: There are several MUSTs here that seem more like statements of fact about the format.

§5.2.1: 

- I’m pretty sure that, when we talk about the digest for a URL, we are talking about a digest over the contents that URL points to, not a digest of the URL itself. Is that stated normatively somewhere that I missed?

Along those lines, I thought that Chris said during the IETF110 STIR meeting that this version puts a 1-level depth limit on externally referenced content. Is there text somewhere that says that, or is that just a side effect of the use of JSON Pointers? (It does seem like the “jcl” examples are really 2 levels deep)

- " Third, when the "rcd" data is verified
   at the Verification Service, it should verify each digest by”

The “should” here seems ambiguous, Seems like that should either say “it MUST verify” or simply “it verifies”.  (Also, antecedent of “it” is unclear).

§5.3.2: " a "mustInclude" for the "rcdi" claim”

Should that be “crn” claim?

§8: 

- general: How does RCS claim extensibility interact with jCARD extensibility? Some of the potential data elements listed in this section already exist in jCard. Can you give guidance about whether to put such info into a jCard or a new RCD extension?

- Last paragraph: Does this belong in section 9?

§9.1, example: What? You didn’t take the opportunity to have SMERH or SPECTRE out Mr. Bond as the third party PASSporT issuer? ;-)

§10: “ Therefore, PASSporTs that carry “rcd" data SHOULD also carry an indication of the relationship of the generator of the PASSporT to the caller. 

How would they do this? If the normative SHOULD is intended, then some mechanism needs to be mentioned. Or is “iss” supposed to indicate that relationship?

§15: General: Since computation of rcdi digests for URLs requires loading the external content, the security considerations probably need to address some sort of sanity checking over that content. E.g. reasonable size limits, avoid reference loops, etc.

§15.1: enhance-rfc8226 added a security consideration to not put rcd and rcdi in excludeClaims. It might be worth mentioning that here as well.

** Editorial Comments **

General: The entire document tends to use future tense (or inconsistent tenses) when describing protocol operations. Consider sticking to present tense unless the future tense really adds information.

§1, last paragraph, first sentence:
s/“will be”/“is”

§4, first paragraphs:

I suspect “Copyright violations” for RCD are more likely “trademark violations”.

§5, general:

Feel free to ignore this: But I find the organization of this section a little hard to follow, due to the inclusion of claim definitions, rcdi procedures and JWtConstraint features all together. If I were to reorganize this, I think i would do a section with more on high level-concepts (especially “authority”), a section describing the claims, a section on creating rcdi digests, and a section on use of JWCConstraints.  

§5.2: 3rd paragraph:
- s/ “will reference” / references
- "JSON that should be used to generate the digest” : s/ “should be” / “is” 

§5.2.3:  I have trouble parsing the first sentence.

§6: " Either the "rcd" or "crn" claim”

or both?

§11.1, last paragraph: “ populates the value of "rcd" and "nam" key,”

Is “rcd” really intended here, given that we are talking about keys inside the rcd claim?

§12.2, last paragraph: "the "rcd" can simply be ignored and disregarded”

Isn’t it normally a _requirement_ to ignore claims you don’t understand unless their support is required by the ppt type? If so, I suggest “can simply be ignored” be “is ignored”.

---

From: Ben Campbell <[email-masked]>
Date: Sat, 27 Mar 2021 16:43:07 -0500

Oops, forgot one:

§2: Still sticking with [RFC6919]?  :-)

> On Mar 27, 2021, at 4:25 PM, Ben Campbell <[email-masked]> wrote:
> 
> Hi,
> 
> _______________________________________________
> stir mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/stir

---

From: Russ Housley <[email-masked]>
Date: Sun, 28 Mar 2021 15:33:18 -0400

The WG Last Call for this document ends today, it looks to me like a few things need to be cleaned up in a revision.

Russ

> On Mar 27, 2021, at 5:25 PM, Ben Campbell <[email-masked]> wrote:
> 
> Hi,
>

---

From: Richard Shockey <[email-masked]>
Date: Sun, 28 Mar 2021 20:31:10 -0400

I'd like to emphasize for the record the extreme interest in this protocol.  

There is no doubt it will be deployed and very very quickly especially among Voice Service Providers in the UK and Europe that never deployed CNAM. [Calling Name Display] 

I would anticipate interoperability testing to begin this year. 

— 
Richard Shockey

Shockey Consulting LLC

Chairman of the Board SIP Forum

www.shockey.us

www.sipforum.org

richard<at>shockey.us

Skype-Linkedin-Facebook –Twitter  rshockey101

PSTN +1 703-593-2683

On 3/28/21, 3:33 PM, "stir on behalf of Russ Housley" <[email-masked] on behalf of [email-masked]> wrote:

    Russ

---

From: Chris Wendt <[email-masked]>
Date: Mon, 29 Mar 2021 12:39:24 -0400

Hi Ben,

Thanks very much for the very detailed review, this brings a lot of good improvements to the document.  I have fixed each of your comments and will submit a new version, some clarifications below.

> Why not MUST NOT?  (and consider saying that ’The “jcd” and “jcl” keys _are_ mutually exclusive.”)

Fixed

> §5.2, 2nd to last paragraph: There are several MUSTs here that seem more like statements of fact about the format.

Fixed

> - I’m pretty sure that, when we talk about the digest for a URL, we are talking about a digest over the contents that URL points to, not a digest of the URL itself. Is that stated normatively somew…

Made this more clear

> Along those lines, I thought that Chris said during the IETF110 STIR meeting that this version puts a 1-level depth limit on externally referenced content. Is there text somewhere that says that, o…

For both “jcd” and “jcl” it’s the jCard object itself that can only have one level deep URI references. So, for the “jcl” URI referencing the jCard object resource, that level of reference is not considered as one of the levels of reference for the rule that only applies to the jCard content (remotely retrieved) itself. I believe i clarified that in new text.

> The “should” here seems ambiguous, Seems like that should either say “it MUST verify” or simply “it verifies”.  (Also, antecedent of “it” is unclear).

Fixed 

> Should that be “crn” claim?

Fixed 

> - general: How does RCS claim extensibility interact with jCARD extensibility? Some of the potential data elements listed in this section already exist in jCard. Can you give guidance about whether…

I will include text that provides the guidance that jCard information should generally be associated with the caller, where as other new claims, similar to “crn” are associated with providing context on the call.

> - Last paragraph: Does this belong in section 9?

I did move it down and made it a MUST that “iss” for third party information is used.

> §9.1, example: What? You didn’t take the opportunity to have SMERH or SPECTRE out Mr. Bond as the third party PASSporT issuer? ;-)

Fixed :)

> How would they do this? If the normative SHOULD is intended, then some mechanism needs to be mentioned. Or is “iss” supposed to indicate that relationship?

Similar to above i upgraded the normative part and made it MUST for the use of “iss"

> §15: General: Since computation of rcdi digests for URLs requires loading the external content, the security considerations probably need to address some sort of sanity checking over that content.…

Added

> §15.1: enhance-rfc8226 added a security consideration to not put rcd and rcdi in excludeClaims. It might be worth mentioning that here as well.

Added 

> ** Editorial Comments **

For following generally fixed or went through document to fix grammatical issues, etc. pointed out