Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Off by one error allows invalid nonce usage #150

Closed
kjvalencik opened this issue Mar 6, 2023 · 2 comments · Fixed by #152
Closed

Off by one error allows invalid nonce usage #150

kjvalencik opened this issue Mar 6, 2023 · 2 comments · Fixed by #152

Comments

@kjvalencik
Copy link

kjvalencik commented Mar 6, 2023

Per the spec, the maximum usable nonce value is 2^64 - 2 and not 2^64 - 1.

The maximum n value (2^64 - 1) is reserved for other use. If incrementing n results in 2^64 - 1, then any further EncryptWithAd() or DecryptWithAd() calls will signal an error to the caller.

http://www.noiseprotocol.org/noise.html#the-cipherstate-object

These two usages should be slightly re-written:

+        if self.n == u64::MAX { return Err(StateProblem::Exhausted); }
         let len = self.cipher.encrypt(self.n, authtext, plaintext, out);
+        self.n += 1;
-        self.n = self.n.checked_add(1).ok_or(StateProblem::Exhausted)?;

self.n = self.n.checked_add(1).ok_or(StateProblem::Exhausted)?;

self.n = self.n.checked_add(1).ok_or(StateProblem::Exhausted)?;

@mcginty
Copy link
Owner

mcginty commented Mar 9, 2023

Oh great catch. Thank you, I'll fix this and push a patch release.

@complexspaces
Copy link
Contributor

I had these changes worked out locally and I've now put them into a PR since I've finished the unit accompanying unit test that I wanted to add.

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

Successfully merging a pull request may close this issue.

3 participants