waku-org / nwaku

Waku node and protocol.
Other
200 stars 52 forks source link

feat(rln): How to securely store RLN credentials cross-client #1278

Closed s1fr0 closed 1 year ago

s1fr0 commented 1 year ago

Background

In

is raised multiple times the question of standardizing the RLN credential format and securely persist it.

Details

nwaku, js-waku and go-waku generates RLN credentials through zerokit FFI/WASM APIs.

While standardization of a format for RLN credentials can be pursued in https://github.com/vacp2p/rfc/issues/543 (a tentative solution was proposed here), their secure persistence and cross-client usage (e.g. decryption) is non-trivial.

As suggested here, nwaku comes with a nimbus keystore implementation that allows to securely encrypt messages (in fact, keys) using a password/mnemonic sentence. However, to allow cross-client usage of encrypted credentials, the underlying employed primitives (scrypt, pbkdf, sha256, aes-128-ctr) should be ported/implemented in js-waku and go-waku too.

On top of this, nimbus keystores don't seem to allow enough flexibility on fields structure to reach something similar to what proposed here and come with primitives that should probably be replaced at least to provide 256 bits of security (I would personally use Argon2 + ChaChaPoly).

Given this, a possible solution could be to implement a WASM-friendly rust module that we can interface, similarly as zerokit, with FFI/WASM implementing all the logic on our custom standardized RLN credential format (most probably JSON with fields content encrypted), so that we avoid multiple implementations of the same crypto primitives.

Nevertheless, if implementation/usage of a browser extension is envisioned for js-rln in order to safely store credentials (e.g., crypt-keeper), then we might opt for separate implementations in nwaku and go-waku of RLN credentials' field encryption, since both already support Noise primitives. In any case, import/export of credentials should be supported too by the browser extension and crypt-keeper doesn't seem to have such feature yet.

Pro and cons

Acceptance criteria

@staheri14 @rymnc @fryorcraken @richard-ramos @oskarth

richard-ramos commented 1 year ago

I don't see an impediment in using an external module for writing/reading external credentials, but IMO the effort of implementing this idea should be compared against the effort of using existing libraries in each language (like it does in go, and nim after extracting it from nimbus)

In Prysm (an eth2 client), they are using https://github.com/wealdtech/go-eth2-wallet-encryptor-keystorev4 https://github.com/prysmaticlabs/prysm/blob/d077483577bc8fdc658940adf71d8d11dfa5949b/tools/keystores/main.go .

For JS, Chainsafe implemented https://www.npmjs.com/package/@chainsafe/bls-keystore which is compatible with EIP2335 and used in Lodestar, and according to its description, it can be used on the browser.

--

in go-waku, while the chat2 example stores the credentials in plaintext (for compatibility across implementations), the wakunode is using go-ethereum keystore to write the credentials https://github.com/status-im/go-waku/blob/master/waku/rln-credentials.go#L50, encrypted with a password that can be passed via the --key-password flag. For reference, this is the go-ethereum implementation: https://github.com/ethereum/go-ethereum/tree/master/accounts/keystore

s1fr0 commented 1 year ago

The encryptDataV3 API used in go-waku encrypts data in a way that doesn't seem to be EIP-2335 compliant (I cannot indeed find its specification there), but formats are however similar:

{"cipher":"aes-128-ctr",
 "ciphertext":"xxxxxxx",
 "cipherparams": {
    "iv":"b674f28ff45d778f2dafe1a294542c21"
 },
 "kdf":"scrypt",
 "kdfparams: { 
   "dklen":32,
   "n":262144,
   "p":1,
   "r":8,
   "salt":"827a1bb45671027d26c314a2b4255bd01436fa9398a273e1e7bf530e2b48675a"
 },
 "mac":"d207eb616402c676e332ea7b8daccb3d2589cddd6c9d1b8ae11771617abb7143"
}

vs.

nimbus-eth/EIP-2335:

{
    "crypto": {
        "kdf": {
            "function": "scrypt",
            "params": {
                "dklen": 32,
                "n": 262144,
                "p": 1,
                "r": 8,
                "salt": "827a1bb45671027d26c314a2b4255bd01436fa9398a273e1e7bf530e2b48675a"
            },
            "message": ""
        },
        "checksum": {
            "function": "sha256",
            "params": {},
            "message": "d207eb616402c676e332ea7b8daccb3d2589cddd6c9d1b8ae11771617abb7143"
        },
        "cipher": {
            "function": "aes-128-ctr",
            "params": {
                "iv": "b674f28ff45d778f2dafe1a294542c21"
            },
            "message": "xxxxxxxxx"
        }
    },
    "description": "",
    "pubkey": "000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000",
    "path": "m/12381/60/3141592653/589793238",
    "uuid": "1d85ae20-35c5-4611-98e8-aa14a633906f",
    "version": 4
  }

In encryptDataV3 there is a field called mac, but in fact is just the keccak256 of the right 16 bytes of the derived key https://github.com/ethereum/go-ethereum/blob/b9ba6f6e4d86d0ee86c63e8f4552e233fe0450aa/accounts/keystore/passphrase.go#L159. It should be equivalent to the EIP field checksum's message: https://github.com/status-im/nimbus-eth2/blob/c585b0a5b1ae4d55af38ad7f4715ad455e791552/beacon_chain/spec/keystore.nim#L905.

So, which approach shall we follow? Fully EIP-compliat (with fields we don't use zeroed, like pubkey, etc.) or the format provided by encryptDataV3? Both are equally possible with minor modifications. I expect more libraries to be able to deal directly with EIP-compliant keystores (think of JSON schema validation), although we're adapting it for a different use than its original purpose and indeed some fields are unnecessary.

richard-ramos commented 1 year ago

Ah! this is because go-waku is not using at the moment EIP2335, but https://github.com/ethereum/wiki/wiki/Web3-Secret-Storage-Definition. This should be implemented in nim in:

s1fr0 commented 1 year ago

Thanks! Same as for nimbus, plaintext is hardcoded to be a secret key, but this can be fixed. The .md states that scrypt is not supported but I can see it implemented, so we should have compatibility! It would be great if you can post 1-2 test-vector (scrypt/pkdf2) with password so that I can check against customized nim-eth implementation!

richard-ramos commented 1 year ago

In https://github.com/ethereum/go-ethereum/blob/master/accounts/keystore/testdata/v3_test_vector.json there are a couple of test vectors with their password. The expected result from decryption can be seen in the attributepriv of each test,

s1fr0 commented 1 year ago

It seems that encrypted credentials returned by go-waku based on encryptDataV3 don't embed the version and id fields in the final JSON and this choice doesn't follow the standard https://github.com/ethereum/wiki/wiki/Web3-Secret-Storage-Definition.

I was able to fully replicate test vectors content (but for arbitrary input data) and produce keyfiles compliant with https://github.com/ethereum/wiki/wiki/Web3-Secret-Storage-Definition.

However we need to decide if we want to keep all fields (even if we don't use some) and refer to the standard, or have a custom/minimal version of the JSON. For me is a minor refactor, I guess what really could make a difference is what libraries are available for js-waku and the output format of keyfiles' JSONs.

s1fr0 commented 1 year ago

As a side note: this PR can proceed in parallel and even be merged before an agreement on the (unencrypted) RLN credential format, i.e. a solution to https://github.com/vacp2p/rfc/issues/543, is found.

Indeed, keyfiles' logic is agnostic from the content they encrypt. Once an agreement on https://github.com/vacp2p/rfc/issues/543 is found, we just replace the input to the keyfile creation routine from current rlnCredentials.txt content to the data in the agreed format.

richard-ramos commented 1 year ago

what really could make a difference is what libraries are available for js-waku and the output format of keyfiles' JSONs.

For JS, the following libraries are available, but would probably require some minor work for them to allow encrypting arbitrary data

richard-ramos commented 1 year ago

It seems that encrypted credentials returned by go-waku based on encryptDataV3 don't embed the version and id fields in the final JSON and this choice doesn't follow the standard https://github.com/ethereum/wiki/wiki/Web3-Secret-Storage-Definition.

The reason why go-waku does not add version and id is because I'm not using the function EncryptKey which adds these attributes and the address. It should be trivial to do the change in go-waku so the encrypted file matches the standard.

s1fr0 commented 1 year ago

The implementation in nwaku of the secure RLN credentials persistence discussed above is now ready for review at https://github.com/status-im/nwaku/pull/1285

fryorcraken commented 1 year ago

I agree with @richard-ramos's comments above. Considering what we are trying to achieve, I am not convinced it is worth using a common library (imported in wasm) across implementations. If we can find an existing standard (e.g. EIP-2335) that fits the purpose then it would make sense.

I see EIP-2335 is BLS specific. Would it makes sense to propose an EIP extension to use it for zk credentials usage too? Has crypt-keeper defined an interface to export/import credentials?

s1fr0 commented 1 year ago

Would it makes sense to propose an EIP extension to use it for zk credentials usage too?

Not sure. At the end we're using a (really minor) revision of https://github.com/ethereum/wiki/wiki/Web3-Secret-Storage-Definition , that is we just removed the version and id fields. But at least in nwaku, those can be easily enabled changing a compilation flag, and probably is worth to just keep these fields too so that we have 1 standard to follow for which there exists reasonably many implementations already.

Has crypt-keeper defined an interface to export/import credentials?

I was unable to find any

fryorcraken commented 1 year ago

Also note that for the browser, https://github.com/vacp2p/zerokit/pull/56#event-7496507160 enables deterministic generation of credentials from an ethereum wallet signature. AFAIK, this strategy is used by several zk projects (e.g. aztec network, zksync).

This enables delegation of the storing of credentials to the Ethereum Wallet. The only missing part being the membership id: I assume this can be retrieve by finding the transaction that inserted the member?

WDYT about using seeded generation as a first step to save credentials in the browser?

AtHeartEngineer commented 1 year ago

Right now, we don't have an import/export format, though that is something we are planning to add. I agree that JSON is probably the best format.

s1fr0 commented 1 year ago

Finding commitments among leaves should be easy. However, the naive linear approach requires 2^19 checks on average for a tree with max 2^20 elements.

Unfortunately the tree is not ordered (this would have required just ~20 checks) since otherwise commitment indexes would keep changing with new registrations.

But a space-time tradeoff is also possible: we maintain a table with keys all values from 0 to n-1 and as values a sequence of indexes. Then, every time a new commitment is added to he tree at position index, we update the table adding index at key commitment mod n (or we just take the log(n) LSB of commitment). In the average case, each key in the table will have 2^19/n indexes, of 4 bytes each, i.e. size of 2^21/n bytes and total table size of 2^21 bytes = 2MB (4 MiB for a full tree). This extra 2MiB brings however the computational costs of finding a commitment from 2^19 checks to 2^19/n.

If we use n of size 2 bytes, i.e. n=256*256 (or we just look at least 2 bytes of commitment), then the indexes we need to try would be 2^19/2^16 = 2^3 = 8, even better than an ordered tree.

Also, given the size of such table it could also be distributed (along with the tree, once we can store it).

fryorcraken commented 1 year ago

Finding commitments among leaves...

Wouldn't it be easier to just find the matching Ethereum transaction sent to the contract from the loaded wallet and look at the receipt of said transaction?

s1fr0 commented 1 year ago

Finding commitments among leaves...

Wouldn't it be easier to just find the matching Ethereum transaction sent to the contract from the loaded wallet and look at the receipt of said transaction?

Not sure easier: to generate valid proofs, assumption here is that you already have the local tree filled/updated and at that point is easier to search the index locally rather than online, since both approaches require linear time if done naively (I don't see how we can optimize the online way, while for the local search we can do something like what I described in my previous comment). If you're just interested in recovering the index regardless of the local tree state and ability to generate proofs, then yes, checking tx logs could be a light way to get it.

In any case, I'm not sure if you need to go through all blocks since contract deployment to get all tx logs (complexity > than searching in local tree) or if you can directly query only tx logs coming from a specific contract address (complexity >= than searching in local tree).

s1fr0 commented 1 year ago

This issue has been addressed by https://github.com/waku-org/nwaku/pull/1285 and its follow-up PR https://github.com/waku-org/nwaku/pull/1466. The solution implements https://github.com/waku-org/nwaku/issues/1238#issuecomment-1278114079, that is a structured JSON keystore supporting multiple membership credentials encrypted with an implementation derived from nimbus keyfile module.