Document: draft-ietf-oauth-sd-jwt-vc Title: SD-JWT-based Verifiable Digital Credentials (SD-JWT VC) Reviewer: Ionuț Mihalcea Review result: Has Issues
I have reviewed this document as part of the security directorate's ongoing effort to review all IETF documents being processed by the IESG. These comments were written primarily for the benefit of the security area directors. Document editors and WG chairs should treat these comments just like any other last call comments. The summary of the review is Has Issues, though this could be predicated on my misunderstanding. The draft builds on top of RFC9901 to define a complete, interoperable data model and ecosystem profile for SD-JWT Verifiable Digital Credentials (VC). It provides not just the model for the VC, but also for metadata for verifying and displaying VCs, as well as credential typing and inheritance mechanics, key and metadata discovery, and more. The document is thorough and quite broad in scope. The shape and flow of the draft seems good. The points below cover the issues and nits that stood out to me. * Section 4 describes the use of a well-known URL to find issuer metadata, and mandates the shape of the starting URL in iss. However, no requirement is made for that URL to be parsed and normalised as per sec. 6 of RFC3986 before the well-known URL is constructed. Is that on purpose? * In section 5.5.1.1, before the bulletpoints you have "The object contains the following properties". Should this be normative (i.e., "The object MUST contain at least one of the following properties"), since all the properties are optional? Asking because Section 5.5.1.2.1 uses normative language in a similar context. * In section 5.5.1.2.2 you give a list of characters that must be escaped before rendering the SVG. However, you should also mandate that invalid XML characters be stripped, or that the credential should be rejected if they're encountered. * The processing of path components in Section 5.6.1.2 and of claim mandatory metadata in 5.6.3 seems prone to issues, when combined. For example, for string path components, you state "If the key does not exist in an element currently selected, remove that element from the selection." This does not take into account whether the element at the end of the path is expected to be mandatory. And actually, 5.6.3 is not clear on this either: if the claim is within elements of an array, MUST all elements include that claim or at least one? Does this apply only at the "leaf element" level, or must all arrays along the path have elements that allow traversal all the way to this mandatory element? Either way, this should be stated explicitly, and the path traversal text should also reflect it. If my description is not clear I can provide an example. * In Section 7.1 you mandate that "the Consumer MUST validate the URL to ensure that it is a valid HTTPS URL and that it does not point to internal resources". Two observations here: this could be subject to a DNS rebinding attack, so it would be good to hint towards that and ask that the destination address get cached and used after check, instead of performing another request for it. Second, this requirement seems to assume that SD-JWT VCs will be used exclusively on the open internet, and never in internal deployments. Is this on purpose? If closed / private environments are out of scope, it would be good to make that explicit. * Section 7.10 mandates that "consuming applications MUST ensure that references to external resources (images, etc.) from within the SVG cannot be used to track users or the usage of credentials." It would be helpful to provide some guidance or references on how to achieve this. Nits: * I'm struggling to parse the first paragraph in Section 5.3. Not certain what "the vct in the reference to the Type Metadata" means. Also, I can't easily tell what the relationship is between the text before the brackets and the text in the brackets. * In Section 5.3.1: "it is at the consumer's discretion whether to attempt retrieve Type Metadata" - should probably be "... to attempt to retrieve ...". * There are two ways in which optional and required properties / claims are indicated throughout the document. In some places (e.g., Section 4.2) it's "claim_name: OPTIONAL. ...". In others (e.g., Section 5.5) it's "claim_name: ... This property is OPTIONAL.". In some places (e.g., Section 5.2) it's both. This is just an observation, I think both work in conveying the intent. _______________________________________________ OAuth mailing list -- [email protected] To unsubscribe send an email to [email protected]
