From 052984bc14c7bf02c48d5722349edb1d54d86ded Mon Sep 17 00:00:00 2001 From: Anders Kaseorg Date: Tue, 18 Jul 2023 15:44:51 -0700 Subject: [PATCH] utils: Remove make_safe_digest wrapper. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- docs/subsystems/caching.md | 6 +++--- zerver/data_import/rocketchat.py | 5 +++-- zerver/lib/avatar_hash.py | 5 ++--- zerver/lib/cache.py | 16 ++++++++-------- zerver/lib/utils.py | 15 +-------------- .../0032_verify_all_medium_avatar_images.py | 3 +-- zerver/models.py | 7 ++++--- 7 files changed, 22 insertions(+), 35 deletions(-) diff --git a/docs/subsystems/caching.md b/docs/subsystems/caching.md index 9579a62f1c..f0e7475e0f 100644 --- a/docs/subsystems/caching.md +++ b/docs/subsystems/caching.md @@ -61,12 +61,12 @@ framework; as you can see, it's very little code on top of our ```python 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) -@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: return UserProfile.objects.select_related().get( email__iexact=email.strip(), realm=realm) diff --git a/zerver/data_import/rocketchat.py b/zerver/data_import/rocketchat.py index 0879abeebf..2ffbd73948 100644 --- a/zerver/data_import/rocketchat.py +++ b/zerver/data_import/rocketchat.py @@ -1,3 +1,4 @@ +import hashlib import logging import os 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.markdown import IMAGE_EXTENSIONS 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 @@ -899,7 +900,7 @@ def map_receiver_id_to_recipient_id( def get_string_huddle_hash(id_list: List[str]) -> str: id_list = sorted(set(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( diff --git a/zerver/lib/avatar_hash.py b/zerver/lib/avatar_hash.py index 9ace9d1096..c500c12173 100644 --- a/zerver/lib/avatar_hash.py +++ b/zerver/lib/avatar_hash.py @@ -2,7 +2,6 @@ import hashlib from django.conf import settings -from zerver.lib.utils import make_safe_digest from zerver.models import UserProfile @@ -13,7 +12,7 @@ def gravatar_hash(email: str) -> str: # outlining internationalization of email addresses, and regardless if we # typo an address or someone manages to give us a non-ASCII address, let's # 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: @@ -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 # it in order to make the hashing scheme different from Gravatar's. 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: diff --git a/zerver/lib/cache.py b/zerver/lib/cache.py index e6cbeb5537..a35fc3a14f 100644 --- a/zerver/lib/cache.py +++ b/zerver/lib/cache.py @@ -30,8 +30,6 @@ from django.http import HttpRequest from django_stubs_ext import QuerySetAny from typing_extensions import ParamSpec -from zerver.lib.utils import make_safe_digest - if TYPE_CHECKING: # These modules have to be imported for type annotations but # 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: # https://github.com/memcached/memcached/blob/master/doc/protocol.txt # 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 regex checks "all characters between ! and ~ in the ascii table", # 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: - 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: @@ -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: - 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: @@ -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: - 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: - 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: @@ -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: - 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: diff --git a/zerver/lib/utils.py b/zerver/lib/utils.py index b545f8892c..a0fd067841 100644 --- a/zerver/lib/utils.py +++ b/zerver/lib/utils.py @@ -1,23 +1,10 @@ -import hashlib import re import secrets -from typing import TYPE_CHECKING, Callable, List, Optional, TypeVar - -if TYPE_CHECKING: - from hashlib import _Hash +from typing import Callable, List, Optional, TypeVar 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: api_key = "" while len(api_key) < 32: diff --git a/zerver/migrations/0032_verify_all_medium_avatar_images.py b/zerver/migrations/0032_verify_all_medium_avatar_images.py index 8de2324a55..d2e3f75572 100644 --- a/zerver/migrations/0032_verify_all_medium_avatar_images.py +++ b/zerver/migrations/0032_verify_all_medium_avatar_images.py @@ -7,7 +7,6 @@ from django.db.backends.base.schema import BaseDatabaseSchemaEditor from django.db.migrations.state import StateApps from zerver.lib.upload import upload_backend -from zerver.lib.utils import make_safe_digest from zerver.models import UserProfile @@ -20,7 +19,7 @@ from zerver.models import UserProfile def patched_user_avatar_path(user_profile: UserProfile) -> str: email = user_profile.email 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) diff --git a/zerver/models.py b/zerver/models.py index 45860f6562..e3202812c7 100644 --- a/zerver/models.py +++ b/zerver/models.py @@ -1,5 +1,6 @@ import ast import datetime +import hashlib import secrets import time from contextlib import suppress @@ -100,7 +101,7 @@ from zerver.lib.types import ( UserFieldElement, 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 ( check_date, check_int, @@ -2811,7 +2812,7 @@ def get_client(name: str) -> Client: 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) @@ -4035,7 +4036,7 @@ class Huddle(models.Model): def get_huddle_hash(id_list: List[int]) -> str: id_list = sorted(set(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: