Add a bandit target to tox.ini

This requires some changes in the Python code, because bandit considers:

    try:
        <some code>
    except <some error>:
        pass

as an issue, since it might be an involuntarily unhandled error.

So this pattern is changed to something else when easily doable. Otherwise,
"nosec" is added when the code is obviously safe.

Change-Id: Ic99f29b944892fb78814f5de515e6ed699979bc1

bandit.yaml

@@ -0,0 +1,393 @@
+# Generated using bandit_conf_generator with the following configuration:
+# profile_name: gate
+#
+profiles:
+  gate:
+    include:
+    - any_other_function_with_shell_equals_true
+    - assert_used
+    - blacklist_calls
+    - blacklist_import_func
+    - blacklist_imports
+    - exec_used
+    - execute_with_run_as_root_equals_true
+    - hardcoded_bind_all_interfaces
+    - hardcoded_password_string
+    - hardcoded_password_funcarg
+    - hardcoded_password_default
+    - hardcoded_sql_expressions
+    - hardcoded_tmp_directory
+    - jinja2_autoescape_false
+    - linux_commands_wildcard_injection
+    - paramiko_calls
+    - password_config_option_not_marked_secret
+    - request_with_no_cert_validation
+    - set_bad_file_permissions
+    - subprocess_popen_with_shell_equals_true
+    - subprocess_without_shell_equals_true
+    - start_process_with_a_shell
+    - start_process_with_no_shell
+    - start_process_with_partial_path
+    - ssl_with_bad_defaults
+    - ssl_with_bad_version
+    - ssl_with_no_version
+    - try_except_pass
+    - use_of_mako_templates
+    - weak_cryptographic_key
+
+exclude_dirs:
+- /tests/
+
+shell_injection:
+  no_shell:
+  - os.execl
+  - os.execle
+  - os.execlp
+  - os.execlpe
+  - os.execv
+  - os.execve
+  - os.execvp
+  - os.execvpe
+  - os.spawnl
+  - os.spawnle
+  - os.spawnlp
+  - os.spawnlpe
+  - os.spawnv
+  - os.spawnve
+  - os.spawnvp
+  - os.spawnvpe
+  - os.startfile
+  shell:
+  - os.system
+  - os.popen
+  - os.popen2
+  - os.popen3
+  - os.popen4
+  - popen2.popen2
+  - popen2.popen3
+  - popen2.popen4
+  - popen2.Popen3
+  - popen2.Popen4
+  - commands.getoutput
+  - commands.getstatusoutput
+  subprocess:
+  - subprocess.Popen
+  - subprocess.call
+  - subprocess.check_call
+  - subprocess.check_output
+  - utils.execute
+  - utils.execute_with_timeout
+
+ssl_with_bad_version:
+  bad_protocol_versions:
+  - PROTOCOL_SSLv2
+  - SSLv2_METHOD
+  - SSLv23_METHOD
+  - PROTOCOL_SSLv3
+  - PROTOCOL_TLSv1
+  - SSLv3_METHOD
+  - TLSv1_METHOD
+
+try_except_pass:
+  check_typed_exception: true
+
+plugin_name_pattern: '*.py'
+
+profiles:
+  gate:
+    include:
+    - any_other_function_with_shell_equals_true
+    - assert_used
+    - blacklist_calls
+    - blacklist_import_func
+    - blacklist_imports
+    - exec_used
+    - execute_with_run_as_root_equals_true
+    - hardcoded_bind_all_interfaces
+    - hardcoded_password_string
+    - hardcoded_password_funcarg
+    - hardcoded_password_default
+    - hardcoded_sql_expressions
+    - hardcoded_tmp_directory
+    - jinja2_autoescape_false
+    - linux_commands_wildcard_injection
+    - paramiko_calls
+    - password_config_option_not_marked_secret
+    - request_with_no_cert_validation
+    - set_bad_file_permissions
+    - subprocess_popen_with_shell_equals_true
+    - subprocess_without_shell_equals_true
+    - start_process_with_a_shell
+    - start_process_with_no_shell
+    - start_process_with_partial_path
+    - ssl_with_bad_defaults
+    - ssl_with_bad_version
+    - ssl_with_no_version
+    - try_except_pass
+    - use_of_mako_templates
+    - weak_cryptographic_key
+
+blacklist_calls:
+  bad_name_sets:
+  - pickle:
+      message: 'Pickle library appears to be in use, possible security issue.
+
+        '
+      qualnames:
+      - pickle.loads
+      - pickle.load
+      - pickle.Unpickler
+      - cPickle.loads
+      - cPickle.load
+      - cPickle.Unpickler
+  - marshal:
+      message: 'Deserialization with the marshal module is possibly dangerous.
+
+        '
+      qualnames:
+      - marshal.load
+      - marshal.loads
+  - md5:
+      message: Use of insecure MD2, MD4, or MD5 hash function.
+      qualnames:
+      - hashlib.md5
+      - Crypto.Hash.MD2.new
+      - Crypto.Hash.MD4.new
+      - Crypto.Hash.MD5.new
+      - cryptography.hazmat.primitives.hashes.MD5
+  - ciphers:
+      level: HIGH
+      message: 'Use of insecure cipher {func}. Replace with a known secure cipher
+        such as AES.
+
+        '
+      qualnames:
+      - Crypto.Cipher.ARC2.new
+      - Crypto.Cipher.ARC4.new
+      - Crypto.Cipher.Blowfish.new
+      - Crypto.Cipher.DES.new
+      - Crypto.Cipher.XOR.new
+      - cryptography.hazmat.primitives.ciphers.algorithms.ARC4
+      - cryptography.hazmat.primitives.ciphers.algorithms.Blowfish
+      - cryptography.hazmat.primitives.ciphers.algorithms.IDEA
+  - cipher_modes:
+      message: Use of insecure cipher mode {func}.
+      qualnames:
+      - cryptography.hazmat.primitives.ciphers.modes.ECB
+  - mktemp_q:
+      message: Use of insecure and deprecated function (mktemp).
+      qualnames:
+      - tempfile.mktemp
+  - eval:
+      message: 'Use of possibly insecure function - consider using safer ast.literal_eval.
+
+        '
+      qualnames:
+      - eval
+  - mark_safe:
+      message: 'Use of mark_safe() may expose cross-site scripting vulnerabilities
+        and should be reviewed.
+
+        '
+      names:
+      - mark_safe
+  - httpsconnection:
+      message: 'Use of HTTPSConnection does not provide security, see https://wiki.openstack.org/wiki/OSSN/OSSN-0033
+
+        '
+      qualnames:
+      - httplib.HTTPSConnection
+      - http.client.HTTPSConnection
+      - six.moves.http_client.HTTPSConnection
+  - yaml_load:
+      message: 'Use of unsafe yaml load. Allows instantiation of arbitrary objects.
+        Consider yaml.safe_load().
+
+        '
+      qualnames:
+      - yaml.load
+  - urllib_urlopen:
+      message: 'Audit url open for permitted schemes. Allowing use of file:/ or custom
+        schemes is often unexpected.
+
+        '
+      qualnames:
+      - urllib.urlopen
+      - urllib.request.urlopen
+      - urllib.urlretrieve
+      - urllib.request.urlretrieve
+      - urllib.URLopener
+      - urllib.request.URLopener
+      - urllib.FancyURLopener
+      - urllib.request.FancyURLopener
+      - urllib2.urlopen
+      - urllib2.Request
+      - six.moves.urllib.request.urlopen
+      - six.moves.urllib.request.urlretrieve
+      - six.moves.urllib.request.URLopener
+      - six.moves.urllib.request.FancyURLopener
+  - random:
+      level: LOW
+      message: 'Standard pseudo-random generators are not suitable for security/cryptographic
+        purposes.
+
+        '
+      qualnames:
+      - random.random
+      - random.randrange
+      - random.randint
+      - random.choice
+      - random.uniform
+      - random.triangular
+  - telnetlib:
+      level: HIGH
+      message: 'Telnet-related funtions are being called. Telnet is considered insecure.
+        Use SSH or some other encrypted protocol.
+
+        '
+      qualnames:
+      - telnetlib.*
+  - xml_bad_cElementTree:
+      message: 'Using {func} to parse untrusted XML data is known to be vulnerable
+        to XML attacks. Replace {func} with its defusedxml equivalent function.
+
+        '
+      qualnames:
+      - xml.etree.cElementTree.parse
+      - xml.etree.cElementTree.iterparse
+      - xml.etree.cElementTree.fromstring
+      - xml.etree.cElementTree.XMLParser
+  - xml_bad_ElementTree:
+      message: 'Using {func} to parse untrusted XML data is known to be vulnerable
+        to XML attacks. Replace {func} with its defusedxml equivalent function.
+
+        '
+      qualnames:
+      - xml.etree.ElementTree.parse
+      - xml.etree.ElementTree.iterparse
+      - xml.etree.ElementTree.fromstring
+      - xml.etree.ElementTree.XMLParser
+  - xml_bad_expatreader:
+      message: 'Using {func} to parse untrusted XML data is known to be vulnerable
+        to XML attacks. Replace {func} with its defusedxml equivalent function.
+
+        '
+      qualnames:
+      - xml.sax.expatreader.create_parser
+  - xml_bad_expatbuilder:
+      message: 'Using {func} to parse untrusted XML data is known to be vulnerable
+        to XML attacks. Replace {func} with its defusedxml equivalent function.
+
+        '
+      qualnames:
+      - xml.dom.expatbuilder.parse
+      - xml.dom.expatbuilder.parseString
+  - xml_bad_sax:
+      message: 'Using {func} to parse untrusted XML data is known to be vulnerable
+        to XML attacks. Replace {func} with its defusedxml equivalent function.
+
+        '
+      qualnames:
+      - xml.sax.parse
+      - xml.sax.parseString
+      - xml.sax.make_parser
+  - xml_bad_minidom:
+      message: 'Using {func} to parse untrusted XML data is known to be vulnerable
+        to XML attacks. Replace {func} with its defusedxml equivalent function.
+
+        '
+      qualnames:
+      - xml.dom.minidom.parse
+      - xml.dom.minidom.parseString
+  - xml_bad_pulldom:
+      message: 'Using {func} to parse untrusted XML data is known to be vulnerable
+        to XML attacks. Replace {func} with its defusedxml equivalent function.
+
+        '
+      qualnames:
+      - xml.dom.pulldom.parse
+      - xml.dom.pulldom.parseString
+  - xml_bad_etree:
+      message: 'Using {func} to parse untrusted XML data is known to be vulnerable
+        to XML attacks. Replace {func} with its defusedxml equivalent function.
+
+        '
+      qualnames:
+      - lxml.etree.parse
+      - lxml.etree.fromstring
+      - lxml.etree.RestrictedElement
+      - lxml.etree.GlobalParserTLS
+      - lxml.etree.getDefaultParser
+      - lxml.etree.check_docinfo
+
+hardcoded_tmp_directory:
+  tmp_dirs:
+  - /tmp
+  - /var/tmp
+  - /dev/shm
+
+blacklist_imports:
+  bad_import_sets:
+  - telnet:
+      imports:
+      - telnetlib
+      level: HIGH
+      message: 'A telnet-related module is being imported.  Telnet is considered insecure.
+        Use SSH or some other encrypted protocol.
+
+        '
+  - info_libs:
+      imports:
+      - pickle
+      - cPickle
+      - subprocess
+      - Crypto
+      level: LOW
+      message: 'Consider possible security implications associated with {module} module.
+
+        '
+  - xml_libs:
+      imports:
+      - xml.etree.cElementTree
+      - xml.etree.ElementTree
+      - xml.sax.expatreader
+      - xml.sax
+      - xml.dom.expatbuilder
+      - xml.dom.minidom
+      - xml.dom.pulldom
+      - lxml.etree
+      - lxml
+      level: LOW
+      message: 'Using {module} to parse untrusted XML data is known to be vulnerable
+        to XML attacks. Replace {module} with the equivalent defusedxml package.
+
+        '
+  - xml_libs_high:
+      imports:
+      - xmlrpclib
+      level: HIGH
+      message: 'Using {module} to parse untrusted XML data is known to be vulnerable
+        to XML attacks. Use defused.xmlrpc.monkey_patch() function to monkey-patch
+        xmlrpclib and mitigate XML vulnerabilities.
+
+        '
+
+include:
+- '*.py'
+- '*.pyw'
+
+password_config_option_not_marked_secret:
+  function_names:
+  - oslo.config.cfg.StrOpt
+  - oslo_config.cfg.StrOpt
+
+hardcoded_password:
+  word_list: '%(site_data_dir)s/wordlist/default-passwords'
+
+execute_with_run_as_root_equals_true:
+  function_names:
+  - ceilometer.utils.execute
+  - cinder.utils.execute
+  - neutron.agent.linux.utils.execute
+  - nova.utils.execute
+  - nova.utils.trycmd

oslo_config/cfg.py

@@ -1761,12 +1761,10 @@ class _Namespace(argparse.Namespace):
         try:
             return self._get_cli_value(names, positional)
         except KeyError:
-            pass
-
-        names = [(g if g is not None else 'DEFAULT', n) for g, n in names]
-        values = self._parser._get(names, multi=multi, normalized=True,
-                                   current_name=current_name)
-        return values if multi else values[-1]
+            names = [(g if g is not None else 'DEFAULT', n) for g, n in names]
+            values = self._parser._get(names, multi=multi, normalized=True,
+                                       current_name=current_name)
+            return values if multi else values[-1]
 
 
 class _CachedArgumentParser(argparse.ArgumentParser):
@@ -2349,7 +2347,7 @@ class ConfigOpts(collections.Mapping):
         if namespace is None:
             try:
                 return self.__cache[key]
-            except KeyError:
+            except KeyError:  # nosec: Valid control flow instruction
                 pass
         value = self._do_get(name, group, namespace)
         self.__cache[key] = value
@@ -2389,7 +2387,7 @@ class ConfigOpts(collections.Mapping):
             group_name = group.name if group else None
             try:
                 return convert(opt._get_from_namespace(namespace, group_name))
-            except KeyError:
+            except KeyError:  # nosec: Valid control flow instruction
                 pass
             except ValueError as ve:
                 raise ConfigFileValueError(

test-requirements.txt

@@ -26,3 +26,6 @@ oslo.i18n>=1.5.0 # Apache-2.0
 
 # mocking framework
 mock>=1.2
+
+# Bandit security code scanner
+bandit>=0.13.2

tox.ini

@@ -1,6 +1,6 @@
 [tox]
 distribute = False
-envlist = py34,py27,pep8
+envlist = py34,py27,pep8,bandit
 
 [testenv]
 deps = -r{toxinidir}/test-requirements.txt
@@ -20,6 +20,10 @@ commands = {posargs}
 [testenv:docs]
 commands = python setup.py build_sphinx
 
+[bandit]
+deps = -r{toxinidir}/test-requirements.txt
+commands = bandit -c bandit.yaml -r oslo_config -n5 -p gate
+
 [flake8]
 show-source = True
 exclude = .tox,dist,doc,*.egg,build