Add support for Rijndael algorithms (encryption methods 14, 15, and 16) #3
Conversation
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
|
I think this is correct, but we'll also need to change the .travis.yml to bring in mcrypt from apt or something, since it doesn't look like it can be built from pypi. |
tmzullinger
force-pushed
the
rijndael-methods-14-15-16
branch
from
46fd95b
to
aa9d812
Compare
|
Ahh, I didn't even notice the .travis.yml file (though I should have noticed that travis is the first line in the README -- cleverly hidden ;). I squashed this in to fix the travis build: diff --git c/.travis.yml w/.travis.yml
index fa923d2..fd07d32 100644
--- c/.travis.yml
+++ w/.travis.yml
@@ -3,6 +3,7 @@ python:
- "2.6"
- "2.7"
install:
- - "sudo apt-get install nsca"
+ - "sudo apt-get install libmcrypt-dev nsca"
- "pip install -r requirements.txt -r requirements-tests.txt --use-mirrors"
+ - "pip install python-mcrypt --allow-external python-mcrypt --allow-unverified python-mcrypt"
script: "nosetests tests"
diff --git c/requirements.txt w/requirements.txt
index 6f01708..3bf0601 100644
--- c/requirements.txt
+++ w/requirements.txt
@@ -1,2 +1 @@
-mcrypt
pycrypto>=2.0Perhaps this needs some documentation updates as well, since python-mcrypt isn't quite as trivially installed as pycrypto? Let me know, or feel free to squash something in on top of this commit or as a follow up. |
The code previously used PyCrypto's AES module to implement these methods. That did not work because they are actually Rijndael ciphers and AES-256 is not the same as Rijndael-256 (and similarly with 128 and 192)¹. It is possible to implement Rijndael-128 using PyCrypto's AES module by setting the key_size to 32. Unfortunately, it is not possible to do so for Rijndael-192 or Rijndael-256. To support all three Rijndael methods, the mcrypt module is needed. Rather than use PyCrypto for just one of the Rijndael methods, replace all three with an mcrypt version. ¹ http://stackoverflow.com/a/8355432
tmzullinger
force-pushed
the
rijndael-methods-14-15-16
branch
from
aa9d812
to
36500e5
Compare
|
The second forced push only fixes a typo in the commit message. I need a better proof-reader, damn it. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The code was previous attempting to use PyCrypto's AES module to
implement these methods. That did not work because they are actually
Rijndael ciphers and AES-256 is not the same as Rijndael-256 (and
similarly with 128 and 192)¹. It is possible to implement Rijndael-128
using PyCrypto's AES module by setting the key_size to 32.
Unfortunately, it is not possible to do so for Rijndael-192 or
Rijndael-256.
To support all three Rijndael methods, the mcrypt module is needed.
Rather than use PyCrypto for just one of the Rijndael methods, replace
all three with an mcrypt version.
¹ http://stackoverflow.com/a/8355432
I attempted to run the unit tests but they failed for me, initially because they rely on a non-upstream patch to nsca from debian, but even when running from an ubuntu system I had no luck.
I have been using this on production systems for a month or so though, FWIW.
Thanks for this module, it's been a great help!