add rate limiting and registration link expiry (#852)

* -add rate limiting to login and register endpoints
-change otp codes to be harder to guess

* allow user to view/change login and registration rate limiting value

* - add registration link expiry
- make it harder for attackers to discover usernames
- disallow empty usernames
- disallow short passwords
- move "generate registration link" button to the bottom to try and make it clearer that it wont update settings

* create "auth" config subsection

* dont show rate limit and registration timeout inputs unless user is admin

* blackify

* fix mistakes in expiration logic

* fix missed changes to use method field of the auth structure

* fix registration_link_timeout input not present when user is admin

Co-authored-by: benk10 <ben.kaufman10@gmail.com>

docs/faq.md

@@ -254,7 +254,10 @@ Simply fill in https://blockstream.info/ to use that block explorer, but you wil
 
 Check the .specter-folder in your Homefolder (or on your mynode/raspiblitz/...). There is a file called `config.json` in there which has a line like this:
 ```
-"auth":"somethingInHere"
+"auth": {
+    "method": "somethingInHere",
+    ...
+},
 ```
 Depending on "what's written in `somethingInHere`:
 * If it's `rpcpasswordaspin`, you can lookup the password in your `bitcoin.conf`-file in a line like `rpcpassword=YourPasswordHere`

src/cryptoadvance/specter/helpers.py

@@ -64,6 +64,13 @@ def alias(name):
     return "".join(x for x in name if x.isalnum() or x == "_").lower()
 
 
+def migrate_config(config):
+    # migrate old "auth" string into new "auth" json subtree
+    if "auth" in config:
+        if isinstance(config["auth"], str):
+            config["auth"] = dict(method=config["auth"])
+
+
 def deep_update(d, u):
     for k, v in six.iteritems(u):
         dv = d.get(k, {})

src/cryptoadvance/specter/server.py

@@ -93,7 +93,7 @@ def init_app(app, hwibridge=False, specter=None):
     app.login = login
     # Attach specter instance so child views (e.g. hwi) can access it
     app.specter = specter
-    if specter.config.get("auth") == "none":
+    if specter.config["auth"].get("method") == "none":
         app.logger.info("Login disabled")
         app.config["LOGIN_DISABLED"] = True
     else:

src/cryptoadvance/specter/server_endpoints/auth.py

@@ -1,4 +1,4 @@
-import random
+import random, time
 from flask import (
     Flask,
     Blueprint,
@@ -16,6 +16,7 @@ from ..user import User, hash_password, verify_password
 
 
 rand = random.randint(0, 1e32)  # to force style refresh
+last_sensitive_request = 0  # to rate limit sensitive requests
 
 # Setup endpoint blueprint
 auth_endpoint = Blueprint("auth_endpoint", __name__)
@@ -25,11 +26,13 @@ auth_endpoint = Blueprint("auth_endpoint", __name__)
 def login():
     """ login """
     if request.method == "POST":
-        if app.specter.config["auth"] == "none":
+        rate_limit()
+        auth = app.specter.config["auth"]
+        if auth["method"] == "none":
             app.login("admin")
             app.logger.info("AUDIT: Successfull Login no credentials")
             return redirect_login(request)
-        if app.specter.config["auth"] == "rpcpasswordaspin":
+        if auth["method"] == "rpcpasswordaspin":
             # TODO: check the password via RPC-call
             if app.specter.rpc is None:
                 flash(
@@ -51,7 +54,7 @@ def login():
                 app.login("admin")
                 app.logger.info("AUDIT: Successfull Login via RPC-credentials")
                 return redirect_login(request)
-        elif app.specter.config["auth"] == "usernamepassword":
+        elif auth["method"] == "usernamepassword":
             # TODO: This way both "User" and "user" will pass as usernames, should there be strict check on that here? Or should we keep it like this?
             username = request.form["username"]
             password = request.form["password"]
@@ -84,23 +87,39 @@ def login():
 def register():
     """ register """
     if request.method == "POST":
+        rate_limit()
         username = request.form["username"]
-        password = hash_password(request.form["password"])
+        password = request.form["password"]
         otp = request.form["otp"]
-        user_id = alias(username)
-        i = 1
-        while app.specter.user_manager.get_user(user_id):
-            i += 1
-            user_id = "{}{}".format(alias(username), i)
-        if app.specter.user_manager.get_user_by_username(username):
-            flash("Username is already taken, please choose another one", "error")
+        if not username:
+            flash(
+                "Please enter a username.",
+                "error",
+            )
             return redirect("register?otp={}".format(otp))
-        if app.specter.burn_new_user_otp(otp):
+        min_chars = int(app.specter.config["auth"]["password_min_chars"])
+        if not password or len(password) < min_chars:
+            flash(
+                "Please enter a password of a least {} characters.".format(min_chars),
+                "error",
+            )
+            return redirect("register?otp={}".format(otp))
+        if app.specter.validate_new_user_otp(otp):
+            user_id = alias(username)
+            i = 1
+            while app.specter.user_manager.get_user(user_id):
+                i += 1
+                user_id = "{}{}".format(alias(username), i)
+            if app.specter.user_manager.get_user_by_username(username):
+                flash("Username is already taken, please choose another one", "error")
+                return redirect("register?otp={}".format(otp))
+            app.specter.remove_new_user_otp(otp)
             config = {
                 "explorers": {"main": "", "test": "", "regtest": "", "signet": ""},
                 "hwi_bridge_url": "/hwi/api/",
             }
-            user = User(user_id, username, password, config)
+            password_hash = hash_password(password)
+            user = User(user_id, username, password_hash, config)
             app.specter.add_user(user)
             flash(
                 "You have registered successfully, \
@@ -132,3 +151,19 @@ def redirect_login(request):
     else:
         response = redirect(url_for("index"))
     return response
+
+
+def rate_limit():
+    global last_sensitive_request
+    limit = int(app.specter.config["auth"]["rate_limit"])
+    if limit < 0:
+        limit = 0
+    now = time.time()
+    if (
+        last_sensitive_request != 0
+        and limit > 0
+        and last_sensitive_request + limit > now
+    ):
+        remaining_time = last_sensitive_request + limit - now
+        time.sleep(remaining_time)
+    last_sensitive_request = time.time()

src/cryptoadvance/specter/server_endpoints/settings.py

@@ -1,4 +1,4 @@
-import json, os, time, random, requests
+import json, os, time, random, requests, secrets
 
 from flask import (
     Flask,
@@ -280,7 +280,7 @@ def tor():
                 tor_connectable = False
         elif action == "toggle_hidden_service":
             if not app.config["DEBUG"]:
-                if app.specter.config.get("auth", "none") == "none":
+                if app.specter.config["auth"].get("method", "none") == "none":
                     flash(
                         "Enabling Tor hidden service will expose your Specter for remote access.<br>It is therefore required that you set up authentication tab for Specter first to prevent unauthorized access.<br><br>Please go to Settings -> Authentication and set up an authentication method and retry.",
                         "error",
@@ -332,9 +332,11 @@ def tor():
 def auth():
     current_version = notify_upgrade(app, flash)
     auth = app.specter.config["auth"]
-    new_otp = -1
+    method = auth["method"]
+    rate_limit = auth["rate_limit"]
+    registration_link_timeout = auth["registration_link_timeout"]
     users = None
-    if current_user.is_admin and auth == "usernamepassword":
+    if current_user.is_admin and method == "usernamepassword":
         users = [user for user in app.specter.user_manager.users if not user.is_admin]
     if request.method == "POST":
         action = request.form["action"]
@@ -347,7 +349,9 @@ def auth():
                 specter_username = None
                 specter_password = None
             if current_user.is_admin:
-                auth = request.form["auth"]
+                method = request.form["method"]
+                rate_limit = request.form["rate_limit"]
+                registration_link_timeout = request.form["registration_link_timeout"]
             if specter_username:
                 if current_user.username != specter_username:
                     if app.specter.user_manager.get_user_by_username(specter_username):
@@ -357,8 +361,9 @@ def auth():
                         )
                         return render_template(
                             "settings/auth_settings.jinja",
-                            auth=auth,
-                            new_otp=new_otp,
+                            method=method,
+                            rate_limit=rate_limit,
+                            registration_link_timeout=registration_link_timeout,
                             users=users,
                             specter=app.specter,
                             current_version=current_version,
@@ -366,12 +371,30 @@ def auth():
                         )
                 current_user.username = specter_username
                 if specter_password:
+                    min_chars = int(auth["password_min_chars"])
+                    if len(specter_password) < min_chars:
+                        flash(
+                            "Please enter a password of a least {} characters.".format(
+                                min_chars
+                            ),
+                            "error",
+                        )
+                        return render_template(
+                            "settings/auth_settings.jinja",
+                            method=method,
+                            rate_limit=rate_limit,
+                            registration_link_timeout=registration_link_timeout,
+                            users=users,
+                            specter=app.specter,
+                            current_version=current_version,
+                            rand=rand,
+                        )
                     current_user.password = hash_password(specter_password)
                 current_user.save_info(app.specter)
             if current_user.is_admin:
-                app.specter.update_auth(auth)
-                if auth == "rpcpasswordaspin" or auth == "usernamepassword":
-                    if auth == "usernamepassword":
+                app.specter.update_auth(method, rate_limit, registration_link_timeout)
+                if method == "rpcpasswordaspin" or method == "usernamepassword":
+                    if method == "usernamepassword":
                         users = [
                             user
                             for user in app.specter.user_manager.users
@@ -387,13 +410,25 @@ def auth():
             app.specter.check()
         elif action == "adduser":
             if current_user.is_admin:
-                new_otp = random.randint(100000, 999999)
+                new_otp = secrets.token_urlsafe(16)
+                now = time.time()
+                timeout = int(registration_link_timeout)
+                timeout = 0 if timeout < 0 else timeout
+                if timeout > 0:
+                    expiry = now + timeout * 60 * 60
+                    if timeout > 1:
+                        expiry_desc = " (expires in {} hours)".format(timeout)
+                    else:
+                        expiry_desc = " (expires in 1 hour)"
+                else:
+                    expiry = 0
+                    expiry_desc = ""
                 app.specter.add_new_user_otp(
-                    {"otp": new_otp, "created_at": time.time()}
+                    {"otp": new_otp, "created_at": now, "expiry": expiry}
                 )
                 flash(
-                    "New user link generated successfully: {}auth/register?otp={}".format(
-                        request.url_root, new_otp
+                    "New user link generated{}: {}auth/register?otp={}".format(
+                        expiry_desc, request.url_root, new_otp
                     ),
                     "info",
                 )
@@ -413,8 +448,9 @@ def auth():
                 flash("Error: Only the admin account can delete users", "error")
     return render_template(
         "settings/auth_settings.jinja",
-        auth=auth,
-        new_otp=new_otp,
+        method=method,
+        rate_limit=rate_limit,
+        registration_link_timeout=registration_link_timeout,
         users=users,
         specter=app.specter,
         current_version=current_version,

src/cryptoadvance/specter/specter.py

@@ -8,7 +8,7 @@ import time
 import zipfile
 import requests
 from io import BytesIO
-from .helpers import deep_update, clean_psbt, is_testnet
+from .helpers import migrate_config, deep_update, clean_psbt, is_testnet
 from .util.checker import Checker
 from .rpc import autodetect_rpc_confs, get_default_datadir, RpcError
 from urllib3.exceptions import NewConnectionError
@@ -114,7 +114,12 @@ class Specter:
                 "host": "localhost",  # localhost
                 "protocol": "http",  # https for the future
             },
-            "auth": "none",
+            "auth": {
+                "method": "none",
+                "password_min_chars": 6,
+                "rate_limit": 10,
+                "registration_link_timeout": 1,
+            },
             "explorers": {"main": "", "test": "", "regtest": "", "signet": ""},
             "proxy_url": "socks5h://localhost:9050",  # Tor proxy URL
             "only_tor": False,
@@ -297,6 +302,7 @@ class Specter:
         if os.path.isfile(self.config_fname):
             with self.lock:
                 self.file_config = read_json_file(self.config_fname)
+                migrate_config(self.file_config)
                 deep_update(self.config, self.file_config)
             # otherwise - create one and assign unique id
         else:
@@ -431,10 +437,15 @@ class Specter:
             self.check(check_all=True)
         return self.rpc is not None
 
-    def update_auth(self, auth):
+    def update_auth(self, method, rate_limit, registration_link_timeout):
         """ simply persisting the current auth-choice """
-        if self.config["auth"] != auth:
-            self.config["auth"] = auth
+        auth = self.config["auth"]
+        if auth["method"] != method:
+            auth["method"] = method
+        if auth["rate_limit"] != rate_limit:
+            auth["rate_limit"] = rate_limit
+        if auth["registration_link_timeout"] != registration_link_timeout:
+            auth["registration_link_timeout"] = registration_link_timeout
         self._save()
 
     def update_explorer(self, explorer, user):
@@ -555,13 +566,30 @@ class Specter:
         self.config["new_user_otps"].append(otp_dict)
         self._save()
 
-    def burn_new_user_otp(self, otp):
-        """ validates an OTP for user registration and removes it if valid"""
+    def validate_new_user_otp(self, otp):
+        """ validates an OTP for user registration and removes it if expired"""
+        if "new_user_otps" not in self.config:
+            return False
+        now = time.time()
+        for i, otp_dict in enumerate(self.config["new_user_otps"]):
+            if otp_dict["otp"] == otp:
+                if (
+                    "expiry" in otp_dict
+                    and otp_dict["expiry"] < now
+                    and otp_dict["expiry"] > 0
+                ):
+                    del self.config["new_user_otps"][i]
+                    self._save()
+                    return False
+                return True
+        return False
+
+    def remove_new_user_otp(self, otp):
+        """ removes an OTP for user registration"""
         if "new_user_otps" not in self.config:
             return False
         for i, otp_dict in enumerate(self.config["new_user_otps"]):
-            # TODO: Validate OTP did not expire based on created_at
-            if otp_dict["otp"] == int(otp):
+            if otp_dict["otp"] == otp:
                 del self.config["new_user_otps"][i]
                 self._save()
                 return True

src/cryptoadvance/specter/templates/base.jinja

@@ -18,7 +18,7 @@
 	</head>
 	<body>
 	<div class="pageloader" id="pageloader"></div>
-	{% if specter.config.auth == "none" or current_user.is_authenticated %}
+	{% if specter.config.auth.method == "none" or current_user.is_authenticated %}
 		{% include "includes/overlay/overlay.html" %}
 		{% include "includes/qr-code.html" %}
 	{% endif %}
@@ -35,7 +35,7 @@
 					<a class="settings-bar-btn" href="{{ url_for('settings_endpoint.settings') }}">
 						<img src="{{ url_for('static', filename='img/settings-status-bar.svg') }}" style="width: 22px;"/>
 					</a>
-					{% if specter.config.auth != "none" %}
+					{% if specter.config.auth.method != "none" %}
 						<a class="settings-bar-btn" href="{{ url_for('auth_endpoint.logout') }}">
 							<img src="{{ url_for('static', filename='img/logout-status-bar.svg') }}" style="width: 22px;"/>
 						</a>

src/cryptoadvance/specter/templates/login.jinja

@@ -7,7 +7,7 @@
     <div class="card">
         <h1>Login to Specter</h1>
         <form action="{{ url_for('auth_endpoint.login') }}" method="POST" role="form">
-            {% if specter.config['auth'] == 'usernamepassword' %}
+            {% if specter.config['auth']['method'] == 'usernamepassword' %}
                 <input id="username" class="form-control" placeholder="Username" name="username" type="text" value=""><br><br>
             {% endif %}
             <input id="password" class="form-control" placeholder="Password" name="password" type="password" value="">
@@ -19,4 +19,4 @@
             </div>
         </form>
     </div>
-{% endblock %}
+{% endblock %}

src/cryptoadvance/specter/templates/register.jinja

@@ -7,7 +7,7 @@
     <div class="card">
         <h1>Register to the Specter Node</h1>
         <form action="{{ url_for('auth_endpoint.register') }}" method="POST" role="form">
-            {% if specter.config['auth'] == 'usernamepassword' %}
+            {% if specter.config['auth']['method'] == 'usernamepassword' %}
                 <input class="form-control" placeholder="Username" name="username" type="text" value=""><br><br>
             {% endif %}
             <input class="form-control" placeholder="Password" name="password" type="password" value="">

src/cryptoadvance/specter/templates/settings/auth_settings.jinja

@@ -5,19 +5,21 @@
 		{% from 'settings/components/settings_menu.jinja' import settings_menu %}
 		{{ settings_menu('auth', current_user) }}
 		<div class="card" style="margin: 20px auto;">
-			{% if specter.config.auth == "none" or current_user.is_admin %}
+			{% if method == "none" or current_user.is_admin %}
 				Authentication:<br>
-				<select name="auth" onchange="toggleUsernamePassword(this)">
-					<option value="none" {% if auth=="none" %} selected="selected"{% endif %}>None</option>
-					<option value="rpcpasswordaspin" {% if auth=="rpcpasswordaspin" %} selected="selected"{% endif %}>RPC password as Pin</option>
-					<option value="usernamepassword" {% if auth=="usernamepassword" %} selected="selected"{% endif %}>Multiple Users</option>
+				<select name="method" onchange="toggleUsernamePassword(this)">
+					<option value="none" {% if method=="none" %} selected="selected"{% endif %}>None</option>
+					<option value="rpcpasswordaspin" {% if method=="rpcpasswordaspin" %} selected="selected"{% endif %}>RPC password as Pin</option>
+					<option value="usernamepassword" {% if method=="usernamepassword" %} selected="selected"{% endif %}>Multiple Users</option>
 				</select>
 				<br><br>
 			{% endif %}
-			<div id="usernamepassword" class="{% if specter.config['auth'] != 'usernamepassword' %}hidden{% endif %}">
-				{% if specter.config.auth == "usernamepassword" and current_user.is_admin %}
-					<button type="submit" class="btn" style="width: 100%;" name="action" value="adduser">Generate New Registration Link</button>
-					<br>
+			<div id="ratelimit" class="{% if method == 'none' or not current_user.is_admin %}hidden{% endif %}">
+				Rate Limiting (seconds between login/register attempts):<br><input id="rate_limit" type="number" name="rate_limit" min="0" step="1" value="{{ rate_limit }}"><br><br>
+			</div>
+			<div id="usernamepassword" class="{% if method != 'usernamepassword' %}hidden{% endif %}">
+				{% if current_user.is_admin %}
+					Registration Link Timeout (hours):<br><input id="registration_link_timeout" type="number" name="registration_link_timeout" min="0" step="1" value="{{ registration_link_timeout }}"><br><br>
 					<span class="note">Default is username: admin, password: admin.</span><br>
 				{% endif %}
 				Specter Username:<br><input id="specter_username" type="text" name="specter_username" type="text" value="{{ current_user.username }}"><br><br>
@@ -42,6 +44,12 @@
 			<div class="row">
 				<button type="submit" class="btn" name="action" value="save">Save</button>
 			</div>
+			{% if method == "usernamepassword" and current_user.is_admin %}
+			<br>
+			<div id="generateregistrationlink" class="row{% if method != 'usernamepassword' %} hidden{% endif %}">
+				<button type="submit" class="btn" name="action" value="adduser">Generate New Registration Link</button>
+			</div>
+			{% endif %}
 		</div>
 	</form>
 {% endblock %}
@@ -49,16 +57,27 @@
 {% block scripts %}
 	<script>
 		function toggleUsernamePassword(select) {
-            var usernamepasswordDiv = document.getElementById("usernamepassword");
-            var specterUsername = document.getElementById("specter_username");
-            var specterPassword = document.getElementById("specter_password");
-            if (select.options[select.selectedIndex].value === 'usernamepassword'){
-                usernamepasswordDiv.style.display = 'block';
-            } else {
-                usernamepasswordDiv.style.display = 'none';
-                specterUsername.value = '{{ current_user.username }}';
-                specterPassword.value = '';
-            }
-        }
+		{% if current_user.is_admin %}
+			var ratelimitDiv = document.getElementById("ratelimit");
+			if (select.options[select.selectedIndex].value !== 'none'){
+				ratelimitDiv.style.display = 'block';
+			} else {
+				ratelimitDiv.style.display = 'none';
+			}
+		{% endif %}
+			var usernamepasswordDiv = document.getElementById("usernamepassword");
+			var specterUsername = document.getElementById("specter_username");
+			var specterPassword = document.getElementById("specter_password");
+			var generateregistrationlinkDiv = document.getElementById("generateregistrationlink");
+			if (select.options[select.selectedIndex].value === 'usernamepassword'){
+				usernamepasswordDiv.style.display = 'block';
+				generateregistrationlinkDiv.style.display = 'flex';
+			} else {
+				usernamepasswordDiv.style.display = 'none';
+				specterUsername.value = '{{ current_user.username }}';
+				specterPassword.value = '';
+				generateregistrationlinkDiv.style.display = 'none';
+			}
+		}
 	</script>
 {% endblock %}

src/cryptoadvance/specter/templates/settings/tor_settings.jinja

@@ -46,7 +46,7 @@
                 <p>Running Specter behind a Tor hidden service allows you to acccess Specter over Tor from anywhere.<br>
                 </p>
                 {% if tor_service_id %}
-                    {% if specter.config.auth == "none" %}
+                    {% if specter.config.auth.method == "none" %}
                         <p class="warning" style="max-width:700px;">
                         ⚠️ Warning!<br>
                         Your are running Specter over Tor with no authentication settings configured.

src/cryptoadvance/specter/util/tor.py

@@ -40,7 +40,7 @@ def start_hidden_service(app):
             f.write("%s.onion" % app.tor_service_id)
     app.tor_service_id = app.tor_service_id
     app.tor_enabled = True
-    if app.specter.config.get("auth", "none") == "none":
+    if app.specter.config["auth"].get("method", "none") == "none":
         print(" * ############################# Warning! #############################")
         print(
             " * Your are running Specter over Tor with no authentication settings configured."

tests/conftest.py

@@ -258,7 +258,9 @@ def specter_regtest_configured(bitcoin_regtest, devices_filled_data_folder):
             "host": bitcoin_regtest.rpcconn.ipaddress,
             "protocol": "http",
         },
-        "auth": "rpcpasswordaspin",
+        "auth": {
+            "method": "rpcpasswordaspin",
+        },
     }
     specter = Specter(data_folder=devices_filled_data_folder, config=config)
     specter.check()