Proposal: `KeyPair` should not use `byte[]` for the private key

Metalnem / noise

.NET Standard 1.3 implementation of the Noise Protocol Framework (revision 33 of the spec)

https://www.nuget.org/packages/Noise.NET

MIT License

152 stars 26 forks source link

Proposal: `KeyPair` should not use `byte[]` for the private key #3

Open danielcrenna opened 4 years ago

danielcrenna commented 4 years ago

In order to facilitate secure memory solutions, KeyPair:

should not copy a given private key into a byte array to make it suitable for KeyPair storage (i.e. in factory methods ala Curve25519.GenerateKeyPair
should not use managed buffer.Clear() to zero out memory on disposal, rather deferring to something like libsodium_free

This is a pre-requisite for enabling secure memory scenarios in this library, and could work for PSKs in the Protocol API methods.

Zetanova commented 3 years ago

danielcrenna's commit https://github.com/Metalnem/noise/commit/6c5fdb68d13e4a36b2f3c9070556359e649a7a54

Zetanova commented 3 years ago

The nsec.net lib uses libsodium too and has the reverse problem. See https://github.com/ektrah/nsec/issues/31

ektrah commented 3 years ago

Fwiw, I think it's totally the right way to do (passing keys around as KeyPair instances rather than byte arrays & storing private keys in sodium_malloc'd memory). The "problem" I'm facing with NSec is that API users are consistently undermining this by recreating the KeyPair instances from byte arrays on every single library invocation. I guess this might be less of a problem in a Noise implementation, which will probably more likely expose a "HandshakeState"- or "Connection"-oriented API rather than an "Algorithm"- and "KeyPair"-API.