From 60740282c9187788fbb9ff6d83a143df4c0bb7cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Beraud?= Date: Wed, 13 Feb 2019 17:40:22 +0100 Subject: [PATCH] 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 --- .zuul.yaml | 24 +++++++++++ cloudkitty/api/app.py | 3 -- cloudkitty/rating/pyscripts/__init__.py | 2 +- .../rating/pyscripts/datamodels/script.py | 4 +- .../75c205f6f1a2_move_from_sha1_to_sha512.py | 43 +++++++++++++++++++ .../rating/pyscripts/db/sqlalchemy/models.py | 4 +- .../rating/pyscripts/gabbits/pyscripts.yaml | 12 +++--- cloudkitty/tests/test_pyscripts.py | 9 ++-- lower-constraints.txt | 1 + ...ndit-security-linter-592faa26f957a3dd.yaml | 6 +++ test-requirements.txt | 1 + tox.ini | 9 +++- 12 files changed, 99 insertions(+), 19 deletions(-) create mode 100644 cloudkitty/rating/pyscripts/db/sqlalchemy/alembic/versions/75c205f6f1a2_move_from_sha1_to_sha512.py create mode 100644 releasenotes/notes/introduce-bandit-security-linter-592faa26f957a3dd.yaml diff --git a/.zuul.yaml b/.zuul.yaml index 5eb22c0d..8f924d93 100644 --- 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: diff --git a/cloudkitty/api/app.py b/cloudkitty/api/app.py index 9a708502..98abcde9 100644 --- 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.'), diff --git a/cloudkitty/rating/pyscripts/__init__.py b/cloudkitty/rating/pyscripts/__init__.py index 399b16a8..ab748e28 100644 --- 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): diff --git a/cloudkitty/rating/pyscripts/datamodels/script.py b/cloudkitty/rating/pyscripts/datamodels/script.py index 4d626727..beffba97 100644 --- 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 diff --git 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 new file mode 100644 index 00000000..2808983e --- /dev/null +++ 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)) diff --git a/cloudkitty/rating/pyscripts/db/sqlalchemy/models.py b/cloudkitty/rating/pyscripts/db/sqlalchemy/models.py index b8a5e145..355c7359 100644 --- 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) diff --git a/cloudkitty/tests/gabbi/rating/pyscripts/gabbits/pyscripts.yaml b/cloudkitty/tests/gabbi/rating/pyscripts/gabbits/pyscripts.yaml index 30378d19..790c4c6a 100644 --- 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 diff --git a/cloudkitty/tests/test_pyscripts.py b/cloudkitty/tests/test_pyscripts.py index 1426e52a..1107b676 100644 --- 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) diff --git a/lower-constraints.txt b/lower-constraints.txt index 7be8c564..9e808f06 100644 --- 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 diff --git a/releasenotes/notes/introduce-bandit-security-linter-592faa26f957a3dd.yaml b/releasenotes/notes/introduce-bandit-security-linter-592faa26f957a3dd.yaml new file mode 100644 index 00000000..8b070e3e --- /dev/null +++ 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. diff --git a/test-requirements.txt b/test-requirements.txt index 1d028b4b..87cad291 100644 --- 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 diff --git a/tox.ini b/tox.ini index 256a411e..10ab65a6 100644 --- 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