CodesInChaos / Chaos.NaCl

Chaos.NaCl cryptography library
Other
131 stars 54 forks source link

Fixes an array-out-of-bounds failure in XSalsa decrypt #3

Closed jasonmccampbell closed 10 years ago

jasonmccampbell commented 10 years ago

I ran into an array-bound exception when sending a short plain text message through. I believe this fix is correct, but I am still getting up-to-speed on the code so it could easily be wrong. The included unit test demonstrates the error on un-patched code.

Related, I have some basic tests for Ed25519.KeyExchange. This is my primary interest in the library in that I want to use Edwards curves for both signing and key exchange. Are there known issues with KeyExchange that basic tests won't tease out? In short I want to add the box create/open functionality by using KeyExchange to generate a shared secret and pass that secret to XSalsa Encrypt/Decrypt for distributing a symmetric encryption key to multiple parties. If KeyExchange is broken I'll have to find an alternative. But this library is very convenient thus far -- thanks!

CodesInChaos commented 10 years ago

I ran into an array-bound exception when sending a short plain text message through. I believe this fix is correct, but I am still getting up-to-speed on the code so it could easily be wrong. The included unit test demonstrates the error on un-patched code

Thanks for the report. I've added a similar fix and a unit test which verifies successful round trips for all message lengths from 0 to 1000.

CodesInChaos commented 10 years ago

I want to use Edwards curves for both signing and key exchange.

There is some concern about using the same key-pair for several purposes. As far as I can tell it's secure, but when I talked to Tanja Lange (one of the original NaCl authors) she recommended not using the same key-pair for Ed25519 and Box.

Related: Using same keypair for Diffie-Hellman and signing on cryptography.stackexchange.com

The conversion between Edwards and Montgomery form might not work correctly for a few exceptional points, but those shouldn't occur in practice.

jasonmccampbell commented 10 years ago

Thanks for the link, very interesting. From a cursory read, my application sounds along the lines of what you describe and may be a reasonable compromise as I would only be encrypting a symmetric encryption key. Like you said, less risk vs. coming up with a more complex protocol. I guess I will think on it.

Closing as a more comprehensive test was added.