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

🔧 Add FIPS compliant flag to md5 call #162

Merged

Conversation

gabor-varga
Copy link
Contributor

Closes #161

@welcome
Copy link

welcome bot commented Sep 8, 2023

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out EBP's Code of Conduct and our Contributing Guide, as this will greatly help the review process.

Welcome to the EBP community! 🎉

@chrisjsewell
Copy link
Member

Heya, usedforsecurity was only added in Python 3.9 https://docs.python.org/3/library/hashlib.html#hash-algorithms, so some handling for <3.9 needs to be implemented 😄

@gabor-varga
Copy link
Contributor Author

Heya, usedforsecurity was only added in Python 3.9 https://docs.python.org/3/library/hashlib.html#hash-algorithms, so some handling for <3.9 needs to be implemented 😄

Ah indeed, and I thought it was just a nice one-line PR :)

@chrisjsewell
Copy link
Member

Ah indeed, and I thought it was just a nice one-line PR

They never are 😂

@gabor-varga
Copy link
Contributor Author

gabor-varga commented Sep 8, 2023

Ah indeed, and I thought it was just a nice one-line PR

They never are 😂

So I am not too proficient with python, maybe you can recommend the best solution for this. This seems to be straightforward:

has_usedforsecurity_support = float(sys.version[:3]) >= 3.9

Or something with the inspect module to check if md5 function supports the usedforsecurity kwargs, although that seems convoluted.

And then just use something like:

md5_kwargs = {"usedforsecurity": False} if has_usedforsecurity_support  else {}
hash = hashlib.md5(content.encode("utf8"), **md5_kwargs ).hexdigest()

@codecov
Copy link

codecov bot commented Sep 8, 2023

Codecov Report

Patch coverage is 100.00% of modified lines.

Files Changed Coverage
sphinx_design/extension.py 100.00%

📢 Thoughts on this report? Let us know!.

@gabor-varga
Copy link
Contributor Author

@chrisjsewell do you think this is sufficient? I'll use my fork until the next release.

@chrisjsewell
Copy link
Member

@chrisjsewell do you think this is sufficient? I'll use my fork until the next release.

Yep I think it's fine but bare with me to merge

@codecov-commenter
Copy link

codecov-commenter commented May 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.01%. Comparing base (074f21f) to head (4651d30).

Current head 4651d30 differs from pull request most recent head f66fd84

Please upload reports for the commit f66fd84 to get more accurate results.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #162      +/-   ##
==========================================
+ Coverage   89.18%   90.01%   +0.82%     
==========================================
  Files          11       11              
  Lines         962      951      -11     
==========================================
- Hits          858      856       -2     
+ Misses        104       95       -9     
Flag Coverage Δ
pytests 90.01% <100.00%> (+0.82%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@chrisjsewell chrisjsewell changed the title Added FIPS compliant flag to md5 call 🔧 Add FIPS compliant flag to md5 call May 21, 2024
Copy link
Member

@chrisjsewell chrisjsewell left a comment

Choose a reason for hiding this comment

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

Sorry for the wait but all good now, and removed 3.9 check as 3.8 was dropped 😄

@chrisjsewell chrisjsewell merged commit af64472 into executablebooks:main May 21, 2024
11 of 15 checks passed
Copy link

welcome bot commented May 21, 2024

Congrats on your first merged pull request in this project! 🎉
congrats

Thank you for contributing, we are very proud of you! ❤️

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.

Add support for FIPS enabled systems
3 participants