From ce67e28f9624dc05d2379f5cff9397e0b6f92e4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gina=20H=C3=A4u=C3=9Fge?= Date: Mon, 27 Oct 2014 09:35:55 +0100 Subject: [PATCH] Generate the salt used for hashing user passwords individually for each server instance --- CHANGELOG.md | 1 + src/octoprint/server/api/__init__.py | 2 +- src/octoprint/settings.py | 1 + src/octoprint/users.py | 42 ++++++++++++++++++++++++++-- 4 files changed, 42 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f9f7ccd1..f0760dd4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ server start and written back into ``config.yaml`` * Event subscriptions are now enabled by default (it was an accident that they weren't) * Generate the key used for session hashing individually for each server instance +* Generate the salt used for hashing user passwords individually for each server instance ### Bug Fixes diff --git a/src/octoprint/server/api/__init__.py b/src/octoprint/server/api/__init__.py index 816d5187..613590a5 100644 --- a/src/octoprint/server/api/__init__.py +++ b/src/octoprint/server/api/__init__.py @@ -120,7 +120,7 @@ def login(): user = octoprint.server.userManager.findUser(username) if user is not None: - if user.check_password(octoprint.users.UserManager.createPasswordHash(password)): + if octoprint.server.userManager.checkPassword(username, password): login_user(user, remember=remember) identity_changed.send(current_app._get_current_object(), identity=Identity(user.get_id())) return jsonify(user.asDict()) diff --git a/src/octoprint/settings.py b/src/octoprint/settings.py index 68ba8b8f..0f70641f 100644 --- a/src/octoprint/settings.py +++ b/src/octoprint/settings.py @@ -114,6 +114,7 @@ default_settings = { }, "accessControl": { "enabled": True, + "salt": None, "userManager": "octoprint.users.FilebasedUserManager", "userfile": None, "autologinLocal": False, diff --git a/src/octoprint/users.py b/src/octoprint/users.py index 58f2f732..dc6e28f8 100644 --- a/src/octoprint/users.py +++ b/src/octoprint/users.py @@ -1,6 +1,9 @@ # coding=utf-8 +from __future__ import absolute_import + __author__ = "Gina Häußge " __license__ = 'GNU Affero General Public License http://www.gnu.org/licenses/agpl.html' +__copyright__ = "Copyright (C) 2014 The OctoPrint Project - Released under terms of the AGPLv3 License" from flask.ext.login import UserMixin from flask.ext.principal import Identity @@ -15,8 +18,38 @@ class UserManager(object): valid_roles = ["user", "admin"] @staticmethod - def createPasswordHash(password): - return hashlib.sha512(password + "mvBUTvwzBzD3yPwvnJ4E4tXNf3CGJvvW").hexdigest() + def createPasswordHash(password, salt=None): + if not salt: + salt = settings().get(["accessControl", "salt"]) + if salt is None: + import string + from random import choice + chars = string.ascii_lowercase + string.ascii_uppercase + string.digits + salt = "".join(choice(chars) for _ in xrange(32)) + settings().set(["accessControl", "salt"], salt) + settings().save() + + return hashlib.sha512(password + salt).hexdigest() + + def checkPassword(self, username, password): + user = self.findUser(username) + if not user: + return False + + hash = UserManager.createPasswordHash(password) + if user.check_password(hash): + # new hash matches, correct password + return True + else: + # new hash doesn't match, but maybe the old one does, so check that! + oldHash = UserManager.createPasswordHash(password, salt="mvBUTvwzBzD3yPwvnJ4E4tXNf3CGJvvW") + if user.check_password(oldHash): + # old hash matches, we migrate the stored password hash to the new one and return True since it's the correct password + self.changeUserPassword(username, password) + return True + else: + # old hash doesn't match either, wrong password + return False def addUser(self, username, password, active, roles): pass @@ -97,7 +130,10 @@ class FilebasedUserManager(UserManager): self._dirty = False self._load() - def addUser(self, username, password, active=False, roles=["user"], apikey=None): + def addUser(self, username, password, active=False, roles=None, apikey=None): + if not roles: + roles = ["user"] + if username in self._users.keys(): raise UserAlreadyExists(username)