Opened 18 months ago

Last modified 2 months ago

#28401 new Cleanup/optimization

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

Reported by: Andrew DiPrinzio Owned by: nobody
Component: Core (Other) Version: master
Severity: Normal Keywords: FIPS, md5
Cc: Triage Stage: Someday/Maybe
Has patch: no 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 (9)

comment:1 Changed 18 months ago by Markus Holtermann

Type: BugCleanup/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 18 months ago by Tim Graham (previous) (diff)

comment:2 Changed 18 months ago by Tim Graham

Component: UncategorizedCore (Other)
Summary: django doesn't work with FIPS kernelsAllow hashlib.md5() calls to work with FIPS kernels
Triage Stage: UnreviewedAccepted

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

comment:3 Changed 18 months ago by Aymeric Augustin

Triage Stage: AcceptedSomeday/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.

comment:4 in reply to:  1 Changed 18 months ago by Andrew DiPrinzio

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 Changed 18 months ago by Tim Graham

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 Changed 2 months ago by Joshua Cornutt

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?

comment:7 Changed 2 months ago by 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.

comment:8 in reply to:  7 Changed 2 months ago by Joshua Cornutt

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 Changed 2 months ago by Carlton Gibson

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

Note: See TracTickets for help on using tickets.
Back to Top