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/discussion: non-extractable CryptoKey output from the prf extension #1895

Closed
stephen opened this issue Jun 5, 2023 · 8 comments
Closed
Assignees
Labels

Comments

@stephen
Copy link

stephen commented Jun 5, 2023

Background

The prf extension creates a path for web applications to support end-to-end encryption via hardware authenticators in the browser, in conjunction with WebCrypto. Encrypting Data in the Browser Using WebAuthn presents some sample code for how this can be achieved in Chrome Canary today.

From reading discussion around prf, E2EE appears to be one of the core use cases, e.g. for password managers or any applications written with a focus on user privacy. (In my case, I'm developing a PWA that does all user data handling in the browser with encrypted backups.)

Problem

When crossing the webauthn <> webcrypto api boundary, we have to expose the key material to the js runtime. From step 2.1 of the article:

const inputKeyMaterial = new Uint8Array(auth1ExtensionResults.prf.results.first);

const keyDerivationKey = await crypto.subtle.importKey(
  "raw",
  inputKeyMaterial,
  "HKDF",
  false, // n.b. Extractable flag from here on is false, but the raw key material is in memory already...
  ["deriveKey"],
);

For this use case, the prf output is ideally never exposed to the js context.

Proposal

I propose adding an option to the prf extension that supports returning an non-extractable CryptoKey:

extensions: {
  prf: {
    eval: {
      first: firstSalt,
      asCryptoKey: true, // New optional field on `AuthenticationExtensionsPRFValues`
    },
  },
},

If asCryptoKey is set true, then the result object returns a non-extractable CryptoKey instead of a BufferSource. The resultant key should match the output of the importKey call above (i.e. using HKDF).

Why

To get concrete about why this change can help, we can think about some attack vectors. Note that "key material" below refers to the prf extension results:

  • Attacker with js code execution (i.e. from XSS) on the page
    • If an attacker can do code execution in your js context, it's game over already (i.e. attacker could also just exfiltrate data later on, etc). Protecting the key material can limit the blast radius of exposure (i.e. decrypting future/past content offline)
  • Attacker with malicious browser extension code execution
    • Again, attacker can't get key material from js, but that may be moot if the user's machine is likely compromised.
  • Buggy code
    • Marking key material unexportable can protect from hidden or obfuscated exposure, e.g. if the developer accidentally logs keys somewhere

Other related issues / alternatives considered

Generalized crypto operations

I've seen the discussion for general crypto operations which seems relevant but much broader and nebulous. I'm proposing a smaller, concrete change to the prf extension spec.

The scope of webauthn

I've seen the previous discussion (#1481) on the scope of the webauthn spec and understand the desire to stay out of webcrypto's territory. At the same time, I do also see the desire to make correct, secure implementations easy to do within the spec. It seems like the only way to keep key material out of the js context is to have the webauthn/the prf extension do the webcrypto call behind the scenes.

Allow arbitrary CryptoKey derivations.

Instead of only allowing HKDF, we could allow passing arbitrary options for use as e.g. RSA or ECDH. This feels out of scope and ties the webauthn api too strongly to the webcrypto one.

Notably, I believe that only outputting HKDF doesn't limit what kinds of constructions folks could implement - they could call deriveKey again from this key, etc.

Don't use prf for this use case

Perhaps this is simply a mismatch in use case, but then what is prf for? I haven't seen much discussion around other use cases. A better fit may be to expose hardware keys in webcrypto and actually decrypt/encrypt bytes on the hardware itself so that keys never leave the HSM, though it's unclear to me if those operations are reasonably performant...

Use the api as-is and do your best to secure the js context

Seems okay? But not as satisfying.

@stephen stephen changed the title Proposal/discussion: getting a non-extractable CryptoKey result from the prf extension Proposal/discussion: non-extractable CryptoKey output from the prf extension Jun 5, 2023
@MasterKale
Copy link
Contributor

I don't know that this would achieve the desired effect. The options could be intercepted by malicious code before the call to .get() and asCryptoKey could then be flipped to false without the RP knowing any better. Then at that point this protection is bypassed and we're back to where we are now :(

@agl
Copy link
Contributor

agl commented Jun 5, 2023

(The hash prefix in the PRF extension was designed to be able to meet needs like this. I.e. opaque keys could use a different prefix to ensure domain separation. I don't think the Chromium team has time to look into this sort of thing for quite a while, but PRF was designed to make it possible to do.)

@stephen
Copy link
Author

stephen commented Jun 5, 2023

I don't know that this would achieve the desired effect. The options could be intercepted by malicious code before the call to .get() and asCryptoKey could then be flipped to false without the RP knowing any better. Then at that point this protection is bypassed and we're back to where we are now :(

aha, fair point - i suppose in the case where there's malicious code in the js context, it's pretty much game over. It might be useful for the "avoid bugs" case in not inadvertently spilling the bytes somewhere? that seems about as worthwhile (or not) as having the extractable flag in webcrypto perhaps?

(The hash prefix in the PRF extension was designed to be able to meet needs like this. I.e. opaque keys could use a different prefix to ensure domain separation. I don't think the Chromium team has time to look into this sort of thing for quite a while, but PRF was designed to make it possible to do.)

sorry - I didn't quite understand this. could you say more about what this means? I didn't see mention of this in the prf extension section of the draft.

@emlun
Copy link
Member

emlun commented Jun 7, 2023

I think what @agl is saying is that because the PRF extension adds a prefix to the input salt, a future update of the PRF extension could add a new feature that uses a different prefix and returns a CryptoKey instead of a raw ByteBuffer. This would prevent the downgrade attack @MasterKale describes: if a script injection disables the CryptoKey setting, then the resulting ByteBuffer would contain a different result than the intended CryptoKey result, because the two are domain-separated by using different prefixes. (But @MasterKale is right that such a downgrade attack would work if there is no such domain separation.)

@nadalin nadalin added the @Risk Items that are at risk for L3 label Jun 27, 2023
@MasterKale
Copy link
Contributor

Hearing that it's possible to make this happen, then I'm all for the addition of something like asCryptoKey 🎉

The one thing I'd suggest is to move asCryptoKey up a level, out of eval, like this:

extensions: {
  prf: {
    eval: {
      first: firstSalt,
    },
    asCryptoKey: true,
  },
},

@emlun
Copy link
Member

emlun commented Aug 24, 2023

I've opened PRs #1945 and #1946 with two possible designs for this. I have some unfortunate news, though: as I was sketching out some ways one might to use this, I concluded that there still seems to be no way to turn an unextractable PRF output into a never-extractable asymmetric private key.

The PRF output is always 32 random bytes, so the only key types it can be directly imported as is AES and HMAC/HKDF keys. HKDF can of course be used to derive new keys, and that's great, but in WebCrypto it is currently not possible to use HKDF to derive asymmetric key pairs. What you can do instead is to generate an unrelated keypair and wrap its private key with a key derived via HKDF - but wrapKey() requires the private key to be created with exportable: true, so there will still be some nonzero duration where the private key is vulnerable. Once you have the wrapped private key you can unwrap it with exportable: false, so that's good, but there's still that critical moment between key generation and the first wrapKey where the private key could be intercepted.

I also found that it's probably not even difficult for a malicious script to intercept the private key - you can just re-assign window.crypto.subtle.generateKey with a function that exfiltrates the private key result before passing it through.

All in all, this still makes it challenging to implement multi-recipient encryption with PRF-derived keys. It's fine as long as you have only one PRF credential, but if you want encrypted data to be decryptable by more than one PRF credential for redundancy, you have to either

  • use symmetric-only encryption, which requires presentation of every PRF credential whenever something is to be encrypted, or
  • accept that each credential's corresponding ECDH (or equivalent) private key is briefly exposed as an exportable key.

It would be nice if WebCrypto could be extended with the ability to use HKDF to derive EC and RSA private keys, that would neatly solve all of these problems.

@MasterKale
Copy link
Contributor

I left this comment in #1945 but figured I'd repost here to better surface it for others following this issue:

WG meeting @ TPAC 9/12: We've spent a lot of time trying to figure out what direction to go with this. However we've not yet identified sufficient interest in a discrete use case (e.g. provider-centric need for prf output) to say that this "swiss army knife" PR (vs the more limited #1946 that got closed) should be pursued any further.

Leaving this open for now, but unless a specific use case for prf can be identified that cannot be served by this extension in its current form then we should probably close it at the next meeting.

@agl
Copy link
Contributor

agl commented Jan 31, 2024

This discussion continued in #1945 which was closed. Thus also closing this bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants