python: Use standard secrets module to generate random tokens.

There are three functional side effects:

• Correct an insignificant but mathematically offensive bias toward
repeated characters in generate_api_key introduced in commit
47b4283c4b4c70ecde4d3c8de871c90ee2506d87; its entropy is increased
from 190.52864 bits to 190.53428 bits.

• Use the base32 alphabet in confirmation.models.generate_key; its
entropy is reduced from 124.07820 bits to the documented 120 bits, but
now it uses 1 syscall instead of 24.

• Use the base32 alphabet in get_bigbluebutton_url; its entropy is
reduced from 51.69925 bits to 50 bits, but now it uses 1 syscall
instead of 10.

(The base32 alphabet is A-Z 2-7.  We could probably replace all of
these with plain secrets.token_urlsafe, since I expect most callers
can handle the full urlsafe_b64 alphabet A-Z a-z 0-9 - _ without
problems.)

Signed-off-by: Anders Kaseorg <anders@zulip.com>

confirmation/models.py

@@ -2,8 +2,8 @@
 
 __revision__ = '$Id: models.py 28 2009-10-22 15:03:02Z jarek.zgoda $'
 import datetime
-import string
-from random import SystemRandom
+import secrets
+from base64 import b32encode
 from typing import Mapping, Optional, Union
 from urllib.parse import urljoin
 
@@ -37,9 +37,8 @@ def render_confirmation_key_error(request: HttpRequest, exception: ConfirmationK
     return render(request, 'confirmation/link_does_not_exist.html')
 
 def generate_key() -> str:
-    generator = SystemRandom()
     # 24 characters * 5 bits of entropy/character = 120 bits of entropy
-    return ''.join(generator.choice(string.ascii_lowercase + string.digits) for _ in range(24))
+    return b32encode(secrets.token_bytes(15)).decode().lower()
 
 ConfirmationObjT = Union[MultiuseInvite, PreregistrationUser, EmailChangeStatus]
 def get_object_from_key(confirmation_key: str,

corporate/lib/stripe.py

@@ -1,6 +1,7 @@
 import logging
 import math
 import os
+import secrets
 from datetime import datetime, timedelta
 from decimal import Decimal
 from functools import wraps
@@ -25,7 +26,6 @@ from corporate.models import (
 )
 from zerver.lib.logging_util import log_to_file
 from zerver.lib.timestamp import datetime_to_timestamp, timestamp_to_datetime
-from zerver.lib.utils import generate_random_token
 from zerver.models import Realm, RealmAuditLog, UserProfile, get_system_bot
 from zproject.config import get_secret
 
@@ -54,7 +54,7 @@ def get_latest_seat_count(realm: Realm) -> int:
     return max(non_guests, math.ceil(guests / 5))
 
 def sign_string(string: str) -> Tuple[str, str]:
-    salt = generate_random_token(64)
+    salt = secrets.token_hex(32)
     signer = Signer(salt=salt)
     return signer.sign(string), salt
 

scripts/setup/generate_secrets.py

@@ -40,9 +40,9 @@ def random_token() -> str:
     # of importing cryptography modules when necessary.
     #
     # This helps optimize noop provision performance.
-    from zerver.lib.utils import generate_random_token
+    import secrets
 
-    return generate_random_token(64)
+    return secrets.token_hex(32)
 
 def generate_django_secretkey() -> str:
     """Secret key generation taken from Django's startproject.py"""

tools/semgrep.yml

@@ -20,7 +20,6 @@ rules:
   - id: dont-import-models-in-migrations
     patterns:
       - pattern-not: from zerver.lib.redis_utils import get_redis_client
-      - pattern-not: from zerver.lib.utils import generate_random_token
       - pattern-not: from zerver.models import filter_pattern_validator
       - pattern-not: from zerver.models import filter_format_validator
       - pattern-not: from zerver.models import generate_email_token_for_stream

zerver/data_import/slack.py

@@ -1,6 +1,7 @@
 import logging
 import os
 import random
+import secrets
 import shutil
 import subprocess
 from collections import defaultdict
@@ -39,7 +40,7 @@ from zerver.data_import.slack_message_conversion import (
 )
 from zerver.lib.emoji import name_to_codepoint
 from zerver.lib.export import MESSAGE_BATCH_CHUNK_SIZE
-from zerver.lib.upload import random_name, resize_logo, sanitize_name
+from zerver.lib.upload import resize_logo, sanitize_name
 from zerver.models import (
     CustomProfileField,
     CustomProfileFieldValue,
@@ -904,7 +905,7 @@ def get_attachment_path_and_content(fileinfo: ZerverFieldsT, realm_id: int) -> T
         'SlackImportAttachment',  # This is a special placeholder which should be kept
                                   # in sync with 'exports.py' function 'import_message_data'
         format(random.randint(0, 255), 'x'),
-        random_name(18),
+        secrets.token_urlsafe(18),
         sanitize_name(fileinfo['name']),
     ])
     attachment_path = f'/user_uploads/{s3_path}'

zerver/lib/cache.py

@@ -1,10 +1,9 @@
 # See https://zulip.readthedocs.io/en/latest/subsystems/caching.html for docs
-import base64
 import hashlib
 import logging
 import os
-import random
 import re
+import secrets
 import sys
 import time
 import traceback
@@ -87,8 +86,7 @@ def get_or_create_key_prefix() -> str:
     filename = os.path.join(settings.DEPLOY_ROOT, "var", "remote_cache_prefix")
     try:
         fd = os.open(filename, os.O_CREAT | os.O_EXCL | os.O_RDWR, 0o444)
-        random_hash = hashlib.sha256(str(random.getrandbits(256)).encode('utf-8')).digest()
-        prefix = base64.b16encode(random_hash)[:32].decode('utf-8').lower() + ':'
+        prefix = secrets.token_hex(16) + ':'
         # This does close the underlying file
         with os.fdopen(fd, 'w') as f:
             f.write(prefix + "\n")

zerver/lib/email_mirror.py

@@ -1,5 +1,6 @@
 import logging
 import re
+import secrets
 from email.headerregistry import AddressHeader
 from email.message import EmailMessage
 from typing import Dict, List, Optional, Tuple
@@ -25,7 +26,6 @@ from zerver.lib.queue import queue_json_publish
 from zerver.lib.rate_limiter import RateLimitedObject
 from zerver.lib.send_email import FromAddress
 from zerver.lib.upload import upload_message_file
-from zerver.lib.utils import generate_random_token
 from zerver.models import (
     Message,
     MissedMessageEmailAddress,
@@ -96,7 +96,7 @@ def log_and_report(email_message: EmailMessage, error_message: str, to: Optional
 # Temporary missed message addresses
 
 def generate_missed_message_token() -> str:
-    return 'mm' + generate_random_token(32)
+    return 'mm' + secrets.token_hex(16)
 
 def is_missed_message_address(address: str) -> bool:
     try:

zerver/lib/import_realm.py

@@ -1,6 +1,7 @@
 import datetime
 import logging
 import os
+import secrets
 import shutil
 from typing import Any, Dict, Iterable, List, Optional, Tuple
 
@@ -30,7 +31,7 @@ from zerver.lib.parallel import run_parallel
 from zerver.lib.server_initialization import create_internal_realm, server_initialized
 from zerver.lib.streams import render_stream_description
 from zerver.lib.timestamp import datetime_to_timestamp
-from zerver.lib.upload import BadImageError, guess_type, random_name, sanitize_name
+from zerver.lib.upload import BadImageError, guess_type, sanitize_name
 from zerver.lib.utils import generate_api_key, process_list_in_batches
 from zerver.models import (
     AlertWord,
@@ -668,7 +669,7 @@ def import_uploads(realm: Realm, import_dir: Path, processes: int, processing_av
             # function 'upload_message_file'
             relative_path = "/".join([
                 str(record['realm_id']),
-                random_name(18),
+                secrets.token_urlsafe(18),
                 sanitize_name(os.path.basename(record['path'])),
             ])
             path_maps['attachment_path'][record['s3_path']] = relative_path

zerver/lib/redis_utils.py

@@ -1,12 +1,11 @@
 import re
+import secrets
 from typing import Any, Dict, Mapping, Optional
 
 import orjson
 import redis
 from django.conf import settings
 
-from zerver.lib.utils import generate_random_token
-
 # Redis accepts keys up to 512MB in size, but there's no reason for us to use such size,
 # so we want to stay limited to 1024 characters.
 MAX_KEY_LENGTH = 1024
@@ -34,7 +33,7 @@ def put_dict_in_redis(redis_client: redis.StrictRedis, key_format: str,
         error_msg = "Requested key too long in put_dict_in_redis. Key format: %s, token length: %s"
         raise ZulipRedisKeyTooLongError(error_msg % (key_format, token_length))
     if token is None:
-        token = generate_random_token(token_length)
+        token = secrets.token_hex(token_length // 2)
     key = key_format.format(token=token)
     with redis_client.pipeline() as pipeline:
         pipeline.set(key, orjson.dumps(data_to_store))

zerver/lib/upload.py

@@ -5,6 +5,7 @@ import logging
 import os
 import random
 import re
+import secrets
 import shutil
 import unicodedata
 import urllib
@@ -30,7 +31,6 @@ from PIL.Image import DecompressionBombError
 
 from zerver.lib.avatar_hash import user_avatar_path
 from zerver.lib.exceptions import ErrorCode, JsonableError
-from zerver.lib.utils import generate_random_token
 from zerver.models import Attachment, Message, Realm, RealmEmoji, UserProfile
 
 DEFAULT_AVATAR_SIZE = 100
@@ -94,9 +94,6 @@ def sanitize_name(value: str) -> str:
     assert value not in {'', '.', '..'}
     return mark_safe(value)
 
-def random_name(bytes: int=60) -> str:
-    return base64.urlsafe_b64encode(os.urandom(bytes)).decode('utf-8')
-
 class BadImageError(JsonableError):
     code = ErrorCode.BAD_IMAGE
 
@@ -362,7 +359,7 @@ class S3UploadBackend(ZulipUploadBackend):
             target_realm = user_profile.realm
         s3_file_name = "/".join([
             str(target_realm.id),
-            random_name(18),
+            secrets.token_urlsafe(18),
             sanitize_name(uploaded_file_name),
         ])
         url = f"/user_uploads/{s3_file_name}"
@@ -591,7 +588,7 @@ class S3UploadBackend(ZulipUploadBackend):
     def upload_export_tarball(self, realm: Optional[Realm], tarball_path: str,
                               percent_callback: Optional[Callable[[Any], None]]=None) -> str:
         # We use the avatar bucket, because it's world-readable.
-        key = self.avatar_bucket.Object(os.path.join("exports", generate_random_token(32),
+        key = self.avatar_bucket.Object(os.path.join("exports", secrets.token_hex(16),
                                                      os.path.basename(tarball_path)))
 
         key.upload_file(tarball_path, Callback=percent_callback)
@@ -672,7 +669,7 @@ class LocalUploadBackend(ZulipUploadBackend):
         path = "/".join([
             str(user_profile.realm_id),
             format(random.randint(0, 255), 'x'),
-            random_name(18),
+            secrets.token_urlsafe(18),
             sanitize_name(uploaded_file_name),
         ])
 
@@ -819,7 +816,7 @@ class LocalUploadBackend(ZulipUploadBackend):
         path = os.path.join(
             'exports',
             str(realm.id),
-            random_name(18),
+            secrets.token_urlsafe(18),
             os.path.basename(tarball_path),
         )
         abs_path = os.path.join(settings.LOCAL_UPLOADS_DIR, 'avatars', path)

zerver/lib/utils.py

@@ -1,10 +1,8 @@
-import base64
 import hashlib
 import heapq
 import itertools
-import os
 import re
-import string
+import secrets
 from itertools import zip_longest
 from time import sleep
 from typing import Any, Callable, Iterator, List, Optional, Sequence, Set, Tuple, TypeVar
@@ -107,14 +105,12 @@ def log_statsd_event(name: str) -> None:
     event_name = f"events.{name}"
     statsd.incr(event_name)
 
-def generate_random_token(length: int) -> str:
-    return str(base64.b16encode(os.urandom(length // 2)).decode('utf-8').lower())
-
 def generate_api_key() -> str:
-    choices = string.ascii_letters + string.digits
-    altchars = ''.join(choices[ord(os.urandom(1)) % 62] for _ in range(2)).encode("utf-8")
-    api_key = base64.b64encode(os.urandom(24), altchars=altchars).decode("utf-8")
-    return api_key
+    api_key = ""
+    while len(api_key) < 32:
+        # One iteration suffices 99.4992% of the time.
+        api_key += secrets.token_urlsafe(3 * 9).replace("_", "").replace("-", "")
+    return api_key[:32]
 
 def has_api_key_format(key: str) -> bool:
     return bool(re.fullmatch(r"([A-Za-z0-9]){32}", key))

zerver/migrations/0260_missed_message_addresses_from_redis_to_db.py

@@ -1,3 +1,5 @@
+import secrets
+
 from django.core.exceptions import ObjectDoesNotExist
 from django.db import migrations
 from django.db.backends.postgresql.schema import DatabaseSchemaEditor
@@ -5,11 +7,10 @@ from django.db.migrations.state import StateApps
 
 # Imported to avoid needing to duplicate redis-related code.
 from zerver.lib.redis_utils import get_redis_client
-from zerver.lib.utils import generate_random_token
 
 
 def generate_missed_message_token() -> str:
-    return 'mm' + generate_random_token(32)
+    return 'mm' + secrets.token_hex(16)
 
 def move_missed_message_addresses_to_database(apps: StateApps, schema_editor: DatabaseSchemaEditor) -> None:
     redis_client = get_redis_client()

zerver/models.py

@@ -1,6 +1,7 @@
 import ast
 import datetime
 import re
+import secrets
 import time
 from collections import defaultdict
 from datetime import timedelta
@@ -78,7 +79,7 @@ from zerver.lib.types import (
     UserFieldElement,
     Validator,
 )
-from zerver.lib.utils import generate_random_token, make_safe_digest
+from zerver.lib.utils import make_safe_digest
 from zerver.lib.validator import (
     check_date,
     check_int,
@@ -1464,7 +1465,7 @@ class PushDeviceToken(AbstractPushDeviceToken):
         unique_together = ("user", "kind", "token")
 
 def generate_email_token_for_stream() -> str:
-    return generate_random_token(32)
+    return secrets.token_hex(16)
 
 class Stream(models.Model):
     MAX_NAME_LENGTH = 60

zerver/tests/test_auth_backends.py

@@ -3,6 +3,7 @@ import copy
 import datetime
 import json
 import re
+import secrets
 import time
 import urllib
 from contextlib import contextmanager
@@ -67,7 +68,6 @@ from zerver.lib.test_helpers import (
 )
 from zerver.lib.upload import MEDIUM_AVATAR_SIZE, resize_avatar
 from zerver.lib.users import get_all_api_keys
-from zerver.lib.utils import generate_random_token
 from zerver.lib.validator import (
     Validator,
     check_bool,
@@ -549,7 +549,7 @@ class RateLimitAuthenticationTests(ZulipTestCase):
         # We have to mock RateLimitedAuthenticationByUsername.key to avoid key collisions
         # if tests run in parallel.
         original_key_method = RateLimitedAuthenticationByUsername.key
-        salt = generate_random_token(32)
+        salt = secrets.token_hex(16)
 
         def _mock_key(self: RateLimitedAuthenticationByUsername) -> str:
             return f"{salt}:{original_key_method(self)}"
@@ -2991,7 +2991,7 @@ class GoogleAuthBackendTest(SocialAuthBase):
             'redirect_to': '',
         }
         with mock.patch("logging.warning") as mock_warn:
-            token = generate_random_token(ExternalAuthResult.LOGIN_TOKEN_LENGTH)
+            token = secrets.token_hex(ExternalAuthResult.LOGIN_TOKEN_LENGTH // 2)
             result = self.get_log_into_subdomain(data, force_token=token)
             mock_warn.assert_called_once_with("log_into_subdomain: Invalid token given: %s", token)
         self.assertEqual(result.status_code, 400)

zerver/tests/test_create_video_call.py

@@ -163,7 +163,7 @@ class TestVideoCall(ZulipTestCase):
 
     def test_create_bigbluebutton_link(self) -> None:
         with mock.patch('zerver.views.video_calls.random.randint', return_value="1"), mock.patch(
-             'zerver.views.video_calls.random.SystemRandom.choice', return_value="A"):
+             'secrets.token_bytes', return_value=b"\x00" * 7):
             response = self.client_get("/json/calls/bigbluebutton/create")
             self.assert_json_success(response)
             self.assertEqual(response.json()['url'],

zerver/tests/test_rate_limiter.py

@@ -1,3 +1,4 @@
+import secrets
 import time
 from typing import Dict, List, Tuple, Type
 from unittest import mock
@@ -12,9 +13,8 @@ from zerver.lib.rate_limiter import (
     remove_ratelimit_rule,
 )
 from zerver.lib.test_classes import ZulipTestCase
-from zerver.lib.utils import generate_random_token
 
-RANDOM_KEY_PREFIX = generate_random_token(32)
+RANDOM_KEY_PREFIX = secrets.token_hex(16)
 
 class RateLimitedTestObject(RateLimitedObject):
     def __init__(self, name: str, rules: List[Tuple[int, int]],

zerver/tests/test_redis_utils.py

@@ -45,7 +45,7 @@ class RedisUtilsTest(ZulipTestCase):
 
         # Trying to put data under an overly long key should get stopped before even
         # generating the random token.
-        with mock.patch("zerver.lib.redis_utils.generate_random_token") as mock_generate:
+        with mock.patch("secrets.token_hex") as mock_generate:
             with self.assertRaises(ZulipRedisKeyTooLongError):
                 put_dict_in_redis(self.redis_client, self.key_format, data,
                                   expiration_seconds=self.expiration_seconds,

zerver/views/auth.py

@@ -1,5 +1,5 @@
 import logging
-import os
+import secrets
 import urllib
 from functools import wraps
 from typing import Any, Dict, List, Mapping, Optional, cast
@@ -289,7 +289,7 @@ def finish_desktop_flow(request: HttpRequest, user_profile: UserProfile,
     result = ExternalAuthResult(user_profile=user_profile)
     token = result.store_data()
     key = bytes.fromhex(otp)
-    iv = os.urandom(12)
+    iv = secrets.token_bytes(12)
     desktop_data = (iv + AESGCM(key).encrypt(iv, token.encode(), b"")).hex()
     context = {'desktop_data': desktop_data,
                'browser_url': reverse('zerver.views.auth.login_page',

zerver/views/home.py

@@ -1,4 +1,5 @@
 import logging
+import secrets
 from typing import Any, Dict, List, Optional, Tuple
 
 from django.conf import settings
@@ -19,7 +20,7 @@ from zerver.lib.push_notifications import num_push_devices_for_user
 from zerver.lib.streams import access_stream_by_name
 from zerver.lib.subdomains import get_subdomain
 from zerver.lib.users import compute_show_invites_and_add_streams
-from zerver.lib.utils import generate_random_token, statsd
+from zerver.lib.utils import statsd
 from zerver.models import PreregistrationUser, Realm, Stream, UserProfile
 from zerver.views.compatibility import is_outdated_desktop_app, is_unsupported_browser
 from zerver.views.portico import hello_view
@@ -200,7 +201,7 @@ def home_real(request: HttpRequest) -> HttpResponse:
 
     request._log_data['extra'] = "[{}]".format(queue_id)
 
-    csp_nonce = generate_random_token(48)
+    csp_nonce = secrets.token_hex(24)
 
     user_permission_info = get_user_permission_info(user_profile)
 

zerver/views/video_calls.py

@@ -1,7 +1,8 @@
 import hashlib
 import json
 import random
-import string
+import secrets
+from base64 import b32encode
 from functools import partial
 from typing import Dict
 from urllib.parse import quote, urlencode, urljoin
@@ -164,7 +165,7 @@ def get_bigbluebutton_url(request: HttpRequest, user_profile: UserProfile) -> Ht
     # https://docs.bigbluebutton.org/dev/api.html#create for reference on the api calls
     # https://docs.bigbluebutton.org/dev/api.html#usage for reference for checksum
     id = "zulip-" + str(random.randint(100000000000, 999999999999))
-    password = ''.join(random.SystemRandom().choice(string.ascii_uppercase + string.digits) for _ in range(10))
+    password = b32encode(secrets.token_bytes(7))[:10].decode()
     checksum = hashlib.sha1(("create" + "meetingID=" + id + "&moderatorPW="
                              + password + "&attendeePW=" + password + "a" + settings.BIG_BLUE_BUTTON_SECRET).encode()).hexdigest()
     url = add_query_to_redirect_url("/calls/bigbluebutton/join", urlencode({