Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Key derivation #889

Merged
merged 9 commits into from
Feb 17, 2022
6 changes: 6 additions & 0 deletions .github/pull_request_template.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Before submitting a pr:
- [ ] Tests passing
- [ ] Black formatting
- [ ] Rebase/merge the `dev` branch
- [ ] Note in the CHANGELOG

5 changes: 4 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,2 +1,5 @@
# Data Delivery System Web / API: Changelog
Please add a _short_ line describing the PR you make, if the PR implements a specific feature or functionality, or refactor. Not needed if you add very small and unnoticable changes.
Please add a _short_ line describing the PR you make, if the PR implements a specific feature or functionality, or refactor. Not needed if you add very small and unnoticable changes.

## Sprint (2022-02-09 - 2022-02-23)
* Secure operations that require cryptographic keys are protected for each user with the user's password ([#889](https://github.com/ScilifelabDataCentre/dds_web/pull/889))
10 changes: 9 additions & 1 deletion dds_web/api/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,15 @@ def get(self):
flask.current_app.logger.debug("Getting the private key.")

return flask.jsonify(
{"private": obtain_project_private_key(auth.current_user(), project).hex().upper()}
{
"private": obtain_project_private_key(
user=auth.current_user(),
project=project,
token=dds_web.security.auth.obtain_current_encrypted_token(),
)
.hex()
.upper()
}
)


Expand Down
9 changes: 2 additions & 7 deletions dds_web/api/schemas/user_schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from dds_web import auth, db, utils
from dds_web import errors as ddserr
from dds_web.database import models
from dds_web.security.project_user_keys import transfer_invite_private_key_to_user
from dds_web.security.project_user_keys import verify_and_transfer_invite_to_user

####################################################################################################
# SCHEMAS ################################################################################ SCHEMAS #
Expand Down Expand Up @@ -196,9 +196,7 @@ def make_user(self, data, **kwargs):
db.session.add(new_user)

# Verify and transfer invite keys to the new user
temporary_key = dds_web.security.auth.verify_invite_key(token)
if temporary_key:
transfer_invite_private_key_to_user(invite, temporary_key, new_user)
if verify_and_transfer_invite_to_user(token, new_user, data.get("password")):
for project_invite_key in invite.project_invite_keys:
project_user_key = models.ProjectUserKeys(
project_id=project_invite_key.project_id,
Expand All @@ -208,9 +206,6 @@ def make_user(self, data, **kwargs):
db.session.add(project_user_key)
db.session.delete(project_invite_key)

# TODO decrypt the user private key using the temp key,
# derive a key from the password and encrypt the user private key with the derived key

flask.session.pop("invite_token", None)

# Delete old invite
Expand Down
25 changes: 17 additions & 8 deletions dds_web/api/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,12 @@ def invite_user(args):
auth.current_user().unit.invites.append(new_invite)
for project in auth.current_user().unit.projects:
if project.is_active:
share_project_private_key(auth.current_user(), new_invite, project)
share_project_private_key(
from_user=auth.current_user(),
to_another=new_invite,
from_user_token=dds_web.security.auth.obtain_current_encrypted_token(),
project=project,
)
else:
db.session.add(new_invite)

Expand Down Expand Up @@ -188,7 +193,12 @@ def add_user_to_project(existing_user, project, role):
owner=owner,
)
)
share_project_private_key(auth.current_user(), existing_user, project)
share_project_private_key(
from_user=auth.current_user(),
to_another=existing_user,
from_user_token=dds_web.security.auth.obtain_current_encrypted_token(),
project=project,
)

try:
db.session.commit()
Expand Down Expand Up @@ -457,7 +467,7 @@ def post(self):
db.session.commit()
except sqlalchemy.exc.SQLAlchemyError as err:
db.session.rollback()
raise DatabaseError(message=str(err))
raise ddserr.DatabaseError(message=str(err))
msg = (
f"The user account {user.username} ({user_email_str}, {user.role}) "
f" has been {action}d successfully been by {current_user.name} ({current_user.role})."
Expand Down Expand Up @@ -542,7 +552,7 @@ def delete_user(user):
db.session.commit()
except sqlalchemy.exc.SQLAlchemyError as err:
db.session.rollback()
raise DatabaseError(message=str(err))
raise ddserr.DatabaseError(message=str(err))


class RemoveUserAssociation(flask_restful.Resource):
Expand Down Expand Up @@ -618,7 +628,8 @@ def get(self):
return {
"message": "Please take this token to /user/second_factor to authenticate with MFA!",
"token": encrypted_jwt_token(
username=auth.current_user().username, sensitive_content=None
username=auth.current_user().username,
sensitive_content=flask.request.authorization.get("password"),
),
}

Expand All @@ -633,9 +644,7 @@ def get(self):

token_schemas.TokenSchema().load(args)

token_claims = dds_web.security.auth.decrypt_and_verify_token_signature(
flask.request.headers["Authorization"].split()[1]
)
token_claims = dds_web.security.auth.obtain_current_encrypted_token_claims()

return {"token": update_token_with_mfa(token_claims)}

Expand Down
2 changes: 1 addition & 1 deletion dds_web/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ class Config(object):
INVITATION_EXPIRES_IN_HOURS = 7 * 24

# 512MiB; at least 4GiB (0x400000) recommended in production
ARGON_MEMORY_COST = os.environ.get("ARGON_MEMORY_COST", 0x80000)
ARGON_KD_MEMORY_COST = os.environ.get("ARGON_KD_MEMORY_COST", 0x80000)

SUPERADMIN_USERNAME = os.environ.get("DDS_SUPERADMIN_USERNAME", "superadmin")
SUPERADMIN_PASSWORD = os.environ.get("DDS_SUPERADMIN_PASSWORD", "password")
Expand Down
12 changes: 8 additions & 4 deletions dds_web/database/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,6 @@ class User(flask_login.UserMixin, db.Model):
hotp_issue_time = db.Column(db.DateTime, unique=False, nullable=True)
active = db.Column(db.Boolean)
kd_salt = db.Column(db.LargeBinary(32), default=None)
temporary_key = db.Column(db.LargeBinary(32), default=None)
nonce = db.Column(db.LargeBinary(12), default=None)
public_key = db.Column(db.LargeBinary(300), default=None)
private_key = db.Column(db.LargeBinary(300), default=None)
Expand Down Expand Up @@ -389,8 +388,6 @@ def __init__(self, **kwargs):
super(User, self).__init__(**kwargs)
if not self.hotp_secret:
self.hotp_secret = os.urandom(20)
if not self.public_key or not self.private_key:
generate_user_key_pair(self)

def get_id(self):
"""Get user id - in this case username. Used by flask_login."""
Expand All @@ -407,7 +404,14 @@ def password(self, plaintext_password):
"""Generate the password hash and save in db."""
pw_hasher = argon2.PasswordHasher(hash_len=32)
self._password_hash = pw_hasher.hash(plaintext_password)
self.kd_salt = os.urandom(32)

# User key pair should only be set from here if the password is lost
# and all the keys associated with the user should be cleaned up
# before setting the password.
# This should help the tests for setup as well.
if not self.public_key or not self.private_key:
self.kd_salt = os.urandom(32)
generate_user_key_pair(self, plaintext_password)

def verify_password(self, input_password):
"""Verifies that the specified password matches the encoded password in the database."""
Expand Down
21 changes: 19 additions & 2 deletions dds_web/development/db_init.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
generate_project_key_pair,
share_project_private_key,
)
from dds_web.security.tokens import encrypted_jwt_token
import dds_web.utils

####################################################################################################
Expand Down Expand Up @@ -154,7 +155,23 @@ def fill_db():

db.session.commit()

share_project_private_key(unituser_1, researchuser_1, project_1)
share_project_private_key(unituser_1, researchuser_2, project_1)
unituser_1_token = encrypted_jwt_token(
username=unituser_1.username,
sensitive_content=password,
)

share_project_private_key(
from_user=unituser_1,
to_another=researchuser_1,
from_user_token=unituser_1_token,
project=project_1,
)

share_project_private_key(
from_user=unituser_1,
to_another=researchuser_2,
from_user_token=unituser_1_token,
project=project_1,
)

db.session.commit()
48 changes: 45 additions & 3 deletions dds_web/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,12 @@
# Installed
from werkzeug import exceptions
import flask
import dds_web
import flask_login
import http
import json
import structlog

# Own modules
from dds_web import actions, auth
from dds_web import auth

####################################################################################################
# LOGGING ################################################################################ LOGGING #
Expand Down Expand Up @@ -73,6 +71,50 @@ def __init__(self, encryption_key_char_length):
general_logger.error(message)


class TokenMissingError(LoggedHTTPException):
"""Errors due to missing token."""

code = http.HTTPStatus.BAD_REQUEST

def __init__(self, message="Token is missing"):
super().__init__(message)

general_logger.warning(message)


class SensitiveContentMissingError(LoggedHTTPException):
"""Errors due to missing sensitive content in the encrypted token."""

code = http.HTTPStatus.BAD_REQUEST

def __init__(self, message="Sensitive content is missing in the encrypted token!"):
super().__init__(message)

general_logger.warning(message)


class KeySetupError(LoggedHTTPException):
"""Errors due to missing keys."""

code = http.HTTPStatus.INTERNAL_SERVER_ERROR

def __init__(self, message="Keys are not properly setup!"):
super().__init__(message)

general_logger.warning(message)


class KeyOperationError(LoggedHTTPException):
"""Errors due to issues in key operations."""

code = http.HTTPStatus.INTERNAL_SERVER_ERROR

def __init__(self, message="A key cannot be processed!"):
super().__init__(message)

general_logger.warning(message)


class AuthenticationError(LoggedHTTPException):
"""Base class for errors due to authentication failure."""

Expand Down
3 changes: 0 additions & 3 deletions dds_web/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,14 @@
# IMPORTS ################################################################################ IMPORTS #

# Standard library
import re

# Installed
import flask_wtf
import flask_login
import wtforms
import marshmallow

# Own modules
from dds_web import utils
from dds_web.database import models

# FORMS #################################################################################### FORMS #

Expand Down
41 changes: 29 additions & 12 deletions dds_web/security/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,8 @@

# Own modules
from dds_web import basic_auth, auth, mail
from dds_web.errors import AuthenticationError, AccessDeniedError, InviteError
from dds_web.errors import AuthenticationError, AccessDeniedError, InviteError, TokenMissingError
from dds_web.database import models
from dds_web.security.project_user_keys import verify_invite_temporary_key
import dds_web.utils

action_logger = structlog.getLogger("actions")
Expand Down Expand Up @@ -135,8 +134,11 @@ def matching_email_with_invite(token, email):
return claims.get("inv") == email


def verify_invite_key(token):
"""Verify token, email, invite and temporary key."""
def extract_token_invite_key(token):
"""Verify token, email and invite.

Return invite and temporary key.
"""
claims = __base_verify_token_for_invite(token=token)

# Verify email in token
Expand All @@ -149,19 +151,32 @@ def verify_invite_key(token):
if not invite:
raise InviteError(message="Invite could not be found!")

# Verify temporary key from token claims
temporary_key = bytes.fromhex(claims.get("sen_con"))
if verify_invite_temporary_key(invite=invite, temporary_key=temporary_key):
return temporary_key
try:
return invite, bytes.fromhex(claims.get("sen_con"))
except ValueError:
raise ValueError("Temporary key is expected be in hexadecimal digits for a byte string.")
i-oden marked this conversation as resolved.
Show resolved Hide resolved


def obtain_current_encrypted_token():
try:
return flask.request.headers["Authorization"].split()[1]
except KeyError:
raise TokenMissingError("Encrypted token is required but missing!")


def obtain_current_encrypted_token_claims():
token = obtain_current_encrypted_token()
if token:
return decrypt_and_verify_token_signature(token)


@auth.verify_token
def verify_token(token):
"""Verify token used in token authencation."""
"""Verify token used in token authentication."""
claims = __verify_general_token(token=token)
user = __user_from_subject(subject=claims.get("sub"))

return handle_multi_factor_authentication(
return __handle_multi_factor_authentication(
user=user, mfa_auth_time_string=claims.get("mfa_auth_time")
)

Expand Down Expand Up @@ -209,7 +224,7 @@ def __user_from_subject(subject):
return user


def handle_multi_factor_authentication(user, mfa_auth_time_string):
def __handle_multi_factor_authentication(user, mfa_auth_time_string):
"""Verify multifactor authentication time frame."""
if user:
if mfa_auth_time_string:
Expand Down Expand Up @@ -245,8 +260,10 @@ def send_hotp_email(user):
return False


def extract_encrypted_token_content(token, username):
def extract_encrypted_token_sensitive_content(token, username):
"""Extract the sensitive content from inside the encrypted token."""
if token is None:
raise TokenMissingError(message="There is no token to extract sensitive content from.")
content = decrypt_and_verify_token_signature(token=token)
if content.get("sub") == username:
return content.get("sen_con")
Expand Down
Loading