Re: [stir] Genart last call review of draft-ietf-stir-passport-rcd-22

[Chris Wendt <[e-mail]>]

Hi Dale,

Thank you very much for the very comprehensive review, comments inline:

> On Oct 20, 2022, at 11:23 PM, Dale Worley via Datatracker <[e-mail]> wrote:
> 
> Reviewer: Dale Worley
> Review result: On the Right Track
> 
> I am the assigned Gen-ART reviewer for this draft. The General Area
> Review Team (Gen-ART) reviews all IETF documents being processed
> by the IESG for the IETF Chair.  Please treat these comments just
> like any other last call comments.
> 
> For more information, please see the FAQ at
> 
> <https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.
> 
> Document:  draft-ietf-stir-passport-rcd-22
> Reviewer:  Dale R. Worley
> Review Date:  2022-10-20
> IETF LC End Date:  2022-10-20
> IESG Telechat date:  (not known)
> 
> Summary:
> 
>    This draft is on the right track but has open issues, described in
>    the review.
> 
> Major issues:
> 
> The document needs to decide on what background is needed to read it.
> As I read it, it requires a thorough understanding of the Passport
> mechanism as well as JWT.  But the document doesn't state at the
> beginning what background is required for understanding it.
> 
>> From my point of view, it would be helpful to provide an example
> showing all the moving parts, e.g. how a SIP INVITE message is
> assembled using this mechanism, as well as describing the roles of all
> the parties (caller, callee, Authentication Service, Verification
> Service) and their actions.  There also is a "certificate" involved,
> but what parties deal with that and how it is involved is not clear.
> Compare to RFC 8224 section 5.1.  Section 9.3 gives some valuable
> examples of the Json data structure that is built but nothing about
> how it is embedded in the larger communication.

I think the first part of your comment is valid and i did modify the text of the first paragraph to help clarify to the reader the scope of understanding needed to understand the rest of the document (i.e. JWT/PASSPorT/STIR framework).  I think the second part of your comment is really covered fully by RFC8224 specific to SIP call flow and how PASSporTs are inserted into identity header fields, etc., so we didn’t want to repeat all of that in this document. PASSporT extensions are supposed to be just that, an extension to the existing mechanisms and don’t stand alone, although RCD is probably a bit more comprehensive than other extensions, it still sits on all of the fundamentals of 8224, 8225, 8226.  Hope that makes sense.

> 
> It would also be very valuable to provide a terminology section that
> introduced the various special terms involved.
> 
> The writing should be carefully revised, especially with regard to
> these points:
> 
> - Checking for forward references and clarifying them.  E.g. some of
>  section 11 discusses "information about the call itself which may
>  derive from analytics", but that only makes sense in the context of
>  section 12, which speaks of parties other than the caller adding
>  Passport information to a call.

This is a good point, since Section 11 is really intended to contain future use and information that could be carried in RCD by future specifications, i moved it to be the last section of the document.

> 
> - Cleaning up various phrasing errors and unclear phrasing.
>  E.g. "externally referenced" or "externally referenced content" is
>  used to describe external data that is referenced by the Passport,
>  even though this isn't correct usage; the phrase means that
>  something external does the referencing.  You want "referenced
>  external content", content that is external and which is referenced
>  by the Jcard.

Fixed

> 
> - Various data items have various relationships that aren't specified
>  clearly at the beginning of the sections about the data items.
>  Often these are described later in the section but not in a succinct
>  way.  Better phrasing would be e.g.
> 
>    "jcd" or "jcl" MAY appear once, but they are mutually exclusive.

Changed to "The "jcd" or "jcl" keys MAY only appear once in the "rcd" claim but MUST be mutually exclusive.”

> 
>    "rcdi" and "rcd" MAY each appear once, but if one appears the other
>    must also.

Changed to ""rcdi" and "rcd" claims MAY each appear once in a PASSporT, but if "rcdi" is included the "rcd" MUST correspondingly be present also."

> 
> - Use RFC 8174 words whenever their meaning is intended.  E.g. there
>  are places in the document where the subjunctive mood is used to
>  indicate RECOMMENDED.  This is one use of the subjunctive mood in
>  English, but clarity demands that 8174 words be used when they would
>  suffice.  E.g.
> 
>    "would recommend xxx" becomes "xxx is RECOMMENDED".
> 
>    "xxx would be discouraged" becomes "xxx is NOT RECOMMENDED".

Fixed 

> 
> - Use only one term for each concept.  A particularly confusing
>  example are these, which seem to me to be intended to have the same
>  meaning, but are all different, and only one has a non-trivial word
>  to describe the relationship between "ppt" and "rcd":
> 
>    a "ppt" value of "rcd"
>    a "ppt" for "rcd"
>    a "ppt" of "rcd"
>    "ppt" does contain an "rcd"
> 
>  A "Terminology" section helps ensure that there is a set term for
>  each concept.

Yes, agree i was using that shortcut a lot, i cleaned up avoiding using “ppt” as the equivalent of “PASSporT extension"

> 
> This document is referenced 18 times in the 33-page document but the
> Datatracker says it expired on 2022-09-08:
> 
>   [I-D.ietf-sipcore-callinfo-rcd]
>              Wendt, C. and J. Peterson, "SIP Call-Info Parameters for
>              Rich Call Data", Work in Progress, Internet-Draft, draft-
>              ietf-sipcore-callinfo-rcd-04, 7 March 2022,
>              <https://www.ietf.org/archive/id/draft-ietf-sipcore-
>              callinfo-rcd-04.txt>.

Yes, we were trying to update and work on both documents in parallel, but it became clear that until this document settled it wasn’t useful/efficient to keep modifying both.  I will be updating the sipcore document very shortly to correspond with the state of passport-rcd document, so we can finish the work on that document in sipcore.

> 
> This statement is in the document:
> 
>   The candidate of designated expert should be like the same
>   designated expert for PASSporT extensions registry which happens to
>   be the first listed author of this document.
> 
> The phrasing is poor, but more importantly, how do this nomination
> of a designated expert, the future approval of the document, and the
> IANA processes interact?

Yes, i’m removing that, that was my misinterpretation of what i needed to say and forgot to remove in last version.

> 
> The document is mostly normative text, although "MUST" is used rarely.
> This is intermixed with discussion of typical use cases, possible
> extensions, and other non-normative text in a way that is not usually
> done.  In most of these cases, it is immediately clear which parts of
> the text have which status.  But there are a few places where the
> transitions are not clear and these should be made more explicit.
> 
> Particular care must be taken with discussions of verification
> processes regarding which behaviors they MUST have, which they SHOULD
> have, and which are RECOMMENDED.

Yes, i have gotten other review comments about normative language usage and using RECOMMENDED and have tried to make the usage more consistent.

> 
> Technical issues:
> 
> Section 6 tells that the "rcdi" datum is used to record hashes of the
> components and subcomponents of the "rcd" datum.  Some questions arise
> when a referenced datum is a string which has the format of a URI.
> Section 6 requires that if the datum is a URI, then the hash is taken
> of the resource retrieved using the URI, but if the datum is a string,
> then the hash is taken of the Json representation of the string.

I believe this is ok if you are using the string of the JSON that incorporates the rules for string vs URL in a standard way, these rules only really apply to “jcd” and “jcl” which are the only claims that can have URLs referencing content that recursively references other URLs.

> 
> Section 6 seems to assume that the hashing algorithm knows whether
> such a datum is a URI or not, but Json does not mark URIs specially.
> Is the hashing algorithm required to know a priori which keys have
> values that are URIs?  Does this cause problems with extensibility of
> the Passport?

As far as I can see, at least for jcard specifically which is currently the only use-case defined that this is relevant, the use of URI is always prescriptively defined, so the a priori knowledge follows the specifications.  I don’t believe the extensibility defined is inteded to be as flexible as you are interpreting it where strings vs URIs are interchangeable for different jcard fields.

> 
> And given that the "jcd" datum is a Jcard, any extension elements of a
> Jcard must be similarly understood by the hashing algorithm.  Looking
> at the example "Rendezvous for Little Nellie" in section 9.3, I see
> that /jcd/1/3/2 is the string "uri", which I suspect is what flags
> that /jcd/1/3/3 should be interpreted as a URI.  To what degree are
> behaviors like this embedded in Jcard, and therefore must be
> understood by the hashing algorithm?

I’m having trouble finding an example with 1/3/2, but in general one URI reference should not influence how another JSON pointer hash should be interpreted.  By definition each JSON pointer in “rcdi”  should have a hash build on only URI references.  

> 
> It seems to me that it would be preferable to alter the definition of
> "rcdi" so that values that are hashes of Json value are explicitly
> distinguished from values that are hashes of retrieved resources.
> Perhaps a quasi-Unix notation could be used by appending "/" to keys
> for the latter hashes.
> 
> (The usage I am referring to is that when there is a Unix symbolic
> link "/foo/bar" pointing to a directory "/baz", using "/foo/bar" in a
> command line usually indicates the link itself, but "/foo/bar/" is
> interpreted to mean the pointed-to directory.  Apparently this was
> originally because the filename parsing algorithm would see the final
> slash and have to verify that "/foo/bar" was a directory in order for
> the string to be declared valid.  But seeing that /foo/bar was a
> symbolic link, the algorithm would follow the link to see what it
> pointed to before deciding whether it was a directory.)

I think this comment is based on the former two comments misinterpretation so hopefully current mechanism is fine. The only case this potentially could apply is for /jcl which has the extra URL reference for the jcard object itself, therefore decided to keep mechanism simple.

> 
> Nits/editorial comments:
> 
>   1.  Introduction
> 
>   ... "rcd PASSporT" ...
> 
> Also, "RCD PASSporT".  At base, this seems to mean a Passport data
> structure containing an "rcd" datum, but since this isn't stated
> explicitly, it's not clear whether this is a category of Passport that
> excludes other, similar categories, or whether it can overlap them.
> The latter case introduces the possibility that multiple
> specifications could be normative for the same Passport.

I went through to make it more consistent that when i refer to an “rcd” PASSporT meaning the “rcd” PASSPorT extension version, i refer to it that way, and when i refer to “rich call data (RCD)” more generally, i use the words or acronym. An “rcd” claim is independent of whether it is in an “rcd” PASSporT or any PASSporT, and i made that reference explicit and consistent.

> 
>   5.1.4.  "jcd" key
> 
>   This jCard object is intended to represent and correspond to
>   the Call-Info header field value defined in
>   [I-D.ietf-sipcore-callinfo-rcd] with a type of "jcard".
> 
> What does "represent and correspond to" mean?

Fixed, i rewrote this to be clearer that the jcard object is intended to exactly match the call-info jcard object.

>  And there is some
> ambiguity in "the Call-Info header field value"; does that mean the
> URI that is physically within the Call-Info header, or the jCard that
> that URI references (typically physically within an additional body
> part of the INVITE request).

I rewrote this better defining the relationship of the jcard JSON object with the URIs and referenced content and the rules around only having a single level of URI references

> 
>   The jCard object value for "jcd" MUST only have referenced content
>   for URI values that do not further reference URIs.
> 
> This is hard to read.  Does it mean "If a jCard object value for "jcd"
> references content via a URI value, the content referenced by the URI
> MUST NOT further reference content via URIs."?

I rewrote this also.

> 
>   other future specifications and protocols are encouraged to be
>   adapted for use of "jcd" (or similarly "jcl" below) key beyond SIP
>   and Call-Info.
> 
> The direction of the relationship between future specifications and
> use of "jcd" is unclear, as the text assumes that "jcd" uses the
> future specifications.  Does this mean:
> 
>   other future specifications and protocols (beyond SIP
>   and Call-Info) are encouraged to use the "jcd"/"jcl" keys.
> 
> --

I changed this to: "Note: even though we refer to {{I-D.ietf-sipcore-callinfo-rcd}} as the definition of the jcard properties for usage in "rcd" claims, using Call-Info as protocol with the addition of an identity header carrying the PASSPorT is not required.  The identity header carrying a PASSporT with "rcd" claim including a "jcd" value can be used as the primary and only transport of the RCD information.” which was the more direct intention of the note


> 
>   5.1.5.  "jcl" key
> 
>   The "jcl" key value is defined to contain a URI that refers the
>   recipient to a jCard [RFC7095] JSON object hosted on a HTTPS enabled
>   web server.
> 
> It is simpler to say "The "jcl" key value is an HTTPS URL that refers
> to a jCard [RFC7095] JSON object on a web server."

Yes, fixed.

> 
>   6.1.1.  "nam" and "apn" elements
> 
>   If used, the JSON key value referenced by the JSON pointer is the
>   string includes the quotes, so quotes MUST be included to compute the
>   digest.
> 
> You might want to say that this is implied by the rules of 6.1:
> 
>   Note that the digest is computed on the Json representation of the
>   string, which necessarily includes the beginning and ending
>   double-quote characters.
> 
> --

Yes, thank you, moved this comment to 6.1 to apply as general note.

> 
>   6.2.  JWT Claim Constraints for "rcd" claims only
> 
>   For the third mode described in the integrity overview Section 4 of
>   this document, where only JWT Claim Constraints for "rcd" claims
>   without an "rcdi" claim is required, the procedure when creating the
>   certificate with the intent to always include an "rcd" claim, to
>   include a JWT Claim Constraints on inclusion of an "rcd" claim with
>   the intended values required to be constrained by the certificate
>   used to compute the signature in the PASSporT.
> 
> I could not parse this paragraph.

I rewrote this a bit to be more specific to the use of JWT Claim Constraints for “rcd” claims, i was aligning it for mode 3 as it was, but now made it more generally applicable to mode 3/4 where you may include JWT Claim Constraints and i think makes more sense.

> 
>   7.  JWT Claim Constraints usage for "rcd" and "rcdi" claims
> 
>   For the case that there should always be both "rcd" and "rcdi" values
>   included in the "rcd" PASSporT, ...
> 
> I could not parse this clause.

Similar changes as noted above, i made this about guidelines when using JWTClaimConstraints with both “rcd” and “rcdi"

> 
>   9.  Rich Call Data Claims Usage Rules
> 
>   ... An example PASSporT header with the "ppt" included is shown as
>   follows:
> 
>   { "typ":"passport",
>     "ppt":"rcd",
>     "alg":"ES256",
>     "x5u":"https://www.example.com/cert.cer" }
> 
> I could not understand this.  I can see that a "ppt" key/name is
> included in this Json, and I assume that is what 'the "ppt"' is
> referring to.  But this paragraph is the first place that "ppt" is
> mentioned and how it related to anything else is unclear.

I added where PASSporT extension “ppt” is defined in RFC8225 Section 8.1

> 
>   9.1.  "rcd" PASSporT Verification
> 
>   An "rcd" PASSporT that uses claims defined in this document, in order
>   to have a successful verification outcome, MUST conform to the
>   following:
> 
> The constraint really isn't a MUST on the PASSporT but a MUST on the
> verifier, e.g. "A verifier that successfully verifies a PASSportT that
> contains an "rcd" claim MUST ensure ..."

Fixed.

> 
>   9.3.  Example "rcd" PASSporTs
> 
>   An example of an "rcd" claims object that includes the "jcd" and also
>   contains URI references to content which requires the inclusion of an
>   "rcdi" claim and corresponding digests.  Note, in this example, the
>   URI referenced content does include integrity protection in the
>   "rcdi" claim.
> 
> {
>   "crn": "Rendezvous for Little Nellie",
>   "orig": { "tn": "12025551000"},
>   "dest": { "tn": ["12155551001"]},
>   "iat": 1443208345,
>   "rcd": {
>     "jcd": ["vcard",
>     [ ["version",{},"text","4.0"],
>       ["fn",{},"text","Q Branch"],
>       ["org",{},"text","MI6;Q Branch Spy Gadgets"],
>       ["photo",{},"uri","https://example.com/photos/q-256x256.png"],
>       ["logo",{},"uri","https://example.com/logos/mi6-256x256.jpg"],
>       ["logo",{},"uri","https://example.com/logos/mi6-64x64.jpg"]
>     ] ],
>     "nam": "Q Branch Spy Gadgets"
>   },
>   "rcdi": {
>     "/jcd/1/3/3": "sha256-RojgWwU6xUtI4q82+kHPyHm1JKbm7+663bMvzymhkl4",
>     "/jcd/1/4/3": "sha256-jL4f47fF82LuwcrOrSyckA4SWrlElfARHkW6kYo1JdI",
>     "/jcd/1/5/3": "sha256-GKNxxqlLRarbyBNh7hc/4lbZAdK6B0kMRf1AMRWPkSo"
>   }
> }
> 
> As in other places, generally correct but the wording needs to be
> fixed.  As far as I can tell from section 4, in mode 4, "rcdi" need
> not be provided regarding the three image URIs in the Passport.  In
> regard to the second sentence, "the URI referenced content" does not
> "include integrity protection in the rcdi claim", but instead "the
> rcdi claim includes integrity protection of the URI referenced
> content".

Mode 4 use of “rcdi” does need to be included for any URI referenced content.  The rules of URI inclusion in content are that the JSON object can have one level of URI reference, but can not include URI referenced content that also has URIs.  Hopefully some of the other review comment fixes have made that clearer in the document, specifically in the “jcd” key 5.1.4 and “jcl” key 5.1.5 sections.

> 
>   10.1.  Compact form of the "rcd" PASSporT claim
> 
>   Compact form of an "rcd" PASSporT claim has some restrictions that
>   will be enumerated below, but mainly follows standard PASSporT
>   compact form procedures.  ...
> 
> It would help if some background/example was given of the distinction
> between "full" and "compact" forms.
> 
> But presumably this is normative text, and for normative text it is
> far too inexact.  Provide a normative description.

I added a reference to RFC8225 Section 7 which defines compact form and RFC8224 Section 4.1 describing PASSporT construction and specifics for compact form, but also did some clarifications in text as well.

> 
> 11.  Further Information Associated with Callers
> 
>   Beyond naming information and the information that can be contained
>   in a jCard [RFC7095] object, there may be additional human-readable
>   information about the calling party that should be rendered to the
>   end user in order to help the called party decide whether or not to
>   pick up the phone.  This is not limited to information about the
>   caller, but includes information about the call itself, which may
>   derive from analytics that determine based on call patterns or
>   similar data if the call is likely to be one the called party wants
>   to receive.  Such data could include:
> 
> It seems like this text could be abbreviated, as the potential data
> seems to be unbounded and none of this is normative (other than a
> couple of sentences in the final paragraph).

I had already moved this text to be the last section in the document based on similar comments from other reviewers.  I agree about the boundedness of the larger issue, but don’t think it hurts to include.

> 
>   12.  Third-Party Uses
> 
>   While rich data about the call can be provided by an originating
>   authentication service, an intermediary in the call path could also
>   acquire rich call data by querying a third-party service.  Such a
>   service effectively acts as a STIR Authentication Service, generating
>   its own PASSporT, and that PASSporT could be attached to a call by
>   either the originating or terminating side.
> 
> Given that this Passport would be generated by "an intermediary in the
> call path", would it not be "attached to a call by" that same
> intermediary (rather than either the originating or terminating side)?

Conceptually this was intended to describe a traditional CNAM type of service where a terminating service provider uses the services of a 3rd party that has CNAM or in this case more generally RCD information.  The intent is to differentiate it from “1st party” authorized data and in the next section use “iss” to make that explicitly distinct.  It is likely the terminating service provider that “attaches” the identity header containing the 3rd party PASSporT, so that is why it is worded that way.  I think the subsequent paragraphs describe this.

> 
>   If the display name in the
>   "rcd" PASSporT object matches the display name in the INVITE, then
>   the name would presumably be rendered to the end user by the
>   terminating user agent.
> 
> As written, this appears to be a very elaborate way of displaying the
> display name in the INVITE.  I think the intended value-added is that
> *if* the display name in the "rcd" (ultimately based on the calling
> telephone number) *does not* match the display name in the INVITE,
> then it is *not* displayed.

This is essentially describing the procedures more generally for PASSporT use in SIP, so maybe could look at it as elaborate, but this comparison is the essence of use of PASSporTs.  I think this paragraph is supposed to be a specific example to explain that the role of the 3rd party data is similar in verifying specifically the display-name info in this example.

> 
> Also, no definition of "matches" is given.

Added ", or is string equivelent to"

> 
> A lot of the information in section 12 seems to be early-stage
> proposals rather than clearly worked out use cases.  So it seems like
> the section could be abbreviated without loss of usefulness.

The intent is to provide a view that guides future specifications or clarifies intent, so i don’t think it hurts to include.

> 
> The descriptions of the "iss" value in sections 12.1 and 13 seem to be
> contradictory.  In 12.1 it is stated to be a name, but in 13 it is
> stated to be the description of a relationship:
> 
>   the
>   value of "iss" however MUST reflect the Subject of the certificate
>   used to sign a third-party PASSporT.  The explicit mechanism for
>   reflecting the subject field of the certificate is out of scope of
>   this document and left to the certificate governance policies that
>   define how to map the "iss" value in the PASSporT to the subject
>   field in the certificate.
> 
>   Therefore, third-party PASSporTs that carry
>   "rcd" data MUST also carry an indication of the relationship of the
>   generator of the PASSporT to the caller in the form of the "iss"
>   claim.
> 
> --

Fixed the description in 13 to be aligned on representing the name of the 3rd party.

> 
>   14.1.  Authentication Service Behavior
> 
>   If
>   the authentication service generates an "rcd" claim containing "nam"
>   with a value that is not equivalent to the From header field display-
>   name value, it MUST use the full form of the PASSporT object in SIP.
> 
> What is the definition of "equivalent" here?

I qualified this to say string equivalent.

> 
>   14.2.  Verification Service Behavior
> 
> This should be clarified regarding what regarding which behaviors a
> verification service MUST have, which it SHOULD have, and which are
> RECOMMENDED and how they integrate into one clearly stated algorithm.

I used this opportunity to make this section (and one above) specific to the RFC8224 and SIP protocol specific verification service behaviors, so actually trimmed down some of the text which was redundant to general PASSporT verification (and authentication) procedures.  I also moved the third party related paragraphs to a new section called “Verification using Third Party RCD” because it was mostly generic to any protocol used.

> 
>   17.3.  PASSporT RCD Types
> 
> This section introduces the term "type" without a definition.  What
> does "type" mean?  As far as I can tell, "type" means the "name"
> values in the Json object structure which Passport uses.  Can this be
> aligned with usual usage?
> 
> 

I qualified the name to be PASSporT RCD Claim Types, because it’s specific to RCD claims vs other JSON object constructs.