-
-
Notifications
You must be signed in to change notification settings - Fork 15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add symmetric capabilities to DecryptedThing #214
Closed
Closed
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
e006ae3
Add symmetric capabilities to DecryptedThing
benjaminpreiss 9bc0fac
Implement HashedKeyEnvelope
benjaminpreiss 34179af
Add .create function to Aes256 key
benjaminpreiss e757cdc
Update packages/utils/crypto/src/encryption.ts
benjaminpreiss d01c1ec
Update packages/utils/crypto/src/encryption.ts
benjaminpreiss File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder whether the Keychain should be improved to also be able to export Ephemeral keys based on their hashes or something
But this is about solving the problem
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting thought... So that means one would also include an Envelope within
EncryptedSymmetricThing
that includes the hash of the SymmetricKey (salted)?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... Well maybe the naming would not be envelope any more as there is no signature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regarding your questions:
I think certainty can be provided by successful deserialization?
When we include the hash of the ephemeral key used for encryption in the "envelope", we have a linear lookup time for the question "Can I decrypt?".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There might be scenarios where the decrypted ciphertext still can deserialize succesfully into your object even though the decryption is invalid. Lets say it is not something you can trust on 100%
But this page is pretty good
https://libsodium.gitbook.io/doc/secret-key_cryptography/secretbox
(which is the function you are using in this PR)
So the solution you have there does the integrity check. I think it bundles some authentication data in the cipher text that depends on the nonce and the secret key used. So that issue would be solved (but one has to test) that this will be sufficient for the verifying it has been decrypted correctly.
That would be an easy way to go about it. But it is not standard thing to do it seems.. (I am lookig around on the web and ask the chatbot). But it feels very intuitive
An alternative thing would be that this is on the app level. So you would a have a keychain locally which is a KV store, and you would put your symmetric keys in there with well chosen IDs, like it could be the db address of the store you want to encrypt. So basically when you want to decrypt a db you would go into this keychain and look for this key that has an id which is the db address and then if it exist, use it to decrypt entries.
Another alternative would be that you would have a shared keychain which is a document store for ephemeral keys which is encrypted with publickey encryption (the encryption you get default with the document store now). With that you can have each document to have an id which you somehow connect with the id you use for the messages you encrypt in another db (this solution would also help you send your ephemeral keys to the other party)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So what I want to continue to say on this topic is that if you want to receiver to have all necessary data to lookup the secretkey from the keystore you practically need to talk on the sidechannel for every message (?) (given that the salt is unique for every message). If you are to talk about the encryption for about every message you might just want to generate a new symmetric key for every message altogether and use the sidechannel to send that over instead.
Using salts is generally done with a slow hashfunction for passwords which have a low entropy. This prevents someone from bruteforcing passwords given that they know they salt and the password + salt hash. But in this case the "password" is high entropy and there is no possibility that an actor will try to generate all 32 byte symmetric keys to match the hash. The only thing would be that if the symmetric key gets lost, then a middle man could have a easy way of figuring out what messages the middle man can decrypt. Having a salt here would not help anything, or making anything harder if the secret key gets lost. For the same reason as the salt for a password db does not help anything if the password in plain text gets lost
i would just keep it simple!
just make it
{ hash: Uint8Array }
Pros:
Will solve a problem where you want to have encryption but share keys on a sidechannel instead of inside the message itself (i.e. use the PublicKeyEnvelope)
The hash will help a receiver to figure out if they have a key in the keystore
Noone can find out what the symmetric key is given the hash
I can loop over all messages efficiently and figure out what messages I can decrypt, if not I can use a sidechannel to ask for the symmetric key by refering to its hash
Cons:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed! That's how I will implement it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you then also just keep one
EncryptedThing
or split up inEncryptedThing
andSymmetricEncryptedThing
?What type would you suggest for the unified
EncryptedThing
encrypt
function?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One encrypted thing, because later lets say the Keystore can both store Symmetric keys and X25519 Keypairs then you can do
without knowing the details of what the Envelope is.
But there is a problem now with the Keystore class is that it depends on the libp2p Keystore object which only can store PeerIds.. and it will also be dropped soon
libp2p/js-libp2p#2084
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There might be a sidequest to this to rework the Keychain to have following features