bitcoin-core / secp256k1

Optimized C library for EC operations on curve secp256k1
MIT License
2.06k stars 1k forks source link

Clear sensitive memory without getting optimized out (revival of #636) #1579

Open theStack opened 1 month ago

theStack commented 1 month ago

This PR picks up #636 (which in turn picked up #448, so this is take number three) and is essentially a rebase on master.

Some changes to the original PR:

So far I haven't looked at any disassembly and possible performance implications yet (there were some concerns expressed in https://github.com/bitcoin-core/secp256k1/pull/636#issuecomment-620118629), happy to go deeper there if this gets Concept ACKed.

The proposed method of using a memory barrier to prevent optimizating away the memset is still used in BoringSSL (where it was originally picked up from) and in the Linux Kernel, see e.g. https://github.com/google/boringssl/blob/5af122c3dfc163b5d1859f1f450756e8e320a142/crypto/mem.c#L335 and https://github.com/torvalds/linux/blob/d4560686726f7a357922f300fc81f5964be8df04/include/linux/string.h#L348 / https://github.com/torvalds/linux/blob/d4560686726f7a357922f300fc81f5964be8df04/include/linux/compiler.h#L102

Fixes #185.

theStack commented 1 month ago

We should call SECP256K1_CHECKMEM_UNDEFINE (

https://github.com/bitcoin-core/secp256k1/blob/b307614401790850b48fb3ba878247290857a975/src/checkmem.h#L27

) in secp256k1_memclear after clearing the memory. Reading cleared memory would clearly be a bug.

Thanks, added that, and rebased on master.