search

summa-tx / coins

Rust implementations of BIP32/39 and Ledger device comms
Other
95 stars 34 forks source link

Secrets Management with Secrecy/Zeroize #15

Open gakonst opened 4 years ago

gakonst commented 4 years ago

Secrets such as Private Keys must remain in memory for as little as possible to minimize chances of extraction via side-channels and similar methods. They must also never be logged.

All such types in the repo should implement Zeroize, so that they're written with 0's once they're dropped (or manually zeroized), and they should be wrapped with Secret so that they're not accidentally logged. The internal type can be accessed via ExposeSecret.

prestwich commented 4 years ago

secrets are out of scope for riemann. I'm on the fence about putting them here/

prestwich commented 4 years ago

Zeroizing is handled by rust-secp but secrets are still leaked by logging. See https://github.com/rust-bitcoin/rust-secp256k1/issues/226

the parity secp ought to be replaced by k256 which will already have zeroize and secrets

elichai commented 4 years ago

Might be of interest to you:

  1. https://github.com/rust-bitcoin/rust-secp256k1/pull/102
  2. https://github.com/bitcoin-core/secp256k1/pull/636

I'll add that AFAICT k256 also doesn't zero out all the needed scalars / field elements in the actual ECDSA computation

prestwich commented 4 years ago

good resources, thanks :)

TheBlueMatt commented 3 years ago

Just implementing a overwrite-on-drop method doesn't accomplish very much in rust - the fact that moves are memcpy in the language definition means you can't readily hook copies of the secret key material. If you want protection your secret keys always need to be in a Box as well, though technically the creation of the box is always on stack before being copied, but that's almost always optimized out.

shamatar commented 3 years ago

With some unsafe code you can guarantee the Box creation doesn't touch a stack, but Box indeed is an only options to guarantee no copies at the moment. May be there is a crate for it, but most likely it needs a type wrapper that is not Clone and not Copy, conditionally compiled for no_std and alloc/std (Box/no Box) and that dereferences to &Secret... or other type in it's current form to use in other API methods

naps62 commented 11 months ago

old issue, but FWIW, I'm using the secrets crate successfully for this. just a wrapper around libsodium

TheBlueMatt commented 11 months ago

Fair enough, though a crate with one maintainer and relatively few outside contributors is probably not a great candidate for use when you need to store things that need cryptographic protection.

naps62 commented 11 months ago

Fair enough, though a crate with one maintainer and relatively few outside contributors is probably not a great candidate for use when you need to store things that need cryptographic protection.

secrets is mostly a thin wrapper around libsodium-sys (which itself is read-only nowadays, but is itself just C-bindings to the C library. deep rabbit hole here)

so while yes, it's a concern, it was also by far the best option I found a few months ago in terms of feature-set + proven track record, security-wise (the underlying libsodium, that is)

main factor for me was that it wasn't just about zeroing memory, but also using os-level features (e.g. mlock) to provide stronger guarantees on that memory region. the rust layer just mainly ensured this fits nicely with the borrow checker

DISCLAIMER: I'm not exactly an expert on this topic, just did quite a bit of research on it recently