Removing bandit.yaml in favor of defaults

Removing old configuration options for build-in defaults of latest
bandit functionality. Also, marking flagged items with _# nosec_
with a descriptive comment on why the code is acceptable as is.

Co-Authored-By: Christopher J Schaefer <cjschaef@us.ibm.com>
Co-Authored-By: Tom Cocozzello <tjcocozz@us.ibm.com>

Change-Id: I138ebd46a8be195177361a9c3306bb70423b639d

bandit.yaml

@@ -1,119 +0,0 @@
-# optional: after how many files to update progress
-#show_progress_every: 100
-
-# optional: plugins directory name
-#plugins_dir: 'plugins'
-
-# optional: plugins discovery name pattern
-plugin_name_pattern: '*.py'
-
-# optional: terminal escape sequences to display colors
-#output_colors:
-#    DEFAULT: '\033[0m'
-#    HEADER: '\033[95m'
-#    INFO: '\033[94m'
-#    WARN: '\033[93m'
-#    ERROR: '\033[91m'
-
-# optional: log format string
-#log_format: "[%(module)s]\t%(levelname)s\t%(message)s"
-
-# globs of files which should be analyzed
-include:
-    - '*.py'
-    - '*.pyw'
-
-# a list of strings, which if found in the path will cause files to be excluded
-# for example /tests/ - to remove all all files in tests directory
-exclude_dirs:
-    - '/tests/'
-
-profiles:
-    gate:
-        include:
-            - blacklist_calls
-            - blacklist_imports
-            - request_with_no_cert_validation
-            - exec_used
-            - set_bad_file_permissions
-            - subprocess_popen_with_shell_equals_true
-            - linux_commands_wildcard_injection
-            - ssl_with_bad_version
-
-blacklist_calls:
-    bad_name_sets:
-        - pickle:
-            qualnames: [pickle.loads, pickle.load, pickle.Unpickler,
-                        cPickle.loads, cPickle.load, cPickle.Unpickler]
-            message: "Pickle library appears to be in use, possible security issue."
-        - marshal:
-            qualnames: [marshal.load, marshal.loads]
-            message: "Deserialization with the marshal module is possibly dangerous."
-        - md5:
-            qualnames: [hashlib.md5]
-            message: "Use of insecure MD5 hash function."
-        - mktemp_q:
-            qualnames: [tempfile.mktemp]
-            message: "Use of insecure and deprecated function (mktemp)."
-        - eval:
-            qualnames: [eval]
-            message: "Use of possibly insecure function - consider using safer ast.literal_eval."
-        - mark_safe:
-            names: [mark_safe]
-            message: "Use of mark_safe() may expose cross-site scripting vulnerabilities and should be reviewed."
-        - httpsconnection:
-            qualnames: [httplib.HTTPSConnection]
-            message: "Use of HTTPSConnection does not provide security, see https://wiki.openstack.org/wiki/OSSN/OSSN-0033"
-        - yaml_load:
-            qualnames: [yaml.load]
-            message: "Use of unsafe yaml load. Allows instantiation of arbitrary objects. Consider yaml.safe_load()."
-        - urllib_urlopen:
-            qualnames: [urllib.urlopen, urllib.urlretrieve, urllib.URLopener, urllib.FancyURLopener, urllib2.urlopen, urllib2.Request]
-            message: "Audit url open for permitted schemes. Allowing use of file:/ or custom schemes is often unexpected."
-
-shell_injection:
-    # Start a process using the subprocess module, or one of its wrappers.
-    subprocess: [subprocess.Popen, subprocess.call, subprocess.check_call,
-                 subprocess.check_output, utils.execute, utils.execute_with_timeout]
-    # Start a process with a function vulnerable to shell injection.
-    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]
-    # Start a process with a function that is not vulnerable to 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]
-
-blacklist_imports:
-    bad_import_sets:
-        - telnet:
-            imports: [telnetlib]
-            level: ERROR
-            message: "Telnet is considered insecure. Use SSH or some other encrypted protocol."
-
-hardcoded_password:
-    word_list: "wordlist/default-passwords"
-
-ssl_with_bad_version:
-    bad_protocol_versions:
-        - 'PROTOCOL_SSLv2'
-        - 'SSLv2_METHOD'
-        - 'SSLv23_METHOD'
-        - 'PROTOCOL_SSLv3'  # strict option
-        - 'PROTOCOL_TLSv1'  # strict option
-        - 'SSLv3_METHOD'    # strict option
-        - 'TLSv1_METHOD'    # strict option
-
-password_config_option_not_marked_secret:
-    function_names:
-        - oslo.config.cfg.StrOpt
-        - oslo_config.cfg.StrOpt
-
-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

keystoneclient/_discover.py

@@ -42,28 +42,30 @@ def get_version_data(session, url, authenticated=None):
 
     try:
         body_resp = resp.json()
-    except ValueError:
+    except ValueError:  # nosec(cjschaef): raise a DiscoveryFailure below
         pass
     else:
         # In the event of querying a root URL we will get back a list of
         # available versions.
         try:
             return body_resp['versions']['values']
-        except (KeyError, TypeError):
+        except (KeyError, TypeError):  # nosec(cjschaef): attempt to return
+            # versions dict or query the endpoint or raise a DiscoveryFailure
             pass
 
         # Most servers don't have a 'values' element so accept a simple
         # versions dict if available.
         try:
             return body_resp['versions']
-        except KeyError:
+        except KeyError:  # nosec(cjschaef): query the endpoint or raise a
+            # DiscoveryFailure
             pass
 
         # Otherwise if we query an endpoint like /v2.0 then we will get back
         # just the one available version.
         try:
             return [body_resp['version']]
-        except KeyError:
+        except KeyError:  # nosec(cjschaef): raise a DiscoveryFailure
             pass
 
     err_text = resp.text[:50] + '...' if len(resp.text) > 50 else resp.text
@@ -77,14 +79,16 @@ def normalize_version_number(version):
     # trim the v from a 'v2.0' or similar
     try:
         version = version.lstrip('v')
-    except AttributeError:
+    except AttributeError:  # nosec(cjschaef): 'version' is not a str, try a
+        # different type or raise a TypeError
         pass
 
     # if it's an integer or a numeric as a string then normalize it
     # to a string, this ensures 1 decimal point
     try:
         num = float(version)
-    except Exception:
+    except Exception:  # nosec(cjschaef): 'version' is not a float, try a
+        # different type or raise a TypeError
         pass
     else:
         version = str(num)
@@ -92,13 +96,15 @@ def normalize_version_number(version):
     # if it's a string (or an integer) from above break it on .
     try:
         return tuple(map(int, version.split('.')))
-    except Exception:
+    except Exception:  # nosec(cjschaef): 'version' is not str (or an int),
+        # try a different type or raise a TypeError
         pass
 
     # last attempt, maybe it's a list or iterable.
     try:
         return tuple(map(int, version))
-    except Exception:
+    except Exception:  # nosec(cjschaef): 'version' is not an expected type,
+        # raise a TypeError
         pass
 
     raise TypeError(_('Invalid version specified: %s') % version)

keystoneclient/access.py

@@ -148,7 +148,7 @@ class AccessInfo(dict):
     def auth_token(self):
         try:
             del self['auth_token']
-        except KeyError:
+        except KeyError:  # nosec(cjschaef): 'auth_token' is not in the dict
             pass
 
     @property
@@ -526,7 +526,8 @@ class AccessInfoV2(AccessInfo):
     def project_name(self):
         try:
             tenant_dict = self['token']['tenant']
-        except KeyError:
+        except KeyError:  # nosec(cjschaef): no 'token' key or 'tenant' key in
+            # token, return the name of the tenant or None
             pass
         else:
             return tenant_dict.get('name')
@@ -534,13 +535,15 @@ class AccessInfoV2(AccessInfo):
         # pre grizzly
         try:
             return self['user']['tenantName']
-        except KeyError:
+        except KeyError:  # nosec(cjschaef): no 'user' key or 'tenantName' in
+            # 'user', attempt 'tenantId' or return None
             pass
 
         # pre diablo, keystone only provided a tenantId
         try:
             return self['token']['tenantId']
-        except KeyError:
+        except KeyError:  # nosec(cjschaef): no 'token' key or 'tenantName' or
+            # 'tenantId' could be found, return None
             pass
 
     @property
@@ -589,7 +592,8 @@ class AccessInfoV2(AccessInfo):
     def project_id(self):
         try:
             tenant_dict = self['token']['tenant']
-        except KeyError:
+        except KeyError:  # nosec(cjschaef): no 'token' key or 'tenant' dict,
+            # attempt to return 'tenantId' or return None
             pass
         else:
             return tenant_dict.get('id')
@@ -597,13 +601,15 @@ class AccessInfoV2(AccessInfo):
         # pre grizzly
         try:
             return self['user']['tenantId']
-        except KeyError:
+        except KeyError:  # nosec(cjschaef): no 'user' key or 'tenantId' in
+            # 'user', attempt to retrive from 'token' or return None
             pass
 
         # pre diablo
         try:
             return self['token']['tenantId']
-        except KeyError:
+        except KeyError:  # nosec(cjschaef): no 'token' key or 'tenantId'
+            # could be found, return None
             pass
 
     @property

keystoneclient/adapter.py

@@ -206,7 +206,8 @@ class LegacyJsonAdapter(Adapter):
 
         try:
             kwargs['json'] = kwargs.pop('body')
-        except KeyError:
+        except KeyError:  # nosec(cjschaef): kwargs doesn't contain a 'body'
+            # key, while 'json' is an optional argument for Session.request
             pass
 
         resp = super(LegacyJsonAdapter, self).request(*args, **kwargs)
@@ -215,7 +216,8 @@ class LegacyJsonAdapter(Adapter):
         if resp.text:
             try:
                 body = jsonutils.loads(resp.text)
-            except ValueError:
+            except ValueError:  # nosec(cjschaef): return None for body as
+                # expected
                 pass
 
         return resp, body

keystoneclient/base.py

@@ -42,7 +42,8 @@ def getid(obj):
     try:
         if obj.uuid:
             return obj.uuid
-    except AttributeError:
+    except AttributeError:  # nosec(cjschaef): 'obj' doesn't contain attribute
+        # 'uuid', return attribute 'id' or the 'obj'
         pass
     try:
         return obj.id
@@ -131,7 +132,9 @@ class Manager(object):
         #           unlike other services which just return the list...
         try:
             data = data['values']
-        except (KeyError, TypeError):
+        except (KeyError, TypeError):  # nosec(cjschaef): keystone data values
+            # not as expected (see comment above), assumption is that values
+            # are already returned in a list (so simply utilize that list)
             pass
 
         return [obj_class(self, res, loaded=True) for res in data if res]
@@ -477,8 +480,8 @@ class Resource(object):
             try:
                 setattr(self, k, v)
                 self._info[k] = v
-            except AttributeError:
-                # In this case we already defined the attribute on the class
+            except AttributeError:  # nosec(cjschaef): we already defined the
+                # attribute on the class
                 pass
 
     def __getattr__(self, k):

keystoneclient/common/cms.py

@@ -60,9 +60,15 @@ def _ensure_subprocess():
             if patcher.already_patched:
                 from eventlet.green import subprocess
             else:
-                import subprocess
+                import subprocess  # nosec(cjschaef): we must be careful when
+                # using subprocess.Popen with possibly untrusted data,
+                # assumption is that the certificate/key files provided are
+                # trustworthy
         except ImportError:
-            import subprocess  # noqa
+            import subprocess  # noqa # nosec(cjschaef): we must be careful
+            # when using subprocess.Popen with possibly untrusted data,
+            # assumption is that the certificate/key files provided are
+            # trustworthy
 
 
 def set_subprocess(_subprocess=None):

keystoneclient/contrib/auth/v3/saml2.py

@@ -13,7 +13,7 @@
 import datetime
 import uuid
 
-from lxml import etree
+from lxml import etree  # nosec(cjschaef): used to create xml, not parse it
 from oslo_config import cfg
 from six.moves import urllib
 
@@ -559,7 +559,8 @@ class ADFSUnscopedToken(_BaseSAMLPlugin):
         """
         try:
             return bool(session.cookies)
-        except AttributeError:
+        except AttributeError:  # nosec(cjschaef): fetch cookies from
+            # underylying requests.Session object, or fail trying
             pass
 
         return bool(session.session.cookies)

keystoneclient/contrib/ec2/utils.py

@@ -71,7 +71,8 @@ class Ec2Signer(object):
                 if (credentials['params']['X-Amz-Algorithm'] ==
                         'AWS4-HMAC-SHA256'):
                     return True
-            except KeyError:
+            except KeyError:  # nosec(cjschaef): in cases of not finding
+                # entries, simply return False
                 pass
 
         return False

keystoneclient/contrib/revoke/model.py

@@ -219,8 +219,9 @@ class RevokeTree(object):
             try:
                 if leaf['issued_before'] > token_data['issued_at']:
                     return True
-            except KeyError:
-                pass
+            except KeyError:  # nosec(cjschaef): 'issued_before' or
+                # 'issued_at' key doesn't exist, try next leaf
+                continue
         # If we made it out of the loop then no element in revocation tree
         # corresponds to our token and it is good.
         return False

keystoneclient/httpclient.py

@@ -31,7 +31,7 @@ from positional import positional
 import requests
 
 try:
-    import pickle
+    import pickle  # nosec(cjschaef): see bug 1534288 for details
 
     # NOTE(sdague): The conditional keyring import needs to only
     # trigger if it's a version of keyring that's supported in global
@@ -129,7 +129,8 @@ class _KeystoneAdapter(adapter.LegacyJsonAdapter):
         # the identity plugin case
         try:
             return self.session.auth.get_access(self.session).user_id
-        except AttributeError:
+        except AttributeError:  # nosec(cjschaef): attempt legacy retrival, or
+            # return None
             pass
 
         # there is a case that we explicity allow (tested by our unit tests)
@@ -138,7 +139,8 @@ class _KeystoneAdapter(adapter.LegacyJsonAdapter):
         # a legacy then self.session.auth is a client and we retrieve user_id.
         try:
             return self.session.auth.user_id
-        except AttributeError:
+        except AttributeError:  # nosec(cjschaef): retrivals failed, return
+            # None
             pass
 
         return None
@@ -629,7 +631,8 @@ class HTTPClient(baseclient.Client, base.BaseAuthPlugin):
                 auth_ref = keyring.get_password("keystoneclient_auth",
                                                 keyring_key)
                 if auth_ref:
-                    auth_ref = pickle.loads(auth_ref)  # nosec
+                    auth_ref = pickle.loads(auth_ref)  # nosec(cjschaef): see
+                    # bug 1534288
                     if auth_ref.will_expire_soon(self.stale_duration):
                         # token has expired, don't use it
                         auth_ref = None
@@ -647,7 +650,8 @@ class HTTPClient(baseclient.Client, base.BaseAuthPlugin):
             try:
                 keyring.set_password("keystoneclient_auth",
                                      keyring_key,
-                                     pickle.dumps(self.auth_ref))
+                                     pickle.dumps(self.auth_ref))  # nosec
+                # (cjschaef): see bug 1534288
             except Exception as e:
                 _logger.warning(
                     _LW("Failed to store token into keyring %s"), e)
@@ -658,8 +662,8 @@ class HTTPClient(baseclient.Client, base.BaseAuthPlugin):
                 service_type='identity',
                 endpoint_type='admin',
                 region_name=region_name)
-        except exceptions.EndpointNotFound:
-            pass
+        except exceptions.EndpointNotFound as e:
+            _logger.debug("Failed to find endpoint for management url %s", e)
 
     def process_token(self, region_name=None):
         """Extract and process information from the new auth_ref.
@@ -872,7 +876,8 @@ class HTTPClient(baseclient.Client, base.BaseAuthPlugin):
     def __getattr__(self, name):
         try:
             var_name = self.deprecated_session_variables[name]
-        except KeyError:
+        except KeyError:  # nosec(cjschaef): try adapter variable or raise
+            # an AttributeError
             pass
         else:
             warnings.warn(
@@ -883,7 +888,7 @@ class HTTPClient(baseclient.Client, base.BaseAuthPlugin):
 
         try:
             var_name = self.deprecated_adapter_variables[name]
-        except KeyError:
+        except KeyError:  # nosec(cjschaef): raise an AttributeError
             pass
         else:
             warnings.warn(
@@ -897,7 +902,8 @@ class HTTPClient(baseclient.Client, base.BaseAuthPlugin):
     def __setattr__(self, name, val):
         try:
             var_name = self.deprecated_session_variables[name]
-        except KeyError:
+        except KeyError:  # nosec(cjschaef): try adapter variable or call
+            # parent class's __setattr__
             pass
         else:
             warnings.warn(
@@ -908,7 +914,7 @@ class HTTPClient(baseclient.Client, base.BaseAuthPlugin):
 
         try:
             var_name = self.deprecated_adapter_variables[name]
-        except KeyError:
+        except KeyError:  # nosec(cjschaef): call parent class's __setattr__
             pass
         else:
             warnings.warn(

keystoneclient/service_catalog.py

@@ -157,7 +157,7 @@ class ServiceCatalog(object):
             if service_name:
                 try:
                     sn = service['name']
-                except KeyError:
+                except KeyError:  # nosec(cjschaef)
                     # assume that we're in v3.0-v3.2 and don't have the name in
                     # the catalog. Skip the check.
                     pass
@@ -268,33 +268,33 @@ class ServiceCatalog(object):
         try:
             return urls[0]
         except Exception:
-            pass
+            if service_name and region_name:
+                msg = (_('%(endpoint_type)s endpoint for %(service_type)s '
+                         'service named %(service_name)s in %(region_name)s '
+                         'region not found') %
+                       {'endpoint_type': endpoint_type,
+                        'service_type': service_type,
+                        'service_name': service_name,
+                        'region_name': region_name})
+            elif service_name:
+                msg = (_('%(endpoint_type)s endpoint for %(service_type)s '
+                         'service named %(service_name)s not found') %
+                       {'endpoint_type': endpoint_type,
+                        'service_type': service_type,
+                        'service_name': service_name})
+            elif region_name:
+                msg = (_('%(endpoint_type)s endpoint for %(service_type)s '
+                         'service in %(region_name)s region not found') %
+                       {'endpoint_type': endpoint_type,
+                        'service_type': service_type,
+                        'region_name': region_name})
+            else:
+                msg = (_('%(endpoint_type)s endpoint for %(service_type)s '
+                         'service not found') %
+                       {'endpoint_type': endpoint_type,
+                        'service_type': service_type})
 
-        if service_name and region_name:
-            msg = (_('%(endpoint_type)s endpoint for %(service_type)s service '
-                     'named %(service_name)s in %(region_name)s region not '
-                     'found') %
-                   {'endpoint_type': endpoint_type,
-                    'service_type': service_type, 'service_name': service_name,
-                    'region_name': region_name})
-        elif service_name:
-            msg = (_('%(endpoint_type)s endpoint for %(service_type)s service '
-                     'named %(service_name)s not found') %
-                   {'endpoint_type': endpoint_type,
-                    'service_type': service_type,
-                    'service_name': service_name})
-        elif region_name:
-            msg = (_('%(endpoint_type)s endpoint for %(service_type)s service '
-                     'in %(region_name)s region not found') %
-                   {'endpoint_type': endpoint_type,
-                    'service_type': service_type, 'region_name': region_name})
-        else:
-            msg = (_('%(endpoint_type)s endpoint for %(service_type)s service '
-                     'not found') %
-                   {'endpoint_type': endpoint_type,
-                    'service_type': service_type})
-
-        raise exceptions.EndpointNotFound(msg)
+            raise exceptions.EndpointNotFound(msg)
 
     @abc.abstractmethod
     def get_data(self):
@@ -343,7 +343,7 @@ class ServiceCatalogV2(ServiceCatalog):
         try:
             token['user_id'] = self.catalog['user']['id']
             token['tenant_id'] = self.catalog['token']['tenant']['id']
-        except Exception:
+        except KeyError:  # nosec(cjschaef)
             # just leave the tenant and user out if it doesn't exist
             pass
         return token
@@ -410,7 +410,7 @@ class ServiceCatalogV3(ServiceCatalog):
             project = self.catalog.get('project')
             if project:
                 token['tenant_id'] = project['id']
-        except Exception:
+        except KeyError:  # nosec(cjschaef)
             # just leave the domain, project and user out if it doesn't exist
             pass
         return token

keystoneclient/session.py

@@ -72,7 +72,7 @@ def _remove_service_catalog(body):
             data['access']['serviceCatalog'] = '<removed>'
             return jsonutils.dumps(data)
 
-    except Exception:
+    except Exception:  # nosec(cjschaef): multiple exceptions can be raised
         # Don't fail trying to clean up the request body.
         pass
     return body
@@ -392,7 +392,7 @@ class Session(object):
 
         try:
             connection_params = self.get_auth_connection_params(auth=auth)
-        except exceptions.MissingAuthPlugin:
+        except exceptions.MissingAuthPlugin:  # nosec(cjschaef)
             # NOTE(jamielennox): If we've gotten this far without an auth
             # plugin then we should be happy with allowing no additional
             # connection params. This will be the typical case for plugins
@@ -579,7 +579,8 @@ class Session(object):
                      'timeout', 'session', 'original_ip', 'user_agent'):
             try:
                 params[attr] = kwargs.pop(attr)
-            except KeyError:
+            except KeyError:  # nosec(cjschaef): we are brute force
+                # identifying possible attributes for kwargs
                 pass
 
         return cls._make(**params)
@@ -725,7 +726,8 @@ class Session(object):
         for arg in ('cert', 'verify'):
             try:
                 kwargs[arg] = params_copy.pop(arg)
-            except KeyError:
+            except KeyError:  # nosec(cjschaef): we are brute force
+                # identifying and removing values in params_copy
                 pass
 
         if params_copy:

keystoneclient/utils.py

@@ -33,7 +33,8 @@ def find_resource(manager, name_or_id):
     # first try the entity as a string
     try:
         return manager.get(name_or_id)
-    except (exceptions.NotFound):
+    except (exceptions.NotFound):  # nosec(cjschaef): try to find 'name_or_id'
+        # as a six.binary_type instead
         pass
 
     # finally try to find entity by name
@@ -94,7 +95,8 @@ def prompt_user_password():
         # Check for Ctl-D
         try:
             password = getpass.getpass('Password: ')
-        except EOFError:
+        except EOFError:  # nosec(cjschaef): return password, which is None if
+            # password was not found
             pass
 
     return password

tox.ini

@@ -19,12 +19,12 @@ whitelist_externals = find
 [testenv:pep8]
 commands =
   flake8
-  bandit -c bandit.yaml -r keystoneclient -n5 -p gate
+  bandit -r keystoneclient -x tests -n5
 
 [testenv:bandit]
 # NOTE(browne): This is required for the integration test job of the bandit
 # project. Please do not remove.
-commands = bandit -c bandit.yaml -r keystoneclient -n5 -p gate
+commands = bandit -r keystoneclient -x tests -n5
 
 [testenv:venv]
 commands = {posargs}