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

Add support to store wallet in browser storage without exporting the key #1838

Open
augustin-cheron opened this issue Nov 12, 2023 · 3 comments

Comments

@augustin-cheron
Copy link

The current implementation of the ed25519 polyfill does not support storing keys in IndexedDB without exporting them. As per the W3C Web Crypto API, a CryptoKey should implement the structured clone algorithm, enabling direct copying without exposing the private key to JavaScript.

To address this, I propose to add a CryptoKey directly into the generated keypair instead of using a memory store.

crypto.subtle.generateKey = async (algorithm, extractable, keyUsages) => {
    if (algorithm !== 'Ed25519') {
        return await originalGenerateKey(algorithm, extractable, keyUsages);
    }
    let rand = crypto.getRandomValues(new Uint8Array(32));
    // Opt for `AES-GCM` key as `HKDF` does not support exporting the key for later use
    let proxyKey = await crypto.subtle.importKey('raw', rand, 'AES-GCM', true, ['wrapKey']);

    const privateKey = {
        algorithm: Object.freeze({ name: 'Ed25519' }),
        extractable,
        type: 'private',
        usages: Object.freeze(['sign']),
        _proxyKey: proxyKey,
    };
    const publicKey = {
        algorithm: Object.freeze({ name: 'Ed25519' }),
        extractable: true,
        type: 'public',
        usages: Object.freeze(['verify']),
        _proxyKey: proxyKey,
    };
    return Object.freeze({
        privateKey: Object.freeze(privateKey),
        publicKey: Object.freeze(publicKey),
    });
}

With this approach, we can identify a polyfill key by checking if _proxyKey exists. Here's an example of how the sign function could be implemented:

crypto.subtle.sign = async (algorithm, key, data) => {
    if (!key._proxyKey) {
        return await originalSign(algorithm, key, data);
    }
    let edPrivate = await crypto.subtle.exportKey('raw', key._proxyKey);
    return ed25519.sign(data, new Uint8Array(edPrivate));
}

This change allow to create or load wallet from index db like this

 function loadKeySecure() {
  return new Promise((resolve, reject) => {
    let request = indexedDB.open('solana-keystore', 1);
    // create keystore if needed
    request.onupgradeneeded = () => {
      request.result.createObjectStore('solana-keystore', {
        keyPath: 'id'
      });
    };

    request.onsuccess = () => {
      var db = request.result;
      var tx = db.transaction('solana-keystore', 'readonly');
      tx.oncomplete = db.close;
      tx.onerror = reject;

      var store = tx.objectStore('solana-keystore');
      var readKey = store.get(1);
      readKey.onerror = console.error;
      readKey.onsuccess = () => {
        if (readKey.result) {
          resolve(readKey.result.keys);
        } else {
          generateKeyPair().then((key) => {
            var tx2 = db.transaction('solana-keystore', 'readwrite');
            tx2.oncomplete = db.close;
            tx2.onerror = reject;
            tx2.objectStore('solana-keystore').put({id: 1, keys: key});
            resolve(key);
          });
        }
      };
    };
  });
}
@steveluscher
Copy link
Collaborator

Oooh. Interesting point! At minimum, we must point this out in the README (#1911).

I love your proxy key solution, but implementing it would pierced the veil of secrecy between the API and the actual bytes of the private key. Anyone who got a reference to the polyfilled key could just exportKey() the private key bytes, which is not true today, and not true of real CryptoKeys.

Until there's support for Ed25519 keys everywhere, I think wherever there's an application where you need to store the keys locally you'll just have to do it in the conventional way (eg. generating your own and storing it, using importPrivateKeyFromBytes() when you need to use it with web3.js (#1813).

@augustin-cheron
Copy link
Author

Thank you for having taken the time to review this.
I loved the fact that you used the Web Crypto API in this new version of web3.js.
I can use my own polyfill and it just works out of the box with the rest of the library.

How about implementing a unique flag at the start of the proxy key, which would prevent key extraction in the polyfilled exportKey function whenever this flag is detected?

With a big enough KEY_HEADER (32 bytes?), real-world key collisions should not be an issue.
The only issue I can foresee is if a user can bypass the polyfill and call the original exportKey function (which will happen when they remove the polyfill from their dependencies).

let rand = crypto.getRandomValues(new Uint8Array(32));
let header = KEY_HEADER;
let key = new Uint8Array(header.length + rand.length);
key.set(header);
key.set(rand,header.length);

let proxyKey = await crypto.subtle.importKey('raw', key, 'AES-GCM', true, ['wrapKey']);
crypto.subtle.exportKey = async (format, key) => {
    let pk = await originalExportKey('raw', key);
    if(pk.slice(0, KEY_HEADER.length) == KEY_HEADER) {
        throw new Error('can not export key');
    }
   // ...
}

@steveluscher
Copy link
Collaborator

The only issue I can foresee is if a user can bypass the polyfill and call the original exportKey function…

Yeah, exactly. And so can an attacker, by grabbing a fresh copy of SubtleCrypto from a new iframe they create.

const frame = document.createElement('iframe');
document.body.appendChild(frame);
const stolenKey = await frame.contentWindow.crypto.subtle.exportKey('raw', proxyKey);

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

No branches or pull requests

2 participants