Opened 4 years ago

Closed 2 years ago

#16008 closed New feature (fixed)

Django does not provide any protection against DNS rebinding

Reported by: adehnert Owned by: nobody
Component: HTTP handling Version: 1.3
Severity: Normal Keywords:
Cc: adehnert Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Django currently does not provide any protection against DNS rebinding attacks. The CsrfProtection page suggests that it make be useful to add such protection.

Attachments (1)

0001-Add-DNS-rebinding-protection.patch (5.5 KB) - added by adehnert 4 years ago.
Patch to provide protection against DNS rebinding attacks

Download all attachments as: .zip

Change History (8)

Changed 4 years ago by adehnert

Patch to provide protection against DNS rebinding attacks

comment:1 Changed 4 years ago by adehnert

  • Has patch set
  • Needs documentation unset
  • Needs tests set
  • Patch needs improvement unset

I've attached a patch that provides some simple protection, in terms of validating that the Host header matches a fixed list of possible values. It probably needs tests and documentation, but I figured feedback on the code first would be good.

comment:2 Changed 4 years ago by adehnert

  • Cc adehnert added

comment:3 Changed 4 years ago by aaugustin

  • Component changed from Uncategorized to HTTP handling
  • Needs documentation set
  • Triage Stage changed from Unreviewed to Design decision needed

comment:4 Changed 4 years ago by lukeplant

  • Triage Stage changed from Design decision needed to Accepted

This is definitely something we are interested in, and the patch is definitely a good start, thanks.

There are some other things to consider, like the possibility of using the Sites framework so that you don't have to specify the domain twice, and there will probably be some discussion on django-devs very shortly about this.

As you say, we'd need docs and tests, and there is no reason not to start writing these already if you are interested. Things will probably need to change, but a solid base for discussing additions and changes is always very helpful, for docs as well as for code. The docs help people to see how hard it is to it up, which is a particularly important consideration for security features.

In terms of what is there already, just one nit: _get_failure_view adds a layer of indirection that we don't need if we are only calling it once - I would inline it into HostMatchMiddleware._reject. (Yes, the same criticism is true of the CSRF code you obviously based this on - that is an historical accident, and should be fixed).

comment:5 Changed 4 years ago by lukeplant

On the subject of needing a whitelist of host names, #13751 is related.

comment:6 Changed 3 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

comment:7 Changed 2 years ago by aaugustin

  • Resolution set to fixed
  • Status changed from new to closed

The last security release introduced a mandatory ALLOWED_HOSTS setting, resolving this ticket.

(I'm not sure to understand what a DNS rebinding attack is, but both the wiki page and the patch propose a Host whitelist as a countermeasure, and that's what ALLOWED_HOSTS does too.)

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