Don't let Debug or Display accidentally log SecretKeys

[prestwich]

Debug and Display print the lowercase hex of the scalar, which can easily lead to accidental key leakage in logs. Consider updating these to print some generic [ redacted ] message instead

[apoelstra]

I think this is probably the right thing for Display .. though this will make .to_string() stop working. It's also worth thinking about whether we should have serde impls for this type.

For Debug I could go either way. It would be useful if users could distinguish between different keys. I wonder if we should hash them and output a "short id" or something?

[prestwich]

A short ID would be ideal, with some other method to print hex that is unlikely to be accidentally invoked. The goal is to make access explicit. C.f. the secrecy crate's expose_secret API.

[thomaseizinger]

We could go the same route as std::path::Path and have a display() method: https://doc.rust-lang.org/std/path/struct.Path.html#method.display

That would make it an explicit action to print them while still being relatively ergonomic to use if you want to opt into it.

[apoelstra]

There's also the question of what to do with the FromStr trait, which I like having implemented, but which I think should be an inverse of ToString (which is implemented in terms of Display).

[prestwich]

would it make sense to have a wrapper type that implements redacted Display and Debug, an an internal type that preserves the ToString/FromStr symmetry?

[tcharding]

We now hash secrets in the Debug implementation if std or bitcoin_hashes is enabled, and elide it if not. Also we provide DisplaySecret to allow explicitly printing the secret.

SharedSecret however still implements Debug by printing contents using impl_raw_debug, is this correct or should we be obfuscating it as well?

Once SharedSecret is handled we should be able to close this issue as resolved.

[dr-orlovsky]

Wasn't it solved in #312?

[apoelstra]

@dr-orlovsky not for SharedSecret (the ECDH type)