Warning: Can't synchronize with repository "(default)" (Unsupported version control system "svn": No module named svn). Look in the Trac log for more information.

Ticket #361 (closed defect: fixed)

Opened 13 years ago

Last modified 12 years ago

Identity (and Catwalk) fail to account for proxy when restricting access by IP address

Reported by: cliff@… Owned by: anonymous
Priority: normal Milestone: 0.9
Component: Identity Version:
Severity: critical Keywords: REMOTE_ADDR HTTP_X_FORWARDED_FOR develix
Cc:

Description

One thing that I think both Catwalk and Identity are probably missing is proxy support. If you just use the REMOTE_ADDR environment variable, all proxied requests appear to come from 127.0.0.1 rather than the real remote IP. What I usually do is check to see if the environment variable HTTP_X_FORWARDED_FOR is set and if so, use that, otherwise use REMOTE_ADDR:

remote_addr = os.environ.get('HTTP_X_FORWARDED_FOR, os.environ.get('REMOTE_ADDR', ''))

Neglecting this means that the features both have for restricting access by IP address won't work behind a proxy.

Attachments

ffx-2%20001s.jpg Download (6.7 KB) - added by anonymous 13 years ago.

Change History

comment:1 Changed 13 years ago by ronald@…

CatWalk takes this into account since changeset 460 (old patch bonone sendt to the mailing list)

comment:2 Changed 13 years ago by Jeff Watkins

Both CatWalk and Identity check the x-forwarded-for header provided in cherrypy.request.headerMap.

Is this somehow different than what is requested in this ticket?

comment:3 Changed 13 years ago by kevin

  • Keywords develix added

Important note: we can't trust the X-Forwarded-For header (or any header, for that matter) from the CherryPy? headers unless we know for sure that we're behind a server that will reliably send us that information. Otherwise, a hacker can just send a request with an X-Forwarded-For: 127.0.0.1 header.

comment:4 Changed 13 years ago by cliff

If TG is truly behind a proxy then the proxy will change the HTTP_X_FORWARDED_FOR header regardless of what the remote user sent, and if TG isn't behind a proxy, then that header should be ignored. I suspect there's no reliable way for TG to determine this for itself.

I think the only general solution would be to have a config file option that specified which header to trust (and perhaps let the user specify the exact header name, since some broken proxies pass HTTP_X_FORWARDED_FOR using a different name). This is so server configuration-specific that the TG config seems the only place to reliably address it.

Probably a genuine authentication (username/password) method is the best way, with IP restrictions just helping to cut down on brute-force attempts anyway.

comment:5 Changed 13 years ago by Jeff Watkins

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

The code has been changed to only check X-Forwarded-For when the remote host is 127.0.0.1.

I agree that IP-based restrictions are probably fraught with peril and probably ought not appear in a production system.

Changed 13 years ago by anonymous

Note: See TracTickets for help on using tickets.