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

Set usedforsecurity=False in hashlib methods (FIPS compliance) #1782

Merged
merged 1 commit into from
Oct 30, 2023

Conversation

Wauplin
Copy link
Contributor

@Wauplin Wauplin commented Oct 27, 2023

This PR aims to make huggingface_hub (and then other libraries from HF-ecosystem) FIPS-compliant.

Issue was first raised by @DueViktor in huggingface/transformers#27034 and huggingface/transformers#27038.

Quoting @DueViktor:

Transformers use MD5 from hashlib, which is not a secure algorithm, but are not specifying that it is for other purposes than security. This is causing issues for organisations following certain security standard. FIPS compliance could be an example.

From https://docs.python.org/3/library/hashlib.html:

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.

This PR copies what has been done in mlflow/mlflow#10106 and mlflow/mlflow#10119. We define a insecure_hashlib module that sets usedforsecurity=False by default in their hash methods. Usage is pretty much the same as hashlib:

# Use
from huggingface_hub.utils.insecure_hashlib import sha256
# instead of
from hashlib import sha256

# Use
from huggingface_hub.utils import insecure_hashlib
# instead of
import hashlib

Once this is merged, the plan is to make a release and then integrate in third-party libraries from the HF-ecosystem (starting by transformers, diffusers, datasets and evaluate). Thanks a lot @DueViktor for raising the question 👍 🙏

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Oct 27, 2023

The documentation is not available anymore as the PR was closed or merged.

Copy link
Member

@LysandreJik LysandreJik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this @Wauplin!

@Wauplin
Copy link
Contributor Author

Wauplin commented Oct 30, 2023

Cool, thanks for the reviews. I'll merge :)

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

Successfully merging this pull request may close these issues.

4 participants