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

Indicate that md5 is not being used for secure purposes—and that's okay (FIPS fix) #10192

Merged
merged 1 commit into from
Apr 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ Changelog
5.1 (xx.xx.xxxx) - IN DEVELOPMENT
~~~~~~~~~~~~~~~~

* Mark calls to `md5` as not being used for secure purposes, to avoid flagging on FIPS-mode systems (Sean Kelly)
* Fix: Prevent choosers from failing when initial value is an unrecognised ID, e.g. when moving a page from a location where `parent_page_types` would disallow it (Dan Braghis)
* Docs: Document how to add non-ModelAdmin views to a `ModelAdminGroup` (Onno Timmerman)
* Docs: Document how to add StructBlock data to a StreamField (Ramon Wenger)
Expand Down
4 changes: 2 additions & 2 deletions docs/releases/5.1.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ depth: 1

### Other features

* ...
* Mark calls to `md5` as not being used for secure purposes, to avoid flagging on FIPS-mode systems (Sean Kelly)

### Bug fixes

Expand All @@ -33,4 +33,4 @@ depth: 1

## Upgrade considerations

### ...
### ...
22 changes: 22 additions & 0 deletions wagtail/coreutils.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
from django.utils.text import capfirst, slugify
from django.utils.translation import check_for_language, get_supported_language_variant
from django.utils.translation import gettext_lazy as _
from hashlib import md5

if TYPE_CHECKING:
from wagtail.models import Site
Expand Down Expand Up @@ -421,6 +422,27 @@ def get_dummy_request(*, path: str = "/", site: "Site" = None) -> HttpRequest:
return RequestFactory(SERVER_NAME=server_name).get(path, SERVER_PORT=server_port)


def safe_md5(data=b"", usedforsecurity=True):
"""
Safely use the MD5 hash algorithm with the given ``data`` and a flag
indicating if the purpose of the digest is for security or not.

On security-restricted systems (such as FIPS systems), insecure hashes
like MD5 are disabled by default. But passing ``usedforsecurity`` as
``False`` tells the underlying security implementation we're not trying
to use the digest for secure purposes and to please just go ahead and
allow it to happen.
"""

# Although ``accepts_kwarg`` works great on Python 3.8+, on Python 3.7 it
# raises a ValueError, saying "no signature found for builtin". So, back
# to the try/except.
try:
return md5(data, usedforsecurity=usedforsecurity)
except TypeError:
return md5(data)


class BatchProcessor:
"""
A class to help with processing of an unknown (and potentially very
Expand Down
6 changes: 2 additions & 4 deletions wagtail/embeds/embeds.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
from datetime import datetime
from hashlib import md5

from django.utils.timezone import now

from wagtail.coreutils import accepts_kwarg
from wagtail.coreutils import accepts_kwarg, safe_md5

from .exceptions import EmbedUnsupportedProviderException
from .finders import get_finders
Expand Down Expand Up @@ -66,8 +65,7 @@ def finder(url, max_width=None, max_height=None):


def get_embed_hash(url, max_width=None, max_height=None):
h = md5()
h.update(url.encode("utf-8"))
h = safe_md5(url.encode("utf-8"), usedforsecurity=False)
if max_width is not None:
h.update(b"\n")
h.update(str(max_width).encode("utf-8"))
Expand Down
8 changes: 5 additions & 3 deletions wagtail/users/utils.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import hashlib

from django.conf import settings
from django.utils.http import urlencode
from django.utils.translation import gettext_lazy as _

from wagtail.compat import AUTH_USER_APP_LABEL, AUTH_USER_MODEL_NAME
from wagtail.coreutils import safe_md5


delete_user_perm = "{0}.delete_{1}".format(
AUTH_USER_APP_LABEL, AUTH_USER_MODEL_NAME.lower()
Expand Down Expand Up @@ -38,9 +38,11 @@ def get_gravatar_url(email, size=50):
if (not email) or (gravatar_provider_url is None):
return None

email_bytes = email.lower().encode("utf-8")
hash = safe_md5(email_bytes, usedforsecurity=False).hexdigest()
gravatar_url = "{gravatar_provider_url}/{hash}?{params}".format(
gravatar_provider_url=gravatar_provider_url.rstrip("/"),
hash=hashlib.md5(email.lower().encode("utf-8")).hexdigest(),
hash=hash,
params=urlencode({"s": size, "d": default}),
)

Expand Down