utils: Remove make_safe_digest wrapper.

It’s unclear what was supposed to be “safe” about this wrapper.  The
hashlib API is fine without it, and we don’t want to encourage further
use of SHA-1.

Signed-off-by: Anders Kaseorg <anders@zulip.com>
This commit is contained in:
Anders Kaseorg 2023-07-18 15:44:51 -07:00 committed by Tim Abbott
parent 143baa4243
commit 052984bc14
7 changed files with 22 additions and 35 deletions

View File

@ -61,12 +61,12 @@ framework; as you can see, it's very little code on top of our
```python ```python
def user_profile_cache_key_id(email: str, realm_id: int) -> str: def user_profile_cache_key_id(email: str, realm_id: int) -> str:
return u"user_profile:%s:%s" % (make_safe_digest(email.strip()), realm_id,) return f"user_profile:{hashlib.sha1(email.strip().encode()).hexdigest()}:{realm_id}"
def user_profile_cache_key(email: str, realm: 'Realm') -> str: def user_profile_cache_key(email: str, realm: "Realm") -> str:
return user_profile_cache_key_id(email, realm.id) return user_profile_cache_key_id(email, realm.id)
@cache_with_key(user_profile_cache_key, timeout=3600*24*7) @cache_with_key(user_profile_cache_key, timeout=3600 * 24 * 7)
def get_user(email: str, realm: Realm) -> UserProfile: def get_user(email: str, realm: Realm) -> UserProfile:
return UserProfile.objects.select_related().get( return UserProfile.objects.select_related().get(
email__iexact=email.strip(), realm=realm) email__iexact=email.strip(), realm=realm)

View File

@ -1,3 +1,4 @@
import hashlib
import logging import logging
import os import os
import random import random
@ -33,7 +34,7 @@ from zerver.data_import.user_handler import UserHandler
from zerver.lib.emoji import name_to_codepoint from zerver.lib.emoji import name_to_codepoint
from zerver.lib.markdown import IMAGE_EXTENSIONS from zerver.lib.markdown import IMAGE_EXTENSIONS
from zerver.lib.upload.base import sanitize_name from zerver.lib.upload.base import sanitize_name
from zerver.lib.utils import make_safe_digest, process_list_in_batches from zerver.lib.utils import process_list_in_batches
from zerver.models import Reaction, RealmEmoji, Recipient, UserProfile from zerver.models import Reaction, RealmEmoji, Recipient, UserProfile
@ -899,7 +900,7 @@ def map_receiver_id_to_recipient_id(
def get_string_huddle_hash(id_list: List[str]) -> str: def get_string_huddle_hash(id_list: List[str]) -> str:
id_list = sorted(set(id_list)) id_list = sorted(set(id_list))
hash_key = ",".join(str(x) for x in id_list) hash_key = ",".join(str(x) for x in id_list)
return make_safe_digest(hash_key) return hashlib.sha1(hash_key.encode()).hexdigest()
def categorize_channels_and_map_with_id( def categorize_channels_and_map_with_id(

View File

@ -2,7 +2,6 @@ import hashlib
from django.conf import settings from django.conf import settings
from zerver.lib.utils import make_safe_digest
from zerver.models import UserProfile from zerver.models import UserProfile
@ -13,7 +12,7 @@ def gravatar_hash(email: str) -> str:
# outlining internationalization of email addresses, and regardless if we # outlining internationalization of email addresses, and regardless if we
# typo an address or someone manages to give us a non-ASCII address, let's # typo an address or someone manages to give us a non-ASCII address, let's
# not error out on it. # not error out on it.
return make_safe_digest(email.lower(), hashlib.md5) return hashlib.md5(email.lower().encode()).hexdigest()
def user_avatar_hash(uid: str) -> str: def user_avatar_hash(uid: str) -> str:
@ -24,7 +23,7 @@ def user_avatar_hash(uid: str) -> str:
# used a hash of the email address, not the user ID, and we salted # used a hash of the email address, not the user ID, and we salted
# it in order to make the hashing scheme different from Gravatar's. # it in order to make the hashing scheme different from Gravatar's.
user_key = uid + settings.AVATAR_SALT user_key = uid + settings.AVATAR_SALT
return make_safe_digest(user_key, hashlib.sha1) return hashlib.sha1(user_key.encode()).hexdigest()
def user_avatar_path(user_profile: UserProfile) -> str: def user_avatar_path(user_profile: UserProfile) -> str:

View File

@ -30,8 +30,6 @@ from django.http import HttpRequest
from django_stubs_ext import QuerySetAny from django_stubs_ext import QuerySetAny
from typing_extensions import ParamSpec from typing_extensions import ParamSpec
from zerver.lib.utils import make_safe_digest
if TYPE_CHECKING: if TYPE_CHECKING:
# These modules have to be imported for type annotations but # These modules have to be imported for type annotations but
# they cannot be imported at runtime due to cyclic dependency. # they cannot be imported at runtime due to cyclic dependency.
@ -190,7 +188,7 @@ def validate_cache_key(key: str) -> None:
# and only "control" characters are strictly disallowed, see: # and only "control" characters are strictly disallowed, see:
# https://github.com/memcached/memcached/blob/master/doc/protocol.txt # https://github.com/memcached/memcached/blob/master/doc/protocol.txt
# However, limiting the characters we allow in keys simiplifies things, # However, limiting the characters we allow in keys simiplifies things,
# and anyway we use make_safe_digest when forming some keys to ensure # and anyway we use a hash function when forming some keys to ensure
# the resulting keys fit the regex below. # the resulting keys fit the regex below.
# The regex checks "all characters between ! and ~ in the ascii table", # The regex checks "all characters between ! and ~ in the ascii table",
# which happens to be the set of all "nice" ascii characters. # which happens to be the set of all "nice" ascii characters.
@ -433,7 +431,7 @@ def bulk_cached_fetch(
def preview_url_cache_key(url: str) -> str: def preview_url_cache_key(url: str) -> str:
return f"preview_url:{make_safe_digest(url)}" return f"preview_url:{hashlib.sha1(url.encode()).hexdigest()}"
def display_recipient_cache_key(recipient_id: int) -> str: def display_recipient_cache_key(recipient_id: int) -> str:
@ -447,7 +445,7 @@ def display_recipient_bulk_get_users_by_id_cache_key(user_id: int) -> str:
def user_profile_cache_key_id(email: str, realm_id: int) -> str: def user_profile_cache_key_id(email: str, realm_id: int) -> str:
return f"user_profile:{make_safe_digest(email.strip())}:{realm_id}" return f"user_profile:{hashlib.sha1(email.strip().encode()).hexdigest()}:{realm_id}"
def user_profile_cache_key(email: str, realm: "Realm") -> str: def user_profile_cache_key(email: str, realm: "Realm") -> str:
@ -455,11 +453,11 @@ def user_profile_cache_key(email: str, realm: "Realm") -> str:
def user_profile_delivery_email_cache_key(delivery_email: str, realm: "Realm") -> str: def user_profile_delivery_email_cache_key(delivery_email: str, realm: "Realm") -> str:
return f"user_profile_by_delivery_email:{make_safe_digest(delivery_email.strip())}:{realm.id}" return f"user_profile_by_delivery_email:{hashlib.sha1(delivery_email.strip().encode()).hexdigest()}:{realm.id}"
def bot_profile_cache_key(email: str, realm_id: int) -> str: def bot_profile_cache_key(email: str, realm_id: int) -> str:
return f"bot_profile:{make_safe_digest(email.strip())}" return f"bot_profile:{hashlib.sha1(email.strip().encode()).hexdigest()}"
def user_profile_by_id_cache_key(user_profile_id: int) -> str: def user_profile_by_id_cache_key(user_profile_id: int) -> str:
@ -700,7 +698,9 @@ def to_dict_cache_key(message: "Message", realm_id: Optional[int] = None) -> str
def open_graph_description_cache_key(content: bytes, request: HttpRequest) -> str: def open_graph_description_cache_key(content: bytes, request: HttpRequest) -> str:
return "open_graph_description_path:{}".format(make_safe_digest(request.META["PATH_INFO"])) return "open_graph_description_path:{}".format(
hashlib.sha1(request.META["PATH_INFO"].encode()).hexdigest()
)
def flush_message(*, instance: "Message", **kwargs: object) -> None: def flush_message(*, instance: "Message", **kwargs: object) -> None:

View File

@ -1,23 +1,10 @@
import hashlib
import re import re
import secrets import secrets
from typing import TYPE_CHECKING, Callable, List, Optional, TypeVar from typing import Callable, List, Optional, TypeVar
if TYPE_CHECKING:
from hashlib import _Hash
T = TypeVar("T") T = TypeVar("T")
def make_safe_digest(string: str, hash_func: "Callable[[bytes], _Hash]" = hashlib.sha1) -> str:
"""
return a hex digest of `string`.
"""
# hashlib.sha1, md5, etc. expect bytes, so non-ASCII strings must
# be encoded.
return hash_func(string.encode()).hexdigest()
def generate_api_key() -> str: def generate_api_key() -> str:
api_key = "" api_key = ""
while len(api_key) < 32: while len(api_key) < 32:

View File

@ -7,7 +7,6 @@ from django.db.backends.base.schema import BaseDatabaseSchemaEditor
from django.db.migrations.state import StateApps from django.db.migrations.state import StateApps
from zerver.lib.upload import upload_backend from zerver.lib.upload import upload_backend
from zerver.lib.utils import make_safe_digest
from zerver.models import UserProfile from zerver.models import UserProfile
@ -20,7 +19,7 @@ from zerver.models import UserProfile
def patched_user_avatar_path(user_profile: UserProfile) -> str: def patched_user_avatar_path(user_profile: UserProfile) -> str:
email = user_profile.email email = user_profile.email
user_key = email.lower() + settings.AVATAR_SALT user_key = email.lower() + settings.AVATAR_SALT
return make_safe_digest(user_key, hashlib.sha1) return hashlib.sha1(user_key.encode()).hexdigest()
@patch("zerver.lib.upload.s3.user_avatar_path", patched_user_avatar_path) @patch("zerver.lib.upload.s3.user_avatar_path", patched_user_avatar_path)

View File

@ -1,5 +1,6 @@
import ast import ast
import datetime import datetime
import hashlib
import secrets import secrets
import time import time
from contextlib import suppress from contextlib import suppress
@ -100,7 +101,7 @@ from zerver.lib.types import (
UserFieldElement, UserFieldElement,
Validator, Validator,
) )
from zerver.lib.utils import generate_api_key, make_safe_digest from zerver.lib.utils import generate_api_key
from zerver.lib.validator import ( from zerver.lib.validator import (
check_date, check_date,
check_int, check_int,
@ -2811,7 +2812,7 @@ def get_client(name: str) -> Client:
def get_client_cache_key(name: str) -> str: def get_client_cache_key(name: str) -> str:
return f"get_client:{make_safe_digest(name)}" return f"get_client:{hashlib.sha1(name.encode()).hexdigest()}"
@cache_with_key(get_client_cache_key, timeout=3600 * 24 * 7) @cache_with_key(get_client_cache_key, timeout=3600 * 24 * 7)
@ -4035,7 +4036,7 @@ class Huddle(models.Model):
def get_huddle_hash(id_list: List[int]) -> str: def get_huddle_hash(id_list: List[int]) -> str:
id_list = sorted(set(id_list)) id_list = sorted(set(id_list))
hash_key = ",".join(str(x) for x in id_list) hash_key = ",".join(str(x) for x in id_list)
return make_safe_digest(hash_key) return hashlib.sha1(hash_key.encode()).hexdigest()
def huddle_hash_cache_key(huddle_hash: str) -> str: def huddle_hash_cache_key(huddle_hash: str) -> str: