Clarify the usage of sha1 as a "secure" hash.

lib/galaxy/managers/users.py

@@ -40,7 +40,7 @@ from galaxy.structured_app import (
     BasicSharedApp,
     MinimalManagerApp,
 )
-from galaxy.util.hash_util import new_secure_hash
+from galaxy.util.hash_util import new_secure_hash_v2
 from galaxy.web import url_for
 
 log = logging.getLogger(__name__)
@@ -197,8 +197,8 @@ class UserManager(base.ModelManager, deletable.PurgableManagerMixin):
         # to identify if it is needed for some reason.
         #
         # Deleting multiple times will re-hash the username/email
-        email_hash = new_secure_hash(user.email + pseudorandom_value)
-        uname_hash = new_secure_hash(user.username + pseudorandom_value)
+        email_hash = new_secure_hash_v2(user.email + pseudorandom_value)
+        uname_hash = new_secure_hash_v2(user.username + pseudorandom_value)
         # We must also redact username
         for role in user.all_roles():
             if self.app.config.redact_username_during_deletion:
@@ -219,15 +219,15 @@ class UserManager(base.ModelManager, deletable.PurgableManagerMixin):
                 .all()
             )
             for addr in user_addresses:
-                addr.desc = new_secure_hash(addr.desc + pseudorandom_value)
-                addr.name = new_secure_hash(addr.name + pseudorandom_value)
-                addr.institution = new_secure_hash(addr.institution + pseudorandom_value)
-                addr.address = new_secure_hash(addr.address + pseudorandom_value)
-                addr.city = new_secure_hash(addr.city + pseudorandom_value)
-                addr.state = new_secure_hash(addr.state + pseudorandom_value)
-                addr.postal_code = new_secure_hash(addr.postal_code + pseudorandom_value)
-                addr.country = new_secure_hash(addr.country + pseudorandom_value)
-                addr.phone = new_secure_hash(addr.phone + pseudorandom_value)
+                addr.desc = new_secure_hash_v2(addr.desc + pseudorandom_value)
+                addr.name = new_secure_hash_v2(addr.name + pseudorandom_value)
+                addr.institution = new_secure_hash_v2(addr.institution + pseudorandom_value)
+                addr.address = new_secure_hash_v2(addr.address + pseudorandom_value)
+                addr.city = new_secure_hash_v2(addr.city + pseudorandom_value)
+                addr.state = new_secure_hash_v2(addr.state + pseudorandom_value)
+                addr.postal_code = new_secure_hash_v2(addr.postal_code + pseudorandom_value)
+                addr.country = new_secure_hash_v2(addr.country + pseudorandom_value)
+                addr.phone = new_secure_hash_v2(addr.phone + pseudorandom_value)
                 self.session().add(addr)
         # Purge the user
         super().purge(user, flush=flush)
@@ -563,7 +563,7 @@ class UserManager(base.ModelManager, deletable.PurgableManagerMixin):
         user = trans.sa_session.query(self.app.model.User).filter(self.app.model.User.table.c.email == email).first()
         activation_token = user.activation_token
         if activation_token is None:
-            activation_token = util.hash_util.new_secure_hash(str(random.getrandbits(256)))
+            activation_token = util.hash_util.new_secure_hash_v2(str(random.getrandbits(256)))
             user.activation_token = activation_token
             trans.sa_session.add(user)
             trans.sa_session.flush()

lib/galaxy/model/__init__.py

@@ -146,7 +146,7 @@ from galaxy.util.form_builder import (
     WorkflowField,
     WorkflowMappingField,
 )
-from galaxy.util.hash_util import new_secure_hash
+from galaxy.util.hash_util import new_insecure_hash
 from galaxy.util.json import safe_loads
 from galaxy.util.sanitize_html import sanitize_html
 
@@ -638,7 +638,7 @@ class User(Base, Dictifiable, RepresentById):
         if User.use_pbkdf2:
             self.password = galaxy.security.passwords.hash_password(cleartext)
         else:
-            self.password = new_secure_hash(text_type=cleartext)
+            self.password = new_insecure_hash(text_type=cleartext)
         self.last_password_change = now()
 
     def set_random_password(self, length=16):

lib/galaxy/tool_util/deps/__init__.py

@@ -431,7 +431,7 @@ class CachedDependencyManager(DependencyManager):
             (dep.name, dep.version, dep.exact, dep.dependency_type) for dep in resolved_dependencies
         ]
         hash_str = json.dumps(sorted(resolved_dependencies))
-        return hash_util.new_secure_hash(hash_str)[:8]  # short hash
+        return hash_util.new_insecure_hash(hash_str)[:8]  # short hash
 
     def get_hashed_dependencies_path(self, resolved_dependencies):
         """

lib/galaxy/util/hash_util.py

@@ -90,9 +90,25 @@ def md5_hash_file(path: Union[str, os.PathLike]) -> Optional[str]:
         return None
 
 
-def new_secure_hash(text_type: Union[bytes, str]) -> str:
+def new_secure_hash_v2(text_type: Union[bytes, str]) -> str:
+    """More modern version of new_secure_hash.
+
+    Certain passwords are set via new_insecure_hash (previously new_secure_hash),
+    so that needs to remain for legacy purposes.
     """
-    Returns the hexdigest of the sha1 hash of the argument `text_type`.
+    assert text_type is not None
+    return sha512(smart_str(text_type)).hexdigest()
+
+
+def new_insecure_hash(text_type: Union[bytes, str]) -> str:
+    """Returns the hexdigest of the sha1 hash of the argument `text_type`.
+
+    Previously called new_secure_hash, but this should not be considered
+    secure - SHA1 is no longer considered a secure hash and has been broken
+    since the early 2000s.
+
+    use_pbkdf2 should be set by default and galaxy.security.passwords should
+    be the default used for passwords in Galaxy.
     """
     assert text_type is not None
     return sha1(smart_str(text_type)).hexdigest()
@@ -110,4 +126,4 @@ def is_hashable(value: Any) -> bool:
     return True
 
 
-__all__ = ("md5", "hashlib", "sha1", "sha", "new_secure_hash", "hmac_new", "is_hashable")
+__all__ = ("md5", "hashlib", "sha1", "sha", "new_insecure_hash", "new_secure_hash_v2", "hmac_new", "is_hashable")

lib/tool_shed/util/admin_util.py

@@ -13,7 +13,7 @@ from galaxy import (
 )
 from galaxy.security.validate_user_input import validate_password
 from galaxy.util import inflector
-from galaxy.util.hash_util import new_secure_hash
+from galaxy.util.hash_util import new_secure_hash_v2
 from galaxy.web.form_builder import CheckboxField
 from galaxy.web.legacy_framework.grids import (
     Grid,
@@ -773,8 +773,8 @@ class Admin:
             compliance_log.info(f"delete-user-event: {user_id}")
             # See lib/galaxy/webapps/tool_shed/controllers/admin.py
             pseudorandom_value = str(int(time.time()))
-            email_hash = new_secure_hash(user.email + pseudorandom_value)
-            uname_hash = new_secure_hash(user.username + pseudorandom_value)
+            email_hash = new_secure_hash_v2(user.email + pseudorandom_value)
+            uname_hash = new_secure_hash_v2(user.username + pseudorandom_value)
             for role in user.all_roles():
                 print(
                     role, self.app.config.redact_username_during_deletion, self.app.config.redact_email_during_deletion

lib/tool_shed/webapp/model/__init__.py

@@ -48,7 +48,7 @@ from galaxy.security.validate_user_input import validate_password_str
 from galaxy.util import unique_id
 from galaxy.util.bunch import Bunch
 from galaxy.util.dictifiable import Dictifiable
-from galaxy.util.hash_util import new_secure_hash
+from galaxy.util.hash_util import new_insecure_hash
 from tool_shed.dependencies.repository import relation_builder
 from tool_shed.util import (
     hg_util,
@@ -152,7 +152,7 @@ class User(Base, Dictifiable, _HasTable):
 
     def check_password(self, cleartext):
         """Check if 'cleartext' matches 'self.password' when hashed."""
-        return self.password == new_secure_hash(text_type=cleartext)
+        return self.password == new_insecure_hash(text_type=cleartext)
 
     def get_disk_usage(self, nice_size=False):
         return 0
@@ -171,7 +171,7 @@ class User(Base, Dictifiable, _HasTable):
         if message:
             raise Exception(f"Invalid password: {message}")
         # Set 'self.password' to the digest of 'cleartext'.
-        self.password = new_secure_hash(text_type=cleartext)
+        self.password = new_insecure_hash(text_type=cleartext)
 
 
 class PasswordResetToken(Base, _HasTable):