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

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

Closed
1 task
s1fr0 opened this issue Oct 18, 2022 · 18 comments · Fixed by #1285
Closed
1 task

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

s1fr0 opened this issue Oct 18, 2022 · 18 comments · Fixed by #1285

Comments

@s1fr0
Copy link
Contributor

s1fr0 commented Oct 18, 2022

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 vacp2p/rfc#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

  • nimubs keystores: ready in nwaku. Require implementations in go/js-waku, but primitives should be ready in libs. Credentials may not be compatible with all clients. Requires some refactor to add custom fields/metadata to credentials.
  • single rust module: more effort for implementation. Reduces implementation mistakes. Credentials compatible with all implementations. Possibility to add custom fields/metadata to credentials.
  • different implementations: more effort, but in parallel. Credentials might not be compatible with all clients. Support for custom fields/metadata to credentials, but may be client-specific.

Acceptance criteria

  • Once RLN credential format is standardized, agree on the best solution for their secure storage, i.e. one among: - use nimbus keystore; - implement a single rust library; - have different implementations in each client.

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

@s1fr0 s1fr0 added the track:rln RLN Track (Secure Messaging/Applied ZK), e.g. relay and applications label Oct 18, 2022
@richard-ramos
Copy link
Member

richard-ramos commented Oct 19, 2022

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

s1fr0 commented Oct 20, 2022

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:

  • go-waku:
{"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
Copy link
Member

richard-ramos commented Oct 20, 2022

@s1fr0
Copy link
Contributor Author

s1fr0 commented Oct 20, 2022

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

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

s1fr0 commented Oct 20, 2022

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

s1fr0 commented Oct 20, 2022

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 vacp2p/rfc#543, is found.

Indeed, keyfiles' logic is agnostic from the content they encrypt. Once an agreement on vacp2p/rfc#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
Copy link
Member

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

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 s1fr0 linked a pull request Oct 22, 2022 that will close this issue
@s1fr0
Copy link
Contributor Author

s1fr0 commented Oct 22, 2022

The implementation in nwaku of the secure RLN credentials persistence discussed above is now ready for review at #1285

@fryorcraken
Copy link
Collaborator

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

s1fr0 commented Oct 27, 2022

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

fryorcraken commented Oct 27, 2022

Also note that for the browser, vacp2p/zerokit#56 (comment) 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
Copy link

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

s1fr0 commented Oct 27, 2022

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

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

s1fr0 commented Nov 9, 2022

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).

@rymnc rymnc added track:rlnp2p and removed track:rln RLN Track (Secure Messaging/Applied ZK), e.g. relay and applications labels Jan 19, 2023
@s1fr0
Copy link
Contributor Author

s1fr0 commented Feb 8, 2023

This issue has been addressed by #1285 and its follow-up PR #1466. The solution implements #1238 (comment), that is a structured JSON keystore supporting multiple membership credentials encrypted with an implementation derived from nimbus keyfile module.

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

Successfully merging a pull request may close this issue.

5 participants