solana-labs / solana-web3.js

Solana JavaScript SDK
https://solana-labs.github.io/solana-web3.js
MIT License
2.21k stars 877 forks source link

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

Closed augustin-cheron closed 3 weeks ago

augustin-cheron commented 1 year ago

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 commented 11 months ago

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 commented 11 months ago

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 commented 11 months ago

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);
steveluscher commented 3 weeks ago

We've pointed out this limitation in the README (#1911).

github-actions[bot] commented 2 weeks ago

Because there has been no activity on this issue for 7 days since it was closed, it has been automatically locked. Please open a new issue if it requires a follow up.