Skip to content
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

Proposal: browser.secureStorage API #154

Open
oliverdunk opened this issue Jan 27, 2022 · 11 comments
Open

Proposal: browser.secureStorage API #154

oliverdunk opened this issue Jan 27, 2022 · 11 comments
Labels
proposal Proposal for a change or new feature supportive: safari Supportive from Safari topic: secure storage Related to the secureStorage API proposal

Comments

@oliverdunk
Copy link
Member

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

  • 2022-01-06: First mention of API in a WECG meeting.
  • 2022-01-06: Initial draft of proposal shared with WECG.
  • 2022-01-27: Issue filed in WECG repo.

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
Copy link
Member

bershanskiy commented Feb 4, 2022

This proposal seems interesting, I have a few 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).
  • do these functions have to be user-activated (e.g., to prevent annoying biometric authentication popups)?
    • I'm not sure if the requirement for user activation would make these functions unavailable in workers.
  • Is it important for your use case that biometric authentication and key retrieval happen in one call to User Agent?
    • It might be more convenient to have two separate APIs, one for biometrics and another just for retrieving hardware-backed secrets. In this case, your code first creates an authentication prompt and then upon success retrieves the secret. I realize that this seemingly relaxes security guarantees because your code could accidentally (or because of XSS) retrieve secrets without biometric authentication, but I'm not sure this relaxation is significant. After all, the total number of LOC in either design would be small enough to audit and ensure you always call biometric authentication. Also, if someone has XSSed you, then they might as well just prompt the user for biometrics and the user would not know any better.
    • Separating these two APIs might prove more convenient for consumers.
      • E.g., 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?
      • Same scenario, but this time we are in a big company and IT department mandated this change. 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?
  • what keys are the records keyed on? Is it just id or id and authentication? For example, if I do this
    browser.secureStorage.store({
     id: "example"
     authentication: ["BIOMETRY_FACE", "BIOMETRY_FINGERPRINT"],
     data: JSON.stringify({ password: "SecurePass" })
    });
    browser.secureStorage.store({
     id: "example"
     data: JSON.stringify({ password: "InsecurePass" })
    });
    
    Does the second call throw because I downgraded security? Do I end up with two "example" records? Intuitively for me, there should ever be only one record equal to the last write (completely disregarding all earlier writes).

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

@bershanskiy
Copy link
Member

bershanskiy commented Feb 4, 2022

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

  • What should be the default value of hardwareBacked? I think it should it be false to match existing users and in case user decides to swap out TPM hardware. Also, hardwareBacked probably should prevent browsers from syncing keys into the cloud?
  • hardwareBacked variable name is completely arbitrary, feel free to propose a better one if you have one.

@Jack-Works
Copy link

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

Large blob storage extension (largeBlob) (proposal)

@oliverdunk
Copy link
Member Author

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:

  • Some operating systems like macOS can enforce biometrics to access data at the operating system level. Being able to make use of these APIs would provide some additional security.
  • Similar to above, it keeps the scope small and makes it more likely we will be able to see implementations of this API in the short term.

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
Copy link
Member Author

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
Copy link
Member Author

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
Copy link
Member

@oliverdunk thanks for the prompt response!

I forgot to ask a few more questions:

  • This API does not provide a way to remove old data or estimate the amount of stored data and remaining free capacity.
    Perhaps, this API could use something similar to storage.<area>.getBytesInUse(), storage.<area>.remove(), and storage.<area>.clear().
  • Should browser.secureStorage be exposed to content scripts?
    Intuitively, I think that secureStorage is too powerful and should be accessible only from the same contexts as chrome.tabs and similar APIs. Here is a more detailed explanation.
  • Can this API support unauthenticated reads?
    I know developers of at least two extensions who probably would use this API without biometric authentication, if that was possible. It would be nice to store some keys, API tokens, session identifiers, etc. in a more secure equivalent of chrome.storage, but requiring biometrics or passwords before every action would be a nuisance.
  • Can extension retrieve multiple pieces of data at once?
    For reference, chrome.storage.<area>.get() accepts a single key or an array of keys.
  • Does this API need new permission in the manifest?

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
Copy link
Member Author

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
Copy link
Member Author

@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
Copy link

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
Copy link
Member Author

With #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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal Proposal for a change or new feature supportive: safari Supportive from Safari topic: secure storage Related to the secureStorage API proposal
Projects
None yet
Development

No branches or pull requests

6 participants