Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhancement Request: Add usedforsecurity Argument to Hash Functions to Indicate Security Context Usage #10073

Open
gxx777 opened this issue Dec 27, 2023 · 4 comments

Comments

@gxx777
Copy link

gxx777 commented Dec 27, 2023

Hello,

Thank you for maintaining the cryptographic library.

I am a research student with an interest in cryptographic engineering. Recently, we developed a Cryptographic APIs Misuse Detector to identify potential misuses. We have also adapted our rules for the mainstream cryptography.io library. In comparing the use of insecure hash functions between the cryptography.io library and the hashlib standard library, we identified numerous misuses in real-world applications. Unfortunately, unlike the hashlib standard library, the cryptography.io library does not have a keyword-only argument like usedforsecurity to explicitly indicate whether it is used in a security context. I believe it would be beneficial to consider adding this feature. As a reference, in the official library, there is a set keyword-only argument usedforsecurity with the default value True. A false value allows the use of insecure and blocked hashing algorithms in restricted environments.

Reference: hashlib

Changed in version 3.9: All hashlib constructors take a keyword-only argument usedforsecurity with default value True. A false value allows the use of insecure and blocked hashing algorithms in restricted environments. False indicates that the hashing algorithm is not used in a security context, e.g. as a non-cryptographic one-way compression function.

Usage

h = hashlib.new('md5',usedforsecurity=False)
h.update(b"Nobody inspects the spammish repetition")
h.hexdigest()

I am grateful for your dedication to the cryptographic library. If this enhancement is considered, I would be delighted to contribute to the Python cryptographic library.

Thank you for your time and consideration.

@facutuesca
Copy link
Contributor

facutuesca commented Apr 22, 2024

Looking a bit into this, here's two ways we could implement it:

  1. Move the insecure hashing algorithms to the decrepit namespace (eventually, using deprecation warnings first). This way, using MD5 (or SHA1) would mean the user has to explicitly import it from decrepit.hashes.MD5.
  2. Use a similar API as the one mentioned above, from hashlib. This could be something like having hazmat.primitives.HashAlgorithm have an extra property HashAlgorithm.is_secure: bool, which MD5 and SHA1 would have as False, and then doing something like:
class HashAlgorithm(metaclass=abc.ABCMeta):
    # ....
    def __init__(self, used_for_security: bool = True):
        if used_for_security and not self.is_secure:
            raise InsecureAlgorithmException()

#....
class SHA1(HashAlgorithm):
    name = "sha1"
    digest_size = 20
    block_size = 64
    is_secure = False

    def __init__(self, used_for_security: bool = True):
        super().__init__(used_for_security)  # new
        if digest_size < 1:
            raise ValueError("digest_size must be a positive integer")

        self._digest_size = digest_size

#...
class SM3(HashAlgorithm):
    name = "sm3"
    digest_size = 32
    block_size = 64
    is_secure = True

    def __init__(self, used_for_security: bool = True):
        super().__init__(used_for_security)

#... etc 

cc @woodruffw @alex @reaperhulk

@woodruffw
Copy link
Contributor

@alex and @reaperhulk might be thinking something different, but IMO mirroring the usedforsecurity kwarg behavior in hashlib would be parsimonious here: rather than having any new exceptions, we could just add the usedforsecurity kwarg and leave it up to users to explicitly set True or False. From there, it's up to external tooling to determine whether the particular hash's use is security sensitive or not (e.g. MD5 is used for security in HMAC-MD5, but is still strong in that setting).

@alex
Copy link
Member

alex commented Apr 22, 2024 via email

@reaperhulk
Copy link
Member

Agreed with @alex, I’d rather structure it with decrepit, although that is also a very long term deprecation cycle to move the hashes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

5 participants