Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

gh-95231: Disable md5 & crypt modules if FIPS is enabled#94742

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
miss-islington merged 3 commits intopython:mainfromsshedi:crypt-fips-fix
Aug 15, 2022
Merged

gh-95231: Disable md5 & crypt modules if FIPS is enabled#94742

miss-islington merged 3 commits intopython:mainfromsshedi:crypt-fips-fix
Aug 15, 2022

Conversation

@sshedi
Copy link
Contributor

@sshedisshedi commentedJul 11, 2022•
edited by miss-islington
Loading

If kernel fips is enabled, we get permission error upon doing
import crypt. So, if kernel fips is enabled, disable the
unallowed hashing methods.

Python 3.9.1 (default, May 10 2022, 11:36:26)
[GCC 10.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.

import crypt
Traceback (most recent call last):
File "", line 1, in
File "/usr/lib/python3.9/crypt.py", line 117, in
_add_method('MD5', '1', 8, 34)
File "/usr/lib/python3.9/crypt.py", line 94, in _add_method
result = crypt('', salt)
File "/usr/lib/python3.9/crypt.py", line 82, in crypt
return _crypt.crypt(word, salt)
PermissionError: [Errno 1] Operation not permitted

Signed-off-by: Shreenidhi Shedisshedi@vmware.com

Automerge-Triggered-By: GH:tiran

arhadthedev reacted with thumbs up emoji
@ghost
Copy link

ghost commentedJul 11, 2022•
edited by ghost
Loading

All commit authors signed the Contributor License Agreement.
CLA signed

@bedevere-bot
Copy link

Most changes to Pythonrequire a NEWS entry.

Please add it using theblurb_it web app or theblurb command-line tool.

@sshedi
Copy link
ContributorAuthor

Hi@vstinner,@erlend-aasland,@rhettinger
Please take a look at my code changes. Do I need to create a github issue for this? This is my first attempt to contribute to upstream python. Please help me out.

@sshedi
Copy link
ContributorAuthor

Hi@gvanrossum@benjaminp@birkenfeld@freddrake,

Please review this PR, need your inputs.

@birkenfeld
Copy link
Member

Please abstain from pinging random developers. Seehttps://www.python.org/dev/core-mentorship/ for getting help with your first contributions and in generalhttps://devguide.python.org/ for documentation of our process.

erlend-aasland and vstinner reacted with thumbs up emoji

@erlend-aasland
Copy link
Contributor

Hi,@sshedi; thanks for your interest in improving CPython! Please create an issue and explain why these changes are needed.

Also, for the future, please follow Georg Brandl's advice: start with the devguide; it contains a lot of important information for new contributors.

@sshedi
Copy link
ContributorAuthor

Thanks@erlend-aasland and@birkenfeld for your response.

I have created an issue here#95231
I will go through the development process, I checked the top contributors and tagged them here to get some insights on my PR.

I believe I have given sufficient info on the issue I'm facing.

@erlend-aaslanderlend-aasland changed the titleLib/crypt.py: disable md5 & crypt modules if kernel fips is enabled.gh-95231: Disable md5 & crypt modules if FIPS is enabledJul 25, 2022
@erlend-aaslanderlend-aasland linked an issueJul 25, 2022 that may beclosed by this pull request
Copy link
Member

@tirantiran left a comment

Choose a reason for hiding this comment

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

The approach with reading from/proc/sys/crypto/fips_enabled and manually disabling MD5 and CRYPT is not portable. The proc interface is Linux specific. The system may block more or less algorithms depending on the local crypto policy.

I recommend that we should catch more errno values in_add_method instead.

diff --git a/Lib/crypt.py b/Lib/crypt.pyindex 46c3de8474b..92e70415e1a 100644--- a/Lib/crypt.py+++ b/Lib/crypt.py@@ -100,6 +100,9 @@ def _add_method(name, *args, rounds=None):         # Not all libc libraries support all encryption methods.         if e.errno == errno.EINVAL:             return False+        # unsupported or blocked by crypto policy+        if e.errno in {errno.EPERM, errno.ENOSYS}:+            return False         raise     if result and len(result) == method.total_size:         methods.append(method)

erlend-aasland reacted with thumbs up emoji
@bedevere-bot
Copy link

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phraseI have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@erlend-aasland
Copy link
Contributor

Also, please add a NEWS entry using theblurb tool.

https://devguide.python.org/getting-started/pull-request-lifecycle/

@sshedi
Copy link
ContributorAuthor

@tiran Valid point. But how other python modules are detecting kernel fips status?
I made my changes using

withopen("/proc/sys/crypto/fips_enabled",encoding="utf-8")asfp:
as reference.

Suggested code works but can we have something to know that we got EPERM because of fips?

@tiran
Copy link
Member

Your proposed solution is solving the wrong problem, or at least a too narrow part of a general problem. It is trying to detect FIPS mode and then hard-codes which algorithms it thinks are disabled in FIPS mode. FIPS is an evolving standard. For example FIPS 140-3 blocks some algorithms are are allowed in FIPS 140-2. There are also more crypto policy standards than FIPS. FIPS is only relevant for the US government. It is irrelevant for the rest of the world and non-government software inside the US.

My solution is simpler and should fix your problem with less code. It is also not tight to FIPS and can handle other crypto policies that block algorithms.

I don't know why libcrypt on your system raises a permission error. I recommend that your contact your vendor and make an inquiry.

erlend-aasland and arhadthedev reacted with thumbs up emoji

@sshedi
Copy link
ContributorAuthor

Thanks@tiran for the detailed explanation, I agree. I will make the suggested changes.

@sshedi
Copy link
ContributorAuthor

The EPERM error is coming from libcrypt and glibc is doing it.

https://github.com/bminor/glibc/blob/ca4d3ea5130d66e66c5af14e958e99341bf20689/crypt/crypt-entry.c#L89

      /* FIPS rules out MD5 password encryption.  */      if (fips_enabled_p ()){  __set_errno (EPERM);  return NULL;}

@erlend-aasland
Copy link
Contributor

erlend-aasland commentedJul 25, 2022•
edited
Loading

For the record; please do not force push PRs, as the GitHub UI do not play well with force-pushes. Please usegit merge --no-ff main instead.

@sshedi ☝🏻

@sshedi
Copy link
ContributorAuthor

Sorry@erlend-aasland , it has become a habit for me now.git push origin -f <my-branch-name>
Will give your suggestion a try.

erlend-aasland and arhadthedev reacted with thumbs up emoji

@sshedi
Copy link
ContributorAuthor

I have made the requested changes; please review again.

@erlend-aasland - Sorry, I amended y changes addressing review comments so I have to force push to my branch.

@bedevere-bot
Copy link

Thanks for making the requested changes!

@tiran: please review the changes made to this pull request.

@bedevere-bot
Copy link

Thanks for making the requested changes!

@erlend-aasland,@tiran: please review the changes made to this pull request.

@erlend-aasland
Copy link
Contributor

Updating branch to retrigger Azure Pipelines CI.

@miss-islingtonmiss-islington merged commit2fa03b1 intopython:mainAug 15, 2022
@miss-islington
Copy link
Contributor

Thanks@sshedi for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Sorry@sshedi, I had trouble checking out the3.11 backport branch.
Please backport usingcherry_picker on command line.
cherry_picker 2fa03b1b0708d5d74630c351ec9abd2aac7550da 3.11

@miss-islingtonmiss-islington self-assigned thisAug 15, 2022
@bedevere-botbedevere-bot removed the needs backport to 3.10only security fixes labelAug 15, 2022
@bedevere-bot
Copy link

GH-95998 is a backport of this pull request to the3.10 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull requestAug 15, 2022
…nGH-94742)If kernel fips is enabled, we get permission error upon doing`import crypt`. So, if kernel fips is enabled, disable theunallowed hashing methods.Python 3.9.1 (default, May 10 2022, 11:36:26)[GCC 10.2.0] on linuxType "help", "copyright", "credits" or "license" for more information.>>> import cryptTraceback (most recent call last):  File "<stdin>", line 1, in <module>  File "/usr/lib/python3.9/crypt.py", line 117, in <module>    _add_method('MD5', '1', 8, 34)  File "/usr/lib/python3.9/crypt.py", line 94, in _add_method    result = crypt('', salt)  File "/usr/lib/python3.9/crypt.py", line 82, in crypt    return _crypt.crypt(word, salt)PermissionError: [Errno 1] Operation not permittedSigned-off-by: Shreenidhi Shedi <sshedi@vmware.com>(cherry picked from commit2fa03b1)Co-authored-by: Shreenidhi Shedi <53473811+sshedi@users.noreply.github.com>
@tirantiran added needs backport to 3.11only security fixes and removed needs backport to 3.11only security fixes labelsAug 15, 2022
@miss-islington
Copy link
Contributor

Thanks@sshedi for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull requestAug 15, 2022
…nGH-94742)If kernel fips is enabled, we get permission error upon doing`import crypt`. So, if kernel fips is enabled, disable theunallowed hashing methods.Python 3.9.1 (default, May 10 2022, 11:36:26)[GCC 10.2.0] on linuxType "help", "copyright", "credits" or "license" for more information.>>> import cryptTraceback (most recent call last):  File "<stdin>", line 1, in <module>  File "/usr/lib/python3.9/crypt.py", line 117, in <module>    _add_method('MD5', '1', 8, 34)  File "/usr/lib/python3.9/crypt.py", line 94, in _add_method    result = crypt('', salt)  File "/usr/lib/python3.9/crypt.py", line 82, in crypt    return _crypt.crypt(word, salt)PermissionError: [Errno 1] Operation not permittedSigned-off-by: Shreenidhi Shedi <sshedi@vmware.com>(cherry picked from commit2fa03b1)Co-authored-by: Shreenidhi Shedi <53473811+sshedi@users.noreply.github.com>
@bedevere-botbedevere-bot removed the needs backport to 3.11only security fixes labelAug 15, 2022
@bedevere-bot
Copy link

GH-95999 is a backport of this pull request to the3.11 branch.

miss-islington added a commit that referenced this pull requestAug 15, 2022
If kernel fips is enabled, we get permission error upon doing`import crypt`. So, if kernel fips is enabled, disable theunallowed hashing methods.Python 3.9.1 (default, May 10 2022, 11:36:26)[GCC 10.2.0] on linuxType "help", "copyright", "credits" or "license" for more information.>>> import cryptTraceback (most recent call last):  File "<stdin>", line 1, in <module>  File "/usr/lib/python3.9/crypt.py", line 117, in <module>    _add_method('MD5', '1', 8, 34)  File "/usr/lib/python3.9/crypt.py", line 94, in _add_method    result = crypt('', salt)  File "/usr/lib/python3.9/crypt.py", line 82, in crypt    return _crypt.crypt(word, salt)PermissionError: [Errno 1] Operation not permittedSigned-off-by: Shreenidhi Shedi <sshedi@vmware.com>(cherry picked from commit2fa03b1)Co-authored-by: Shreenidhi Shedi <53473811+sshedi@users.noreply.github.com>
@sshedisshedi deleted the crypt-fips-fix branchAugust 18, 2022 07:34
miss-islington added a commit that referenced this pull requestAug 30, 2022
If kernel fips is enabled, we get permission error upon doing`import crypt`. So, if kernel fips is enabled, disable theunallowed hashing methods.Python 3.9.1 (default, May 10 2022, 11:36:26)[GCC 10.2.0] on linuxType "help", "copyright", "credits" or "license" for more information.>>> import cryptTraceback (most recent call last):  File "<stdin>", line 1, in <module>  File "/usr/lib/python3.9/crypt.py", line 117, in <module>    _add_method('MD5', '1', 8, 34)  File "/usr/lib/python3.9/crypt.py", line 94, in _add_method    result = crypt('', salt)  File "/usr/lib/python3.9/crypt.py", line 82, in crypt    return _crypt.crypt(word, salt)PermissionError: [Errno 1] Operation not permittedSigned-off-by: Shreenidhi Shedi <sshedi@vmware.com>(cherry picked from commit2fa03b1)Co-authored-by: Shreenidhi Shedi <53473811+sshedi@users.noreply.github.com>
@mixmind
Copy link

mixmind commentedMay 3, 2024•
edited
Loading

Hi everyone, any chance of backport it to 3.9 too??

@vstinner
Copy link
Member

Hi everyone, any chance of backport it to 3.9 too??

That's somehow a new feature, and 3.9 only accept security fixes:https://devguide.python.org/versions/

mixmind reacted with confused emoji

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

Reviewers

@erlend-aaslanderlend-aaslanderlend-aasland approved these changes

@tirantiranAwaiting requested review from tiran

Assignees

@miss-islingtonmiss-islington

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

crypt module fails to import in FIPS mode

8 participants

@sshedi@bedevere-bot@birkenfeld@erlend-aasland@tiran@miss-islington@mixmind@vstinner
[8]ページ先頭
©2009-2025 Movatter.jp