Skip to content

Commit

Permalink
Add PyCryptodome to import blacklists
Browse files Browse the repository at this point in the history
PyCryptodome is a direct fork of PyCrypto, and is generally
considered to not be the replacement for it.  It is recommended
that projects move to pyca/cryptography instead, as this may be
exposing folks to the same inherent issues that PyCrypto was
deprecated because of.

Signed-off-by: John 'Warthog9' Hawley <warthog9@eaglescrag.net>
Signed-off-by: John 'Warthog9' Hawley <jhawley@vmware.com>
Signed-off-by: Terri Oda <terri.oda@intel.com>
  • Loading branch information
warthog9 authored and ericwb committed May 16, 2018
1 parent 69a209b commit 1c716be
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 0 deletions.
1 change: 1 addition & 0 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,7 @@ Usage::
B411 import_xmlrpclib
B412 import_httpoxy
B413 import_pycrypto
B414 import_pycryptodome
B501 request_with_no_cert_validation
B502 ssl_with_bad_version
B503 ssl_with_bad_defaults
Expand Down
34 changes: 34 additions & 0 deletions bandit/blacklists/imports.py
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,26 @@
| | | - Crypto.Util | |
+------+---------------------+------------------------------------+-----------+
B414: import_pycryptodome
-------------------------
pycryptodome is a direct fork of pycrypto that has not fully addressed
the issues inherent in PyCrypto. It seems to exist, mainly, as an API
compatible continuation of pycrypto and should be deprecated in favor
of pyca/cryptography which has more support among the Python community.
+------+---------------------+------------------------------------+-----------+
| ID | Name | Imports | Severity |
+======+=====================+====================================+===========+
| B414 | import_pycryptodome | - Cryptodome.Cipher | high |
| | | - Cryptodome.Hash | |
| | | - Cryptodome.IO | |
| | | - Cryptodome.Protocol | |
| | | - Cryptodome.PublicKey | |
| | | - Cryptodome.Random | |
| | | - Cryptodome.Signature | |
| | | - Cryptodome.Util | |
+------+---------------------+------------------------------------+-----------+
"""

from bandit.blacklists import utils
Expand Down Expand Up @@ -302,4 +322,18 @@ def gen_blacklist():
'maintained and have been deprecated. '
'Consider using pyca/cryptography library.', 'HIGH'))

sets.append(utils.build_conf_dict(
'import_pycryptodome', 'B414',
['Cryptodome.Cipher',
'Cryptodome.Hash',
'Cryptodome.IO',
'Cryptodome.Protocol',
'Cryptodome.PublicKey',
'Cryptodome.Random',
'Cryptodome.Signature',
'Cryptodome.Util'],
'The pycryptodome library is not considered a secure alternative '
'to pycrypto.'
'Consider using pyca/cryptography library.', 'HIGH'))

return {'Import': sets, 'ImportFrom': sets, 'Call': sets}
11 changes: 11 additions & 0 deletions examples/pycryptodome.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
from Cryptodome.Cipher import AES
from Cryptodome import Random

from . import CryptoMaterialsCacheEntry


def test_pycrypto():
key = b'Sixteen byte key'
iv = Random.new().read(AES.block_size)
cipher = pycrypto_arc2.new(key, AES.MODE_CFB, iv)
factory = CryptoMaterialsCacheEntry()
8 changes: 8 additions & 0 deletions tests/functional/test_functional.py
Original file line number Diff line number Diff line change
Expand Up @@ -729,3 +729,11 @@ def test_blacklist_pycrypto(self):
'CONFIDENCE': {'UNDEFINED': 0, 'LOW': 0, 'MEDIUM': 0, 'HIGH': 2}
}
self.check_example('pycrypto.py', expect)

def test_blacklist_pycryptodome(self):
'''Test importing pycryptodome module'''
expect = {
'SEVERITY': {'UNDEFINED': 0, 'LOW': 0, 'MEDIUM': 0, 'HIGH': 2},
'CONFIDENCE': {'UNDEFINED': 0, 'LOW': 0, 'MEDIUM': 0, 'HIGH': 2}
}
self.check_example('pycryptodome.py', expect)

1 comment on commit 1c716be

@frispete
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think, this is not the correct approach.

Weak algorithms are available in any crypto library out there. PyCryptodome is a fairly well maintained library, that does not depend on any other crypto package, with mostly any methods implemented in (portable) Python, falling back to optimized C code for performance critical parts only. You should parse the AST down to the crypto methods used, and flag weak ones only.

To be fair, you need to flag pyca/cryptography package as highly vunerable as well, since it contains similar methods, that even depend on a 3rd party package (openssl). openssl directly qualifies, what features are available, therefore deciding about pyca/cryptography, you need to take the openssl version and revision into account. Do you?

Obviously, this is more based on a political decision, not on rational/technical reasons. In case, I'm wrong, leave us a note on the real technical reasons here, please.

Apart from watching and using this project in my own projects since January 2017, I'm not affiliated to PyCryptodome in any further way.

Please sign in to comment.