Usage of insecure hash functions: sha1 and md5

[rdimaio]

Description

The following insecure hash functions are currently in use within Rucio:

• sha1
  • https://crypto.stackexchange.com/questions/48289/how-secure-is-sha1-what-are-the-chances-of-a-real-exploit
  • Vulnerable to collision attacks: https://www.nist.gov/news-events/news/2022/12/nist-retires-sha-1-cryptographic-algorithm
• md5
  • Vulnerable to collision attacks:
    • https://security.stackexchange.com/questions/138363/why-is-md5-considered-a-vulnerable-algorithm
    • https://stackoverflow.com/questions/30496061/why-not-use-md5-for-password-hashing
    • https://security.stackexchange.com/questions/19906/is-md5-considered-insecure
    • https://security.stackexchange.com/questions/106843/have-weaknesses-in-sha-1-and-md5-ever-actually-been-successfully-used-in-an-atta

The linter log below lists the uses identified by the linter, but there are other instances as well. For example, get_auth_token_user_pass expects a SHA1 hash of the password:

rucio/lib/rucio/api/authentication.py

Lines 124 to 147 in 834a7c0

	def get_auth_token_user_pass(account, username, password, appid, ip=None, vo='def', *, session: "Session"):
	"""
	Authenticate a Rucio account temporarily via username and password.
	The token lifetime is 1 hour.
	:param account: Account identifier as a string.
	:param username: Username as a string.
	:param password: SHA1 hash of the password as a string.
	:param appid: The application identifier as a string.
	:param ip: IP address of the client as a string.
	:param vo: The VO to act on.
	:param session: The database session in use.
	:returns: A dict with token and expires_at entries.
	"""
	kwargs = {'account': account, 'username': username, 'password': password}
	if not permission.has_permission(issuer=account, vo=vo, action='get_auth_token_user_pass', kwargs=kwargs, session=session):
	raise exception.AccessDenied('User with identity %s can not log to account %s' % (username, account))
	account = InternalAccount(account, vo=vo)
	return authentication.get_auth_token_user_pass(account, username, password, appid, ip, session=session)

To do

In each instance of sha1 and md5 usages:

• If possible, migrate to a stronger hash algorithm
  • For password hashing in particular, see here for recommendations: https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html
  • For most cases, SHA-256 is a good compromise between speed and safety
• Otherwise, add the inline comment # noqa: S324

Linter log

❯ ruff check --select S324                                                                                                                                                                                                                                                                                               ─╯
lib/rucio/common/dumper/data_models.py:175:13: S324 Probable use of insecure hash functions in `hashlib`: `sha1`
lib/rucio/common/utils.py:303:16: S324 Probable use of insecure hash functions in `hashlib`: `md5`
lib/rucio/core/did.py:98:20: S324 Probable use of insecure hash functions in `hashlib`: `md5`
lib/rucio/core/oidc.py:131:11: S324 Probable use of insecure hash functions in `hashlib`: `md5`
lib/rucio/daemons/auditor/hdfs.py:58:13: S324 Probable use of insecure hash functions in `hashlib`: `sha1`
lib/rucio/daemons/auditor/srmdumps.py:249:9: S324 Probable use of insecure hash functions in `hashlib`: `sha1`
lib/rucio/daemons/c3po/c3po.py:220:48: S324 Probable use of insecure hash functions in `hashlib`: `md5`
lib/rucio/rse/protocols/protocol.py:114:16: S324 Probable use of insecure hash functions in `hashlib`: `md5`
Found 8 errors.

Also other cases - e.g. get_auth_token_user_pass.

See also

• https://docs.astral.sh/ruff/rules/hashlib-insecure-hash-function/

Steps to reproduce

N/A

Rucio Version

No response

Additional Information

No response

[bari12]

The majority of these are just for the lfn2pfn methods, there it should be fine. The oidc one which shows up is something to check though.

[dchristidis]

The oidc one which shows up is something to check though.

A hash function is used to simplify avoiding they key restrictions of either dogpile.cache or Memcached (we could have devised a function that removes or replaces unacceptable characters instead). The payload that is hashed cannot be affected by a user, whether directly or indirectly. Given that MD5 is less expensive for the CPU, I don’t see sufficient motivation to replace it.