sentry: Ignore all SuspiciousOperation loggers.

django.security.DisallowedHost is only one of a set of exceptions that
are "SuspiciousOperation" exceptions; all return a 400 to the user
when they bubble up[1]; all of them are uninteresting to Sentry.
While they may, in bulk, show a mis-configuration of some sort of the
application, such a failure should be detected via the increase in
400's, not via these, which are uninteresting individually.

While all of these are subclasses of SuspiciousOperation, we enumerate
them explicitly for a number of reasons:

 - There is no one logger we can ignore that captures all of them.
   Each of the errors uses its own logger, and django does not supply
   a `django.security` logger that all of them feed into.

 - Nor can we catch this by examining the exception object.  The
   SuspiciousOperation exception is raised too early in the stack for
   us to catch the exception by way of middleware and check
   `isinstance`.  But at the Sentry level, in `add_context`, it is no
   longer an exception but a log entry, and as such we have no
   `isinstance` that can be applied; we only know the logger name.

 - Finally, there is the semantic argument that while we have decided
   to ignore this set of security warnings, we _may_ wish to log new
   ones that may be added at some point in the future.  It is better
   to opt into those ignores than to blanket ignore all messages from
   the security logger.

This moves the DisallowedHost `ignore_logger` to be adjacent to its
kin, and not on the middleware that may trigger it.  Consistency is
more important than locality in this case.

Of these, the DisallowedHost logger if left as the only one that is
explicitly ignored in the LOGGING configuration in
`computed_settings.py`; it is by far the most frequent, and the least
likely to be malicious or impactful (unlike, say, RequestDataTooBig).

[1] https://docs.djangoproject.com/en/3.0/ref/exceptions/#suspiciousoperation
This commit is contained in:
Alex Vandiver 2020-08-11 19:06:04 -07:00 committed by Tim Abbott
parent 4e7eda0539
commit 596cf2580b
2 changed files with 14 additions and 4 deletions

View File

@ -384,10 +384,6 @@ class FlushDisplayRecipientCache(MiddlewareMixin):
return response
class HostDomainMiddleware(MiddlewareMixin):
def __init__(self, get_response: Callable[[Any, WSGIRequest], Union[HttpResponse, BaseException]]) -> None:
super().__init__(get_response)
ignore_logger("django.security.DisallowedHost")
def process_request(self, request: HttpRequest) -> Optional[HttpResponse]:
# Match against ALLOWED_HOSTS, which is rather permissive;
# failure will raise DisallowedHost, which is a 400.

View File

@ -3,6 +3,7 @@ from typing import TYPE_CHECKING, Optional
import sentry_sdk
from sentry_sdk.integrations import Integration
from sentry_sdk.integrations.django import DjangoIntegration
from sentry_sdk.integrations.logging import ignore_logger
from sentry_sdk.integrations.redis import RedisIntegration
from sentry_sdk.integrations.sqlalchemy import SqlalchemyIntegration
from sentry_sdk.utils import capture_internal_exceptions
@ -44,3 +45,16 @@ def setup_sentry(dsn: Optional[str], *integrations: Integration) -> None:
],
before_send=add_context,
)
# Ignore all of the loggers from django.security that are for user
# errors; see https://docs.djangoproject.com/en/3.0/ref/exceptions/#suspiciousoperation
ignore_logger("django.security.DisallowedHost")
ignore_logger("django.security.DisallowedModelAdminLookup")
ignore_logger("django.security.DisallowedModelAdminToField")
ignore_logger("django.security.DisallowedRedirect")
ignore_logger("django.security.InvalidSessionKey")
ignore_logger("django.security.RequestDataTooBig")
ignore_logger("django.security.SuspiciousFileOperation")
ignore_logger("django.security.SuspiciousMultipartForm")
ignore_logger("django.security.SuspiciousSession")
ignore_logger("django.security.TooManyFieldsSent")