Skip to content
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

Rotate nonces #6

Closed
felinira opened this issue Mar 18, 2022 · 6 comments
Closed

Rotate nonces #6

felinira opened this issue Mar 18, 2022 · 6 comments

Comments

@felinira
Copy link
Contributor

Currently only one nonce is used for the entire file. As I understand it this is not really a good idea. To further improve security, maybe the nonce should be changed for each packet that is sent. I tried something like this in my fork, this is working but maybe not the way you would want to do it. It would change the way encryption works though. The file wouldn't be encrypted in one operation but rather during the data transmission. I think this would probably be more performant anyways though, especially for large files.

@landhb
Copy link
Owner

landhb commented Mar 18, 2022

I actually don't think this is a security issue since the entire file is encrypted at once. So it's one nonce & one pass. The AEAD_CHACHA20_POLY1305 RFC states:

a nonce value cannot be used securely more than once with the same key.

Which fits the current use case. Since the nonce + key are both only used once to encrypt the entire file, and never used again.

For performance I think I did have it encrypt per chunk during development at one point and it was actually slower than a single call to encrypt_in_place_detached. But it would be worth creating & running some benchmarks to see if there is any speedup.

@piegamesde
Copy link

@felinira You appear to be reworking the protocol at the moment. May I suggest you to have a deep look at croc and Magic Wormhole first (if not already done so)? A lot of valuable lessons were learned in the making of there w.r.t to security, features and backwards compatibility. I wouldn't want you to learn these the hard way …

@felinira
Copy link
Contributor Author

I actually don't think this is a security issue since the entire file is encrypted at once. So it's one nonce & one pass. The AEAD_CHACHA20_POLY1305 RFC states:

a nonce value cannot be used securely more than once with the same key.

You might be right. As long as "using once" means one invocation of the algorithm which it probably does?

@felinira
Copy link
Contributor Author

@felinira You appear to be reworking the protocol at the moment. May I suggest you to have a deep look at croc and Magic Wormhole first (if not already done so)? A lot of valuable lessons were learned in the making of there w.r.t to security, features and backwards compatibility. I wouldn't want you to learn these the hard way …

I was only trying things out for fun. But I will definitely look more in depth there, thanks.

@landhb
Copy link
Owner

landhb commented Mar 20, 2022

Going to close the issue for now since currently the nonce is only used once. If we move to per-chunk encryption later on we'll need to generate a nonce per-chunk.

For the new metadata PR, since it's encrypted separately it uses an entirely different nonce: https://github.com/landhb/portal/blob/encmetadata/lib/src/lib.rs#L381

@landhb landhb closed this as completed Mar 20, 2022
@landhb
Copy link
Owner

landhb commented Mar 31, 2022

Per-chunk encryption has now been implemented in #11, so unique nonces per-chunk are now generated via NonceSequence

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants