-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
What would OpenID4VP look like over the Digital Credentials API? #80
Comments
This looks good, but not loving all the Is it possible to simplify that to something like: {
spec: "org.iso.18013.5.1",
field: "family_name"
} Or is that stuff standardized? |
Are those COSE algs? Because if so, EdDSA and ES256 are ambiguous without a key type. If they are JOSE algs, ES256 is not ambiguous but EdDSA still is. |
response type is necessary, even though the result is being intercepted, and the presentation is being extracted... For completeness you should show the presentation being submitted to the RP. |
The json path stuff seems weird for mDoc, I don't know if CBOR path has ever been specified in an RFC, the presentation exchange spec does not cover that part I think. |
I guess the larger question is: is any of the structural representation of mDoc in OpenID4VP standardized? Be nice if we could simplify it a bit. |
Regarding JSON Path standardization, it's in Auth48 at IETF right now ... https://datatracker.ietf.org/doc/html/draft-ietf-jsonpath-base-21 See section 4.1 for attacks. |
Using "JSON path" for something that's not JSON seems... strange? What am I missing? Also, we should probably not make path be an array? Might be simpler to just declare fields independently. Anyway, we should probably resolve a lot of this on a call... I have a lot of questions 😸 |
JSONPath indeed is for JSON values.
(implied:) or in some other way, e.g., by ingesting an encoded CBOR data item that limits itself to the JSON generic data model. Why would the full syntax of JSONPath be needed here? [“$['org.iso.18013.5.1']['family_name']”] …could be expressed simpler by: [“org.iso.18013.5.1”, “family_name”] plus a sentence that the elements of the array descend down a nested map structure. |
It is, you're not missing anything. Things are currently badly specified with the ground shifting fairly regularly. One consideration for the query language is avoiding things like regex bomb attacks, which can be performed in a variety of the existing query languages (like JSON Path) that support arbitrary regular expressions. Subsetting the language requires effort as well. JSON Pointer is almost certainly a better choice (because it doesn't support arbitrary state machines in the query language). All that to say, I don't think OID4VP in it's current state (which we implement) is a safe thing to build on if the query language is going to be executed in a browser environment. We are going to need a more constrained query language that is fit for purpose for browser environments. |
Yes. We did all this work already. JSONPath (RFC 9535-to-be) uses I-Regexp (RFC 9485), which addresses ReDoS. However, I think you don't need to introduce new syntax here, neither JSONPath nor JSON Pointer (RFC 6901), because all you need appears to be descending through maps. A simple array of map keys solves this problem nicely, without adding a parser that increases the attack surface. |
Just a simple key is not enough in my experience, just think about data structures like addresses and there are much deeper structured schemas in education space. However, based on the experience we gathered so far ourselves and from other implementers, simple path expressions determining claims within a credential are sufficient.
<updated to remove email artifacts>
|
That comes from OID4VP's use of Presentation Exchange 2.0 (and its use of JSONPath) as the credential request/query language. OID4VP's dependency on Presentation Exchange is a topic with a lot of diverse opinions (to put it mildly). |
Ok, yeah, JSONPath is not something I'd want to implement in WebKit (given the above responses, it seems completely unnecessary and adds security risks). @cabo wrote:
I'd like to avoid arrays if possible (too easy to mess up, not clear what the relationship is between values). If we can do 1:1 mappings, that would be better (below). @tlodderstedt wrote:
That's my feeling too. If we can get this down to: {
spec: "org.iso.18013.5.1",
field: "family_name"
} That would be much safer, explicit, simpler, and legible. It would be good to run through scenarios to see if that above covers all requests possible request attributes. |
My larger question remains: "is any of the structural representation of mDoc in OpenID4VP standardized?" Specifically, I'm guessing @Sakurann and @tplooker would definitively know. If not, then we should do that work here instead to make the request format suitable/safe for ingestion and interpretation by web browsers. It can then be upstreamed to OpenID4VP or whatever once we determine it's safe. |
This might work for credential formats that dont support any nested attributes but wont work for say formats like SD-JWT.
I'd be interested in discussing this further as an array of strings where the ordering implies the path through the data structure doesn't seem like a bad idea, nor one that would be too complex for implementations. As I said above 1:1 mappings IIUC won't work for credential formats that aren't just a flat map of attributes like mDocs.
OpenID4VP is standards track at OIDF so no it hasn't completed this process but a lot of work has gone into the protocol to date including multiple rounds of implementation feedback. |
as others have described, currently, OpenID4VP uses DIF Presentation Exchange v2.0 as a query language for any credential format, including mdocs.
Given that Presentation Exchange has been on track to be updated to v3.0 anyway (mainly to move away from JSON Path), I would prefer to take that as an opportunity to update query language syntax in OpenID4VP and address issues of PE v2.0. One of which is making it work with CBOR-based formats, ie mdocs. Need to put more thought, but currently would prefer array of strings approach as mentioned by @cabo |
Please make it work for flat and nested maps, and allow it to support integer and unicode keys.... or explain what the legal values are, and what code points are allowed and forbidden. |
I think we will get to that when we do a mapping from the request format to WebIDL. That is, we can use enums as needed, DOMString or USVString, the appropriate number format from WebIDL, etc... first we just need the object hierarchy, and the rest will follow. |
See my related comment on tailoring OpenID4VP to fit with the WICG credential presentation API at #52 (comment). |
I would like to let you know the Digital Credentials Protocols (DCP) Working Group at OpenID Foundation has started to work on a proposal for an OpenID 4 VP profile for the Digital Credentials API. The respective issue is openid/OpenID4VP#125. Feel free to chime in and comment on the draft. We plan to contribute this proposal as work item to the DCP WG and will present it to this group as soon as possible. We are investigating how OpenID 4 VP could be best fit into the |
@marcoscaceres have you taken a look at how 18013-7 profiled PE for mdocs? It does not use JSON Path:
18013-7 also mandates |
That's great to hear! Thanks for kicking this off @tlodderstedt ! I'm going to close this issue and point any open questions and discussions to this proposal instead, where we'll find a community of experts that can help us resolve them. |
@marcoscaceres and I were wondering what (a profile of?) OpenID4VP may look like over the Digital Credentials API, to get a sense of whether we'd missing big architectural elements in the API.
Reading the OpenID4VP spec and trying to reconcile with the Digital Credentials API spec, here is more or less what I think a request would look like:
@Sakurann @awoie @tlodderstedt @tplooker does this look more or less right to you?
The text was updated successfully, but these errors were encountered: