Add bandit for security static analysis and fix potential security issues

This change adds a basic bandit config for cloudkitty. It can be invoked by running the tox environment for bandit; tox -e bandit These changes also fix potential security issues find during bandit checks. - binding to all interface: remove useless host_ip option to avoid issue - hash function issue: switch from sha1 to sha512 - use of exec: can't be removed for moment so using #nosec comment Change-Id: Iae7d7604457345fe6d482cf48311c9b75fdde947
2019-02-13 17:40:22 +01:00 · 2019-02-13 17:40:22 +01:00 · 60740282c9
commit 60740282c9
parent 7c7ff81cd9
12 changed files with 99 additions and 19 deletions
--- a/.zuul.yaml
+++ b/.zuul.yaml
@ -40,6 +40,28 @@
        DEVSTACK_GATE_USE_PYTHON3: "True"
        USE_PYTHON3: "True"

+- job:
+    name: cloudkitty-tox-bandit
+    parent: openstack-tox
+    timeout: 2400
+    vars:
+      tox_envlist: bandit
+    required-projects:
+      - openstack/requirements
+    irrelevant-files:
+      - ^.*\.rst$
+      - ^.*\.txt$
+      - ^api-ref/.*$
+      - ^apidocs/.*$
+      - ^contrib/.*$
+      - ^doc/.*$
+      - ^etc/.*$
+      - ^releasenotes/.*$
+      - ^setup.cfg$
+      - ^tools/.*$
+      - ^cloudkitty/hacking/.*$
+      - ^cloudkitty/tests/scenario/.*$
+      - ^cloudkitty/tests/unittests/.*$

 - project:
    templates:
@ -55,6 +77,8 @@
      jobs:
        - cloudkitty-tempest-full
        - cloudkitty-tempest-full-python3
+        - cloudkitty-tox-bandit:
+            voting: false
    gate:
      queue: cloudkitty
      jobs:
--- a/cloudkitty/api/app.py
+++ b/cloudkitty/api/app.py
@ -42,9 +42,6 @@ auth_opts = [
 ]

 api_opts = [
-    cfg.IPOpt('host_ip',
-              default='0.0.0.0',
-              help='The listen IP for the cloudkitty API server.'),
    cfg.PortOpt('port',
                default=8889,
                help='The port for the cloudkitty API server.'),
--- a/cloudkitty/rating/pyscripts/init.py
+++ b/cloudkitty/rating/pyscripts/init.py
@ -77,7 +77,7 @@ class PyScripts(rating.RatingProcessorBase):

    def start_script(self, code, data):
        context = {'data': data}
-        exec(code, context)
+        exec(code, context)  # nosec
        return data

    def process(self, data):
--- a/cloudkitty/rating/pyscripts/datamodels/script.py
+++ b/cloudkitty/rating/pyscripts/datamodels/script.py
@ -42,7 +42,9 @@ class Script(wtypes.Base):
        sample = cls(script_id='bc05108d-f515-4984-8077-de319cbf35aa',
                     name='policy1',
                     data='return 0',
-                     checksum='da39a3ee5e6b4b0d3255bfef95601890afd80709')
+                     checksum='cf83e1357eefb8bdf1542850d66d8007d620e4050b5715d'
+                              'c83f4a921d36ce9ce47d0d13c5d85f2b0ff8318d2877eec'
+                              '2f63b931bd47417a81a538327af927da3e')
        return sample


--- a/cloudkitty/rating/pyscripts/db/sqlalchemy/alembic/versions/75c205f6f1a2_move_from_sha1_to_sha512.py
+++ b/cloudkitty/rating/pyscripts/db/sqlalchemy/alembic/versions/75c205f6f1a2_move_from_sha1_to_sha512.py
@ -0,0 +1,43 @@
+# Copyright 2019 OpenStack Foundation
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+#
+
+"""move from sha1 to sha512
+
+Revision ID: 75c205f6f1a2
+Revises: 4f9efa4601c0
+Create Date: 2019-03-25 13:53:23.398755
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = '75c205f6f1a2'
+down_revision = '4f9efa4601c0'
+
+from alembic import op
+import sqlalchemy as sa
+
+
+def upgrade():
+    with op.batch_alter_table('pyscripts_scripts') as batch_op:
+        batch_op.alter_column('checksum',
+                              existing_type=sa.VARCHAR(length=40),
+                              type_=sa.String(length=128))
+
+
+def downgrade():
+    with op.batch_alter_table('pyscripts_scripts') as batch_op:
+        batch_op.alter_column('checksum',
+                              existing_type=sa.String(length=128),
+                              type_=sa.VARCHAR(length=40))
--- a/cloudkitty/rating/pyscripts/db/sqlalchemy/models.py
+++ b/cloudkitty/rating/pyscripts/db/sqlalchemy/models.py
@ -82,7 +82,7 @@ class PyScriptsScript(Base, PyScriptsBase):
                              sqlalchemy.LargeBinary(),
                              nullable=False)
    _checksum = sqlalchemy.Column('checksum',
-                                  sqlalchemy.String(40),
+                                  sqlalchemy.String(128),
                                  nullable=False)

    @hybrid.hybrid_property
@ -92,7 +92,7 @@ class PyScriptsScript(Base, PyScriptsBase):

    @data.setter
    def data(self, value):
-        sha_check = hashlib.sha1()
+        sha_check = hashlib.sha512()
        sha_check.update(value)
        self._checksum = sha_check.hexdigest()
        self._data = zlib.compress(value)
--- a/cloudkitty/tests/gabbi/rating/pyscripts/gabbits/pyscripts.yaml
+++ b/cloudkitty/tests/gabbi/rating/pyscripts/gabbits/pyscripts.yaml
@ -38,7 +38,7 @@ tests:
      $.script_id: "6c1b8a30-797f-4b7e-ad66-9879b79059fb"
      $.name: "policy1"
      $.data: "a = 0"
-      $.checksum: "5ae340c1b3bb81955db1cb593cdc78540082c526"
+      $.checksum: "4c612e33c0e40b7bf53cf95fad47dbfbeab9dd62f9bc181a9d1c6f40a087782223c23f793e747b0466b9e6998c6ea54f4edbd20febd13edb13b55074b5ee1a5a"
    response_headers:
      location: '$SCHEME://$NETLOC/v1/rating/module_config/pyscripts/scripts/6c1b8a30-797f-4b7e-ad66-9879b79059fb'

@ -62,7 +62,7 @@ tests:
      $.scripts[0].script_id: "6c1b8a30-797f-4b7e-ad66-9879b79059fb"
      $.scripts[0].name: "policy1"
      $.scripts[0].data: "a = 0"
-      $.scripts[0].checksum: "5ae340c1b3bb81955db1cb593cdc78540082c526"
+      $.scripts[0].checksum: "4c612e33c0e40b7bf53cf95fad47dbfbeab9dd62f9bc181a9d1c6f40a087782223c23f793e747b0466b9e6998c6ea54f4edbd20febd13edb13b55074b5ee1a5a"

  - name: list scripts excluding data
    url: /v1/rating/module_config/pyscripts/scripts?no_data=true
@ -70,7 +70,7 @@ tests:
    response_json_paths:
      $.scripts[0].script_id: "6c1b8a30-797f-4b7e-ad66-9879b79059fb"
      $.scripts[0].name: "policy1"
-      $.scripts[0].checksum: "5ae340c1b3bb81955db1cb593cdc78540082c526"
+      $.scripts[0].checksum: "4c612e33c0e40b7bf53cf95fad47dbfbeab9dd62f9bc181a9d1c6f40a087782223c23f793e747b0466b9e6998c6ea54f4edbd20febd13edb13b55074b5ee1a5a"

  - name: get script
    url: /v1/rating/module_config/pyscripts/scripts/6c1b8a30-797f-4b7e-ad66-9879b79059fb
@ -79,7 +79,7 @@ tests:
      $.script_id: "6c1b8a30-797f-4b7e-ad66-9879b79059fb"
      $.name: "policy1"
      $.data: "a = 0"
-      $.checksum: "5ae340c1b3bb81955db1cb593cdc78540082c526"
+      $.checksum: "4c612e33c0e40b7bf53cf95fad47dbfbeab9dd62f9bc181a9d1c6f40a087782223c23f793e747b0466b9e6998c6ea54f4edbd20febd13edb13b55074b5ee1a5a"

  - name: modify script
    url: /v1/rating/module_config/pyscripts/scripts/6c1b8a30-797f-4b7e-ad66-9879b79059fb
@ -95,7 +95,7 @@ tests:
      $.script_id: "6c1b8a30-797f-4b7e-ad66-9879b79059fb"
      $.name: "policy1"
      $.data: "a = 1"
-      $.checksum: "b88f1ec0c9fe96fde96a6f9dabcbeee661dd7afe"
+      $.checksum: "acb3095e24b13960484e75bce070e13e8a7728760517c31b34929a6f732841c652e9d2cc4d186bd02ef2e7495fab3c4850673bedc945cee7c74fea85eabd542c"

  - name: modify unknown script
    url: /v1/rating/module_config/pyscripts/scripts/42
@ -120,7 +120,7 @@ tests:
      $.script_id: "6c1b8a30-797f-4b7e-ad66-9879b79059fb"
      $.name: "policy1"
      $.data: "a = 1"
-      $.checksum: "b88f1ec0c9fe96fde96a6f9dabcbeee661dd7afe"
+      $.checksum: "acb3095e24b13960484e75bce070e13e8a7728760517c31b34929a6f732841c652e9d2cc4d186bd02ef2e7495fab3c4850673bedc945cee7c74fea85eabd542c"

  - name: delete script
    url: /v1/rating/module_config/pyscripts/scripts/6c1b8a30-797f-4b7e-ad66-9879b79059fb
--- a/cloudkitty/tests/test_pyscripts.py
+++ b/cloudkitty/tests/test_pyscripts.py
@ -84,11 +84,11 @@ CK_RESOURCES_DATA = [{
                    "unit": "instance"}}]}}]

 TEST_CODE1 = 'a = 1'.encode('utf-8')
-TEST_CODE1_CHECKSUM = hashlib.sha1(TEST_CODE1).hexdigest()
+TEST_CODE1_CHECKSUM = hashlib.sha512(TEST_CODE1).hexdigest()
 TEST_CODE2 = 'a = 0'.encode('utf-8')
-TEST_CODE2_CHECKSUM = hashlib.sha1(TEST_CODE2).hexdigest()
+TEST_CODE2_CHECKSUM = hashlib.sha512(TEST_CODE2).hexdigest()
 TEST_CODE3 = 'if a == 1: raise Exception()'.encode('utf-8')
-TEST_CODE3_CHECKSUM = hashlib.sha1(TEST_CODE3).hexdigest()
+TEST_CODE3_CHECKSUM = hashlib.sha512(TEST_CODE3).hexdigest()

 COMPLEX_POLICY1 = """
 import decimal
@ -238,7 +238,8 @@ class PyScriptsRatingTest(tests.TestCase):
            setattr,
            script,
            'checksum',
-            'da39a3ee5e6b4b0d3255bfef95601890afd80709')
+            'cf83e1357eefb8bdf1542850d66d8007d620e4050b5715dc83f4a921d36ce9ce4'
+            '7d0d13c5d85f2b0ff8318d2877eec2f63b931bd47417a81a538327af927da3e')

    def test_update_checksum(self):
        self._db_api.create_script('policy1', TEST_CODE1)
--- a/lower-constraints.txt
+++ b/lower-constraints.txt
@ -45,3 +45,4 @@ reno==1.8.0  # Apache2
 sphinxcontrib-httpdomain==1.6.0 # Apache-2.0
 doc8==0.6.0 # Apache-2.0
 Pygments==2.2.0 # BSD
+bandit==1.1.0 # Apache-2.0
--- a/releasenotes/notes/introduce-bandit-security-linter-592faa26f957a3dd.yaml
+++ b/releasenotes/notes/introduce-bandit-security-linter-592faa26f957a3dd.yaml
@ -0,0 +1,6 @@
+---
+security:
+  - |
+    Introduce bandit security checks and fix potential security issues detected
+    by bandit linter. Remove unused option where host_ip was a binding to all
+    interfaces. Using of insecure hash function, switch from sha1 to sha512.
--- a/test-requirements.txt
+++ b/test-requirements.txt
@ -20,3 +20,4 @@ sphinxcontrib-pecanwsme>=0.8 # Apache-2.0
 reno>=1.8.0 # Apache-2.0
 doc8>=0.6.0 # Apache-2.0
 Pygments>=2.2.0 # BSD license
+bandit>=1.1.0 # Apache-2.0
--- a/tox.ini
+++ b/tox.ini
@ -24,8 +24,13 @@ commands = oslo_debug_helper {posargs}
 [testenv:pep8]
 basepython = python3
 commands =
-  flake8 {posargs} cloudkitty
-  doc8 {posargs}
+    flake8 {posargs} cloudkitty
+    doc8 {posargs}
+
+[testenv:bandit]
+basepython = python3
+deps = -r{toxinidir}/test-requirements.txt
+commands = bandit -r cloudkitty -n5 -x tests -ll

 [testenv:cover]
 basepython = python3