From 4455a7f22cba08f6af72ea47a87cf7fc9aeb8467 Mon Sep 17 00:00:00 2001
From: Julian Rother <julianr@fsmpi.rwth-aachen.de>
Date: Sun, 4 Oct 2020 00:24:17 +0200
Subject: [PATCH] recovery codes, finished 2fa ui, lot of cleanup

---
 uffd/mfa/models.py                     | 22 +++++++
 uffd/mfa/templates/auth.html           | 25 +++++---
 uffd/mfa/templates/disable.html        | 12 ++++
 uffd/mfa/templates/setup.html          | 82 ++++++++++++++++++++++----
 uffd/mfa/templates/setup_recovery.html | 25 ++++++++
 uffd/mfa/templates/setup_totp.html     |  3 -
 uffd/mfa/views.py                      | 73 +++++++++++++++++------
 uffd/template_helper.py                |  5 ++
 uffd/templates/base.html               |  2 +
 9 files changed, 208 insertions(+), 41 deletions(-)
 create mode 100644 uffd/mfa/templates/disable.html
 create mode 100644 uffd/mfa/templates/setup_recovery.html

diff --git a/uffd/mfa/models.py b/uffd/mfa/models.py
index feffe466..d48c8d3b 100644
--- a/uffd/mfa/models.py
+++ b/uffd/mfa/models.py
@@ -1,6 +1,7 @@
 import enum
 import datetime
 import secrets, time, struct, hmac, hashlib, base64, urllib.parse
+import crypt
 
 from flask import request, current_app
 from sqlalchemy import Column, Integer, Enum, Boolean, String, DateTime, Text
@@ -11,6 +12,7 @@ from uffd.database import db
 from uffd.user.models import User
 
 class MFAType(enum.Enum):
+	RECOVERY_CODE = 0
 	TOTP = 1
 	WEBAUTHN = 2
 
@@ -39,6 +41,26 @@ class MFAMethod(db.Model):
 	def user(self, u):
 		self.dn = u.dn
 
+class RecoveryCodeMethod(MFAMethod):
+	code_salt = Column('recovery_salt', String(64))
+	code_hash = Column('recovery_hash', String(256))
+
+	__mapper_args__ = {
+		'polymorphic_identity': MFAType.RECOVERY_CODE
+	}
+
+	def __init__(self, user):
+		super().__init__(user, None)
+		self.code = secrets.token_hex(8).replace(' ', '').lower()
+		self.code_hash = crypt.crypt(self.code)
+
+	def verify(self, code):
+		code = code.replace(' ', '').lower()
+		if crypt.crypt(code, self.code_hash) == self.code_hash:
+			return True
+		else:
+			return False
+
 def _hotp(counter, key, digits=6):
 	'''Generates HMAC-based one-time password according to RFC4226
 	
diff --git a/uffd/mfa/templates/auth.html b/uffd/mfa/templates/auth.html
index d5d9a652..8e359995 100644
--- a/uffd/mfa/templates/auth.html
+++ b/uffd/mfa/templates/auth.html
@@ -8,24 +8,29 @@
 		<div class="text-center">
 			<img src="{{ url_for("static", filename="chaosknoten.png") }}" class="col-lg-8 col-md-12" >
 		</div>
-		<div class="col-12">
+		<div class="col-12 mb-3">
 			<h2 class="text-center">Two-Factor Authentication</h2>
 		</div>
 		{% if webauthn_methods %}
-		<div class="form-group col-12 d-none webauthn-group">
-			<div id="webauthn-alert" class="alert alert-warning d-none" role="alert">
+		<noscript>
+			<div class="form-group col-12">
+				<div id="webauthn-nojs" class="alert alert-warning" role="alert">Enable javascript for authentication with U2F/FIDO2 devices</div>
 			</div>
-			<label for="webauthn-btn">FIDO token</label>
+		</noscript>
+		<div id="webauthn-unsupported" class="form-group col-12 d-none">
+			<div class="alert alert-warning" role="alert">Authentication with U2F/FIDO2 devices is not supported by your browser</div>
+		</div>
+		<div class="form-group col-12 d-none webauthn-group">
+			<div id="webauthn-alert" class="alert alert-warning d-none" role="alert"></div>
 			<button type="button" id="webauthn-btn" class="btn btn-primary btn-block">
 				<span id="webauthn-spinner" class="spinner-border spinner-border-sm d-none" role="status" aria-hidden="true"></span>
-				<span id="webauthn-btn-text">Authenticate with FIDO token</span>
+				<span id="webauthn-btn-text">Authenticate with U2F/FIDO2 device</span>
 			</button>
 		</div>
-		<div class="text-center text-muted d-none webauthn-group">- or -</div>
+		<div class="text-center text-muted d-none webauthn-group mb-3">- or -</div>
 		{% endif %}
-		<div class="form-group col-12">
-			<label for="mfa-code">Authentication code</label>
-			<input type="text" class="form-control" id="mfa-code" name="code" required="required" tabindex="1">
+		<div class="form-group col-12 mb-2">
+			<input type="text" class="form-control" id="mfa-code" name="code" required="required" tabindex="1" placeholder="Code from your authenticator app or recovery code">
 		</div>
 		<div class="form-group col-12">
 			<button type="submit" class="btn btn-primary btn-block" tabindex="2">Verify</button>
@@ -82,6 +87,8 @@ function begin_webauthn() {
 $('#webauthn-btn').on('click', begin_webauthn);
 if (typeof(PublicKeyCredential) != "undefined") {
 	$('.webauthn-group').removeClass('d-none');
+} else {
+	$('#webauthn-unsupported').removeClass('d-none');
 }
 </script>
 {% endif %}
diff --git a/uffd/mfa/templates/disable.html b/uffd/mfa/templates/disable.html
new file mode 100644
index 00000000..679ff1ba
--- /dev/null
+++ b/uffd/mfa/templates/disable.html
@@ -0,0 +1,12 @@
+{% extends 'base.html' %}
+
+{% block body %}
+
+<p>When you proceed, all recovery codes, registered authenticator applications and devices will be invalidated. 
+You can later generate new recovery codes and setup your applications and devices again.</p>
+
+<form class="form" action="{{ url_for('mfa.disable_confirm') }}" method="POST">
+	<button type="submit" class="btn btn-danger btn-block">Disable two-factor authentication</button>
+</form>
+
+{% endblock %}
diff --git a/uffd/mfa/templates/setup.html b/uffd/mfa/templates/setup.html
index 5ca7003a..80cb5d58 100644
--- a/uffd/mfa/templates/setup.html
+++ b/uffd/mfa/templates/setup.html
@@ -1,14 +1,72 @@
 {% extends 'base.html' %}
 
+{# Two-factor auth can be in three states:
+
+mfa_init: The user has not setup any two-factor methods or recovery codes
+mfa_setup: The user has setup recovery codes but no two-factor methods. Two-factor authentication is still disabled.
+mfa_enabled: The user has setup at least one two-factor method. Two-factor authentication is enabled.
+
+#}
+
+{% set mfa_enabled = totp_methods or webauthn_methods %}
+{% set mfa_init = not recovery_methods and not mfa_enabled %}
+{% set mfa_setup = recovery_methods and not mfa_enabled %}
+
 {% block body %}
-{% if totp_methods or webauthn_methods %}
-<p>Two-factor authentication is currently <strong>enabled</strong>. Delete all registered methods to disable it.</p>
-{% else %}
-<p>Two-factor authentication is currently <strong>disabled</strong>. Setup an authentication method below to enable it.</p>
+<p>Two-factor authentication is currently <strong>{{ 'enabled' if mfa_enabled else 'disabled' }}</strong>.
+{% if mfa_init %}
+You need to generate recovery codes and setup at least one authentication method to enable two-factor authentication.
+{% elif mfa_setup %}
+You need to setup at least one authentication method to enable two-factor authentication.
+{% endif %}
+</p>
+{% if mfa_setup or mfa_enabled %}
+<div class="clearfix">
+	{% if mfa_enabled %}
+	<form class="form float-right" action="{{ url_for('mfa.disable') }}">
+		<button type="submit" class="btn btn-danger mb-2">Disable two-factor authentication</button>
+	</form>
+	{% else %}
+	<form class="form float-right" action="{{ url_for('mfa.disable') }}" method="POST">
+		<button type="submit" class="btn btn-light mb-2">Reset two-factor configuration</button>
+	</form>
+	{% endif %}
+</div>
 {% endif %}
 
 <hr>
 
+<div class="row mt-3">
+	<div class="col-12 col-md-5">
+		<h4>Recovery Codes</h4>
+		<p>Recovery codes allow you to login and setup new two-factor methods when you lost your registered second factor.</p>
+		<p>
+			{% if mfa_init %}<strong>{% endif %}
+			You need to setup recovery codes before you can setup up authenticator apps or U2F/FIDO2 devices.
+			{% if mfa_init %}</strong>{% endif %}
+			Each code can only be used once.
+		</p>
+	</div>
+
+	<div class="col-12 col-md-7">
+		<form class="form" action="{{ url_for('mfa.setup_recovery') }}" method="POST">
+			{% if mfa_init %}
+			<button type="submit" class="btn btn-primary mb-2 col">Generate recovery codes to enable two-factor authentication</button>
+			{% else %}
+			<button type="submit" class="btn btn-primary mb-2 col">Generate new recovery codes</button>
+			{% endif %}
+		</form>
+
+		{% if recovery_methods %}
+		<p>{{ recovery_methods|length }} recovery codes remain</p>
+		{% elif not recovery_methods and mfa_enabled %}
+		<p><strong>You have no remaining recovery codes.</strong></p>
+		{% endif %}
+	</div>
+</div>
+
+<hr>
+
 <div class="row mt-3">
 	<div class="col-12 col-md-5">
 		<h4>Authenticator Apps</h4>
@@ -18,11 +76,11 @@
 	</div>
 
 	<div class="col-12 col-md-7">
-		<form class="form mb-3" action="{{ url_for('mfa.setup_totp') }}">
+		<form class="form mb-2" action="{{ url_for('mfa.setup_totp') }}">
 			<div class="row m-0">
 				<label class="sr-only" for="totp-name">Name</label>
-				<input type="text" name="name" class="form-control mb-2 col-12 col-lg-auto mr-2" style="width: 15em;" id="totp-name" placeholder="Name" required>
-				<button type="submit" id="totp-submit" class="btn btn-primary mb-2 col">Setup new authenticator</button>
+				<input type="text" name="name" class="form-control mb-2 col-12 col-lg-auto mr-2" style="width: 15em;" id="totp-name" placeholder="Name" required {{ 'disabled' if mfa_init }}>
+				<button type="submit" id="totp-submit" class="btn btn-primary mb-2 col" {{ 'disabled' if mfa_init }}>Setup new authenticator</button>
 			</div>
 		</form>
 
@@ -67,10 +125,10 @@
 			<div class="alert alert-warning" role="alert">Enable javascript in your browser to use U2F and FIDO2 devices!</div>
 		</noscript>
 		<div id="webauthn-alert" class="alert alert-warning d-none" role="alert"></div>
-		<form id="webauthn-form" class="form mb-3">
+		<form id="webauthn-form" class="form mb-2">
 			<div class="row m-0">
 				<label class="sr-only" for="webauthn-name">Name</label>
-				<input type="text" class="form-control mb-2 col-12 col-lg-auto mr-2" style="width: 15em;" id="webauthn-name" placeholder="Name" required>
+				<input type="text" class="form-control mb-2 col-12 col-lg-auto mr-2" style="width: 15em;" id="webauthn-name" placeholder="Name" required {{ 'disabled' if mfa_init }}>
 				<button type="submit" id="webauthn-btn" class="btn btn-primary mb-2 col" disabled>
 					<span id="webauthn-spinner" class="spinner-border spinner-border-sm d-none" role="status" aria-hidden="true"></span>
 					<span id="webauthn-btn-text">Setup new device</span>
@@ -104,9 +162,6 @@
 	</div>
 </div>
 
-<!-- spacer for floating footer -->
-<div class="mb-5"></div>
-
 <script src="{{ url_for('static', filename="cbor.js") }}"></script>
 <script>
 
@@ -152,7 +207,10 @@ $('#webauthn-form').on('submit', function(e) {
 });
 
 if (typeof(PublicKeyCredential) != "undefined") {
+	{% if not mfa_init %}
 	$('#webauthn-btn').prop('disabled', false);
+	$('#webauthn-name').prop('disabled', false);
+	{% endif %}
 } else {
 	$('#webauthn-alert').text('U2F and FIDO2 devices are not supported by your browser!');
 	$('#webauthn-alert').removeClass('d-none');
diff --git a/uffd/mfa/templates/setup_recovery.html b/uffd/mfa/templates/setup_recovery.html
new file mode 100644
index 00000000..b5e00467
--- /dev/null
+++ b/uffd/mfa/templates/setup_recovery.html
@@ -0,0 +1,25 @@
+{% extends 'base.html' %}
+
+{% block body %}
+
+<h1 class="d-none d-print-block">Recovery Codes</h1>
+
+<p>Recovery codes allow you to login when you lose access to your authenticator app or U2F/FIDO device. Each code can only be used once.</p>
+
+<div class="text-monospace">
+	<ul>
+	{% for method in methods %}
+		<li>{{ method.code }}</li>
+	{% endfor %}
+	</ul>
+</div>
+
+<p>These are your new recovery codes. Make sure to store them in a safe place or you risk losing access to your account. All previous recovery codes are now invalid.</p>
+
+<div class="btn-toolbar">
+	<a class="ml-auto mb-2 btn btn-primary d-print-none" href="{{ url_for('mfa.setup') }}">Continue</a>
+	<a class="ml-2 mb-2 btn btn-light d-print-none" href="{{ methods|map(attribute='code')|join('\n')|datauri }}" download="uffd-recovery-codes">Download codes</a>
+	<button class="ml-2 mb-2 btn btn-light d-print-none" type="button" onClick="window.print()">Print codes</button>
+</div>
+
+{% endblock %}
diff --git a/uffd/mfa/templates/setup_totp.html b/uffd/mfa/templates/setup_totp.html
index db83e46b..fef0c439 100644
--- a/uffd/mfa/templates/setup_totp.html
+++ b/uffd/mfa/templates/setup_totp.html
@@ -32,7 +32,4 @@
 	</div>
 </form>
 
-<!-- spacer for floating footer -->
-<div class="mb-5"></div>
-
 {% endblock %}
diff --git a/uffd/mfa/views.py b/uffd/mfa/views.py
index a46a6802..170653bb 100644
--- a/uffd/mfa/views.py
+++ b/uffd/mfa/views.py
@@ -8,7 +8,7 @@ from fido2.ctap2 import AttestationObject, AuthenticatorData
 from fido2 import cbor
 
 from uffd.database import db
-from uffd.mfa.models import TOTPMethod, WebauthnMethod
+from uffd.mfa.models import MFAMethod, TOTPMethod, WebauthnMethod, RecoveryCodeMethod
 from uffd.session.views import get_current_user, login_required
 
 bp = Blueprint('mfa', __name__, template_folder='templates', url_prefix='/mfa/')
@@ -17,9 +17,37 @@ bp = Blueprint('mfa', __name__, template_folder='templates', url_prefix='/mfa/')
 @login_required()
 def setup():
 	user = get_current_user()
+	recovery_methods = RecoveryCodeMethod.query.filter_by(dn=user.dn).all()
 	totp_methods = TOTPMethod.query.filter_by(dn=user.dn).all()
 	webauthn_methods = WebauthnMethod.query.filter_by(dn=user.dn).all()
-	return render_template('setup.html', totp_methods=totp_methods, webauthn_methods=webauthn_methods)
+	return render_template('setup.html', totp_methods=totp_methods, webauthn_methods=webauthn_methods, recovery_methods=recovery_methods)
+
+@bp.route('/setup/disable', methods=['GET'])
+@login_required()
+def disable():
+	return render_template('disable.html')
+
+@bp.route('/setup/disable', methods=['POST'])
+@login_required()
+def disable_confirm():
+	user = get_current_user()
+	MFAMethod.query.filter_by(dn=user.dn).delete()
+	db.session.commit()
+	return redirect(url_for('mfa.setup'))
+
+@bp.route('/setup/recovery', methods=['POST'])
+@login_required()
+def setup_recovery():
+	user = get_current_user()
+	for method in RecoveryCodeMethod.query.filter_by(dn=user.dn).all():
+		db.session.delete(method)
+	methods = []
+	for _ in range(10):
+		method = RecoveryCodeMethod(user)
+		methods.append(method)
+		db.session.add(method)
+	db.session.commit()
+	return render_template('setup_recovery.html', methods=methods)
 
 @bp.route('/setup/totp', methods=['GET'])
 @login_required()
@@ -33,8 +61,10 @@ def setup_totp():
 @login_required()
 def setup_totp_finish():
 	user = get_current_user()
-	method = TOTPMethod(user, name=request.values['name'], key=session['mfa_totp_key'])
-	del session['mfa_totp_key']
+	if not RecoveryCodeMethod.query.filter_by(dn=user.dn).all():
+		flash('Generate recovery codes first!')
+		return redirect(url_for('mfa.setup'))
+	method = TOTPMethod(user, name=request.values['name'], key=session.pop('mfa_totp_key'))
 	if method.verify(request.form['code']):
 		db.session.add(method)
 		db.session.commit()
@@ -51,12 +81,6 @@ def delete_totp(id):
 	db.session.commit()
 	return redirect(url_for('mfa.setup'))
 
-@bp.route('/setup/webauthn', methods=['GET'])
-@login_required()
-def setup_webauthn():
-	user = get_current_user()
-	return render_template('setup_webauthn.html')
-
 def get_webauthn_server():
 	return Fido2Server(PublicKeyCredentialRpEntity(urllib.parse.urlsplit(request.url).hostname, "uffd"))
 
@@ -64,6 +88,8 @@ def get_webauthn_server():
 @login_required()
 def setup_webauthn_begin():
 	user = get_current_user()
+	if not RecoveryCodeMethod.query.filter_by(dn=user.dn).all():
+		abort(403)
 	methods = WebauthnMethod.query.filter_by(dn=user.dn).all()
 	creds = [method.cred_data.credential_data for method in methods]
 	server = get_webauthn_server()
@@ -72,13 +98,12 @@ def setup_webauthn_begin():
 			"id": user.loginname.encode(),
 			"name": user.loginname,
 			"displayName": user.displayname,
-			"icon": "https://example.com/image.png",
 		},
 		creds,
 		user_verification=UserVerificationRequirement.DISCOURAGED,
 		authenticator_attachment="cross-platform",
 	)
-	session["state"] = state
+	session["webauthn-state"] = state
 	return cbor.encode(registration_data)
 
 @bp.route('/setup/webauthn/complete', methods=['POST'])
@@ -89,7 +114,7 @@ def setup_webauthn_complete():
 	data = cbor.decode(request.get_data())
 	client_data = ClientData(data["clientDataJSON"])
 	att_obj = AttestationObject(data["attestationObject"])
-	auth_data = server.register_complete(session["state"], client_data, att_obj)
+	auth_data = server.register_complete(session["webauthn-state"], client_data, att_obj)
 	method = WebauthnMethod(user, auth_data, name=data['name'])
 	db.session.add(method)
 	db.session.commit()
@@ -115,7 +140,7 @@ def auth_webauthn_begin():
 	if not creds:
 		abort(404)
 	auth_data, state = server.authenticate_begin(creds, user_verification=UserVerificationRequirement.DISCOURAGED)
-	session["state"] = state
+	session["webauthn-state"] = state
 	return cbor.encode(auth_data)
 
 @bp.route("/auth/webauthn/complete", methods=["POST"])
@@ -148,19 +173,33 @@ def auth_webauthn_complete():
 @login_required()
 def auth():
 	user = get_current_user()
+	recovery_methods = RecoveryCodeMethod.query.filter_by(dn=user.dn).all()
 	totp_methods = TOTPMethod.query.filter_by(dn=user.dn).all()
 	webauthn_methods = WebauthnMethod.query.filter_by(dn=user.dn).all()
 	return render_template('auth.html', ref=request.values.get('ref'), totp_methods=totp_methods,
-			webauthn_methods=webauthn_methods)
+			webauthn_methods=webauthn_methods, recovery_methods=recovery_methods)
 
 @bp.route('/auth', methods=['POST'])
 @login_required()
 def auth_finish():
 	user = get_current_user()
-	methods = TOTPMethod.query.filter_by(dn=user.dn).all()
-	for method in methods:
+	recovery_methods = RecoveryCodeMethod.query.filter_by(dn=user.dn).all()
+	totp_methods = TOTPMethod.query.filter_by(dn=user.dn).all()
+	for method in totp_methods:
+		if method.verify(request.form['code']):
+			session['mfa_verifed'] = True
+			return redirect(request.values.get('ref', url_for('index')))
+	for method in recovery_methods:
 		if method.verify(request.form['code']):
+			db.session.delete(method)
+			db.session.commit()
 			session['mfa_verifed'] = True
+			if len(recovery_methods) <= 1:
+				flash('You have exhausted your recovery codes. Please generate new ones now!')
+				return redirect(url_for('mfa.setup'))
+			elif len(recovery_methods) <= 5:
+				flash('You only have a few recovery codes remaining. Make sure to generate new ones before they run out.')
+				return redirect(url_for('mfa.setup'))
 			return redirect(request.values.get('ref', url_for('index')))
 	flash('Two-factor authentication failed')
 	return redirect(url_for('mfa.auth', ref=request.values.get('ref')))
diff --git a/uffd/template_helper.py b/uffd/template_helper.py
index 63bad80e..ebd06596 100644
--- a/uffd/template_helper.py
+++ b/uffd/template_helper.py
@@ -4,6 +4,7 @@ import qrcode, qrcode.image.svg
 
 import random
 import subprocess
+import base64
 from datetime import timedelta, datetime
 import io
 
@@ -22,6 +23,10 @@ def register_template_helper(app):
 		img.save(buf)
 		return Markup(buf.getvalue().decode())
 
+	@app.template_filter()
+	def datauri(data, mimetype='text/plain'):
+		return Markup('data:%s;base64,%s'%(mimetype, base64.b64encode(data.encode()).decode()))
+
 	@app.url_defaults
 	def static_version_inject(endpoint, values): #pylint: disable=unused-variable
 		if endpoint == 'static':
diff --git a/uffd/templates/base.html b/uffd/templates/base.html
index 22360863..ac9e2af2 100644
--- a/uffd/templates/base.html
+++ b/uffd/templates/base.html
@@ -93,6 +93,8 @@
 		<main role="main" class="container">
 			{% block body %}
 			{% endblock body %}
+			<!-- spacer for floating footer -->
+			<div class="mb-5"></div>
 		</main>
 
 		<footer class="footer">
-- 
GitLab