Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ETag checksum fails on FIPS-enabled systems when using MD5 #1365

Closed
2 tasks done
landtuna opened this issue Dec 14, 2021 · 13 comments · Fixed by #1366 or #1410
Closed
2 tasks done

ETag checksum fails on FIPS-enabled systems when using MD5 #1365

landtuna opened this issue Dec 14, 2021 · 13 comments · Fixed by #1366 or #1410
Labels
help wanted Feel free to help

Comments

@landtuna
Copy link

landtuna commented Dec 14, 2021

Checklist

  • The bug is reproducible against the latest release and/or master.
  • There are no similar issues or pull requests to fix it yet.

Describe the bug

The ETag checksum fails when using MD5. This is causing Starlette to not work at all under Red Hat Enterprise Linux when FIPS mode is enabled.

Debugging material

Here's the exception that's thrown:

INFO:     10.42.1.7:34422 - "GET /app/static/foo.html HTTP/1.1" 500 Internal Server Error
ERROR:    Exception in ASGI application
Traceback (most recent call last):
  File "/usr/local/lib/python3.8/site-packages/uvicorn/protocols/http/h11_impl.py", line 373, in run_asgi
    result = await app(self.scope, self.receive, self.send)
  File "/usr/local/lib/python3.8/site-packages/uvicorn/middleware/proxy_headers.py", line 75, in __call__
    return await self.app(scope, receive, send)
  File "/usr/local/lib/python3.8/site-packages/fastapi/applications.py", line 208, in __call__
    await super().__call__(scope, receive, send)
  File "/usr/local/lib/python3.8/site-packages/starlette/applications.py", line 112, in __call__
    await self.middleware_stack(scope, receive, send)
  File "/usr/local/lib/python3.8/site-packages/starlette/middleware/errors.py", line 181, in __call__
    raise exc
  File "/usr/local/lib/python3.8/site-packages/starlette/middleware/errors.py", line 159, in __call__
    await self.app(scope, receive, _send)
  File "/usr/local/lib/python3.8/site-packages/starlette/exceptions.py", line 82, in __call__
    raise exc
  File "/usr/local/lib/python3.8/site-packages/starlette/exceptions.py", line 71, in __call__
    await self.app(scope, receive, sender)
  File "/usr/local/lib/python3.8/site-packages/starlette/routing.py", line 656, in __call__
    await route.handle(scope, receive, send)
  File "/usr/local/lib/python3.8/site-packages/starlette/routing.py", line 408, in handle
    await self.app(scope, receive, send)
  File "/usr/local/lib/python3.8/site-packages/starlette/staticfiles.py", line 97, in __call__
    response = await self.get_response(path, scope)
  File "/usr/local/lib/python3.8/site-packages/starlette/staticfiles.py", line 118, in get_response
    return self.file_response(full_path, stat_result, scope)
  File "/usr/local/lib/python3.8/site-packages/starlette/staticfiles.py", line 173, in file_response
    response = FileResponse(
  File "/usr/local/lib/python3.8/site-packages/starlette/responses.py", line 267, in __init__
    self.set_stat_headers(stat_result)
  File "/usr/local/lib/python3.8/site-packages/starlette/responses.py", line 273, in set_stat_headers
    etag = hashlib.md5(etag_base.encode()).hexdigest()
ValueError: [digital envelope routines: EVP_DigestInit_ex] disabled for FIPS

Environment

  • OS: Red Hat Enterprise Linux 8 in FIPS mode
  • Python version: 3.8.8
  • Starlette version: 0.16.0
@Kludex
Copy link
Sponsor Member

Kludex commented Dec 14, 2021

nginx uses the following logic:

Unit’s ETag response header fields use the MTIME-FILESIZE format, where MTIME stands for file modification timestamp and FILESIZE stands for file size in bytes, both in hexadecimal.

Ref.: https://unit.nginx.org/configuration/#static-files

I've found this article stating the same problem for Django. Which lead me to https://code.djangoproject.com/ticket/28401, and finally to this PR (merged 2 months ago) that solves the problem above.

We should probably follow the same path? As we're not on stable release, we can also change the algorithm. Check more about it here.

@Kludex
Copy link
Sponsor Member

Kludex commented Dec 14, 2021

What I can review, prior discussion, is a PR that supports "Red Hat Enterprise Linux when FIPS mode is enabled" using the same solution as Django, i.e. checking usedforsecurity flag.

As for changing the algorithm, we need further discussion, and other members should be involved.

I'm going to share this on our Gitter.

@adriangb
Copy link
Member

The ETag checksum should use a more secure hash than MD5

@landtuna could you clarify is this is a statement of fact that you are making based on your expertise in security or if this is just something that you think may be true? My understanding (which I believe is also what @Kludex is getting at above) is that ETags may be used for caching and such, but should not be considered secure / foolproof (for such a use case, the client should get the data and hash it with a secure hash algorithm). But I'm happy to be wrong, so just checking.

@landtuna
Copy link
Author

@adriangb sorry, you're right. "Should" was too strong, but if you don't change the algorithm, you'll have to wrap or monkey patch hashlib. I guess you'd want to stick with MD5 if changing away from it is a performance concern. Looks like django inspects hashlib and wraps it.

@tomchristie
Copy link
Member

We probably want to add the usedforsecurity=False flag, which is available in Python 3.9 onwards.

@landtuna landtuna changed the title ETag checksum shouldn't use MD5 ETag checksum fails on FIPS-enabled systems when using MD5 Dec 15, 2021
@Kludex Kludex added the help wanted Feel free to help label Dec 15, 2021
adriangb added a commit that referenced this issue Dec 17, 2021
… systems (#1366)

See #1365 

Co-authored-by: Tom Christie <tom@tomchristie.com>
@Kludex
Copy link
Sponsor Member

Kludex commented Dec 17, 2021

This will be available on Starlette 0.18.0.

@tomchristie
Copy link
Member

Indeed. I'll highlight again that it's only supported on Python 3.9+, so if you're using FIPs mode you need that.

@landtuna
Copy link
Author

@tomchristie I think that's not quite accurate. I believe RHEL was the first implementer and backported this feature, and it's on the RHEL family where everything has this problem. I know it's in some of the RHEL Python 2.7 libs, but I'm not positive about the others.

Since the implementation didn't check the Python version but did use try/except, I think this fix works for everyone who needs it to to work.

@adriangb
Copy link
Member

Since the implementation didn't check the Python version but did use try/except, I think this fix works for everyone who needs it to to work.

Yep indeed, I believe you are right. That said, could you test off of master on Python 3.6 and confirm @landtuna? I'd be good to know!

@landtuna
Copy link
Author

@adriangb Almost a month later, and I don't have an answer for you yet. The customer who was running in FIPS mode turned it off for a while, but I think they're going to turn it back on soon. 🤷

@landtuna
Copy link
Author

@adriangb Finally was able to test this, and the fix doesn't work. What happens is that the check uses usedforsecurity=True, but that always triggers an exception on FIPS-enabled systems. The check in _compat.py should instead use usedforsecurity=False.

File "/usr/local/lib/python3.8/site-packages/fastapi/__init__.py", line 7, in <module>
    from .applications import FastAPI as FastAPI
  File "/usr/local/lib/python3.8/site-packages/fastapi/applications.py", line 3, in <module>
    from fastapi import routing
  File "/usr/local/lib/python3.8/site-packages/fastapi/routing.py", line 22, in <module>
    from fastapi.dependencies.models import Dependant
  File "/usr/local/lib/python3.8/site-packages/fastapi/dependencies/models.py", line 3, in <module>
    from fastapi.security.base import SecurityBase
  File "/usr/local/lib/python3.8/site-packages/fastapi/security/__init__.py", line 1, in <module>
    from .api_key import APIKeyCookie as APIKeyCookie
  File "/usr/local/lib/python3.8/site-packages/fastapi/security/api_key.py", line 5, in <module>
    from starlette.exceptions import HTTPException
  File "/usr/local/lib/python3.8/site-packages/starlette/exceptions.py", line 7, in <module>
    from starlette.responses import PlainTextResponse, Response
  File "/usr/local/lib/python3.8/site-packages/starlette/responses.py", line 14, in <module>
    from starlette._compat import md5_hexdigest
  File "/usr/local/lib/python3.8/site-packages/starlette/_compat.py", line 14, in <module>
    hashlib.md5(b"data", usedforsecurity=True)  # type: ignore[call-arg]
ValueError: [digital envelope routines: EVP_DigestInit_ex] disabled for FIPS

@adriangb
Copy link
Member

Ah yes I see.
What we are trying to check is if the Python version supports the parameter (hence catching the TypeError).
But we are inadvertently triggering that ValueError.
I'll submit a PR with the change.
Thank you for testing this!

@landtuna
Copy link
Author

Confirmed that version 0aef172 with usedforsecurity=False in _compat.py works on the FIPS system. I don't think I can check master (after your merge) because it doesn't work for me with the latest release of FastAPI. But I think we're good to go now with regards to _compat.py!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Feel free to help
Projects
None yet
4 participants