feat: add Zeroize support to key types, and create new shared secret type
#137
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
AaronFeickert
changed the title
Zeroize support to key typesZeroize support to key types
This was referenced Oct 22, 2022
Closed
|
utACK |
AaronFeickert
changed the title
Zeroize support to key typesZeroize support to key types, and create new shared secret type
This was referenced Oct 24, 2022
stringhandler
pushed a commit
to tari-project/tari
that referenced
this pull request
Nov 7, 2022
Description --- Ensures safer use of ECDH shared secrets by switching to the new `DiffieHellmanSharedSecret` type. Updates `tari-crypto` to v0.15.7 to accomplish this. Motivation and Context --- Currently, an ECDH secret used for message keys is produced as a `RistrettoPublicKey`, converted to bytes, and returned as a byte array. However, neither the `RistrettoPublicKey` nor the byte array are cleared when dropped. In conjunction with `tari-crypto` [PR 137](tari-project/tari-crypto#137), this work ensures both the `RistrettoPublicKey` and byte array representations of the ECDH secret are zeroized on drop by using that PR's new `DiffieHellmanSharedSecret` type. How Has This Been Tested? --- Tested after applying `tari-crypto` [PR 137](tari-project/tari-crypto#137), which adds the new `DiffieHellmanSharedSecret` generic type.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds support for
Zeroizeto both theRistrettoSecretKeyandRistrettoPublicKeytypes, and creates a new shared secret type. Adds a test for both key types.This work adds derived
Zeroizesupport toRistrettoSecretKeyto support the use case where we want to clear a secret key in scope.It also adds custom
Zeroizesupport toRistrettoPublicKey. In this case, we zeroize both the underlyingRistrettoPointand theCompressedRistrettoPointcontained in aOnceCell. This is useful in the case where aRistrettoPublicKeyrepresents secret data, like in the case of a Diffie-Hellman shared secret. Note that we do not zeroize on drop, so this use case must be handled manually, ideally through the use of aZeroizingwrapper.Finally, it adds a new generic
DiffieHellmanSharedSecret<P>type forP: Zeroize. This type zeroizes on drop. It does not provide direct access to the underlying public key, but only gives anas_bytesarray view. This removes the need forzeroizesupport by implementations, and provides safer use of shared secrets.