Add provider validation
Closes #152 Closes #130
The following tasks have been completed:
- [ ] Modified Web platform tests (link)
Implementation commitment:
- [ ] WebKit (link to issue)
- [ ] Chromium (link to issue)
- [ ] Gecko (link to issue)
Documentation and checks
- [ ] Affects privacy
- [ ] Affects security
- [ ] Pinged MDN
- [ ] Updated Explainer
You're the best @TallTed! Thank you again for those eagle eyes. 🦅
See https://github.com/openid/OpenID4VP/issues/224 for a concrete example of what I'm expecting here...
@marcoscaceres did you intentionally request review? Not only this branch has merge conflicts, but it also uses the old "provider" terminology.
Fixed up terminology... good catch. Hopefully better now.
met with @mohamedamir, discussed changing this...
@marcoscaceres to add a validatedRequest list, gather the validated results, and then continue using the validated requests.
@marcoscaceres I have updated the PR to collect valid requests in a list before proceeding as discussed in our last call. However, looking at the discussion on this PR, it seems that it was decided to omit the parts regarding "Validate |request| against any validation rules set forth in the corresponding specification."
Did anything change in that front? Otherwise, I would update this PR and remove those lines.
WDYT?
So, I still don't agree with the assessment here that it's not needed. All the formats require some validation to happen at some point, and that needs to be codified somewhere because it results in some DOM Exception or TypeError.
As per collecting the valid request, I don't think we actually need to collect them because the result of failing validation is always a some error, which halts all processing:
- the protocol is not supported - error.
- the protocol is known, but not allowed - error.
- the request's payload is invalid (per the validation rules set forth in whatever spec) - error.
Added back to agenda+ for discussion.
So, I still don't agree with the assessment here that it's not needed. All the formats require some validation to happen at some point, and that needs to be codified somewhere because it results in some DOM Exception or TypeError.
As per collecting the valid request, I don't think we actually need to collect them because the result of failing validation is always a some error, which halts all processing:
- the protocol is not supported - error.
- the protocol is known, but not allowed - error.
- the request's payload is invalid (per the validation rules set forth in whatever spec) - error.
Regarding the protocol validation: I am not convinced there needs to be a full protocol-level validation in the Browser. There should probably be security related checks regarding size, valid json etc., but protocol-level validation only needs to happen on the smartphone/wallet side imho. Enforcing such checks in the browser will make it even more difficult to evolve the ecosystem (e.g., by introducing a v1.1 of the interaction protocol) for imho very little practical benefit.
Regarding the protocol validation: I am not convinced there needs to be a full protocol-level validation in the Browser.
The validation check is not necessarily done by the browser: In the case of WebKit, the validation happens through the OS, but contained in the "content process" sandbox.
There should probably be security related checks regarding size, valid json etc., but protocol-level validation only needs to happen on the smartphone/wallet side imho.
That's not how it works at least on Apple's platforms. The initial validation is done by OS level framework. Eventually, once deemed safe, the data is passed to the wallet, which may perform its own checks. However, no arbitrary data is passed to the wallet to minimize the risk of the wallet also becoming an attack vector and/or target.
Enforcing such checks in the browser will make it even more difficult to evolve the ecosystem (e.g., by introducing a v1.1 of the interaction protocol) for imho very little practical benefit.
The intent is to protect users, to follow the principle of "put user needs first" priority of constituencies. Just as a refresher:
"User needs come before the needs of web page authors, which come before the needs of user agent implementors, which come before the needs of specification writers, which come before theoretical purity."
Ecosystem evolution concerns sits towards the end of the list of priority of constituencies.
WebKit's implementation won't pass arbitrary data to the wallet: it only initially passes enough data to create the presentation request. However, before that happens, the data must be validated, which can result in an error. This part of the specification is just codifying that a main thread security check can take place and what to do about that check if it fails.
The way to evolve the ecosystem is to go through the standardization process, reach consensus, have things implemented, etc... basically what we are doing here.
That's not how it works at least on Apple's platforms. The initial validation is done by OS level framework. Eventually, once deemed safe, the data is passed to the wallet, which may perform its own checks. However, no arbitrary data is passed to the wallet to minimize the risk of the wallet also becoming an attack vector and/or target.
In today's world without DC API, Wallets already parse potentially unsafe QRCodes/Requests and deal with the security concerns. I understand the wish to add additional security checks/layers, especially from the perspective of a user agent or OS implementor where the main focus is to provide a smooth and secure user experience. Part of the question is which layers should be active, what happens in those layers and at what cost/for what benefit.
"User needs come before the needs of web page authors, which come before the needs of user agent implementors, which come before the needs of specification writers, which come before theoretical purity."
I absolutely agree, but part of the "user need" is also the freedom of choice which wallet or in that case underlying technology to use. If a country/region chooses other protocols or especially other credential/proof formats than the user agent implementors, then the user should still be able to benefit from the security & UX advantages of DC API. The security benefits/guarantees might be slightly worse (more dependent on wallet implementations) than with additional checks along the chain, but it would still be a big improvement, especially for cross device flows.
The way to evolve the ecosystem is to go through the standardization process, reach consensus, have things implemented, etc... basically what we are doing here.
I fully agree, but who decides which standards should be used/supported? With your proposal, there is no consensus on support. The Browser/OS would become the gatekeeper to applications (wallets) that already fully implement and understand the protocols and formats in that space. Said applications would become fully dependent on platform support for any changes (like for example the introduction of zero-knowledge proofs for stronger privacy guarantees).
For the upcoming discussion: Could you describe where exactly what checks are happening or planned to happen especially for a cross device flow?
Discussed on the FedID WG DC API Series B call
To clarify since that came up in the discussion last Thursday and I was asked to add it as a comment here in the discussion:
One of the biggest fears or concerns I've heard people bring up in discussions regarding DC API is that its usage might/will be dependant on platform (OS & Browser) providers enabling support for specific protocols / credential formats / document types. Especially in the context of state issued wallets and credentials, that becomes a topic of governmental sovereignty. We should try to make sure we address these concerns and provide some level of guarantee in regards to the openness/neutrality of this interface in the spec. That is imho the biggest risk for the adoption of this API.
@c2bo, let's approach this differently. Let's say I have the following openid4vp request:
{
response_type: "this is wrong",
// other stuff
}
When would be the appropriate time to throw an error because response_type is wrong?
Closing... maybe no interop issues will emerge with processing formats 🤷