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

[sshedi]

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 Shedi [email protected]

• Issue: crypt module fails to import in FIPS mode #95231

Automerge-Triggered-By: GH:tiran

[ghost]

All commit authors signed the Contributor License Agreement.

[bedevere-bot]

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

[sshedi]

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]

Hi @gvanrossum @benjaminp @birkenfeld @freddrake,

Please review this PR, need your inputs.

[birkenfeld]

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

[erlend-aasland]

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]

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.

[tiran]

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.py
index 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)

[bedevere-bot]

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 phrase I 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]

Also, please add a NEWS entry using the blurb tool.

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

[sshedi]

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

cpython/Lib/test/pythoninfo.py

Line 840 in 310f948

with open("/proc/sys/crypto/fips_enabled", encoding="utf-8") as fp:

as reference.

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

[tiran]

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.

[sshedi]

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

[sshedi]

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]

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

@sshedi ☝🏻

[sshedi]

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

[sshedi]

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]

Thanks for making the requested changes!

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

[bedevere-bot]

Thanks for making the requested changes!

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

[erlend-aasland]

Updating branch to retrigger Azure Pipelines CI.

[miss-islington]

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

[miss-islington]

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

[bedevere-bot]

GH-95998 is a backport of this pull request to the 3.10 branch.

[miss-islington]

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

[bedevere-bot]

GH-95999 is a backport of this pull request to the 3.11 branch.

[mixmind]

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

[vstinner]

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/