status-im / nimbus-eth2

Nim implementation of the Ethereum Beacon Chain
https://nimbus.guide
Other
535 stars 231 forks source link

Protection of users private keys #545

Open mratsim opened 4 years ago

mratsim commented 4 years ago

Context

It is very important than under no circumstances that Nimbus never leaks private keys. Ideally, Nimbus never has knowledge of those and all private key manipulations are done in a separate library or even hardware.

Issues

At the moment, Nimbus does manipulate the private key in non-test/mocking files for the ValidatorPool (search: https://github.com/status-im/nim-beacon-chain/search?p=1&q=ValidatorPrivKey&unscoped_q=ValidatorPrivKey).

https://github.com/status-im/nim-beacon-chain/blob/136a11f681508dde4fb1f030cc673259312757ab/beacon_chain/beacon_node_types.nim#L219-L241

It is also used in genRandaoReveal but that is much easier to offload to a library: https://github.com/status-im/nim-beacon-chain/blob/63e621c27d66f8f61796033a602eb3999b9e94c9/beacon_chain/validator_pool.nim#L67-L73

Furthermore the data is heap-allocated, this means that it could be written to disk via core dumps or when swapping (see our recommendation to use lot of swap for Raspberry Pi).

Recommendation

Create a set of API that will be exposed by an external library that will be in charge of just handling private keys and where we could focus special security efforts.

In particular, if heap allocation is needed, it should not be dumpable or swappable and erasing/wiping it should not be optimized away by the compiler.

Unknown

GDB and other debuggers. While we can strongly recommend to setuid or equivalent to prevent attaching, there doesn't seem to be a way to prevent secrets from leaking when gdb is launched by root.

References

CERT Secure coding rules for C: https://wiki.sei.cmu.edu/confluence/display/c/MEM06-C.+Ensure+that+sensitive+data+is+not+written+out+to+disk

Encrypting secrets in-memory: https://spacetime.dev/encrypting-secrets-in-memory

Implementation in Go: https://github.com/awnumar/memguard

Memory retention attacks: https://spacetime.dev/memory-retention-attacks

tersec commented 4 years ago

https://lwn.net/Articles/804658/ has a relevant article from November 15, which @stefantalpalaru has linked elsewher:

One of the many responsibilities of the operating system is to help processes keep secrets from each other. Operating systems often fail in this regard, sometimes due to factors — such as hardware bugs and user-space vulnerabilities — that are beyond their direct control. It is thus unsurprising that there is an increasing level of interest in ways to improve the ability to keep data secret, perhaps even from the operating system itself. The MAP_EXCLUSIVE patch set from Mike Rapoport is one example of the work that is being done in this area; it also shows that the development community has not yet really begun to figure out how this type of feature should work.

mratsim commented 4 years ago

I wasn't aware of MAP_EXCLUSIVE.

On OpenBSD there is MAP_CONCEAL since June: https://undeadly.org/cgi?action=article;sid=20190605110020

And on Linux you can exclude some pages from dump with madvise and MADV_DONTDUMP.

I'm not too sure on Windows. And I'm not too sure on preventing dump of what is on the stack as well.

Edit: another attack vector, it's a rabbit hole: http://people.eecs.berkeley.edu/~shwetas/publications/pfdefense_asiaccs16.pdf

tersec commented 4 years ago

For those inclined to try with SGX, https://www.plundervolt.com/ offers yet another attack vector.

tersec commented 4 years ago

"Keeping secrets in memfd areas" from https://lwn.net/SubscriberLink/812325/b642e849751b9068/ describes a proposed Linux kernel mechanism in more detail. Not (yet/potentially) merged, but it looks like something along these lines will be available.

mratsim commented 4 years ago

Relevant, the bitcoin/libsecp256k1 to clear secrets from memory: https://github.com/bitcoin-core/secp256k1/pull/636

onqtam commented 4 years ago

Just implemented the remote ValidatorKind - the beacon node spawns a validator_client process (in a special push mode - nothing in common with the standard VC functionality which other clients have) which loads the keys and they communicate via RPC: https://github.com/status-im/nim-beacon-chain/pull/1490 - not sure what else would need to be done for security

mratsim commented 4 years ago

The isolated process is done in #1522 which makes the goal of this PR much more practical.

mratsim commented 4 years ago

Erasing secrets in memory: https://github.com/status-im/nim-beacon-chain/issues/1714

mratsim commented 3 years ago

New primitive memfd_secret to store cryptographic keys in a place that even the kernel can't read https://lwn.net/Articles/865256/