Use object for request #97
Conversation
marcoscaceres
requested review from
samuelgoto,
RByers and
timcappalli
as code owners
marcoscaceres
changed the title
|
What is the benefit of making the request an object? |
IIUC this proposal, I think developer use-ability of the API is significantly improved here by making this an object rather then a string. Requiring the developer to serialise their request in order to use this API, is a step we can avoid if we let the browser do it. I'd also point out that many Web API's including Web Crypto and Web Authn operate in this manner. |
|
It also gives browser nice security properties by allowing the request structure to the converted to WebIDL dictionaries and type checked. This means that only well-defined and known request structures ever get passed to the OS or to a Wallet - so handler applications need to do much less error checking and get some data assurances. |
|
I've sent a couple of patches to WebKit making these changes already. |
Thanks for the explanation. It confirms my concerns regarding this change. I'm concerned it will make rollout of protocols and protocol changes unnecessary complex without a compelling advantage. I don't think the browser needs to protect wallets from malformed requests, wallet implementations handle such requests today. They can continue to do so. And I think they will want, especially, if the alternative is to depend on browser changes before rolling out changes to the exchange protocol. I suggest we postpone this change after it is clear what the impact on the overall lifecycle management for protocol implementations is. It might make sense to involve more wallet implementors in the discussion. |
Compelling to who? I literally said that it's critical for security. You're concerned that you will need to secure the protocol? That's literally our (standards engineers) job.
It absolutely does. It would be extremely irresponsible if it didn't. No API on the web just blindly passes raw data to the OS (and specially not to third party apps, like wallets). That's burnt us browser vendors in the past. It's a non-stater from a security standpoint to suggest otherwise.
Right, but from an implementation perspective it's a non-starter. The wallet still gets the data, so I don't see what the problem is with the browser doing the security checks and type conversions? |
|
@tlodderstedt, let's approach this a different way. Which request format are you concerned about not being to represent as an object (or would require a string)? Also, if there is a request format that would require passing a secure string, then it's trivial to again add |
|
@marcoscaceres my concern is not about whether a protocol can be represented in JSON. That's trivial. I'm concerned about the fact you want to use WebIDL on top of the JSON structure to enforce structural conditions/constraints in the browser. To be more precisely, I'm concerned what this might mean for the overall ecosystem. Let's assume the DCP WG decides to introduce a new parameter in a OID4VP request. Today, there are two parties involved in implementing a new protocol feature, the verifier and the wallet. So a certain wallet ecosystem or just business partners can decide to support the new feature. What if the ecosystem uses the browser API? Would a request with the new feature be blocked by the browser? I think enforcing constraints on the content of the |
|
While I certainly prefer the developer ergonomics of an object over a string, the use of string in creating an extensibility point as @tlodderstedt describes was absolutely intentional on Chrome's part. As I said here, I personally believe it's important to enable this level of separation of concerns and freedom to move independently that Torsten is describing.
The URL string for custom scheme URL navigations, the data passed to the Web Share API, and simple file downloads are the most obvious counter-examples to me. Of course web sites (servers) and apps already communicate directly through the generic OS capability of a TCP socket. If we don't allow such flexibility here, I would expect wallets to continue preferring these other existing generic mechanisms instead of adopting our API. That said, I don't think anyone is arguing this needs to be "blind" (I am also opposed to request encryption which has been suggested). Browsers could still use heuristics to warn about known dangerous patterns (as we do, for example, when downloading files from the Internet before letting arbitrary applications open them), that's just details which are out of scope for any specification IMHO (we don't try to standardize our precise downloaded malware protection mechanisms for example). |
|
I’m still not seeing any actual use case for it being a string. Right now all the cited examples are have been JSON based. So, why not make it a string when we actually need it to be string, instead of some hypothetical future extension? It’s totally fine if Chrome wants to accept strings, but we should get consensus on this in the standard. It doesn’t appear we have that (or at worst, it should accept a DOMString or object, though again the ask for DOMString remains dubious at best). Just need to point out that web share heavily restricted what URLs can be shared (because it got hacked and ended up exposing user passwords): https://www.w3.org/TR/web-share/#validate-share-data that was literally the example I was thinking of where passing unbound data went terribly wrong. There’s every reason to fear that will happen again here, and there’s little chance I could take this to our security folks with a straight face to approve this being a DOMString. About WebIDL, there’s still no way we won’t be doing the conversion in WebKit. I understand the extensibility argument, but it’s absolutely not counter to any extensibility goals. |
i think these are great questions, and all pretty trivial to address. I’d be happy to work through these together. This doesn’t have any impact on this being an object though (or doing IDL conversions, if we plan extensions safely and correctly). |
at IIW? |
|
Yep! At IIW. Looking forward to discussing in person. Let’s grab a bunch of us and work together on cases. We could maybe do like a session on extensibility if we wanted to. |
|
sounds good to me |
I'd like to understand better, do you know of a write up anywhere of how this got hacked & exposed user passwords please? (I'd be very happy to join any session IIW sessions on this.) |
|
I think I can empathize with both sides of this discussion here, but I wanted to note that what Marcos is suggesting in using objects isn't, in practice, more or less restrictive than strings, unless you want to deviate from JSON. An object is NOT an opinionated WebIDL dictionary, meaning that you get the benefits of extensibility while having a firmer structure. So, unless we want to support requests that aren't JSON, then I wanted to note in this discussion that there is a middle ground between strings and dictionaries, which doesn't have the extensibility challenges that are being pointed out (again, within the constrain of opining on JSON - which seems reasonable, at least for OpenID4VP?). |
|
I can understand both points of views, but wanted to add that I do believe this statement created the biggest concerns:
Imho, the biggest concern from protocols building on top of this API would be that if the browser wants to be able to fully parse/understand the request, that would mean that you add another entity that needs to be updated when protocol changes occur. We have seen that it can take a lot of time until those changes propagate to the end-users for browsers. I do believe it would help the discussion to understand what exactly that statement would mean or how far the checks on the browser side would go - how much knowledge of the protocols would be necessary in the browser. My 2 cents on the security considerations: The user interaction would be happening on the wallet side (and it would be the task of the wallet to make sure the request is valid), so I do believe we should have a bit different security assumption than e.g., for the share-data API. That said, I can definitely understand the general thought of wanting to make sure that the browser protects their users to a certain extent - we just need to be careful in figuring out what that means (benefits & risks). |
…bject https://bugs.webkit.org/show_bug.cgi?id=272443 rdar://126244307 Reviewed by Abrar Rahman Protyasha. Changed IdentityRequestProvider's request member to an object. Spec change: WICG/digital-credentials#97 * LayoutTests/http/wpt/identity/identitycredentialscontainer-get-basics.https.html: * LayoutTests/http/wpt/identity/idl.https.html: * Source/WebCore/Modules/identity/IdentityRequestProvider.h: * Source/WebCore/Modules/identity/IdentityRequestProvider.idl: Canonical link: https://commits.webkit.org/277472@main
That doesn't have to be the case (e.g., this change landing within days in WebKit). But routing around browser's security and privacy processes and assurances because things "can take a lot of time" is not a reason to exclude browsers from participating in standardizing and facilitating the request/response structure - or any extension, to what are supposed to be "standardized" formats. Things take as long as they take because browser makers need to make sure what's being proposed/implemented is not going to end up with end-users getting hacked or attacked. The Web is an extremely hostile ecosystem: browsers take extra care to secure things for end-users, and I don't think any browser maker makes any apologies for making sure things are done right, and users are kept safe, and that meaning things sometimes take a little longer. The way to get things in browsers quickly is to work directly with implementers and the rest of the community. Excluding any party or trying to side step the process is what leads to delays.
That's correct. But that doesn't absolve the browser from participating in trying to assure that what's being passed is not malicious: it's the role of the browser to also make sure no weirdness reaches the wallet in the first place. The browsers role is to be the first line of defense against malicious sites trying to attack the OS or Wallets. We filter out any garbage, intentional or not, being passed to the OS or Wallet. Similarly, we would protect the RP, as much as possible, from receiving garbage from a Wallet. Protection goes both ways.
Yeah, it's basically that. The working assumption is that the wallets are not secure and will be attacked because they contain high-value information. The other assumption its that there is no guarantee that the wallet is doing validity checks (or doing them properly) - hence limiting the potential for things going wrong. The Wallets are operating outside the browser's sandbox, hence could (through an extended request format) potentially be confused or otherwise coerced to perform unintended actions by a malicious RP. |
|
Thanks for the links Marcos! I realise we're already getting off topic for this issue... but if I may ask another question: I think the intention is to create a .idl file that represents the OpenID for Verifiable Credentials request? Is there any precedence for how that should be created and who owns it? e.g. would it live in WICG, or is it something that should become part of (or at least live alongside) the OID4VP specification? |
Is it actually the OS that might do this? (I'm trying to figure out if these protections also apply when the verifier is a native app.) |
Yes and no: obviously, the OS doesn't speak JavaScript and the entry point isn't the browser API. Eventually, of course, the browser calls into the OS - and yes, OSs impose their own protections. But the expectation is that the OS gets objects in a structure it understands (i.e., not JS objects or DOMStrings), and with a best effort by the entrusted caller (the browser) to not send it garbage, or try to attack it, or confuse it. Getting consistent behavior before these structures hit the OS is also critical for interop (e.g., for error handling/reporting). |
|
There are two discussions tied up here that have subtle differences, and I don't think are perfectly tied together. First, what should this field be- an Object, DOMString, or either (Object or DOMString)? Second, should the browser have a say in what request structures can be passed? I think most would agree the ergonomics of an object argument are better for protocols we know about and can define webidl for. So I see no reason we shouldn't at least have If the browser knows all request formats, we can provide guarantees to the OS and Wallet that the request is well formed. I agree that while the wallets do and will continue having to handle requests as untrusted input, however defense in depth can be useful, especially when you can eliminate classes of vulnerabilities. The extent of that benefit is dependent on the power of as-of-yet-uninvented protocols, I fear. However, the point stands that it is one more party to the addition of a field or a protocol to the supported ecosystem. Whether or not this is a negative doesn't have agreement. Wallets certainly don't like it, but as a browser, I'd like to know what requests are being made so that I can provide context to the request such that I feel comfortable passing it on to a wallet that wouldn't pass EUID audits and abandoned user agency. Rick's point about being able to introspect on requests mitigates this some, but having consistent behaviors on what constitutes harmful patterns seems desirable. Still, I don't know what the answer should be there. Even if we say the browser has no say in the request structure, we can still make it an Object! Just set the requirements for updating the IDL low enough that anyone can do it and it is far from the hardest part of updating the protocol. Webidl-only changes are very easy in Gecko when the webidl is already in a spec. We do lose backward compatibility with this on older browser versions however. Another interesting middle-ground is making it a |
|
I just wanted to chime in again and clarify my approval and support of this PR is more from the perspective that shifting from a string to an object as the format of the request is much better from a developer experience perspective and that is why I am supportive. What the browser validates from the request is a seperate consideration as that could apply whether the request is passed as a string or object and so should be discussed separately. IMO unless there are objections that argue the request being passed as a string rather then an object is better from a developer experience, there wouldn't appear to be a reason to hold up this PR? |
|
Yeah, just taking what @tplooker said, it's unfortunate to see in places people needlessly doing: {
request: JSON.stringify(object)
}That's why I also think So, again, is there a format that requires a |
|
Agreed that an object is fine. Probably the place we should be having this particular debate isn't this PR, but a PR on an explainer that lays out the important design properties. I'll try to get something up for that ASAP. |
|
Right, let's put aside the post-ingestion WebIDL conversion for now. It may be something we build up once we actually have a request format. We still need to understand what extensibility there is going to be, how are those standardized, how quickly do they change, etc. |
|
I opened #100 to hopefully progress the discussion around request validation seperately. |
|
Can we remove the data response type changes and keep this PR exclusively about the request type change, so that we can keep this PR as narrow as possible? Otherwise, object for requests also LGTM. |
marcoscaceres
changed the title
|
Thanks everyone! I'll spin up a separate issue on managing extensibility. @samuelgoto, here's the other PR #101 (please ✅) |
|
Filed #102 |
|
FWIW, just as an update, this has now been implemented in Chrome: https://chromium-review.googlesource.com/c/chromium/src/+/5546326 |
Closes #75
The following tasks have been completed:
Implementation commitment:
Preview | Diff