Missing zeroization for private keys in monad-secp

[dnkolegov-ar]

secp256k1 library and KeyPair wrapper don't implement zeroization for private keys. Private keys remain in memory after use.
The criticality of this issue is low, but we recommend implementing zeroization to mitigate potential key leakage.

See the original issue.

Solution

Some projects use partial mitigation as here.

[dshulyak]

so practically in monad key can be zeroed only on the node shutdown?
i think in bitcoin it is used in wallet software, which is very different use case

[dnkolegov-ar]

The idea of zeroization of the private key is to remove secret keys from memory as soon and robustly as possible in the case of software compromise and extracting sensitive information from memory, as in the Heartbleed attack.

[dshulyak]

i understand what it means in general. i will try to elaborate, maybe i misunderstood something else

in monad-node this lib is used for signing network messages, so the private key needs to be kept in memory, or loaded from disk every time the message is signed. so practically when would you zero the memory of the key? thats why i said that it is likely means that it can be done only when monad-node shutdown.

in wallet software it is quite different, since you can zero the key after signing transaction.

is there anything that can be done better, perhaps something that exists on solana/ethereum validator software?

[dnkolegov-ar]

You’re right that a validator/node must keep its identity key resident for the entire process-lifetime.
What we can do is make sure the secret material is handled as defensively as possible while it is in memory and guarantee it’s scrubbed the moment the node no longer needs it.

[dshulyak]

it is quite hard to get any guarantees here as discussed in bitcoin thread.
i think the best we can do is to copy secp256k1::KeyPair on heap (with box) immediately after creation, and erase secp256k1::KeyPair that was on stack. then any moves should not affect boxed key pair, though i am not sure about that.

just doing partial mitigation as in https://github.com/TDSoftware/substrate-ipfs/blob/5e0a0744c7b510a68c899ba98d7e73f04f90e691/primitives/core/src/ecdsa.rs#L567-L576 seems as good as doing nothing

[dshulyak]

this comment seems like the best summary that i've found rust-bitcoin/rust-secp256k1#262 (comment) . in short it confirms what i wrote above, and doing that is still best effort and won't be enough to guarantee that secret key is always erased.

we probably can just do that custom drop implementation, it will erase one copy, which might be considered an improvement