From 00b3a3b0a962fbd11e16710f86a358bc7c6e8837 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Sp=C3=B6ttel?= <1682504+fspoettel@users.noreply.github.com> Date: Tue, 29 Sep 2020 19:39:40 +0200 Subject: [PATCH 1/3] Remove unique key constraint on foreign key user_id in mfa table --- setup/mail-users.sh | 2 +- setup/migrate.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/setup/mail-users.sh b/setup/mail-users.sh index b2625a34..48b3ef32 100755 --- a/setup/mail-users.sh +++ b/setup/mail-users.sh @@ -22,7 +22,7 @@ if [ ! -f $db_path ]; then echo Creating new user database: $db_path; echo "CREATE TABLE users (id INTEGER PRIMARY KEY AUTOINCREMENT, email TEXT NOT NULL UNIQUE, password TEXT NOT NULL, extra, privileges TEXT NOT NULL DEFAULT '');" | sqlite3 $db_path; echo "CREATE TABLE aliases (id INTEGER PRIMARY KEY AUTOINCREMENT, source TEXT NOT NULL UNIQUE, destination TEXT NOT NULL, permitted_senders TEXT);" | sqlite3 $db_path; - echo "CREATE TABLE mfa (id INTEGER PRIMARY KEY AUTOINCREMENT, user_id INTEGER NOT NULL UNIQUE, type TEXT NOT NULL, secret TEXT NOT NULL, mru_token TEXT, label TEXT, FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE);" | sqlite3 $db_path; + echo "CREATE TABLE mfa (id INTEGER PRIMARY KEY AUTOINCREMENT, user_id INTEGER NOT NULL, type TEXT NOT NULL, secret TEXT NOT NULL, mru_token TEXT, label TEXT, FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE);" | sqlite3 $db_path; fi # ### User Authentication diff --git a/setup/migrate.py b/setup/migrate.py index e4a253dd..c52ac967 100755 --- a/setup/migrate.py +++ b/setup/migrate.py @@ -184,7 +184,7 @@ def migration_12(env): def migration_13(env): # Add the "mfa" table for configuring MFA for login to the control panel. db = os.path.join(env["STORAGE_ROOT"], 'mail/users.sqlite') - shell("check_call", ["sqlite3", db, "CREATE TABLE mfa (id INTEGER PRIMARY KEY AUTOINCREMENT, user_id INTEGER NOT NULL UNIQUE, type TEXT NOT NULL, secret TEXT NOT NULL, mru_token TEXT, label TEXT, FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE);"]) + shell("check_call", ["sqlite3", db, "CREATE TABLE mfa (id INTEGER PRIMARY KEY AUTOINCREMENT, user_id INTEGER NOT NULL, type TEXT NOT NULL, secret TEXT NOT NULL, mru_token TEXT, label TEXT, FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE);"]) ########################################################### From be5032ffbe70f087b195b41a9224928ab5fa57b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Sp=C3=B6ttel?= <1682504+fspoettel@users.noreply.github.com> Date: Tue, 29 Sep 2020 19:46:02 +0200 Subject: [PATCH 2/3] Don't expose mru_token and secret for enabled mfas over HTTP --- api/mailinabox.yml | 6 +----- management/daemon.py | 4 ++-- management/mfa.py | 8 ++++++++ 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/api/mailinabox.yml b/api/mailinabox.yml index 118c0ce9..15a048f9 100644 --- a/api/mailinabox.yml +++ b/api/mailinabox.yml @@ -2637,10 +2637,6 @@ components: type: string type: type: string - secret: - type: string - mru_token: - type: string label: type: string nullable: true @@ -2681,4 +2677,4 @@ components: type: string nullable: true MfaDisableSuccessResponse: - type: string \ No newline at end of file + type: string diff --git a/management/daemon.py b/management/daemon.py index bc519789..04b109f7 100755 --- a/management/daemon.py +++ b/management/daemon.py @@ -9,7 +9,7 @@ import auth, utils, mfa from mailconfig import get_mail_users, get_mail_users_ex, get_admins, add_mail_user, set_mail_password, remove_mail_user from mailconfig import get_mail_user_privileges, add_remove_mail_user_privilege from mailconfig import get_mail_aliases, get_mail_aliases_ex, get_mail_domains, add_mail_alias, remove_mail_alias -from mfa import get_mfa_state, provision_totp, validate_totp_secret, enable_mfa, disable_mfa +from mfa import get_public_mfa_state, provision_totp, validate_totp_secret, enable_mfa, disable_mfa env = utils.load_environment() @@ -403,7 +403,7 @@ def ssl_provision_certs(): @authorized_personnel_only def mfa_get_status(): return json_response({ - "enabled_mfa": get_mfa_state(request.user_email, env), + "enabled_mfa": get_public_mfa_state(request.user_email, env), "new_mfa": { "totp": provision_totp(request.user_email, env) } diff --git a/management/mfa.py b/management/mfa.py index 4db0ac9e..b7f29bce 100644 --- a/management/mfa.py +++ b/management/mfa.py @@ -21,6 +21,14 @@ def get_mfa_state(email, env): for r in c.fetchall() ] +def get_public_mfa_state(email, env): + c = open_database(env) + c.execute('SELECT id, type, label FROM mfa WHERE user_id=?', (get_user_id(email, c),)) + return [ + { "id": r[0], "type": r[1], "label": r[2] } + for r in c.fetchall() + ] + def enable_mfa(email, type, secret, token, label, env): if type == "totp": validate_totp_secret(secret) From ada2167d08b5072b6447181b5558788c9cce9e71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Sp=C3=B6ttel?= <1682504+fspoettel@users.noreply.github.com> Date: Tue, 29 Sep 2020 20:05:58 +0200 Subject: [PATCH 3/3] Only update mru_token for matched mfa row --- management/mfa.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/management/mfa.py b/management/mfa.py index b7f29bce..541fbc26 100644 --- a/management/mfa.py +++ b/management/mfa.py @@ -43,9 +43,9 @@ def enable_mfa(email, type, secret, token, label, env): c.execute('INSERT INTO mfa (user_id, type, secret, label) VALUES (?, ?, ?, ?)', (get_user_id(email, c), type, secret, label)) conn.commit() -def set_mru_token(email, token, env): +def set_mru_token(email, mfa_id, token, env): conn, c = open_database(env, with_connection=True) - c.execute('UPDATE mfa SET mru_token=? WHERE user_id=?', (token, get_user_id(email, c))) + c.execute('UPDATE mfa SET mru_token=? WHERE user_id=? AND id=?', (token, get_user_id(email, c), mfa_id)) conn.commit() def disable_mfa(email, mfa_id, env): @@ -127,7 +127,7 @@ def validate_auth_mfa(email, request, env): continue # On success, record the token to prevent a replay attack. - set_mru_token(email, token, env) + set_mru_token(email, mfa_mode['id'], token, env) return (True, []) # On a failed login, indicate failure and any hints for what the user can do instead.