Skip to content

Allow make_template_fragment_key() to work on FIPS systems#10605

Closed
01000101 wants to merge 5 commits intodjango:masterfrom
01000101:osp13-fips-min
Closed

Allow make_template_fragment_key() to work on FIPS systems#10605
01000101 wants to merge 5 commits intodjango:masterfrom
01000101:osp13-fips-min

Conversation

@01000101
Copy link
Copy Markdown

@01000101 01000101 commented Nov 4, 2018

Systems in "FIPS mode" (fips=1 kernel flag set) cannot use MD5. There's some exception for allowed use cases, but that's mostly academic in nature considering software like OpenSSL will block MD5 regardless of intended use on FIPS mode systems. This patch specifically addresses an issue encountered on a deployment of Red Hat OpenStack 13, albeit using an older Red Hat packaged version of python-django (1.8.18-1.el7ost).

@01000101
Copy link
Copy Markdown
Author

01000101 commented Nov 4, 2018

I see there's some (seemingly unrelated) Jenkins errors. Can anyone shed some light as to why?

@matthiask
Copy link
Copy Markdown
Contributor

Nice, that was a daylight savings time error. You could rebase your changes and force-push them again, or add an empty git commit on top (git commit --allow-empty) and push again.

That being said, here's already an issue with some discussion how to approach the FIPS problem: https://code.djangoproject.com/ticket/28401


# Try to generate an MD5 hash but fall up to SHA256 if in FIPS mode
try:
args = hashlib.md5(key.encode())
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Rather than try this each time could we not include the is_fips_mode method in this module and do a conditional check? We can wrap it in an @lru_cache to ensure it's not called more than once.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I completely agree, but it sounds like first some decisions about the overall approach (crafting a solution vs. waiting for hashlib to implement FIPS-awareness) are still in flux. Once that's resolved, I'd be happy to implement something more complete, optimized.

def test_without_vary_on(self):
key = make_template_fragment_key('a.fragment')
self.assertEqual(key, 'template.cache.a.fragment.d41d8cd98f00b204e9800998ecf8427e')
if is_fips_mode():
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

As we don't run any Jenkins executors in FIPS mode we could instead use some subtests here and mock the return value of is_fips_mode if it's included in cache/utils.py?

@timgraham
Copy link
Copy Markdown
Member

I'm not convinced about the solution and other areas of Django are affected. See https://code.djangoproject.com/ticket/28401 for some discussion (all patches like this should be linked to a Trac ticket anyway).

@01000101
Copy link
Copy Markdown
Author

01000101 commented Nov 4, 2018 via email

Joshua Cornutt added 2 commits November 4, 2018 13:39
@felixxm
Copy link
Copy Markdown
Member

felixxm commented Jul 10, 2020

Closing per ticket. I also think that it's not a good approach. You can start a discussion on DevelopersMailingList if you don't agree.

@felixxm felixxm closed this Jul 10, 2020
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.

5 participants