#5199 closed enhancement (fixed)
IPBlacklistFilterStrategy and Use X-Forwarded-For
Reported by: | Owned by: | ||
---|---|---|---|
Priority: | normal | Milestone: | plugin - spam-filter |
Component: | plugin/spamfilter | Version: | |
Severity: | normal | Keywords: | apache proxy x-forwarded-for |
Cc: | Branch: | ||
Release Notes: | |||
API Changes: | |||
Internal Changes: |
Description
I am running tracd behind a local Apache proxy. this means that all the IP addresses trac sees are 127.0.0.1, however, Apache adds the header X-Forwarded-For that says what was the original IP address. this cause a problem to the IPBlacklistFilterStrategy. I suggest that the IPBlacklistFilterStrategy will take the X-Forwarded-For header into consideration. if the ip address is a LAN address (127.0.0.1, 10.0.0.*, 192.168.* etc) it should ignore it and get the next one from the XFF header.
I have logic that does this for my plugin which can be used: http://firestats.cc/browser/branches/firestats-1.1/integration/trac/firestats/hitlogger.py
Attachments (0)
Change History (11)
comment:1 by , 18 years ago
Component: | general → spamfilter |
---|---|
Milestone: | → not applicable |
Owner: | changed from | to
comment:2 by , 15 years ago
Owner: | changed from | to
---|---|
Summary: | IPBlacklistFilterStrategy and Use X-Forwarded-For → [PATCH] IPBlacklistFilterStrategy and Use X-Forwarded-For |
comment:3 by , 15 years ago
Summary: | [PATCH] IPBlacklistFilterStrategy and Use X-Forwarded-For → IPBlacklistFilterStrategy and Use X-Forwarded-For |
---|
Aargh, That comment was for #8032. Sorry. The ticket name is so equal :-)
comment:4 by , 15 years ago
Milestone: | not applicable → spam-filter-plugin |
---|---|
Owner: | removed |
comment:6 by , 14 years ago
Keywords: | apache proxy x-forwarded-for added |
---|---|
Resolution: | fixed |
Status: | closed → reopened |
r9890 doesn't look optimal, as it changes the existing interface in a very intrusive way (r9891, r9892) and therefore will break the other plugins relying on it.
I think that in this situation, the X-Forwarded-For
should rather be used to replace req.remote_addr
, as the original one is useless (we could still keep that in req._forwarded_by_addr
eventually).
So if we would do the check done in [9890#file2] in Request.remote_addr, then we could support this feature in a transparent way. Also, all the other uses of remote_addr
would benefit from this fix as well (e.g. storing the IP of wiki authors in wiki changes, it doesn't help to store 127.0.0.1 either).
What do you think?
follow-up: 8 comment:7 by , 14 years ago
I had also another reason to do the intrusive change regarding the IP parameter. Actually the IP is used by the filters to do decisions, so it also should be passed as explicit parameter (even if that means an interface change - are there really other plugins relying on spamfilter test/train API?) It was not so clean as I wished, but nothing is for free. The train() interface does a hack with setting REMOTE_ADDRESS to work around this missing argument.
If only for that bug report I would have replaced it inline where needed.
Handling X-Forwarded-For in Trac itself is nevertheless a good idea I would think. It must not necessarily mean to revert the IP argument :-)
comment:8 by , 14 years ago
Replying to anonymous:
I had also another reason to do the intrusive change regarding the IP parameter. Actually the IP is used by the filters to do decisions, so it also should be passed as explicit parameter (even if that means an interface change - are there really other plugins relying on spamfilter test/train API?)
Ah, you're right, I confused that with the other interface. Yeah, this is mainly an internal interface I suppose.
It was not so clean as I wished, but nothing is for free. The train() interface does a hack with setting REMOTE_ADDRESS to work around this missing argument.
author
and ip
could be keys in a submitter
dictionary, for example.
If only for that bug report I would have replaced it inline where needed.
Handling X-Forwarded-For in Trac itself is nevertheless a good idea I would think. It must not necessarily mean to revert the IP argument :-)
Ok, I'll do this.
Oh, just a quick note though: for your commit messages, please take the habit of using the SpamFilter: prefix for the first line (take a look at log:/plugins for examples).
comment:9 by , 14 years ago
guys, keep in mind that X-Forwarded-For can be easily be spoofed by spam bots.
each proxy appends itself the the X-Forwarded-For, but the user should be able to control exactly how this is handled.
my original bug report was about having tracd behind apache proxy. since in this case we know that there is an apache proxy we can safely strip the first (or is it the last?) ip address from the XFF list.
I think the implementation I suggested may be too naive for the general case.
comment:10 by , 14 years ago
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
I made it configurable in r9934. I think we should leave it there for some time and if it works as wanted, then maybe move it to Request directly.
comment:11 by , 13 years ago
Hello there!
I have a standalone tracd which runs behind Nginx proxy. It sets a proper X-Real-IP and X-Forwarded-For headers, but it seems that BlogSpam filter sets negative karma for a new tickets. I can see something like that on the admin/spamfilter/monitor page:
BlogSpamFilterStrategy (-5): BlogSpam says content is spam (badip:state/blacklist.d/127.0.0.1)
Below details about my Trac box:
- Debian 6.0 Squeeze
- Python 2.6.6
- Trac 0.12.2-1~bpo60+1
- TracSpamFilter-0.4.8dev_r10882
I've resolved the issue by hacking tracd source code:
--- trac/web/standalone.py.old 2011-12-12 14:01:33.546973494 +0000 +++ trac/web/standalone.py 2011-12-12 14:00:50.554973513 +0000 @@ -112,6 +112,11 @@ server_version = 'tracd/' + VERSION def address_string(self): + # Get remote address of client if tracd is behind a proxy + ip = self.headers.get("X-Real-IP") or \ + self.headers.get("X-Forwarded-For") + if ip: + return ip # Disable reverse name lookups return self.client_address[:2][0]
Now tracd shows me a good remote IP address of client and I don't have a problem with a karma.