Opened at 2010-07-03T01:28:37Z
Closed at 2012-02-07T23:43:30Z
#40 closed enhancement (fixed)
xsalsa20 wrapper
Reported by: | dragonxue | Owned by: | xue yu |
---|---|---|---|
Priority: | major | Milestone: | 0.6.0 |
Version: | 0.5.19 | Keywords: | xsalsa20 review-needed |
Cc: | zooko@…, lloyd@… | Launchpad Bug: |
Description
python wrapper for cryptopp's xsalsa20 stream cipher
Attachments (10)
Change History (23)
Changed at 2010-07-03T01:31:38Z by dragonxue
Changed at 2010-07-03T01:32:06Z by dragonxue
Changed at 2010-07-03T01:32:33Z by dragonxue
Changed at 2010-07-03T01:37:53Z by dragonxue
Changed at 2010-07-03T02:50:20Z by dragonxue
Changed at 2010-07-03T02:50:57Z by dragonxue
Changed at 2010-07-03T02:51:27Z by dragonxue
comment:1 follow-up: ↓ 2 Changed at 2010-07-06T19:27:35Z by davidsarah
- Keywords review-needed added
I've just skimmed it, but can't see anything wrong. Where do the test vectors come from?
comment:2 in reply to: ↑ 1 ; follow-up: ↓ 3 Changed at 2010-07-06T22:50:29Z by davidsarah
Replying to davidsarah:
I've just skimmed it, but can't see anything wrong. Where do the test vectors come from?
Jack Lloyd wrote on tahoe-dev:
The test vector is from Crypto++'s TestVectors/salsa.txt, comment there is:
Source: created by Wei Dai using naclcrypto-20090308
naclcrypto being DJB's crypto library and of course DJB designed XSalsa20
Thanks. dragonxue: please add a comment about this to test_xsalsa.py and testx1.txt.
comment:3 in reply to: ↑ 2 Changed at 2010-07-06T22:54:18Z by davidsarah
Replying to davidsarah:
dragonxue: please add a comment about this to test_xsalsa.py and testx1.txt.
... or if it's inconvenient to put comments in textx1.txt, just in test_xsalsa.py is sufficient.
comment:4 Changed at 2010-07-20T05:44:35Z by zooko
comment:5 Changed at 2011-12-09T22:49:43Z by zooko
- Milestone set to 0.6.0
comment:6 Changed at 2012-02-06T03:49:53Z by warner
https://github.com/warner/pycryptopp/tree/40-xsalsa20 has this work integrated into trunk. All tests pass.
comment:7 Changed at 2012-02-07T04:58:19Z by warner
I ran a quick test (using the attached attachment:compare_with_pynacl.py script) to compare the pycryptopp XSalsa20 implementation against pynacl's crypto_stream_xor() function (using random keys, IVs, messages, and chunk sizes), and it passed 10k cases (took about 4 seconds total on my laptop).
I also verified that the built-in test_xsalsa.py fails if I modify one of the salsa.txt test vectors, or if I modify the key in test_zero_XSalsa.
For reference, it looks like the salsa.txt test vectors are a reformatted subset of the upstream cryptopp-5.6.1/TestVectors/salsa.txt (at least I eyeballed the two and found several keys in common). Upstream uses a funky data format (more of a miniature test-executing language), and includes vectors for reduced-round variants (Salsa12 and Salsa08). It'd be reassuring if pycryptopp could have an exact copy of the upstream vectors, but I don't think it's worth the effort to reverse-engineer Crypto++'s testing language.
comment:8 follow-up: ↓ 10 Changed at 2012-02-07T05:28:31Z by warner
I've updated my branch with some improvements:
- add a power-on self-test
- fix the same wrong-length-IV bug that was present in the AES code (#70)
I skimmed xsalsamodule.cpp looking for memory problems, and nothing jumped out at me.
One question came to mind, though: do we want this to be named "xsalsa", or would "XSalsa20" be better? (Or even "Salsa20"? I'm not sure exactly what the "X" means, and calling it Salsa20 would distinguish it from the reduced-round Salsa12/Salsa08 variants).
comment:9 Changed at 2012-02-07T05:33:32Z by warner
I also ran a few hundred GB through XSalsa().process() and didn't see the memory consumption increase at all, so I'm not worried about memory leaks.
comment:10 in reply to: ↑ 8 Changed at 2012-02-07T07:38:59Z by zooko
Brian:
Thank you for doing this work!
One question came to mind, though: do we want this to be named "xsalsa", or would "XSalsa20" be better? (Or even "Salsa20"? I'm not sure exactly what the "X" means, and calling it Salsa20 would distinguish it from the reduced-round Salsa12/Salsa08 variants).
The 'X' denotes an incompatible new variant of the algorithm, with a 192-bit nonce instead of 64-bit (so that implementors can pick random nonces instead of maintaining a counter). See the paper on http://cr.yp.to/snuffle.html denoted "[xsalsa]". We should include 'X' in the name. I agree about the '20', so let's name it 'XSalsa20'.
comment:11 Changed at 2012-02-07T22:05:14Z by warner
- Resolution set to fixed
- Status changed from new to closed
Ok, that's all been landed to trunk (https://github.com/warner/pycryptopp/commit/c4d510126c59afa801d8defa2f97337c44957d80) in a set of 7 patches finishing with [c4d51012/git]. Done!
comment:12 Changed at 2012-02-07T22:06:46Z by warner
- Resolution fixed deleted
- Status changed from closed to reopened
oops, the buildbot is red.. some sort of compile error. Will investigate.
comment:13 Changed at 2012-02-07T23:43:30Z by warner
- Resolution set to fixed
- Status changed from reopened to closed
ah, it was a #include problem, xsalsa20module.cpp was using an obsolete #ifdef USE_NAME_CRYPTO_PLUS_PLUS. I removed that and replaced it with the current #ifdef DISABLE_EMBEDDED_CRYPTOPP (matching the usage in aesmodule.cpp), and now the buildbot is turning green again.
header file