Re: [stir] AD Evaluation: draft-ietf-stir-passport-rcd

Hi Murray,

Many thanks for the review, comments inline, i have submitted a new 21 version of draft with specific fixes:

> On Sep 25, 2022, at 11:39 PM, Murray S. Kucherawy <superuser@gmail.com> wrote:
> 
> Hi all, here are my review comments for this document.  Thanks for the work put into it.
> 
> -MSK
> 
> General:
> 
> I found several places where "indexes" was used instead of "indices".

Fixed

> 
> How do we know that the reason placed in a "crn" claim is true?  Could this be abused?  If so, should this be called out in Section 13 or 18 (or somewhere else)?

Section 8.1 was intended to address this use of constraints if the signer is “non-authoritative”.  The general concept is that RCD information should be vetted by trusted participants in the eco-system.  “crn” is a claim that i think we conceptually provide a bit more latitude with just because call reason may more generally be a more dynamic field, but there is nothing in the specification that doesn’t allow for an eco-system using this specification to mandate the use of constraints.  This is a balance we have needed to support for most of the specs to allow for flexibility depending on use of the specs.

> 
> Section 1:
> 
> The last sentence of the third paragraph, namely "However, both are ..., and therefore is ...", is a bit hard for me to parse.

Yes, rewrote to this simpler statement: "In this document, we utilize the STIR framework to more generally extend the assertion of an extensible set of identity information not limited to but including calling name."

> 
> In the fourth paragraph, "In" has been reduced to "n".

Fixed

> 
> Section 3:
> 
> * The last sentence of the first paragraph could use some commas, usually right before each "or".

Fixed

> 
> * "P-Asserted-ID" should be "P-Asserted-Identity".

Fixed

> 
> Section 4:
> 
> * "Often there is policy and restrictions ..." -- s/is/are/

Fixed

> 
> * "(Auth represents authoritative in this table)" -- I suggest quoting "Auth" and "authoritative", and adding a period after "table".

Fixed

> 
> Section 5.1.3:
> 
> * "... the call-info header field ..." -- s/call-info/Call-Info/

Fixed

> 
> Section 6:
> 
> The last paragraph describes the syntax of the values in this claim.  It says there is a delimiter in the value, namely a "minus" character.  I know what that means but I've never heard that character identified that way; it's usually "dash" or "hyphen".  In this era of Unicode, might it be better to identify it as ASCII 45?

Fixed

> 
> For that matter, would ABNF be useful here?

Likely would but we haven’t included it in other relevant specs, i would hate to add at last minute.

> 
> Section 8.1:
> 
> The "must" in here probably needs to be a "MUST".

Fixed

> 
> Section 13:
> 
> * "Even in first party cases, this admits of some complexity: ..." -- I don't understand the part after the comma; should "of" be dropped?

Yes, this was likely copy and paste error or something, Fixed by removing "this admits of some complexity: "

> 
> Section 14.1:
> 
> "Identity header" should be "Identity header field" (at least two instances).

Fixed

> 
> Section 14.2:
> 
> The "SHOULD" presents an implementer with a choice, but includes no guidance about how to make that choice.  I suggest adding some, or reconsidering whether SHOULD is appropriate here.

I did change this to a MUST, i think in the specific context of compact form procedures a MUST level is appropriate.

> 
> Section 17.2:
> 
> There isn't a registry called "PASSporT Types".  I only see "Personal Assertion Token (PASSporT) Extensions" and "PASSporT Resource Priority Header (rph) Types".  Which one are we modifying here?

Fixed

> 
> Section 17.3:
> 
> This should say that the new registry should be added to the "Personal Assertion Token (PASSporT)" group.

Fixed

> 
> A "Specification Required" registry will require nomination of one or more Designated Experts.  Do you have anyone to nominate?  Moreover, Section 4.6 of BCP 26 says you should provide guidance to the DE to be applied to new registrations once the registry is created.  That seems to be missing here.

Added and Fixed

> 
> You should also specify explicitly that new registrations consist only of a name and a reference document.

Fixed

> 
> Section 18:
> 
> I can't parse the first sentence of this section.

Rewrote this paragraph:
The process of signing information contained in a RCD PASSporT, whether its identities, alternate identities, images, logos, physical addresses, or otherwise should follow some vetting process in which an authoritative entity should follow an appropriate consistent policy defined and governed by the eco-system using RCD and the STIR framework. This can be of many forms, depending on the setup and constraints of the policy requirements of the eco-system and is therefore out-of-scope of this document. However, the general chain of trust that signers of RCD PASSporT are either directly authoritative or have been delegated authority through certificates using JWT Claim Constraints and integrity mechanisms defined in this and related documents is critical to maintain the integrity of the eco-system utilizing this and other STIR related specifications.

> 
> _______________________________________________
> stir mailing list
> stir@ietf.org
> https://www.ietf.org/mailman/listinfo/stir