Context Navigation

← Previous Ticket
Next Ticket →

#28401 closed Cleanup/optimization (fixed)

Allow hashlib.md5() calls to work with FIPS kernels

Reported by:	Andrew DiPrinzio	Owned by:	Ade Lee
Component:	Core (Other)	Version:	dev
Severity:	Normal	Keywords:	FIPS, md5, python3.9
Cc:		Triage Stage:	Ready for checkin
Has patch:	yes	Needs documentation:	no
Needs tests:	no	Patch needs improvement:	no
Easy pickings:	no	UI/UX:	no

Description

Specifically, every use of hashlib.md5() is an issue for FIPS kernels which lack openssl support for md5. However, at least on RHEL and Centos, hashlib.new() supports the usedforsecurity=False flag that allows you to bypass the FIPS prohibition on md5. Many of the cases in which the md5 function is used qualify as non-security uses since it is used to mainly truncate values. Of course the md5 auth backends would constitute a security usages and should not include the usedforsecurity=False and thus should fail on FIPS systems.

Therefore i propose that we add the usedforsecurity=False flag where warranted and handling in case some versions of python do not support this flag. (I tested python 2.7 on OSX and it errors when the usedforsecurity flag is set). If everyone this this is a good plan I will go ahead and make a PR.

list of all useages of hashlib.md5
https://github.com/django/django/search?q=hashlib.md5&type=Code&utf8=%E2%9C%93

django-developers thread
https://groups.google.com/forum/#!msg/django-developers/dlUIPzQgnpM/Mtl7CQbPAQAJ
django-users thread
https://groups.google.com/forum/#!topic/django-users/THJdhaKo-ng

Example stack trace:

Operations to perform:
  Apply all migrations: admin, auth, contenttypes, dashboard, kombu_transport_django, sessions
Running migrations:
  Applying contenttypes.0001_initial...Traceback (most recent call last):
  File "manage.py", line 10, in <module>
    execute_from_command_line(sys.argv)
  File "/usr/lib64/python2.7/site-packages/django/core/management/__init__.py", line 367, in execute_from_command_line
    utility.execute()
  File "/usr/lib64/python2.7/site-packages/django/core/management/__init__.py", line 359, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/usr/lib64/python2.7/site-packages/django/core/management/base.py", line 294, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/usr/lib64/python2.7/site-packages/django/core/management/base.py", line 345, in execute
    output = self.handle(*args, **options)
  File "/usr/lib64/python2.7/site-packages/django/core/management/commands/migrate.py", line 204, in handle
    fake_initial=fake_initial,
  File "/usr/lib64/python2.7/site-packages/django/db/migrations/executor.py", line 115, in migrate
    state = self._migrate_all_forwards(state, plan, full_plan, fake=fake, fake_initial=fake_initial)
  File "/usr/lib64/python2.7/site-packages/django/db/migrations/executor.py", line 145, in _migrate_all_forwards
    state = self.apply_migration(state, migration, fake=fake, fake_initial=fake_initial)
  File "/usr/lib64/python2.7/site-packages/django/db/migrations/executor.py", line 244, in apply_migration
    state = migration.apply(state, schema_editor)
  File "/usr/lib64/python2.7/site-packages/django/db/migrations/migration.py", line 129, in apply
    operation.database_forwards(self.app_label, schema_editor, old_state, project_state)
  File "/usr/lib64/python2.7/site-packages/django/db/migrations/operations/models.py", line 532, in database_forwards
    getattr(new_model._meta, self.option_name, set()),
  File "/usr/lib64/python2.7/site-packages/django/db/backends/base/schema.py", line 333, in alter_unique_together
    self.execute(self._create_unique_sql(model, columns))
  File "/usr/lib64/python2.7/site-packages/django/db/backends/base/schema.py", line 913, in _create_unique_sql
    "name": self.quote_name(self._create_index_name(model, columns, suffix="_uniq")),
  File "/usr/lib64/python2.7/site-packages/django/db/backends/base/schema.py", line 819, in _create_index_name
    index_unique_name = '_%s' % self._digest(table_name, *column_names)
  File "/usr/lib64/python2.7/site-packages/django/db/backends/base/schema.py", line 123, in _digest
    h = hashlib.md5()
ValueError: error:060800A3:digital envelope routines:EVP_DigestInit_ex:disabled for fips

Change History (20)

follow-up: 4 comment:1 by Markus Holtermann, 7 years ago

Type:	Bug → Cleanup/optimization

Thanks for the report.

While I like the idea, useforsecurity doesn't seem to be part of the official Python package but rather something that Red Hat added and thus is only available on RHEL, Centos, etc.

Last edited 7 years ago by Tim Graham (previous) (diff)

comment:2 by Tim Graham, 7 years ago

Component:	Uncategorized → Core (Other)
Summary:	django doesn't work with FIPS kernels → Allow hashlib.md5() calls to work with FIPS kernels
Triage Stage:	Unreviewed → Accepted

If the Python patch is merged, we could use the new parameter.

comment:3 by Aymeric Augustin, 7 years ago

Triage Stage:	Accepted → Someday/Maybe

Considering that the patch for Python isn't merged, it could take a bit of time until there's a Python release including it.

in reply to: 1 comment:4 by Andrew DiPrinzio, 7 years ago

Replying to Markus Holtermann:

Thanks for the report.

While I like the idea, useforsecurity doesn't seem to be part of the official Python package but rather something that Red Hat added and thus is only available on RHEL, Centos, etc.

Thanks all for your insights! I continued to do research after opening this ticket. You are correct that this flag is only in some fedora distributions. Also i think it is fair to say that this not really a bug in django but rather a deficiency in python's hashlib. I will try and move that ticket forward on the python side. https://bugs.python.org/issue9216 However since some some distributions have this flag would it be acceptable to add support for this flag? something like this?

try:
  hashlib.md5("blah")
except ValueError e:
  # the fedora fix throws value errors for issues with FIPS
  hashlib.md5("blah", usedforsecurity=False)

comment:5 by Tim Graham, 7 years ago

If the patch is merged to Python, I don't mind adding a wrapper function for that pattern until Django drops support for Python versions without the new parameter, however, I don't want to support the parameter until the Python issue resolved.

comment:6 by Joshua Cornutt, 5 years ago

This ticket was referenced in my PR for a similar fix/feature - https://github.com/django/django/pull/10605
Was there ever any consensus on the best way to handle these sort of FIPS-related fixes going forward?

follow-up: 8 comment:7 by Tim Graham, 5 years ago

I'm not aware of any discussion of the issue outside of the ticket. You can write to the DevelopersMailingList if you want to get a second opinion on your proposal. As a general solution (replacing all hashlib.md5() calls in Django with a fallback to sha256), I don't like the idea because of inconsistency possibilities in mixed (fips and non-fips) system environments the performance differences (perhaps insignificant). Of course, there's also the issue that md5 and sha256 sums are different lengths. Perhaps truncating the latter to be the length of md5 would help eliminate possible bugs due to differences.

in reply to: 7 comment:8 by Joshua Cornutt, 5 years ago

Replying to Tim Graham:

I'm not aware of any discussion of the issue outside of the ticket. You can write to the DevelopersMailingList if you want to get a second opinion on your proposal. As a general solution (replacing all hashlib.md5() calls in Django with a fallback to sha256), I don't like the idea because of inconsistency possibilities in mixed (fips and non-fips) system environments the performance differences (perhaps insignificant). Of course, there's also the issue that md5 and sha256 sums are different lengths. Perhaps truncating the latter to be the length of md5 would help eliminate possible bugs due to differences.

I agree that a full, sweeping change from MD5 to SHA-256 is a Bad Idea (tm), but some code segments can be pretty easily moved over with minimal/no impact. I'm a fan of keeping things like this user-configurable and leaving the default as whatever the code uses today (MD5). That way it spreads some of the responsibility out. Since this is my first commit to the Django project, do you (or anyone else) have any tips/hints as to how to proceed with the change I proposed? How does it get a stamp of approval and merged?

comment:9 by Carlton Gibson, 5 years ago

...do you (or anyone else) have any tips/hints as to how to proceed with the change I proposed? How does it get a stamp of approval and merged?

Tim’s suggestion to mail the DevelopersMailingList is the best approach. There’s lots of people there that can input, and it’s likely a discussion can lead to a way forward (especially if it’s pressing).

Can I ask you to make sure you include enough backstory and context to bring people (including me 😀) up to speed on the issue? Otherwise you limit the response to those with lots of time (which isn’t many).

comment:10 by David Davis, 5 years ago

The usedforsecurity flag will be available in Python 3.9+:

https://github.com/python/cpython/pull/16044/

Last edited 5 years ago by David Davis (previous) (diff)

comment:11 by Mariusz Felisiak, 3 years ago

Keywords:	python3.9 added

comment:12 by Lars Erhardt, 3 years ago

So since 'usedforsecurity' flag will be available in python 3.9 we might as well go forward and add up support.
FIPS compliance will be interesting for government organizations or for everything else that has to deal with sensitive data.
It would also be a good advancement for data security, auditing, and compliance in general.
Every developer that uses RHEL, Fedora, or CentOS would profit from this change as well.
We should not swap the algorithms since this would remove backwards compatibility.
We also don't have to since Python's Hashlibrary allows to pass the flag with the new method as the second argument.

( By the way, this is my first comment and contribution to Django so let me know if I do something wrong)

Version 0, edited 3 years ago by Lars Erhardt (next)

comment:17 by Lars Erhardt, 3 years ago

Ok, so what are the next steps? According to the docs, someone should mail the developer mail list?

comment:18 by Ade Lee, 3 years ago

Owner:	changed from nobody to Ade Lee
Status:	new → assigned

comment:19 by Ade Lee, 3 years ago

I have added the following pull request to resolve this issue. Thanks!

This will allow django to run in fips enabled kernels. Eventually, when support for this
attribute is available everywhere, we can remove the wrapper function and just use what
is in hashlib.

https://github.com/django/django/pull/14763

comment:20 by Ade Lee, 3 years ago

Has patch:	set

comment:21 by Mariusz Felisiak, 3 years ago

Triage Stage:	Someday/Maybe → Accepted

The usedforsecurity argument is available in Python 3.9+, see https://github.com/python/cpython/commit/7cad53e6b084435a220e6604010f1fa5778bd0b1.

comment:22 by Mariusz Felisiak, 3 years ago

Patch needs improvement:	set

comment:23 by Mariusz Felisiak, 3 years ago

Patch needs improvement:	unset
Triage Stage:	Accepted → Ready for checkin

comment:24 by Mariusz Felisiak <felisiak.mariusz@…>, 3 years ago

Resolution:	→ fixed
Status:	assigned → closed

In d10c7bfe:

Fixed #28401 -- Allowed hashlib.md5() calls to work with FIPS kernels.

md5 is not an approved algorithm in FIPS mode, and trying to instantiate
a hashlib.md5() will fail when the system is running in FIPS mode.

md5 is allowed when in a non-security context. There is a plan to add a
keyword parameter (usedforsecurity) to hashlib.md5() to annotate whether
or not the instance is being used in a security context.

In the case where it is not, the instantiation of md5 will be allowed.
See https://bugs.python.org/issue9216 for more details.

Some downstream python versions already support this parameter. To
support these versions, a new encapsulation of md5() has been added.
This encapsulation will pass through the usedforsecurity parameter in
the case where the parameter is supported, and strip it if it is not.

Co-authored-by: Mariusz Felisiak <felisiak.mariusz@…>

Note: See TracTickets for help on using tickets.

Download in other formats:

Issues