diff --git a/homeassistant/auth/providers/homeassistant.py b/homeassistant/auth/providers/homeassistant.py index ce252497901..c743a5b7f65 100644 --- a/homeassistant/auth/providers/homeassistant.py +++ b/homeassistant/auth/providers/homeassistant.py @@ -5,13 +5,16 @@ import hashlib import hmac from typing import Any, Dict, List, Optional, cast +import bcrypt import voluptuous as vol from homeassistant.const import CONF_ID from homeassistant.core import callback, HomeAssistant from homeassistant.exceptions import HomeAssistantError +from homeassistant.util.async_ import run_coroutine_threadsafe from . import AuthProvider, AUTH_PROVIDER_SCHEMA, AUTH_PROVIDERS, LoginFlow + from ..models import Credentials, UserMeta from ..util import generate_secret @@ -74,8 +77,7 @@ class Data: Raises InvalidAuth if auth invalid. """ - hashed = self.hash_password(password) - + dummy = b'$2b$12$CiuFGszHx9eNHxPuQcwBWez4CwDTOcLTX5CbOpV6gef2nYuXkY7BO' found = None # Compare all users to avoid timing attacks. @@ -84,22 +86,55 @@ class Data: found = user if found is None: - # Do one more compare to make timing the same as if user was found. - hmac.compare_digest(hashed, hashed) + # check a hash to make timing the same as if user was found + bcrypt.checkpw(b'foo', + dummy) raise InvalidAuth - if not hmac.compare_digest(hashed, - base64.b64decode(found['password'])): + user_hash = base64.b64decode(found['password']) + + # if the hash is not a bcrypt hash... + # provide a transparant upgrade for old pbkdf2 hash format + if not (user_hash.startswith(b'$2a$') + or user_hash.startswith(b'$2b$') + or user_hash.startswith(b'$2x$') + or user_hash.startswith(b'$2y$')): + # IMPORTANT! validate the login, bail if invalid + hashed = self.legacy_hash_password(password) + if not hmac.compare_digest(hashed, user_hash): + raise InvalidAuth + # then re-hash the valid password with bcrypt + self.change_password(found['username'], password) + run_coroutine_threadsafe( + self.async_save(), self.hass.loop + ).result() + user_hash = base64.b64decode(found['password']) + + # bcrypt.checkpw is timing-safe + if not bcrypt.checkpw(password.encode(), + user_hash): raise InvalidAuth - def hash_password(self, password: str, for_storage: bool = False) -> bytes: - """Encode a password.""" + def legacy_hash_password(self, password: str, + for_storage: bool = False) -> bytes: + """LEGACY password encoding.""" + # We're no longer storing salts in data, but if one exists we + # should be able to retrieve it. salt = self._data['salt'].encode() # type: ignore hashed = hashlib.pbkdf2_hmac('sha512', password.encode(), salt, 100000) if for_storage: hashed = base64.b64encode(hashed) return hashed + # pylint: disable=no-self-use + def hash_password(self, password: str, for_storage: bool = False) -> bytes: + """Encode a password.""" + hashed = bcrypt.hashpw(password.encode(), bcrypt.gensalt(rounds=12)) \ + # type: bytes + if for_storage: + hashed = base64.b64encode(hashed) + return hashed + def add_auth(self, username: str, password: str) -> None: """Add a new authenticated user/pass.""" if any(user['username'] == username for user in self.users): diff --git a/homeassistant/package_constraints.txt b/homeassistant/package_constraints.txt index fdff380301f..70fb519eef4 100644 --- a/homeassistant/package_constraints.txt +++ b/homeassistant/package_constraints.txt @@ -2,6 +2,7 @@ aiohttp==3.4.0 astral==1.6.1 async_timeout==3.0.0 attrs==18.1.0 +bcrypt==3.1.4 certifi>=2018.04.16 jinja2>=2.10 PyJWT==1.6.4 diff --git a/requirements_all.txt b/requirements_all.txt index 691bbf62246..e1b5b1c70ec 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -3,6 +3,7 @@ aiohttp==3.4.0 astral==1.6.1 async_timeout==3.0.0 attrs==18.1.0 +bcrypt==3.1.4 certifi>=2018.04.16 jinja2>=2.10 PyJWT==1.6.4 diff --git a/setup.py b/setup.py index 8e2ad008cc6..b1b0af70319 100755 --- a/setup.py +++ b/setup.py @@ -36,6 +36,7 @@ REQUIRES = [ 'astral==1.6.1', 'async_timeout==3.0.0', 'attrs==18.1.0', + 'bcrypt==3.1.4', 'certifi>=2018.04.16', 'jinja2>=2.10', 'PyJWT==1.6.4', diff --git a/tests/auth/providers/test_homeassistant.py b/tests/auth/providers/test_homeassistant.py index c92f8539b17..935c5e50dd5 100644 --- a/tests/auth/providers/test_homeassistant.py +++ b/tests/auth/providers/test_homeassistant.py @@ -1,6 +1,7 @@ """Test the Home Assistant local auth provider.""" from unittest.mock import Mock +import base64 import pytest from homeassistant import data_entry_flow @@ -132,3 +133,91 @@ async def test_new_users_populate_values(hass, data): user = await manager.async_get_or_create_user(credentials) assert user.name == 'hello' assert user.is_active + + +async def test_new_hashes_are_bcrypt(data, hass): + """Test that newly created hashes are using bcrypt.""" + data.add_auth('newuser', 'newpass') + found = None + for user in data.users: + if user['username'] == 'newuser': + found = user + assert found is not None + user_hash = base64.b64decode(found['password']) + assert (user_hash.startswith(b'$2a$') + or user_hash.startswith(b'$2b$') + or user_hash.startswith(b'$2x$') + or user_hash.startswith(b'$2y$')) + + +async def test_pbkdf2_to_bcrypt_hash_upgrade(hass_storage, hass): + """Test migrating user from pbkdf2 hash to bcrypt hash.""" + hass_storage[hass_auth.STORAGE_KEY] = { + 'version': hass_auth.STORAGE_VERSION, + 'key': hass_auth.STORAGE_KEY, + 'data': { + 'salt': '09c52f0b120eaa7dea5f73f9a9b985f3d493b30a08f3f2945ef613' + '0b08e6a3ea', + 'users': [ + { + 'password': 'L5PAbehB8LAQI2Ixu+d+PDNJKmljqLnBcYWYw35onC/8D' + 'BM1SpvT6A8ZFael5+deCt+s+43J08IcztnguouHSw==', + 'username': 'legacyuser' + } + ] + }, + } + data = hass_auth.Data(hass) + await data.async_load() + + # verify the correct (pbkdf2) password successfuly authenticates the user + await hass.async_add_executor_job( + data.validate_login, 'legacyuser', 'beer') + + # ...and that the hashes are now bcrypt hashes + user_hash = base64.b64decode( + hass_storage[hass_auth.STORAGE_KEY]['data']['users'][0]['password']) + assert (user_hash.startswith(b'$2a$') + or user_hash.startswith(b'$2b$') + or user_hash.startswith(b'$2x$') + or user_hash.startswith(b'$2y$')) + + +async def test_pbkdf2_to_bcrypt_hash_upgrade_with_incorrect_pass(hass_storage, + hass): + """Test migrating user from pbkdf2 hash to bcrypt hash.""" + hass_storage[hass_auth.STORAGE_KEY] = { + 'version': hass_auth.STORAGE_VERSION, + 'key': hass_auth.STORAGE_KEY, + 'data': { + 'salt': '09c52f0b120eaa7dea5f73f9a9b985f3d493b30a08f3f2945ef613' + '0b08e6a3ea', + 'users': [ + { + 'password': 'L5PAbehB8LAQI2Ixu+d+PDNJKmljqLnBcYWYw35onC/8D' + 'BM1SpvT6A8ZFael5+deCt+s+43J08IcztnguouHSw==', + 'username': 'legacyuser' + } + ] + }, + } + data = hass_auth.Data(hass) + await data.async_load() + + orig_user_hash = base64.b64decode( + hass_storage[hass_auth.STORAGE_KEY]['data']['users'][0]['password']) + + # Make sure invalid legacy passwords fail + with pytest.raises(hass_auth.InvalidAuth): + await hass.async_add_executor_job( + data.validate_login, 'legacyuser', 'wine') + + # Make sure we don't change the password/hash when password is incorrect + with pytest.raises(hass_auth.InvalidAuth): + await hass.async_add_executor_job( + data.validate_login, 'legacyuser', 'wine') + + same_user_hash = base64.b64decode( + hass_storage[hass_auth.STORAGE_KEY]['data']['users'][0]['password']) + + assert orig_user_hash == same_user_hash