Re: [art] Artart last call review of draft-ietf-stir-passport-rcd-21

[Chris Wendt <[e-mail]>]

Hi Harald,

Thanks for the review, comments inline:

> On Oct 13, 2022, at 7:02 AM, Harald Alvestrand via Datatracker <[e-mail]> wrote:
> 
> Reviewer: Harald Alvestrand
> Review result: Ready with Issues
> 
> This document is clearly a product of a fairly mature standards process, but as
> always happens with fresh eyes, there are a number of points to call out.
> 
> I'll try to not revisit items unlikely to change, such as the excessive use of
> 3-letter tags (I suppose for shorter headers, despite the fact that compression
> would make that irrelevant) and the use of the term "rich", which by this time
> is as meaningless as that other IETF cozy-word "simple".

“rich” is in the eye of the beholder :)

> 
> With that off my chest - commentary about details and nits follow. This
> document is, to my mind, close to being "ready" - but fixing the issues would,
> I think, make it better.
> 
> By section:
> Section 4 mode 3-4 uses the term “forward control”, which is a confusing term.
> The term seems to indicate that “the entity that signs the RCD object can
> include within the JWT object a statement about what the signer of the PASSporT
> object can claim to include”. This mechanism (and the need for it) is still
> obscure to me after reading the spec. See also comments on section 7.

I can certainly flush out what 'forward control' means better for modes 3-4, but the intent is that in particular with certificate delegation with a party using a certificate that was delegated to them in association with a process where they are vetted to be who they are, vetted to have the rights to use a particular calling name and logo, etc. that the constraints and integrity is meant to be an explicit governing of their use of the authority of the delegate certificate with the associated rich call data.  That process of determining what that information is and how its communicated between the authoritative certificate issuer and the delegated to party is something we didn’t want to define too much as part of this specification.

> 
> Section 5.1 talks about fields being “derived from” things like display-name
> component of From header field value of SIP. This seems like an improper
> constraint on how applications are built. More likely usage is that the
> passport object is signed first, and the display-name is set from the same data
> source; instead of saying “derived from”, a spec should say “the same as” -
> what comes first and last is not a concern for the spec.

That’s fair, i did replace those terms as you suggest, i used terms that were appropriate in the specific sentences including match, determined by.

> 
> Section 6 - spec says MUST SHA512, MUST NOT SHA-1. Given that this is a
> document format, is this the right place for that kind of statement? If specs
> that use it already specify something about hash functions, they are unlikely
> to change their usages just because this document says so. What will not be
> obeyed should not be stated.

These specific MUSTs are specific to the hashes defined as part of the integrity “rcdi” values which is newly defined in this document.  We do inherit the requirements for algorithms used for PASSporT signatures from RFC8225 more generally, so i think we are ok there.

> 
> Section 6 says that it uses “JSON pointer with a minor additional rule”, is it
> clear what the additional rule is? If it’s only the 6.1.4 “jump the indirect”,
> that might be worth calling out.

I clarified that by adding a reference to that section but did say it was specific to ‘jcl’
"The key value references a specific object within the "rcd" claim
   value using a JSON pointer defined in [RFC6901] with a minor
   additional rule to support external URI references that include JSON
   objects themselves, for the specific case of the use of "jcl",
   defined in Section 6.1.4"

> 
> Section 7, The JWT Claim Constraints section, talks about “constrain the
> signer”. That’s an odd form if this is a document format description. RFC 8226
> seems to say that it is part of the format for requesting a signature (the
> unsigned or self-signed version of the cert), and is part of the cert only to
> verify what the signer was asked to sign.

This is related to comments above about Section 4 modes 3-4.  If i read this comment, i believe you are saying the policies and usage of constraints are defined by 8226 and we are repeating it in this document unnecessarily?

> 
> Section 9.1 talks about “the certificate used to sign the PASSporT”. This
> terminology seems suspect. RFC 7515 section 5.1 is the “compute the signature”
> step. It doesn’t talk about a certificate used as input.

Agree, i switched to "used to compute the signature in the PASSporT” it actually occurred a few places in document.

> 12.1 also contains
> this odd construct. Does it mean "the certificate containing the public key
> associated with the private key used to sign the PASSporT"? If so, it should
> say that this is what it means. (An alternative would be "the PKCS#11 object
> that contains both the public and private keys" - but that would seem like a
> *really* odd construct.)

The purpose of this is really more about the potential of using “rcd” PASSporT in a different trust model from the end-to-end call model. So, nothing to do with keys, more to do with who is issuing certs aligned with the required trust model, for example, a terminating provider may trust their third party directly and not need a certificate that has all of the requirements of the larger PKI eco-system as long as they can accommodate it in their implementation. 

> 
> Section 9.3 examples - it would be good to say which elements are protected by
> what for each case - for instance, example 2 will have the protection for th
> PASSPorT object of the icn URL, but has no protection for the content of the
> image - it’s not verifiable - while examples 3 and 4 have protected the content
> of the external references using “rcdi”.

Included that level of detail for examples.

> 
> The note in section 10.1 about reconstructing “nam” from the display-name
> string means that use of this format with the SIP protocol requires them to be
> identical, which seems to be a common assumption (see comment on 5.1).

Correct and did fix to make it explicit that it should match for other comment

> 
> Section 13 introduced “first” and “third” parties without defining them.
> Presumably the "first" party is the signer of the PASSPorT, while the "third"
> party is the signer of the RCD - making this explicit would be good.

Changed first sentence to clarify: "As "rcd" can be provided by either first party authorized providers that are directly authorized to sign PASSporTs or third party providers that are indirectly or delegated authority to sign PASSporTs."

> 
> 14.2 says that one has to extract From and use it to check the rcd (which
> constrains it to be the same, no?) - while the next para talks about pulling
> the nam out and displaying it instead of the name in From. This is odd, given
> that they are identical.

Yes chicken or egg, which came first, which i think is part of issue. Either approach is valid and i think depending how you implement your SIP network and how you provision and authenticate this information you may think from either perspective, but your point of them being identical is correct.
I modified the sentence in second paragraph to be the following: "If the signature validates, then the verification service compares the value of the "rcd" "nam" key with the From header field value or the preferred value depending on local policy of the SIP network technique that conveys the display name string through a field other than the From header field to interoperate with this specification (e.g. P-Asserted-Identity)"

> 
> 14.2 at first seems to constrain what a relying entity shold do, but the
> section ends with a statement equivalent to “but do what you want” ("It's going
> to be determined by policy"), so what’s the point of the section?

I agree, i think some of the text and the last paragraph in particular was motivated by industry discussions on policies that have for the most part been settled and normalized and in the STIR specific context don’t provide value in this document. I plan to delete the last paragraph and clarify a bit of the text in this section to be more relevant to verification of “rcd” in the context of other PASSporTs that may be present or represent different authority in the eco-system. I’m looking to others to provide comments on whether there is disagreement, but i think i’m pretty safe in making those changes.

> 
> All that said - this specification seems to be clear for its purpose. It could
> just be somewhat better.
> 
> 
>