w3c / webextensions

Charter and administrivia for the WebExtensions Community Group (WECG)
Other
576 stars 50 forks source link

Proposal: `browser.secureStorage` API #154

Open oliverdunk opened 2 years ago

oliverdunk commented 2 years ago

Summary

At the 2022-01-06 meeting, I mentioned 1Password's interest in proposing a new API to store data in hardware backed secure storage. We now have an initial proposal which is available here: https://docs.google.com/document/d/11ERMp8ErCF2_l-V1YSE73UKKZeGoBC3QeMUoCTACElc/edit?usp=sharing

I'm opening this issue to provide a place for discussion on the API.

Progress

I've been slowly reaching out to those representing browser vendors at WECG, as well as individual engineers, to solicit feedback on the API. I think things are sounding promising but I will let them comment here instead of trying to speak on their behalf.

bershanskiy commented 2 years ago

This proposal seems interesting, I have a few questions:

Below I propose a slightly different approach based on extending Web Authentication API SubtleCrypto interfaces which have exportable attribute with a hardwareBacked attribute.

bershanskiy commented 2 years ago

Extending Web Crypto API SubtleCrypto interfaces with a boolean hardwareBacked attribute

Motivation

SubtleCrypto is a mature and widely supported API accessible in a wide range of contexts (regular windows, extension frames, extension MV2 background pages, extension MV3 service workers). Simply extending it would make this feature easily adoptable by existing users. Also, this API change is rather small hopefully making it simple to implement.

Proposal

SubtleCrypto represents keys as CryptoKey object which have boolean attribute CryptoKey.extractable. Key is "extractable" if and only if it was marked as extractable during generation or import (generateKey(), deriveKey(), importKey(), or unwrapKey()) and denotes whether the key supports exportKey() and wrapKey(). We might as well create a new attribute hardwareBacked which denotes whether or not key is stored in a hardware-based storage. Of course, hardwareBacked being true implies extractable being false.

Feature detection

'hardwareBacked' in CryptoKey.prototype should be true if this feature is supported by current hardware/software combo and enabled in the current context, false otherwise.

Extension for generateKey(), deriveKey(), importKey(), or unwrapKey()
Input parameter dictionary gets a new optional boolean attribute hardwareBacked. The following combinations of hardwareBacked and extractable are possible: extractable hardwareBacked Result
false false Same as before
true false Same as before
false true Key is hardware-backed, or Error is thrown if storage of hardware-backed keys is impossible.
true true Error
Extension to CryptoKey

CryptoKey gets a new attribute hardwareBacked matching hardwareBacked value passed into generateKey(), deriveKey(), importKey(), or unwrapKey().

Remaining questions

Jack-Works commented 2 years ago

Hi please be aware of https://www.w3.org/TR/webauthn-2/#sctn-large-blob-extension

Large blob storage extension (largeBlob) (proposal)

oliverdunk commented 2 years ago

Hey @bershanskiy 👋 Thanks for the questions!

is it important to attach this API to browser instead of a regular window or even worker scope? I feel like regular sites might want to store secrets in hardware-backed storage as well (like they do in Web Crypto API).

One of my goals with this proposal has been to keep the scope limited, with the hope of seeing some implementations in the short to medium term (3-6 months). It would be a real game changer for us at 1Password, and I believe many other extensions, so I'm keen to see it out there. I have no problem with making this available to the larger web - in fact I'd love it - but that comes with additional formalities which do (rightly) slow things down. I think my preferred approach would be to start in browser extensions where we can iterate quickly and consider a web standard further down the line.

do these functions have to be user-activated (e.g., to prevent annoying biometric authentication popups)?

I don't think so. Extensions are already fairly privileged installs and imply some level of user consent. In the API overview doc for the new chrome.action.openPopup API (http://crbug.com/1245093), it says that they do not intend mitigations for "attacks of annoyance" because "extensions can already do far worse than this with the tabs API and window creation". I'd argue that applies here too.

Another good example is the permissions request chip coming to Chrome: https://developer.chrome.com/blog/permissions-chip/. There are exceptions if "the permission is deemed essential and generally non-spammy" and I would argue biometrics fall under this.

Is it important for your use case that biometric authentication and key retrieval happen in one call to User Agent?

I think yes. While splitting these APIs may be useful, and we would likely still benefit from them in that case, combining them has two benefits:

the user stores a bunch of unimportant passwords into 1Password, and disables biometric authentication for convenience. Some time later, the user decides to store something important and wants to enable biometrics. What do you do? Export all passwords from existing records and then re-import them again?

In our use case, we would store the key used to encrypt data in secureStorage, and the data itself would live elsewhere. Removing the secureStorage key would prevent us from accessing the key through it but we could still derive the key from the long account password a user types and this would allow us to decrypt the extension data.

They initially allowed to use both face and fingerprint, but then decided to forbid face and use only fingerprint. Would you prefer to make the upgrade seamless or would you not mind nagging user to input biometrics one extra time?

Good question. I think this falls in to the next point but we could potentially allow removing authentication methods without a re-prompt.

what keys are the records keyed on? Is it just id or id and authentication? For example, if I do this

Great question. I think this is good feedback and something we should document. I don't think authentication should be part of the key and we should either throw on writing a duplicate ID or overwrite the data like you suggested.

Will reply to your SubtleCrypto proposal in a second comment.

oliverdunk commented 2 years ago

On SubtleCrypto @bershanskiy - are you thinking that we use these existing types with Proposal 1, or is this a full counter-proposal? I think the types make sense and could be re-used but I'm unclear how keys would be stored between browser sessions and where biometrics comes in to the mix.

Worth quickly noting - while I think Proposal 1 is still worth discussing, the overwhelming feedback from browser vendors so far has been that they prefer Proposal 2. It avoids keys which makes it harder for users to bite themselves and keeps things simple so the API is applicable to more extensions.

oliverdunk commented 2 years ago

Hi please be aware of https://www.w3.org/TR/webauthn-2/#sctn-large-blob-extension

Large blob storage extension (largeBlob) (proposal)

Thanks for mentioning this Jack! It's actually touched on under the "Could a web API like WebAuthn cater for this use case?" section of the proposal.

One of the biggest issues here is that some browsers don't support platform authenticators (Firefox on macOS) and those that do don't support largeBlob within it (Chrome for example). I have spoken to some people involved in WebAuthn to see if there was interest in solving these problems, but it didn't sound like the work was close and the overwhelming advice so far has been that an extension API seems like the better solution. We'll be able to build something better catered to the use case and easier for developers to use.

bershanskiy commented 2 years ago

@oliverdunk thanks for the prompt response!

I forgot to ask a few more questions:

One of my goals with this proposal has been to keep the scope limited, with the hope of seeing some implementations in the short to medium term (3-6 months).

That makes sense.

do these functions have to be user-activated (e.g., to prevent annoying biometric authentication popups)?

I don't think so.

I agree with this. Also, user activation would be impossible if API is called from the background context. Also, chrome.permissions.request() requires user activation making it more annoying to use without much benefit for the user.

we could potentially allow removing authentication methods without a re-prompt.

Makes sense.

oliverdunk commented 2 years ago

thanks for the prompt response!

No problem @bershanskiy! Enjoy chatting about this.

This API does not provide a way to remove old data or estimate the amount of stored data and remaining free capacity.

That's fair. We definitely need a delete call and if there are storage limits, providing a way to access them makes sense. For this (and all of the other feedback that needs changes) I'll try to do a pass soon or we may be able to collaborate if the proposal is ever moved to a GitHub repo.

Should browser.secureStorage be exposed to content scripts?

I think I agree with your no. We don't have a use case for this in content scripts and I don't expect many others would. It seems like an unnecessary risk to expose it there.

Can this API support unauthenticated reads?

Yes, absolutely! I very briefly cover this in the proposal by saying "optionally protected by biometrics", but I could probably make that clearer. There's a bit in the FAQ as well.

Does this API need new permission in the manifest?

Another great question, and one I don't have a great answer for. Since most APIs have a corresponding permission it seems sensible. At the same time, one thing we've found at 1Password is that requesting new permissions is often best avoided (in the worst case, an extension can automatically disable itself on update). Using an existing permission or allowing it to be passively added would be nice.

oliverdunk commented 2 years ago

@xeenon, I wanted to quickly address the question you asked at the last meeting, on if being able to access this storage from native code is important. It was an interesting one! I discussed it internally at 1Password and we still feel the same as I did in the meeting - it may be interesting and allow us to simplify the extension's setup on first install, but it isn't a requirement.

bradcush commented 2 years ago

Does this API need new permission in the manifest?

Another great question, and one I don't have a great answer for. Since most APIs have a corresponding permission it seems sensible. At the same time, one thing we've found at 1Password is that requesting new permissions is often best avoided (in the worst case, an extension can automatically disable itself on update). Using an existing permission or allowing it to be passively added would be nice.

I would advocate for not requiring a new permission here. As you said Oliver, requesting new permissions as it can result in disabled extensions in the worst case. Here I think the storage permission can work well. I'm just worried about how different the API can be where everything previously under browser.storage.<area> maps quite closely to each other. (Thinking browser.storage.secure in that case as well)

More of a reason to potentially align them as much as possible where it makes sense. Regarding the new In-memory storage API that was recently added, that also doesn't require an additional permission and is granted as part of storage which can help make the case for this to be the same. Depending on how limits are treated, as Proposal#2 is favored by browser vendors, the unlimitedStorage permission could also be leveraged to go beyond any default limit.

oliverdunk commented 2 years ago

With https://github.com/w3c/webextensions/pull/167 merged, I think we should move towards opening PRs when we can and opening issues if we need more discussion. I'm going to leave this one open as an overall tracking issue but I'll try to break out some smaller issues based on the feedback here.