use secrets to safely generate random tokens

The usage of os.urandom was fine to generate a salt, but using secrets
here makes sure that the intent is carried across.

For the share tokens, using random might be insecure. We should err on
the side of caution and use a secure PRNG here, so now we use secrets
here as well.

For tokens (password reset, ...), UUID4 is probably also fine, so we'll
leave that for now.

4 files changed, 13 insertions, 12 deletions

diff --git a/fietsboek/models/user.py b/fietsboek/models/user.py
index fa6bd7f..83a36e0 100644
--- a/fietsboek/models/user.py
+++ b/fietsboek/models/user.py
@@ -2,7 +2,7 @@
 import datetime
 import enum
 import uuid
-import os
+import secrets
 
 from sqlalchemy import (
     Column,
@@ -45,6 +45,7 @@ SCRYPT_PARAMETERS = {
     'r': 8,
     'p': 1,
 }
+SALT_LENGTH = 32
 
 
 friends_assoc = Table(
@@ -130,7 +131,7 @@ class User(Base):
         :type new_password: str
         """
         new_password = new_password.encode('utf-8')
-        salt = os.urandom(16)
+        salt = secrets.token_bytes(SALT_LENGTH)
         scrypt = Scrypt(salt=salt, **SCRYPT_PARAMETERS)
         password = scrypt.derive(new_password)
         self.salt = salt
diff --git a/fietsboek/util.py b/fietsboek/util.py
index ddc4832..71f5d16 100644
--- a/fietsboek/util.py
+++ b/fietsboek/util.py
@@ -1,10 +1,9 @@
 """Various utility functions."""
-import random
-import string
 import datetime
 import re
 import os
 import unicodedata
+import secrets
 
 # Compat for Python < 3.9
 import importlib_resources
@@ -213,16 +212,17 @@ def month_name(request, month):
     return locale.months["stand-alone"]["wide"][month]
 
 
-def random_alphanum_string(length=20):
-    """Draws a random string that consists of alphanumerical characters.
+def random_link_secret(nbytes=20):
+    """Safely generates a secret suitable for the link share.
 
-    :param length: Length of the string.
-    :type length: int
+    The returned string consists of characters that are safe to use in a URL.
+
+    :param nbytes: Number of random bytes to use.
+    :type nbytes: int
     :return: A randomly drawn string.
     :rtype: str
     """
-    candidates = string.ascii_letters + string.digits
-    return ''.join(random.choice(candidates) for _ in range(length))
+    return secrets.token_urlsafe(nbytes)
 
 
 def retrieve_multiple(dbsession, model, params, name):
diff --git a/fietsboek/views/detail.py b/fietsboek/views/detail.py
index f213231..6eba3ce 100644
--- a/fietsboek/views/detail.py
+++ b/fietsboek/views/detail.py
@@ -73,7 +73,7 @@ def invalidate_share(request):
     :rtype: pyramid.response.Response
     """
     track = request.context
-    track.link_secret = util.random_alphanum_string()
+    track.link_secret = util.random_link_secret()
     return HTTPFound(request.route_url('details', track_id=track.id))
 
 
diff --git a/fietsboek/views/upload.py b/fietsboek/views/upload.py
index ea05c74..95008b4 100644
--- a/fietsboek/views/upload.py
+++ b/fietsboek/views/upload.py
@@ -155,7 +155,7 @@ def do_finish_upload(request):
         visibility=Visibility[request.params["visibility"]],
         description=request.params["description"],
         badges=badges,
-        link_secret=util.random_alphanum_string(),
+        link_secret=util.random_link_secret(),
         tagged_people=tagged_people,
     )
     track.date = date