Key serialization and deserialization

[sgeisler]

You already have some internal serde implementations for field elements (FieldWrap) and functions for converting Fr to a string and parsing it again. I think it would be nice to also have serde impls for secret/public keys and key shares (e.g. for config files of HBBFT nodes). Are you interested in PRs adding this functionality?

Or is the lack of these impls intentional and I should just define newtypes? I could imagine that to be the case because reliable memory zeroing is probably harder if you impl serde traits.

EDIT: I'm currently implementing serde for SecretKeyShares and haven't looked into the other key types yet, maybe they have serde impls. But at least for SecretKeyShare it's missing.

[afck]

The public ones do have serde implementations. For the secret ones we were hesitating: It makes it a bit easier to accidentally store or send secrets. But I agree that there's plenty of valid use cases for that, of course. I'm not sure what's the best approach:

• We could just implement the serde traits—in the end it's the user's responsibility to use them correctly.
• We could add something like SecretKey::export or reveal_and_export that returns a serializable wrapper, so at least you have to do that consciously and explicitly in the code. (And implement Deserialize directly, because that's less dangerous?)
• We could add a SecretKey::reveal_inner that returns the contained field (Fr) element.

[vkomenda]

@sgeisler, we are interested in your proposal and a PR. The absence of serialisation methods for secret keys was justified by an idea that it would direct the application developer towards more secure applications. But as you rightfully note, serialisation is still required. I think the latter two options given by @afck - either a wrapper or an explicit method that reveals the Fr element - would be fine from the standpoint.

[sgeisler]

Currently @afck's second option would be good enough for my use case. But hbbft also supports dynamic federations, so having both Serialize and Deserialize implemented for the same struct would be more useful since you could update your config in memory (adding peers/keys/...) and serialize it afterwards.

Maybe a wrapper SerdeSecret<T: serde::Deserialize + SerializeSecret>(pub T) with an appropriate warning in the docs would be ok?

I don't really like the SecretKey::reveal_inner idea, because this would remove semantic meaning from the key material.

[afck]

I like te wrapper idea. It could implement Deref<T>, so it could be used as a secret key [share], and it would still force the user to make the serialization aspect explicit.

[sgeisler]

I began to work on the wrapper solution in #74 . If you could have a look at it whenever you find the time that would be great since I'm not familiar with the code base and might be doing things in a wrong/too complicated way.

Please feel free to add secret key structs that should implement SerializeSecret to the list in the PR.