From 9363cadb095a769c70fa75d791ddd1f3743b2da5 Mon Sep 17 00:00:00 2001 From: PrajwalM2212 Date: Sat, 20 Feb 2021 12:17:05 -0800 Subject: [PATCH 01/29] Add functionality to hash passwords on server side --- .../cc/environment/user_creds.py | 5 +++-- .../monkey_island/cc/resources/auth/auth.py | 10 ++++++---- .../cc/ui/src/services/AuthService.js | 20 ++++--------------- 3 files changed, 13 insertions(+), 22 deletions(-) diff --git a/monkey/monkey_island/cc/environment/user_creds.py b/monkey/monkey_island/cc/environment/user_creds.py index 98a23a14a..a5c905f70 100644 --- a/monkey/monkey_island/cc/environment/user_creds.py +++ b/monkey/monkey_island/cc/environment/user_creds.py @@ -1,6 +1,7 @@ from __future__ import annotations import json +from hashlib import sha3_512 from typing import Dict from monkey_island.cc.resources.auth.auth_user import User @@ -30,8 +31,8 @@ class UserCreds: creds = UserCreds() if "user" in data_dict: creds.username = data_dict["user"] - if "password_hash" in data_dict: - creds.password_hash = data_dict["password_hash"] + if "password" in data_dict: + creds.password_hash = sha3_512(data_dict["password"].encode("utf-8")).hexdigest() return creds @staticmethod diff --git a/monkey/monkey_island/cc/resources/auth/auth.py b/monkey/monkey_island/cc/resources/auth/auth.py index 29d2d9e89..597c73d60 100644 --- a/monkey/monkey_island/cc/resources/auth/auth.py +++ b/monkey/monkey_island/cc/resources/auth/auth.py @@ -1,6 +1,7 @@ import json import logging from functools import wraps +from hashlib import sha3_512 import flask_jwt_extended import flask_restful @@ -25,7 +26,7 @@ def init_jwt(app): class Authenticate(flask_restful.Resource): """ - Resource for user authentication. The user provides the username and hashed password and we + Resource for user authentication. The user provides the username and password and we give them a JWT. See `AuthService.js` file for the frontend counterpart for this code. """ @@ -33,7 +34,7 @@ class Authenticate(flask_restful.Resource): @staticmethod def _authenticate(username, secret): user = user_store.UserStore.username_table.get(username, None) - if user and safe_str_cmp(user.secret.encode("utf-8"), secret.encode("utf-8")): + if user and safe_str_cmp(user.secret, secret): return user def post(self): @@ -41,13 +42,14 @@ class Authenticate(flask_restful.Resource): Example request: { "username": "my_user", - "password": "343bb87e553b05430e5c44baf99569d4b66..." + "password": "mypassword...." } """ credentials = json.loads(request.data) # Unpack auth info from request username = credentials["username"] - secret = credentials["password"] + password = credentials["password"] + secret = sha3_512(password.encode("utf-8")).hexdigest() # If the user and password have been previously registered if self._authenticate(username, secret): access_token = flask_jwt_extended.create_access_token( diff --git a/monkey/monkey_island/cc/ui/src/services/AuthService.js b/monkey/monkey_island/cc/ui/src/services/AuthService.js index 52658f5a9..27a8100bd 100644 --- a/monkey/monkey_island/cc/ui/src/services/AuthService.js +++ b/monkey/monkey_island/cc/ui/src/services/AuthService.js @@ -2,17 +2,14 @@ import {SHA3} from 'sha3'; import decode from 'jwt-decode'; export default class AuthService { - // SHA3-512 of '1234567890!@#$%^&*()_nothing_up_my_sleeve_1234567890!@#$%^&*()' - NO_AUTH_CREDS = - '55e97c9dcfd22b8079189ddaeea9bce8125887e3237b800c6176c9afa80d2062' + - '8d2c8d0b1538d2208c1444ac66535b764a3d902b35e751df3faec1e477ed3557'; + NO_AUTH_CREDS = 'loginwithoutpassword'; SECONDS_BEFORE_JWT_EXPIRES = 20; AUTHENTICATION_API_ENDPOINT = '/api/auth'; REGISTRATION_API_ENDPOINT = '/api/registration'; login = (username, password) => { - return this._login(username, this.hashSha3(password)); + return this._login(username, password); }; authFetch = (url, options) => { @@ -25,12 +22,6 @@ export default class AuthService { } }; - hashSha3(text) { - let hash = new SHA3(512); - hash.update(text); - return this._toHexStr(hash.digest()); - } - _login = (username, password) => { return this._authFetch(this.AUTHENTICATION_API_ENDPOINT, { method: 'POST', @@ -52,7 +43,7 @@ export default class AuthService { register = (username, password) => { if (password !== '') { - return this._register(username, this.hashSha3(password)); + return this._register(username, password); } else { return this._register(username, password); } @@ -63,7 +54,7 @@ export default class AuthService { method: 'POST', body: JSON.stringify({ 'user': username, - 'password_hash': password + 'password': password }) }).then(res => { if (res.status === 200) { @@ -156,7 +147,4 @@ export default class AuthService { return localStorage.getItem('jwt') } - _toHexStr(byteArr) { - return byteArr.reduce((acc, x) => (acc + ('0' + x.toString(0x10)).slice(-2)), ''); - } } From 2ee6315bb8f8ca683fdd8ad72edebe6579f53fba Mon Sep 17 00:00:00 2001 From: PrajwalM2212 Date: Sat, 20 Feb 2021 13:09:10 -0800 Subject: [PATCH 02/29] Changes --- monkey/monkey_island/cc/resources/auth/auth.py | 2 +- monkey/monkey_island/cc/ui/src/services/AuthService.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/monkey/monkey_island/cc/resources/auth/auth.py b/monkey/monkey_island/cc/resources/auth/auth.py index 597c73d60..43cbf3b0e 100644 --- a/monkey/monkey_island/cc/resources/auth/auth.py +++ b/monkey/monkey_island/cc/resources/auth/auth.py @@ -34,7 +34,7 @@ class Authenticate(flask_restful.Resource): @staticmethod def _authenticate(username, secret): user = user_store.UserStore.username_table.get(username, None) - if user and safe_str_cmp(user.secret, secret): + if user and safe_str_cmp(user.secret.encode('utf-8'), secret.encode('utf-8')): return user def post(self): diff --git a/monkey/monkey_island/cc/ui/src/services/AuthService.js b/monkey/monkey_island/cc/ui/src/services/AuthService.js index 27a8100bd..7a99ba819 100644 --- a/monkey/monkey_island/cc/ui/src/services/AuthService.js +++ b/monkey/monkey_island/cc/ui/src/services/AuthService.js @@ -2,7 +2,7 @@ import {SHA3} from 'sha3'; import decode from 'jwt-decode'; export default class AuthService { - NO_AUTH_CREDS = 'loginwithoutpassword'; + NO_AUTH_CREDS = '1234567890!@#$%^&*()_nothing_up_my_sleeve_1234567890!@#$%^&*()'; SECONDS_BEFORE_JWT_EXPIRES = 20; AUTHENTICATION_API_ENDPOINT = '/api/auth'; From b5236d14c924b68cc7c300754d85366ee91cdf7c Mon Sep 17 00:00:00 2001 From: Shreya Date: Mon, 3 May 2021 19:29:58 +0530 Subject: [PATCH 03/29] Use bcrypt for password hashing for authentication --- monkey/monkey_island/cc/environment/user_creds.py | 7 +++++-- monkey/monkey_island/cc/resources/auth/auth.py | 12 +++++------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/monkey/monkey_island/cc/environment/user_creds.py b/monkey/monkey_island/cc/environment/user_creds.py index a5c905f70..fac911cdd 100644 --- a/monkey/monkey_island/cc/environment/user_creds.py +++ b/monkey/monkey_island/cc/environment/user_creds.py @@ -1,9 +1,10 @@ from __future__ import annotations import json -from hashlib import sha3_512 from typing import Dict +import bcrypt + from monkey_island.cc.resources.auth.auth_user import User @@ -32,7 +33,9 @@ class UserCreds: if "user" in data_dict: creds.username = data_dict["user"] if "password" in data_dict: - creds.password_hash = sha3_512(data_dict["password"].encode("utf-8")).hexdigest() + creds.password_hash = bcrypt.hashpw( + data_dict["password"].encode("utf-8"), bcrypt.gensalt() + ) return creds @staticmethod diff --git a/monkey/monkey_island/cc/resources/auth/auth.py b/monkey/monkey_island/cc/resources/auth/auth.py index 43cbf3b0e..b6221c417 100644 --- a/monkey/monkey_island/cc/resources/auth/auth.py +++ b/monkey/monkey_island/cc/resources/auth/auth.py @@ -1,14 +1,13 @@ import json import logging from functools import wraps -from hashlib import sha3_512 +import bcrypt import flask_jwt_extended import flask_restful from flask import make_response, request from flask_jwt_extended.exceptions import JWTExtendedException from jwt import PyJWTError -from werkzeug.security import safe_str_cmp import monkey_island.cc.environment.environment_singleton as env_singleton import monkey_island.cc.resources.auth.user_store as user_store @@ -32,9 +31,9 @@ class Authenticate(flask_restful.Resource): """ @staticmethod - def _authenticate(username, secret): + def _authenticate(username, password): user = user_store.UserStore.username_table.get(username, None) - if user and safe_str_cmp(user.secret.encode('utf-8'), secret.encode('utf-8')): + if user and bcrypt.checkpw(password.encode("utf-8"), user.secret.encode("utf-8")): return user def post(self): @@ -42,16 +41,15 @@ class Authenticate(flask_restful.Resource): Example request: { "username": "my_user", - "password": "mypassword...." + "password": "my_password" } """ credentials = json.loads(request.data) # Unpack auth info from request username = credentials["username"] password = credentials["password"] - secret = sha3_512(password.encode("utf-8")).hexdigest() # If the user and password have been previously registered - if self._authenticate(username, secret): + if self._authenticate(username, password): access_token = flask_jwt_extended.create_access_token( identity=user_store.UserStore.username_table[username].id ) From 09a37292b585e758c93d9b56f4b2ee1f272e7097 Mon Sep 17 00:00:00 2001 From: Shreya Date: Mon, 3 May 2021 19:32:08 +0530 Subject: [PATCH 04/29] Remove unused import and repeated code in monkey_island/cc/ui/src/services/AuthService.js --- monkey/monkey_island/cc/ui/src/services/AuthService.js | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/monkey/monkey_island/cc/ui/src/services/AuthService.js b/monkey/monkey_island/cc/ui/src/services/AuthService.js index 7a99ba819..d7d1b9c2f 100644 --- a/monkey/monkey_island/cc/ui/src/services/AuthService.js +++ b/monkey/monkey_island/cc/ui/src/services/AuthService.js @@ -1,4 +1,3 @@ -import {SHA3} from 'sha3'; import decode from 'jwt-decode'; export default class AuthService { @@ -42,11 +41,7 @@ export default class AuthService { }; register = (username, password) => { - if (password !== '') { - return this._register(username, password); - } else { - return this._register(username, password); - } + return this._register(username, password); }; _register = (username, password) => { From b5d05a1a7844ad365ff1c82744da2ab2ebefd6bd Mon Sep 17 00:00:00 2001 From: Shreya Date: Mon, 3 May 2021 19:35:10 +0530 Subject: [PATCH 05/29] Add bcrypt to monkey_island/Pipfile and monkey_island/Pipfile.lock --- monkey/monkey_island/Pipfile | 1 + monkey/monkey_island/Pipfile.lock | 15 ++++++++++++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/monkey/monkey_island/Pipfile b/monkey/monkey_island/Pipfile index 70c0f304c..65eeaae4b 100644 --- a/monkey/monkey_island/Pipfile +++ b/monkey/monkey_island/Pipfile @@ -6,6 +6,7 @@ name = "pypi" [packages] pyinstaller = "==3.6" awscli = "==1.18.131" +bcrypt = "==3.2.0" boto3 = "==1.14.54" botocore = "==1.17.54" cffi = ">=1.8,!=1.11.3" diff --git a/monkey/monkey_island/Pipfile.lock b/monkey/monkey_island/Pipfile.lock index 55f5e4502..46479e643 100644 --- a/monkey/monkey_island/Pipfile.lock +++ b/monkey/monkey_island/Pipfile.lock @@ -1,7 +1,7 @@ { "_meta": { "hash": { - "sha256": "4fdfe90af14139cf855a0363ad0acbe7fb307b35b038e2c099c4d1227322a13b" + "sha256": "3057235b1e85593ee307d9e5a2e0d15e26f13437bb709529303c7c900d3c7b41" }, "pipfile-spec": 6, "requires": { @@ -60,6 +60,19 @@ "index": "pypi", "version": "==1.18.131" }, + "bcrypt": { + "hashes": [ + "sha256:5b93c1726e50a93a033c36e5ca7fdcd29a5c7395af50a6892f5d9e7c6cfbfb29", + "sha256:63d4e3ff96188e5898779b6057878fecf3f11cfe6ec3b313ea09955d587ec7a7", + "sha256:81fec756feff5b6818ea7ab031205e1d323d8943d237303baca2c5f9c7846f34", + "sha256:a67fb841b35c28a59cebed05fbd3e80eea26e6d75851f0574a9273c80f3e9b55", + "sha256:c95d4cbebffafcdd28bd28bb4e25b31c50f6da605c81ffd9ad8a3d1b2ab7b1b6", + "sha256:cd1ea2ff3038509ea95f687256c46b79f5fc382ad0aa3664d200047546d511d1", + "sha256:cdcdcb3972027f83fe24a48b1e90ea4b584d35f1cc279d76de6fc4b13376239d" + ], + "index": "pypi", + "version": "==3.2.0" + }, "boto3": { "hashes": [ "sha256:4196b418598851ffd10cf9d1606694673cbfeca4ddf8b25d4e50addbd2fc60bf", From d2083149ddd5bb7d009afff38b2cc23f87e677fe Mon Sep 17 00:00:00 2001 From: Shreya Date: Mon, 3 May 2021 20:08:11 +0530 Subject: [PATCH 06/29] Convert hashed pwd to string before storing in server_config.json --- monkey/monkey_island/cc/environment/user_creds.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/monkey/monkey_island/cc/environment/user_creds.py b/monkey/monkey_island/cc/environment/user_creds.py index fac911cdd..1574f6e8e 100644 --- a/monkey/monkey_island/cc/environment/user_creds.py +++ b/monkey/monkey_island/cc/environment/user_creds.py @@ -35,7 +35,7 @@ class UserCreds: if "password" in data_dict: creds.password_hash = bcrypt.hashpw( data_dict["password"].encode("utf-8"), bcrypt.gensalt() - ) + ).decode() return creds @staticmethod From 02f3b15c6413b147b7a11ce79fe3b875cea9e4ed Mon Sep 17 00:00:00 2001 From: Shreya Date: Mon, 3 May 2021 23:28:55 +0530 Subject: [PATCH 07/29] Split `get_from_dict()` into 2 functions as per usage --- .../cc/environment/environment_config.py | 2 +- monkey/monkey_island/cc/environment/user_creds.py | 13 +++++++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/monkey/monkey_island/cc/environment/environment_config.py b/monkey/monkey_island/cc/environment/environment_config.py index 70d27e546..183d10932 100644 --- a/monkey/monkey_island/cc/environment/environment_config.py +++ b/monkey/monkey_island/cc/environment/environment_config.py @@ -38,7 +38,7 @@ class EnvironmentConfig: self._load_from_dict(data) def _load_from_dict(self, dict_data: Dict): - user_creds = UserCreds.get_from_dict(dict_data) + user_creds = UserCreds.get_from_dict_server_config(dict_data) aws = dict_data["aws"] if "aws" in dict_data else None data_dir = dict_data["data_dir"] if "data_dir" in dict_data else DEFAULT_DATA_DIR diff --git a/monkey/monkey_island/cc/environment/user_creds.py b/monkey/monkey_island/cc/environment/user_creds.py index 1574f6e8e..385083311 100644 --- a/monkey/monkey_island/cc/environment/user_creds.py +++ b/monkey/monkey_island/cc/environment/user_creds.py @@ -28,7 +28,7 @@ class UserCreds: return User(1, self.username, self.password_hash) @staticmethod - def get_from_dict(data_dict: Dict) -> UserCreds: + def get_from_dict_new_registration(data_dict: Dict) -> UserCreds: creds = UserCreds() if "user" in data_dict: creds.username = data_dict["user"] @@ -38,7 +38,16 @@ class UserCreds: ).decode() return creds + @staticmethod + def get_from_dict_server_config(data_dict: Dict) -> UserCreds: + creds = UserCreds() + if "user" in data_dict: + creds.username = data_dict["user"] + if "password_hash" in data_dict: + creds.password_hash = data_dict["password_hash"] + return creds + @staticmethod def get_from_json(json_data: bytes) -> UserCreds: cred_dict = json.loads(json_data) - return UserCreds.get_from_dict(cred_dict) + return UserCreds.get_from_dict_new_registration(cred_dict) From 9c1096daa1f3b67019f3a782dec2ef266c2f5978 Mon Sep 17 00:00:00 2001 From: Shreya Date: Mon, 3 May 2021 23:49:44 +0530 Subject: [PATCH 08/29] Add CHANGELOG entry for bcrypt work --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5bf4916e6..20fb693f9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). collection time. #1102 - Changed default BB test suite: if `--run-performance-tests` flag is not specified, performance tests are skipped. +- Authentication mechanism to use bcrypt on server side. #1139 ### Fixed - Attempted to delete a directory when monkey config reset was called. #1054 From 7684a2dcf8470da546fa557cdac3fc69408ebd74 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 4 May 2021 11:58:58 -0400 Subject: [PATCH 09/29] island: Make return values of Authenticate._authenticate() explicit --- monkey/monkey_island/cc/resources/auth/auth.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/monkey/monkey_island/cc/resources/auth/auth.py b/monkey/monkey_island/cc/resources/auth/auth.py index b6221c417..0d1ac3d30 100644 --- a/monkey/monkey_island/cc/resources/auth/auth.py +++ b/monkey/monkey_island/cc/resources/auth/auth.py @@ -34,7 +34,9 @@ class Authenticate(flask_restful.Resource): def _authenticate(username, password): user = user_store.UserStore.username_table.get(username, None) if user and bcrypt.checkpw(password.encode("utf-8"), user.secret.encode("utf-8")): - return user + return True + + return False def post(self): """ From 83f7f04929c17bb9631afa60b17e438011d36204 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 4 May 2021 12:00:10 -0400 Subject: [PATCH 10/29] island: Change order of methods in Authenticate to follow stepdown rule --- monkey/monkey_island/cc/resources/auth/auth.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/monkey/monkey_island/cc/resources/auth/auth.py b/monkey/monkey_island/cc/resources/auth/auth.py index 0d1ac3d30..4fd9b66a0 100644 --- a/monkey/monkey_island/cc/resources/auth/auth.py +++ b/monkey/monkey_island/cc/resources/auth/auth.py @@ -30,14 +30,6 @@ class Authenticate(flask_restful.Resource): See `AuthService.js` file for the frontend counterpart for this code. """ - @staticmethod - def _authenticate(username, password): - user = user_store.UserStore.username_table.get(username, None) - if user and bcrypt.checkpw(password.encode("utf-8"), user.secret.encode("utf-8")): - return True - - return False - def post(self): """ Example request: @@ -62,6 +54,14 @@ class Authenticate(flask_restful.Resource): else: return make_response({"error": "Invalid credentials"}, 401) + @staticmethod + def _authenticate(username, password): + user = user_store.UserStore.username_table.get(username, None) + if user and bcrypt.checkpw(password.encode("utf-8"), user.secret.encode("utf-8")): + return True + + return False + # See https://flask-jwt-extended.readthedocs.io/en/stable/custom_decorators/ def jwt_required(fn): From 39c274c4d91d9f7af28a5b43847b17c8f1470329 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 4 May 2021 12:03:33 -0400 Subject: [PATCH 11/29] island: Extract method get_credentials_from_request() from post() --- monkey/monkey_island/cc/resources/auth/auth.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/monkey/monkey_island/cc/resources/auth/auth.py b/monkey/monkey_island/cc/resources/auth/auth.py index 4fd9b66a0..0137b5b36 100644 --- a/monkey/monkey_island/cc/resources/auth/auth.py +++ b/monkey/monkey_island/cc/resources/auth/auth.py @@ -38,10 +38,8 @@ class Authenticate(flask_restful.Resource): "password": "my_password" } """ - credentials = json.loads(request.data) - # Unpack auth info from request - username = credentials["username"] - password = credentials["password"] + + (username, password) = Authenticate._get_credentials_from_request(request) # If the user and password have been previously registered if self._authenticate(username, password): access_token = flask_jwt_extended.create_access_token( @@ -54,6 +52,15 @@ class Authenticate(flask_restful.Resource): else: return make_response({"error": "Invalid credentials"}, 401) + @staticmethod + def _get_credentials_from_request(request): + credentials = json.loads(request.data) + + username = credentials["username"] + password = credentials["password"] + + return (username, password) + @staticmethod def _authenticate(username, password): user = user_store.UserStore.username_table.get(username, None) From a8646fc0561960d90e7f41b9e3b77ba86ddd471a Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 4 May 2021 12:09:04 -0400 Subject: [PATCH 12/29] island: Give _authenticate() more descriptive name and remove comment --- monkey/monkey_island/cc/resources/auth/auth.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/monkey/monkey_island/cc/resources/auth/auth.py b/monkey/monkey_island/cc/resources/auth/auth.py index 0137b5b36..a6f395424 100644 --- a/monkey/monkey_island/cc/resources/auth/auth.py +++ b/monkey/monkey_island/cc/resources/auth/auth.py @@ -38,10 +38,9 @@ class Authenticate(flask_restful.Resource): "password": "my_password" } """ - (username, password) = Authenticate._get_credentials_from_request(request) - # If the user and password have been previously registered - if self._authenticate(username, password): + + if self._credentials_match_registered_user(username, password): access_token = flask_jwt_extended.create_access_token( identity=user_store.UserStore.username_table[username].id ) @@ -62,7 +61,7 @@ class Authenticate(flask_restful.Resource): return (username, password) @staticmethod - def _authenticate(username, password): + def _credentials_match_registered_user(username, password): user = user_store.UserStore.username_table.get(username, None) if user and bcrypt.checkpw(password.encode("utf-8"), user.secret.encode("utf-8")): return True From c7d47fee9cdb3a1ec959ffd09ad1688506b49943 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 4 May 2021 12:14:25 -0400 Subject: [PATCH 13/29] island: Extract method _create_access_token() from _get_credentials_from_request() --- monkey/monkey_island/cc/resources/auth/auth.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/monkey/monkey_island/cc/resources/auth/auth.py b/monkey/monkey_island/cc/resources/auth/auth.py index a6f395424..d04b94da4 100644 --- a/monkey/monkey_island/cc/resources/auth/auth.py +++ b/monkey/monkey_island/cc/resources/auth/auth.py @@ -41,12 +41,7 @@ class Authenticate(flask_restful.Resource): (username, password) = Authenticate._get_credentials_from_request(request) if self._credentials_match_registered_user(username, password): - access_token = flask_jwt_extended.create_access_token( - identity=user_store.UserStore.username_table[username].id - ) - logger.debug( - f"Created access token for user {username} that begins with {access_token[:4]}" - ) + access_token = Authenticate._create_access_token(username) return make_response({"access_token": access_token, "error": ""}, 200) else: return make_response({"error": "Invalid credentials"}, 401) @@ -68,6 +63,17 @@ class Authenticate(flask_restful.Resource): return False + @staticmethod + def _create_access_token(username): + access_token = flask_jwt_extended.create_access_token( + identity=user_store.UserStore.username_table[username].id + ) + logger.debug( + f"Created access token for user {username} that begins with {access_token[:4]}" + ) + + return access_token + # See https://flask-jwt-extended.readthedocs.io/en/stable/custom_decorators/ def jwt_required(fn): From 904e51a3652ed24e67b84da8176d85563ec8cb0e Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 4 May 2021 12:28:17 -0400 Subject: [PATCH 14/29] island: Replace private static functions in Authenticator with functions In python, private static methods serve no purpose. Python has first-class functions; let's use them. --- .../monkey_island/cc/resources/auth/auth.py | 46 +++++++++---------- 1 file changed, 22 insertions(+), 24 deletions(-) diff --git a/monkey/monkey_island/cc/resources/auth/auth.py b/monkey/monkey_island/cc/resources/auth/auth.py index d04b94da4..4eedaa61f 100644 --- a/monkey/monkey_island/cc/resources/auth/auth.py +++ b/monkey/monkey_island/cc/resources/auth/auth.py @@ -38,41 +38,39 @@ class Authenticate(flask_restful.Resource): "password": "my_password" } """ - (username, password) = Authenticate._get_credentials_from_request(request) + (username, password) = _get_credentials_from_request(request) - if self._credentials_match_registered_user(username, password): - access_token = Authenticate._create_access_token(username) + if _credentials_match_registered_user(username, password): + access_token = _create_access_token(username) return make_response({"access_token": access_token, "error": ""}, 200) else: return make_response({"error": "Invalid credentials"}, 401) - @staticmethod - def _get_credentials_from_request(request): - credentials = json.loads(request.data) - username = credentials["username"] - password = credentials["password"] +def _get_credentials_from_request(request): + credentials = json.loads(request.data) - return (username, password) + username = credentials["username"] + password = credentials["password"] - @staticmethod - def _credentials_match_registered_user(username, password): - user = user_store.UserStore.username_table.get(username, None) - if user and bcrypt.checkpw(password.encode("utf-8"), user.secret.encode("utf-8")): - return True + return (username, password) - return False - @staticmethod - def _create_access_token(username): - access_token = flask_jwt_extended.create_access_token( - identity=user_store.UserStore.username_table[username].id - ) - logger.debug( - f"Created access token for user {username} that begins with {access_token[:4]}" - ) +def _credentials_match_registered_user(username, password): + user = user_store.UserStore.username_table.get(username, None) + if user and bcrypt.checkpw(password.encode("utf-8"), user.secret.encode("utf-8")): + return True - return access_token + return False + + +def _create_access_token(username): + access_token = flask_jwt_extended.create_access_token( + identity=user_store.UserStore.username_table[username].id + ) + logger.debug(f"Created access token for user {username} that begins with {access_token[:4]}") + + return access_token # See https://flask-jwt-extended.readthedocs.io/en/stable/custom_decorators/ From 060c4b0c40fc76607781ffddb79fa73ab6fe13c6 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 4 May 2021 12:32:07 -0400 Subject: [PATCH 15/29] island: Minor formatting fix --- monkey/monkey_island/cc/resources/auth/auth.py | 1 + 1 file changed, 1 insertion(+) diff --git a/monkey/monkey_island/cc/resources/auth/auth.py b/monkey/monkey_island/cc/resources/auth/auth.py index 4eedaa61f..5ffa4516c 100644 --- a/monkey/monkey_island/cc/resources/auth/auth.py +++ b/monkey/monkey_island/cc/resources/auth/auth.py @@ -58,6 +58,7 @@ def _get_credentials_from_request(request): def _credentials_match_registered_user(username, password): user = user_store.UserStore.username_table.get(username, None) + if user and bcrypt.checkpw(password.encode("utf-8"), user.secret.encode("utf-8")): return True From 502bc3b296ccb9356cd77c2b738315be2ef579d7 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 4 May 2021 14:24:49 -0400 Subject: [PATCH 16/29] island: Enable standard mode with bcrypted passwords --- monkey/monkey_island/cc/environment/standard.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/monkey/monkey_island/cc/environment/standard.py b/monkey/monkey_island/cc/environment/standard.py index 35ca84a34..7cca21c87 100644 --- a/monkey/monkey_island/cc/environment/standard.py +++ b/monkey/monkey_island/cc/environment/standard.py @@ -1,3 +1,5 @@ +import bcrypt + from monkey_island.cc.environment import Environment from monkey_island.cc.resources.auth.auth_user import User @@ -7,11 +9,10 @@ __author__ = "itay.mizeretz" class StandardEnvironment(Environment): _credentials_required = False - # SHA3-512 of '1234567890!@#$%^&*()_nothing_up_my_sleeve_1234567890!@#$%^&*()' - NO_AUTH_CREDS = ( - "55e97c9dcfd22b8079189ddaeea9bce8125887e3237b800c6176c9afa80d2062" - "8d2c8d0b1538d2208c1444ac66535b764a3d902b35e751df3faec1e477ed3557" - ) + NO_AUTH_USER = "1234567890!@#$%^&*()_nothing_up_my_sleeve_1234567890!@#$%^&*()" + NO_AUTH_SECRET = bcrypt.hashpw( + NO_AUTH_USER.encode("utf-8"), b"$2b$12$frH7uEwV3jkDNGgReW6j2u" + ).decode() def get_auth_users(self): - return [User(1, StandardEnvironment.NO_AUTH_CREDS, StandardEnvironment.NO_AUTH_CREDS)] + return [User(1, StandardEnvironment.NO_AUTH_USER, StandardEnvironment.NO_AUTH_SECRET)] From f28cd5305c852cfd6f31cdbb62363cac2979f975 Mon Sep 17 00:00:00 2001 From: VakarisZ Date: Fri, 30 Apr 2021 15:14:44 +0300 Subject: [PATCH 17/29] Refactored test_user_creds.py to pytest from unittests --- .../cc/environment/test_user_creds.py | 46 ++++++++----------- 1 file changed, 18 insertions(+), 28 deletions(-) diff --git a/monkey/tests/monkey_island/cc/environment/test_user_creds.py b/monkey/tests/monkey_island/cc/environment/test_user_creds.py index 93da16e24..f295f245a 100644 --- a/monkey/tests/monkey_island/cc/environment/test_user_creds.py +++ b/monkey/tests/monkey_island/cc/environment/test_user_creds.py @@ -1,37 +1,27 @@ -from unittest import TestCase - from monkey_island.cc.environment.user_creds import UserCreds -class TestUserCreds(TestCase): - def test_to_dict(self): - user_creds = UserCreds() - self.assertDictEqual(user_creds.to_dict(), {}) +def test_to_dict_empty_creds(): + user_creds = UserCreds() + assert user_creds.to_dict() == {} - user_creds = UserCreds(username="Test") - self.assertDictEqual(user_creds.to_dict(), {"user": "Test"}) - user_creds = UserCreds(password_hash="abc1231234") - self.assertDictEqual(user_creds.to_dict(), {"password_hash": "abc1231234"}) +def test_to_dict_full_creds(): + user_creds = UserCreds(username="Test", password_hash="abc1231234") + assert user_creds.to_dict() == {"user": "Test", "password_hash": "abc1231234"} - user_creds = UserCreds(username="Test", password_hash="abc1231234") - self.assertDictEqual(user_creds.to_dict(), {"user": "Test", "password_hash": "abc1231234"}) - def test_to_auth_user(self): - user_creds = UserCreds(username="Test", password_hash="abc1231234") - auth_user = user_creds.to_auth_user() - self.assertEqual(auth_user.id, 1) - self.assertEqual(auth_user.username, "Test") - self.assertEqual(auth_user.secret, "abc1231234") +def test_to_auth_user_full_credentials(): + user_creds = UserCreds(username="Test", password_hash="abc1231234") + auth_user = user_creds.to_auth_user() + assert auth_user.id == 1 + assert auth_user.username == "Test" + assert auth_user.secret == "abc1231234" - user_creds = UserCreds(username="Test") - auth_user = user_creds.to_auth_user() - self.assertEqual(auth_user.id, 1) - self.assertEqual(auth_user.username, "Test") - self.assertEqual(auth_user.secret, "") - user_creds = UserCreds(password_hash="abc1231234") - auth_user = user_creds.to_auth_user() - self.assertEqual(auth_user.id, 1) - self.assertEqual(auth_user.username, "") - self.assertEqual(auth_user.secret, "abc1231234") +def test_to_auth_user_username_only(): + user_creds = UserCreds(username="Test") + auth_user = user_creds.to_auth_user() + assert auth_user.id == 1 + assert auth_user.username == "Test" + assert auth_user.secret == "" From 1be07a4828d3f4a47f7f6a92888bc74e7d2c8872 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 4 May 2021 14:43:11 -0400 Subject: [PATCH 18/29] monkey: Rename `get_from...()` methods in UserCreds to be more readable --- monkey/monkey_island/cc/environment/environment_config.py | 2 +- monkey/monkey_island/cc/environment/user_creds.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/monkey/monkey_island/cc/environment/environment_config.py b/monkey/monkey_island/cc/environment/environment_config.py index 183d10932..65c6e57b8 100644 --- a/monkey/monkey_island/cc/environment/environment_config.py +++ b/monkey/monkey_island/cc/environment/environment_config.py @@ -38,7 +38,7 @@ class EnvironmentConfig: self._load_from_dict(data) def _load_from_dict(self, dict_data: Dict): - user_creds = UserCreds.get_from_dict_server_config(dict_data) + user_creds = UserCreds.get_from_server_config_dict(dict_data) aws = dict_data["aws"] if "aws" in dict_data else None data_dir = dict_data["data_dir"] if "data_dir" in dict_data else DEFAULT_DATA_DIR diff --git a/monkey/monkey_island/cc/environment/user_creds.py b/monkey/monkey_island/cc/environment/user_creds.py index 385083311..dbd7b7422 100644 --- a/monkey/monkey_island/cc/environment/user_creds.py +++ b/monkey/monkey_island/cc/environment/user_creds.py @@ -28,7 +28,7 @@ class UserCreds: return User(1, self.username, self.password_hash) @staticmethod - def get_from_dict_new_registration(data_dict: Dict) -> UserCreds: + def get_from_new_registration_dict(data_dict: Dict) -> UserCreds: creds = UserCreds() if "user" in data_dict: creds.username = data_dict["user"] @@ -39,7 +39,7 @@ class UserCreds: return creds @staticmethod - def get_from_dict_server_config(data_dict: Dict) -> UserCreds: + def get_from_server_config_dict(data_dict: Dict) -> UserCreds: creds = UserCreds() if "user" in data_dict: creds.username = data_dict["user"] @@ -50,4 +50,4 @@ class UserCreds: @staticmethod def get_from_json(json_data: bytes) -> UserCreds: cred_dict = json.loads(json_data) - return UserCreds.get_from_dict_new_registration(cred_dict) + return UserCreds.get_from_new_registration_dict(cred_dict) From 5fa08f04472f6a2dc07339d875545a69ec78971a Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 4 May 2021 15:14:41 -0400 Subject: [PATCH 19/29] island: Add UserCreds.from_cleartext() --- monkey/monkey_island/cc/environment/user_creds.py | 6 ++++++ .../monkey_island/cc/environment/test_user_creds.py | 11 +++++++++++ 2 files changed, 17 insertions(+) diff --git a/monkey/monkey_island/cc/environment/user_creds.py b/monkey/monkey_island/cc/environment/user_creds.py index dbd7b7422..498511675 100644 --- a/monkey/monkey_island/cc/environment/user_creds.py +++ b/monkey/monkey_island/cc/environment/user_creds.py @@ -27,6 +27,12 @@ class UserCreds: def to_auth_user(self) -> User: return User(1, self.username, self.password_hash) + @classmethod + def from_cleartext(cls, username, cleartext_password): + password_hash = bcrypt.hashpw(cleartext_password.encode("utf-8"), bcrypt.gensalt()).decode() + + return cls(username, password_hash) + @staticmethod def get_from_new_registration_dict(data_dict: Dict) -> UserCreds: creds = UserCreds() diff --git a/monkey/tests/monkey_island/cc/environment/test_user_creds.py b/monkey/tests/monkey_island/cc/environment/test_user_creds.py index f295f245a..359bc3741 100644 --- a/monkey/tests/monkey_island/cc/environment/test_user_creds.py +++ b/monkey/tests/monkey_island/cc/environment/test_user_creds.py @@ -1,5 +1,9 @@ +import bcrypt + from monkey_island.cc.environment.user_creds import UserCreds +TEST_SALT = b"$2b$12$JA7GdT1iyfIsquF2cTZv2." + def test_to_dict_empty_creds(): user_creds = UserCreds() @@ -25,3 +29,10 @@ def test_to_auth_user_username_only(): assert auth_user.id == 1 assert auth_user.username == "Test" assert auth_user.secret == "" + + +def test_get_from_cleartext(monkeypatch): + monkeypatch.setattr(bcrypt, "gensalt", lambda: TEST_SALT) + + creds = UserCreds.from_cleartext("Test", "Test_Password") + assert creds.password_hash == "$2b$12$JA7GdT1iyfIsquF2cTZv2.NdGFuYbX1WGfQAOyHlpEsgDTNGZ0TXG" From 4b3b7af3d20eedeb6ae363487067d92afa302dc5 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 4 May 2021 15:17:36 -0400 Subject: [PATCH 20/29] island: Remove coupling between EnvironmentConfig and UserCreds --- .../monkey_island/cc/environment/environment_config.py | 10 ++++++++-- monkey/monkey_island/cc/environment/user_creds.py | 9 --------- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/monkey/monkey_island/cc/environment/environment_config.py b/monkey/monkey_island/cc/environment/environment_config.py index 65c6e57b8..1f9602d22 100644 --- a/monkey/monkey_island/cc/environment/environment_config.py +++ b/monkey/monkey_island/cc/environment/environment_config.py @@ -38,13 +38,12 @@ class EnvironmentConfig: self._load_from_dict(data) def _load_from_dict(self, dict_data: Dict): - user_creds = UserCreds.get_from_server_config_dict(dict_data) aws = dict_data["aws"] if "aws" in dict_data else None data_dir = dict_data["data_dir"] if "data_dir" in dict_data else DEFAULT_DATA_DIR self.server_config = dict_data["server_config"] self.deployment = dict_data["deployment"] - self.user_creds = user_creds + self.user_creds = _get_user_credentials_from_config(dict_data) self.aws = aws self.data_dir = data_dir @@ -75,3 +74,10 @@ class EnvironmentConfig: def get_users(self) -> List[User]: auth_user = self.user_creds.to_auth_user() return [auth_user] if auth_user else [] + + +def _get_user_credentials_from_config(dict_data: Dict): + username = dict_data.get("user", "") + password_hash = dict_data.get("password_hash", "") + + return UserCreds(username, password_hash) diff --git a/monkey/monkey_island/cc/environment/user_creds.py b/monkey/monkey_island/cc/environment/user_creds.py index 498511675..6cd483f70 100644 --- a/monkey/monkey_island/cc/environment/user_creds.py +++ b/monkey/monkey_island/cc/environment/user_creds.py @@ -44,15 +44,6 @@ class UserCreds: ).decode() return creds - @staticmethod - def get_from_server_config_dict(data_dict: Dict) -> UserCreds: - creds = UserCreds() - if "user" in data_dict: - creds.username = data_dict["user"] - if "password_hash" in data_dict: - creds.password_hash = data_dict["password_hash"] - return creds - @staticmethod def get_from_json(json_data: bytes) -> UserCreds: cred_dict = json.loads(json_data) From 1aed5f37d1bc9840da410b06cf60ba1e8c559420 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 4 May 2021 15:27:52 -0400 Subject: [PATCH 21/29] monkey: Remove coupling between Registration and UserCreds --- .../monkey_island/cc/environment/user_creds.py | 17 ----------------- .../cc/resources/auth/registration.py | 14 +++++++++++++- 2 files changed, 13 insertions(+), 18 deletions(-) diff --git a/monkey/monkey_island/cc/environment/user_creds.py b/monkey/monkey_island/cc/environment/user_creds.py index 6cd483f70..a86472cd9 100644 --- a/monkey/monkey_island/cc/environment/user_creds.py +++ b/monkey/monkey_island/cc/environment/user_creds.py @@ -1,6 +1,5 @@ from __future__ import annotations -import json from typing import Dict import bcrypt @@ -32,19 +31,3 @@ class UserCreds: password_hash = bcrypt.hashpw(cleartext_password.encode("utf-8"), bcrypt.gensalt()).decode() return cls(username, password_hash) - - @staticmethod - def get_from_new_registration_dict(data_dict: Dict) -> UserCreds: - creds = UserCreds() - if "user" in data_dict: - creds.username = data_dict["user"] - if "password" in data_dict: - creds.password_hash = bcrypt.hashpw( - data_dict["password"].encode("utf-8"), bcrypt.gensalt() - ).decode() - return creds - - @staticmethod - def get_from_json(json_data: bytes) -> UserCreds: - cred_dict = json.loads(json_data) - return UserCreds.get_from_new_registration_dict(cred_dict) diff --git a/monkey/monkey_island/cc/resources/auth/registration.py b/monkey/monkey_island/cc/resources/auth/registration.py index e5ca99232..8803a7d12 100644 --- a/monkey/monkey_island/cc/resources/auth/registration.py +++ b/monkey/monkey_island/cc/resources/auth/registration.py @@ -1,3 +1,5 @@ +import json + import flask_restful from flask import make_response, request @@ -11,9 +13,19 @@ class Registration(flask_restful.Resource): return {"needs_registration": env_singleton.env.needs_registration()} def post(self): - credentials = UserCreds.get_from_json(request.data) + credentials = _get_user_credentials_from_request(request) + try: env_singleton.env.try_add_user(credentials) return make_response({"error": ""}, 200) except (InvalidRegistrationCredentialsError, RegistrationNotNeededError) as e: return make_response({"error": str(e)}, 400) + + +def _get_user_credentials_from_request(request): + cred_dict = json.loads(request.data) + + username = cred_dict.get("user", "") + password = cred_dict.get("password", "") + + return UserCreds.from_cleartext(username, password) From d56cb5cd754e4bb4a4997be2e124c28af39b115b Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 4 May 2021 16:48:53 -0400 Subject: [PATCH 22/29] island: Simplify UserCreds constructor by removing defaults The default values were only really used by the test code. We can simplify the Usercreds's interface and test code by removing functionality (read: complication) we don't really need. --- monkey/monkey_island/cc/environment/user_creds.py | 2 +- .../monkey_island/cc/environment/test_environment.py | 8 +++++--- .../monkey_island/cc/environment/test_user_creds.py | 10 +--------- 3 files changed, 7 insertions(+), 13 deletions(-) diff --git a/monkey/monkey_island/cc/environment/user_creds.py b/monkey/monkey_island/cc/environment/user_creds.py index a86472cd9..f1989a6a4 100644 --- a/monkey/monkey_island/cc/environment/user_creds.py +++ b/monkey/monkey_island/cc/environment/user_creds.py @@ -8,7 +8,7 @@ from monkey_island.cc.resources.auth.auth_user import User class UserCreds: - def __init__(self, username="", password_hash=""): + def __init__(self, username, password_hash): self.username = username self.password_hash = password_hash diff --git a/monkey/tests/monkey_island/cc/environment/test_environment.py b/monkey/tests/monkey_island/cc/environment/test_environment.py index 0c98d1ccf..8d4c4147e 100644 --- a/monkey/tests/monkey_island/cc/environment/test_environment.py +++ b/monkey/tests/monkey_island/cc/environment/test_environment.py @@ -20,6 +20,8 @@ PARTIAL_CREDENTIALS = None STANDARD_WITH_CREDENTIALS = None STANDARD_ENV = None +EMPTY_CREDS = UserCreds("", "") + # This fixture is a dirty hack that can be removed once these tests are converted from # unittest to pytest. Instead, the appropriate fixtures from conftest.py can be used. @@ -67,7 +69,7 @@ def get_server_config_file_path_test_version(): class TestEnvironment(TestCase): class EnvironmentCredentialsNotRequired(Environment): def __init__(self): - config = StubEnvironmentConfig("test", "test", UserCreds()) + config = StubEnvironmentConfig("test", "test", EMPTY_CREDS) super().__init__(config) _credentials_required = False @@ -77,7 +79,7 @@ class TestEnvironment(TestCase): class EnvironmentCredentialsRequired(Environment): def __init__(self): - config = StubEnvironmentConfig("test", "test", UserCreds()) + config = StubEnvironmentConfig("test", "test", EMPTY_CREDS) super().__init__(config) _credentials_required = True @@ -101,7 +103,7 @@ class TestEnvironment(TestCase): credentials = UserCreds(username="test", password_hash="1231234") env.try_add_user(credentials) - credentials = UserCreds(username="test") + credentials = UserCreds(username="test", password_hash="") with self.assertRaises(InvalidRegistrationCredentialsError): env.try_add_user(credentials) diff --git a/monkey/tests/monkey_island/cc/environment/test_user_creds.py b/monkey/tests/monkey_island/cc/environment/test_user_creds.py index 359bc3741..87286fc60 100644 --- a/monkey/tests/monkey_island/cc/environment/test_user_creds.py +++ b/monkey/tests/monkey_island/cc/environment/test_user_creds.py @@ -6,7 +6,7 @@ TEST_SALT = b"$2b$12$JA7GdT1iyfIsquF2cTZv2." def test_to_dict_empty_creds(): - user_creds = UserCreds() + user_creds = UserCreds("", "") assert user_creds.to_dict() == {} @@ -23,14 +23,6 @@ def test_to_auth_user_full_credentials(): assert auth_user.secret == "abc1231234" -def test_to_auth_user_username_only(): - user_creds = UserCreds(username="Test") - auth_user = user_creds.to_auth_user() - assert auth_user.id == 1 - assert auth_user.username == "Test" - assert auth_user.secret == "" - - def test_get_from_cleartext(monkeypatch): monkeypatch.setattr(bcrypt, "gensalt", lambda: TEST_SALT) From e223126c1611887d8fc412730a07ddffbb7496f5 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 4 May 2021 17:14:54 -0400 Subject: [PATCH 23/29] island: Add tests for UserCreds.__bool__() --- .../cc/environment/test_user_creds.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/monkey/tests/monkey_island/cc/environment/test_user_creds.py b/monkey/tests/monkey_island/cc/environment/test_user_creds.py index 87286fc60..16fa9ee61 100644 --- a/monkey/tests/monkey_island/cc/environment/test_user_creds.py +++ b/monkey/tests/monkey_island/cc/environment/test_user_creds.py @@ -5,6 +5,22 @@ from monkey_island.cc.environment.user_creds import UserCreds TEST_SALT = b"$2b$12$JA7GdT1iyfIsquF2cTZv2." +def test_bool_true(): + assert UserCreds("Test", "abc1231234") + + +def test_bool_false_empty_password_hash(): + assert not UserCreds("Test", "") + + +def test_bool_false_empty_user(): + assert not UserCreds("", "abc1231234") + + +def test_bool_false_empty_user_and_password_hash(): + assert not UserCreds("", "") + + def test_to_dict_empty_creds(): user_creds = UserCreds("", "") assert user_creds.to_dict() == {} From c4c0b7217d0a7b27724a735481930be28b73acb4 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 4 May 2021 17:17:54 -0400 Subject: [PATCH 24/29] island: Add test for members of UserCreds --- .../tests/monkey_island/cc/environment/test_user_creds.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/monkey/tests/monkey_island/cc/environment/test_user_creds.py b/monkey/tests/monkey_island/cc/environment/test_user_creds.py index 16fa9ee61..0ea6a6464 100644 --- a/monkey/tests/monkey_island/cc/environment/test_user_creds.py +++ b/monkey/tests/monkey_island/cc/environment/test_user_creds.py @@ -44,3 +44,11 @@ def test_get_from_cleartext(monkeypatch): creds = UserCreds.from_cleartext("Test", "Test_Password") assert creds.password_hash == "$2b$12$JA7GdT1iyfIsquF2cTZv2.NdGFuYbX1WGfQAOyHlpEsgDTNGZ0TXG" + + +def test_member_values(monkeypatch): + monkeypatch.setattr(bcrypt, "gensalt", lambda: TEST_SALT) + + creds = UserCreds("Test", "abc1231234") + assert creds.username == "Test" + assert creds.password_hash == "abc1231234" From e4dec5501ec64b119fc84513a84cfc2ca903b7f5 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 4 May 2021 17:20:03 -0400 Subject: [PATCH 25/29] island: Add constants for user and hash to UserCreds tests --- .../cc/environment/test_user_creds.py | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/monkey/tests/monkey_island/cc/environment/test_user_creds.py b/monkey/tests/monkey_island/cc/environment/test_user_creds.py index 0ea6a6464..3d1f2ea34 100644 --- a/monkey/tests/monkey_island/cc/environment/test_user_creds.py +++ b/monkey/tests/monkey_island/cc/environment/test_user_creds.py @@ -2,19 +2,21 @@ import bcrypt from monkey_island.cc.environment.user_creds import UserCreds +TEST_USER = "Test" +TEST_HASH = "abc1231234" TEST_SALT = b"$2b$12$JA7GdT1iyfIsquF2cTZv2." def test_bool_true(): - assert UserCreds("Test", "abc1231234") + assert UserCreds(TEST_USER, TEST_HASH) def test_bool_false_empty_password_hash(): - assert not UserCreds("Test", "") + assert not UserCreds(TEST_USER, "") def test_bool_false_empty_user(): - assert not UserCreds("", "abc1231234") + assert not UserCreds("", TEST_HASH) def test_bool_false_empty_user_and_password_hash(): @@ -27,28 +29,28 @@ def test_to_dict_empty_creds(): def test_to_dict_full_creds(): - user_creds = UserCreds(username="Test", password_hash="abc1231234") - assert user_creds.to_dict() == {"user": "Test", "password_hash": "abc1231234"} + user_creds = UserCreds(username=TEST_USER, password_hash=TEST_HASH) + assert user_creds.to_dict() == {"user": TEST_USER, "password_hash": TEST_HASH} def test_to_auth_user_full_credentials(): - user_creds = UserCreds(username="Test", password_hash="abc1231234") + user_creds = UserCreds(username=TEST_USER, password_hash=TEST_HASH) auth_user = user_creds.to_auth_user() assert auth_user.id == 1 - assert auth_user.username == "Test" - assert auth_user.secret == "abc1231234" + assert auth_user.username == TEST_USER + assert auth_user.secret == TEST_HASH def test_get_from_cleartext(monkeypatch): monkeypatch.setattr(bcrypt, "gensalt", lambda: TEST_SALT) - creds = UserCreds.from_cleartext("Test", "Test_Password") + creds = UserCreds.from_cleartext(TEST_USER, "Test_Password") assert creds.password_hash == "$2b$12$JA7GdT1iyfIsquF2cTZv2.NdGFuYbX1WGfQAOyHlpEsgDTNGZ0TXG" def test_member_values(monkeypatch): monkeypatch.setattr(bcrypt, "gensalt", lambda: TEST_SALT) - creds = UserCreds("Test", "abc1231234") - assert creds.username == "Test" - assert creds.password_hash == "abc1231234" + creds = UserCreds(TEST_USER, TEST_HASH) + assert creds.username == TEST_USER + assert creds.password_hash == TEST_HASH From f73b048169078ed5bf3ce8b45eecbfeae0ca8be3 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 4 May 2021 17:21:05 -0400 Subject: [PATCH 26/29] island: Remove parameter names from UserCreds() init in tests --- monkey/tests/monkey_island/cc/environment/test_user_creds.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/monkey/tests/monkey_island/cc/environment/test_user_creds.py b/monkey/tests/monkey_island/cc/environment/test_user_creds.py index 3d1f2ea34..60bb2afa2 100644 --- a/monkey/tests/monkey_island/cc/environment/test_user_creds.py +++ b/monkey/tests/monkey_island/cc/environment/test_user_creds.py @@ -29,12 +29,12 @@ def test_to_dict_empty_creds(): def test_to_dict_full_creds(): - user_creds = UserCreds(username=TEST_USER, password_hash=TEST_HASH) + user_creds = UserCreds(TEST_USER, TEST_HASH) assert user_creds.to_dict() == {"user": TEST_USER, "password_hash": TEST_HASH} def test_to_auth_user_full_credentials(): - user_creds = UserCreds(username=TEST_USER, password_hash=TEST_HASH) + user_creds = UserCreds(TEST_USER, TEST_HASH) auth_user = user_creds.to_auth_user() assert auth_user.id == 1 assert auth_user.username == TEST_USER From 0f49a2c96ae8d6b6b092c7d057e8a275a7d3de21 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 4 May 2021 18:53:43 -0400 Subject: [PATCH 27/29] island: Remove UserCreds.from_cleartext() --- monkey/monkey_island/cc/environment/user_creds.py | 8 -------- .../monkey_island/cc/resources/auth/registration.py | 4 +++- .../monkey_island/cc/environment/test_user_creds.py | 11 ----------- 3 files changed, 3 insertions(+), 20 deletions(-) diff --git a/monkey/monkey_island/cc/environment/user_creds.py b/monkey/monkey_island/cc/environment/user_creds.py index f1989a6a4..aba349f2d 100644 --- a/monkey/monkey_island/cc/environment/user_creds.py +++ b/monkey/monkey_island/cc/environment/user_creds.py @@ -2,8 +2,6 @@ from __future__ import annotations from typing import Dict -import bcrypt - from monkey_island.cc.resources.auth.auth_user import User @@ -25,9 +23,3 @@ class UserCreds: def to_auth_user(self) -> User: return User(1, self.username, self.password_hash) - - @classmethod - def from_cleartext(cls, username, cleartext_password): - password_hash = bcrypt.hashpw(cleartext_password.encode("utf-8"), bcrypt.gensalt()).decode() - - return cls(username, password_hash) diff --git a/monkey/monkey_island/cc/resources/auth/registration.py b/monkey/monkey_island/cc/resources/auth/registration.py index 8803a7d12..8c7ca5054 100644 --- a/monkey/monkey_island/cc/resources/auth/registration.py +++ b/monkey/monkey_island/cc/resources/auth/registration.py @@ -1,5 +1,6 @@ import json +import bcrypt import flask_restful from flask import make_response, request @@ -27,5 +28,6 @@ def _get_user_credentials_from_request(request): username = cred_dict.get("user", "") password = cred_dict.get("password", "") + password_hash = bcrypt.hashpw(password.encode("utf-8"), bcrypt.gensalt()).decode() - return UserCreds.from_cleartext(username, password) + return UserCreds(username, password_hash) diff --git a/monkey/tests/monkey_island/cc/environment/test_user_creds.py b/monkey/tests/monkey_island/cc/environment/test_user_creds.py index 60bb2afa2..802c13416 100644 --- a/monkey/tests/monkey_island/cc/environment/test_user_creds.py +++ b/monkey/tests/monkey_island/cc/environment/test_user_creds.py @@ -1,5 +1,3 @@ -import bcrypt - from monkey_island.cc.environment.user_creds import UserCreds TEST_USER = "Test" @@ -41,16 +39,7 @@ def test_to_auth_user_full_credentials(): assert auth_user.secret == TEST_HASH -def test_get_from_cleartext(monkeypatch): - monkeypatch.setattr(bcrypt, "gensalt", lambda: TEST_SALT) - - creds = UserCreds.from_cleartext(TEST_USER, "Test_Password") - assert creds.password_hash == "$2b$12$JA7GdT1iyfIsquF2cTZv2.NdGFuYbX1WGfQAOyHlpEsgDTNGZ0TXG" - - def test_member_values(monkeypatch): - monkeypatch.setattr(bcrypt, "gensalt", lambda: TEST_SALT) - creds = UserCreds(TEST_USER, TEST_HASH) assert creds.username == TEST_USER assert creds.password_hash == TEST_HASH From 9024a512b01b89be0529af2a8bd347c75f6db14f Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Tue, 4 May 2021 21:26:06 -0400 Subject: [PATCH 28/29] island: Move all bcrypt dependencies to password_utils --- monkey/monkey_island/cc/environment/standard.py | 6 +----- monkey/monkey_island/cc/resources/auth/auth.py | 4 ++-- .../cc/resources/auth/password_utils.py | 12 ++++++++++++ .../monkey_island/cc/resources/auth/registration.py | 4 ++-- 4 files changed, 17 insertions(+), 9 deletions(-) create mode 100644 monkey/monkey_island/cc/resources/auth/password_utils.py diff --git a/monkey/monkey_island/cc/environment/standard.py b/monkey/monkey_island/cc/environment/standard.py index 7cca21c87..2f337fbf0 100644 --- a/monkey/monkey_island/cc/environment/standard.py +++ b/monkey/monkey_island/cc/environment/standard.py @@ -1,5 +1,3 @@ -import bcrypt - from monkey_island.cc.environment import Environment from monkey_island.cc.resources.auth.auth_user import User @@ -10,9 +8,7 @@ class StandardEnvironment(Environment): _credentials_required = False NO_AUTH_USER = "1234567890!@#$%^&*()_nothing_up_my_sleeve_1234567890!@#$%^&*()" - NO_AUTH_SECRET = bcrypt.hashpw( - NO_AUTH_USER.encode("utf-8"), b"$2b$12$frH7uEwV3jkDNGgReW6j2u" - ).decode() + NO_AUTH_SECRET = "$2b$12$frH7uEwV3jkDNGgReW6j2udw8hy/Yw1SWAqytrcBYK48kn1V5lQIa" def get_auth_users(self): return [User(1, StandardEnvironment.NO_AUTH_USER, StandardEnvironment.NO_AUTH_SECRET)] diff --git a/monkey/monkey_island/cc/resources/auth/auth.py b/monkey/monkey_island/cc/resources/auth/auth.py index 5ffa4516c..064395eaf 100644 --- a/monkey/monkey_island/cc/resources/auth/auth.py +++ b/monkey/monkey_island/cc/resources/auth/auth.py @@ -2,7 +2,6 @@ import json import logging from functools import wraps -import bcrypt import flask_jwt_extended import flask_restful from flask import make_response, request @@ -10,6 +9,7 @@ from flask_jwt_extended.exceptions import JWTExtendedException from jwt import PyJWTError import monkey_island.cc.environment.environment_singleton as env_singleton +import monkey_island.cc.resources.auth.password_utils as password_utils import monkey_island.cc.resources.auth.user_store as user_store logger = logging.getLogger(__name__) @@ -59,7 +59,7 @@ def _get_credentials_from_request(request): def _credentials_match_registered_user(username, password): user = user_store.UserStore.username_table.get(username, None) - if user and bcrypt.checkpw(password.encode("utf-8"), user.secret.encode("utf-8")): + if user and password_utils.password_matches_hash(password, user.secret): return True return False diff --git a/monkey/monkey_island/cc/resources/auth/password_utils.py b/monkey/monkey_island/cc/resources/auth/password_utils.py new file mode 100644 index 000000000..f470fd882 --- /dev/null +++ b/monkey/monkey_island/cc/resources/auth/password_utils.py @@ -0,0 +1,12 @@ +import bcrypt + + +def hash_password(plaintext_password): + salt = bcrypt.gensalt() + password_hash = bcrypt.hashpw(plaintext_password.encode("utf-8"), salt) + + return password_hash.decode() + + +def password_matches_hash(plaintext_password, password_hash): + return bcrypt.checkpw(plaintext_password.encode("utf-8"), password_hash.encode("utf-8")) diff --git a/monkey/monkey_island/cc/resources/auth/registration.py b/monkey/monkey_island/cc/resources/auth/registration.py index 8c7ca5054..121b03d71 100644 --- a/monkey/monkey_island/cc/resources/auth/registration.py +++ b/monkey/monkey_island/cc/resources/auth/registration.py @@ -1,10 +1,10 @@ import json -import bcrypt import flask_restful from flask import make_response, request import monkey_island.cc.environment.environment_singleton as env_singleton +import monkey_island.cc.resources.auth.password_utils as password_utils from common.utils.exceptions import InvalidRegistrationCredentialsError, RegistrationNotNeededError from monkey_island.cc.environment.user_creds import UserCreds @@ -28,6 +28,6 @@ def _get_user_credentials_from_request(request): username = cred_dict.get("user", "") password = cred_dict.get("password", "") - password_hash = bcrypt.hashpw(password.encode("utf-8"), bcrypt.gensalt()).decode() + password_hash = password_utils.hash_password(password) return UserCreds(username, password_hash) From 7772ea6e4e76ef2477f27355fd982d9c9dbab918 Mon Sep 17 00:00:00 2001 From: Mike Salvatore Date: Wed, 5 May 2021 07:21:46 -0400 Subject: [PATCH 29/29] island: Add FULL_USER_CREDENTIALS to test_environment.py --- .../monkey_island/cc/environment/test_environment.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/monkey/tests/monkey_island/cc/environment/test_environment.py b/monkey/tests/monkey_island/cc/environment/test_environment.py index 8d4c4147e..6b648ddee 100644 --- a/monkey/tests/monkey_island/cc/environment/test_environment.py +++ b/monkey/tests/monkey_island/cc/environment/test_environment.py @@ -20,7 +20,8 @@ PARTIAL_CREDENTIALS = None STANDARD_WITH_CREDENTIALS = None STANDARD_ENV = None -EMPTY_CREDS = UserCreds("", "") +EMPTY_USER_CREDENTIALS = UserCreds("", "") +FULL_USER_CREDENTIALS = UserCreds(username="test", password_hash="1231234") # This fixture is a dirty hack that can be removed once these tests are converted from @@ -69,7 +70,7 @@ def get_server_config_file_path_test_version(): class TestEnvironment(TestCase): class EnvironmentCredentialsNotRequired(Environment): def __init__(self): - config = StubEnvironmentConfig("test", "test", EMPTY_CREDS) + config = StubEnvironmentConfig("test", "test", EMPTY_USER_CREDENTIALS) super().__init__(config) _credentials_required = False @@ -79,7 +80,7 @@ class TestEnvironment(TestCase): class EnvironmentCredentialsRequired(Environment): def __init__(self): - config = StubEnvironmentConfig("test", "test", EMPTY_CREDS) + config = StubEnvironmentConfig("test", "test", EMPTY_USER_CREDENTIALS) super().__init__(config) _credentials_required = True @@ -100,7 +101,7 @@ class TestEnvironment(TestCase): @patch.object(target=EnvironmentConfig, attribute="save_to_file", new=MagicMock()) def test_try_add_user(self): env = TestEnvironment.EnvironmentCredentialsRequired() - credentials = UserCreds(username="test", password_hash="1231234") + credentials = FULL_USER_CREDENTIALS env.try_add_user(credentials) credentials = UserCreds(username="test", password_hash="") @@ -108,7 +109,7 @@ class TestEnvironment(TestCase): env.try_add_user(credentials) env = TestEnvironment.EnvironmentCredentialsNotRequired() - credentials = UserCreds(username="test", password_hash="1231234") + credentials = FULL_USER_CREDENTIALS with self.assertRaises(RegistrationNotNeededError): env.try_add_user(credentials)