diff --git a/.travis.yml b/.travis.yml index bf630a5..ad05a89 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,7 +1,6 @@ -# We only use Python for coveralls; both Python 2.7 and 3.6 are -# tested but that is done within docker containers +# We only use Python for coveralls language: python -python: 3.6 +python: 3.7 services: - docker diff --git a/Dockerfile b/Dockerfile index 4810b50..97409db 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM python:3.6 +FROM python:3.7 VOLUME /opt WORKDIR /opt diff --git a/Makefile b/Makefile index 3c8f97e..86b155c 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -.PHONY: attach build build_tester clean coverage create_network docs psql release_pypi release_pypitest release_quay remove_network start_postgres stop_postgres test test_one_pg_version test27 test36 view_docs wait_for_postgres +.PHONY: attach build build_tester clean coverage create_network docs psql release_pypi release_pypitest release_quay remove_network start_postgres stop_postgres test test_one_pg_version pytest view_docs wait_for_postgres SUPPORTED_PG_VERSIONS ?= 9.5.13 9.6.4 10.4 # The default Postgres that will be used in individual targets @@ -26,15 +26,10 @@ build: clean . build_tester: - @echo "Building the tester27 and tester36 docker images" + @echo "Building the tester docker image" docker build . \ -f tests/Dockerfile \ - --build-arg PYTHON_VERSION=2.7 \ - -t tester27 - docker build . \ - -f tests/Dockerfile \ - --build-arg PYTHON_VERSION=3.6 \ - -t tester36 + -t tester clean: @echo "Cleaning the repo" @@ -94,7 +89,7 @@ stop_postgres: @echo "Stopping postgres (if it is running)" @-docker stop $(POSTGRES_HOST) || true -test_one_pg_version: start_postgres wait_for_postgres test27 test36 remove_network clean +test_one_pg_version: start_postgres wait_for_postgres pytest remove_network clean test: clean build_tester @for pg_version in ${SUPPORTED_PG_VERSIONS}; do \ @@ -102,25 +97,15 @@ test: clean build_tester $(MAKE) test_one_pg_version POSTGRES_VERSION="$$pg_version"; \ done -test27: - @echo "Running pytest with Python 2.7" - @docker run \ - --rm \ - -e WITHIN_DOCKER_FLAG=true \ - -e POSTGRES_PORT=5432 \ - -v $(shell pwd):/opt \ - --net=$(COMPOSED_NETWORK) \ - tester27 - -test36: - @echo "Running pytest with Python 3.6" +pytest: + @echo "Running pytest" @docker run \ --rm \ -e WITHIN_DOCKER_FLAG=true \ -e POSTGRES_PORT=5432 \ -v $(shell pwd):/opt \ --net=$(COMPOSED_NETWORK) \ - tester36 + tester wait_for_postgres: @echo 'Sleeping while postgres starts up'; diff --git a/pgbedrock/attributes.py b/pgbedrock/attributes.py index a53181b..39ad178 100644 --- a/pgbedrock/attributes.py +++ b/pgbedrock/attributes.py @@ -1,6 +1,8 @@ import copy import datetime as dt import hashlib +import hmac +import base64 import logging import click @@ -87,6 +89,14 @@ def create_md5_hash(rolename, value): salted_input = (value + rolename).encode('utf-8') return 'md5' + hashlib.md5(salted_input).hexdigest() +def create_scram_hash(password, salt, iterations): + digest = hashlib.pbkdf2_hmac("sha256", password.encode(), salt, iterations) + client_key = hmac.digest(digest, "Client Key".encode(), hashlib.sha256) + stored_key = base64.b64encode(hashlib.sha256(client_key).digest()).decode() + server_key = base64.b64encode(hmac.digest(digest, "Server Key".encode(), hashlib.sha256)).decode() + + return "SCRAM-SHA-256${}:{}${}:{}".format(iterations, base64.b64encode(salt).decode(), stored_key, server_key) + def is_valid_forever(val): if val is None or val == 'infinity': @@ -194,13 +204,47 @@ def get_attribute_value(self, attribute): return value def is_same_password(self, value): - """ Convert the input value into a postgres rolname-salted md5 hash and compare - it with the currently stored hash """ - if value is None: - return self.current_attributes.get('rolpassword') is None + """ Compare stored password hash to the new password. + + Returns ``True`` if both ``value`` and the stored password are ``None`` (empty) + **or** ``value`` is a plain text password its hash matches the stored password + hash **or** ``value`` is a hashed password and matches the stored password hash + type and value. ``False`` in any other case. """ + current_value = self.current_attributes.get('rolpassword') + + if value is None or current_value is None: + return value == current_value + + if hmac.compare_digest(current_value, value): + return True + + if current_value.startswith("SCRAM-SHA-256$"): + if value.startswith("md5"): + return False + + if value.startswith("SCRAM-SHA-256$"): + return hmac.compare_digest(current_value, value) + + hash_parts = current_value.split("$") + + iterations, salt = hash_parts[1].split(":") + iterations = int(iterations) + salt = base64.b64decode(salt) + + return hmac.compare_digest(current_value, create_scram_hash(value, salt, iterations)) + + if current_value.startswith("md5"): + if value.startswith("SCRAM-SHA-256$"): + return False + + if value.startswith("md5"): + return hmac.compare_digest(current_value, value) + + return hmac.compare_digest(current_value, create_md5_hash(self.rolename, value)) - md5_hash = create_md5_hash(self.rolename, value) - return self.current_attributes.get('rolpassword') == md5_hash + # There’s currently only two hash algorithm supported by Postgres (md5 + # and SCRAM-SHA-256) so we should never reach this. + return False def role_exists(self): # If current_attributes is empty then the rolname wasn't in pg_authid, i.e. it doesn't exist diff --git a/requirements-dev.txt b/requirements-dev.txt index 16b680f..ee3c7ef 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -2,3 +2,4 @@ pytest==3.1.3 pytest-cov==2.5.1 -r requirements-docs.txt wheel==0.33.6 +psycopg2==2.7.7 diff --git a/requirements.txt b/requirements.txt index 4fcc6a0..f808bfe 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,6 +1,6 @@ Cerberus==1.1 click==6.7 Jinja2==2.10.1 -MarkupSafe==1.0 +MarkupSafe==1.1.1 psycopg2==2.7.3 PyYAML==5.2 diff --git a/setup.py b/setup.py index de26ed4..8528fb1 100644 --- a/setup.py +++ b/setup.py @@ -42,9 +42,7 @@ def get_version(): install_requires=required, classifiers=[ 'Programming Language :: Python', - 'Programming Language :: Python :: 2', 'Programming Language :: Python :: 3', - 'Programming Language :: Python :: 2.7', - 'Programming Language :: Python :: 3.6', + 'Programming Language :: Python :: 3.7', ], ) diff --git a/tests/Dockerfile b/tests/Dockerfile index 9913906..cb8ce9a 100644 --- a/tests/Dockerfile +++ b/tests/Dockerfile @@ -1,5 +1,4 @@ -ARG PYTHON_VERSION -FROM python:$PYTHON_VERSION +FROM python:3.7 VOLUME /opt WORKDIR /opt diff --git a/tests/test_attributes.py b/tests/test_attributes.py index 7688786..921b46d 100644 --- a/tests/test_attributes.py +++ b/tests/test_attributes.py @@ -382,14 +382,38 @@ def test_set_attribute_value_valid_until(roleconf): rolpassword=attr.create_md5_hash(ROLE1, 'supersecret'), )) @pytest.mark.parametrize('desired_value, expected', [ - ('supersecret', True), - ('incorrect_password', False)]) -def test_is_same_password(roleconf, desired_value, expected): + ('supersecret', True), + ('md5c85aa4317b187e73a31e8ab775a10833', True), + ('incorrect_password', False), + ('SCRAM-SHA-256$4096:c3VwZXJzYWx0$E6lZT4K2olotsu19xYcF825iMPGdJQDYaklVS2mR6js=:Pe9DLNf8idnP59Q5l8Xmz3H+6LrTuiq//bcujQPGsRM=', False), + ] +) +def test_is_same_password_md5(roleconf, desired_value, expected): assert roleconf.is_same_password(desired_value) == expected -def test_is_same_password_if_empty(roleconf): - assert roleconf.is_same_password(None) is True +@nondefault_attributes(dict( + rolpassword=attr.create_scram_hash('supersecret', 'supersalt'.encode(), 4096), +)) +@pytest.mark.parametrize('desired_value, expected', [ + ('supersecret', True), + ('SCRAM-SHA-256$4096:c3VwZXJzYWx0$E6lZT4K2olotsu19xYcF825iMPGdJQDYaklVS2mR6js=:Pe9DLNf8idnP59Q5l8Xmz3H+6LrTuiq//bcujQPGsRM=', True), + ('incorrect_password', False), + ('md5c85aa4317b187e73a31e8ab775a10833', False), + ] +) +def test_is_same_password_scram(roleconf, desired_value, expected): + assert roleconf.is_same_password(desired_value) == expected + +@pytest.mark.parametrize('desired_value, expected', [ + (None, True), + ('incorrect_password', False), + ('md5c85aa4317b187e73a31e8ab775a10833', False), + ('SCRAM-SHA-256$4096:c3VwZXJzYWx0$E6lZT4K2olotsu19xYcF825iMPGdJQDYaklVS2mR6js=:Pe9DLNf8idnP59Q5l8Xmz3H+6LrTuiq//bcujQPGsRM=', False), + ] +) +def test_is_same_password_if_empty(roleconf, desired_value, expected): + assert roleconf.is_same_password(desired_value) == expected @nondefault_attributes(dict(