409942 - Page history shows everyone with the same IP address

Status: VERIFIED FIXED

(support.mozilla.org :: Knowledge Base Software, task, P4)

Description

[Chris Ilias [:cilias]]

Go to any article.
Click on "History".
The number in the IP column is the same for everyone.

Comment 1

[Nelson Ko [:nkoth]]

This also has the problem of us not being able to ban users by IP (through tiki-admin_banning.php). morgamic, can you consult IT if we can do something about this? That IP is our forward proxy right?

Comment 2

[Laura Thomson :laura]

morgamic: ping?

Comment 3

[Dave Miller [:justdave] ([e-mail])]

FWIW, I just got a password reset email that said this:

Someone coming from IP Address 10.2.81.4 requested password reset for your account  (support.mozilla.com).

The IP address listed is that of the internal address on the Netscaler of course.

Comment 5

[Laura Thomson :laura]

Server-ops, is there a way around this?

Comment 6

[Wil Clouser [:clouserw]]

09:29 <@clouserw> laura: you should be able to use $_SERVER['HTTP_X_FORWARDED_FOR'] for bug 409942
09:32 <laura> clouserw: oh cool
09:32 <laura> could you comment in the bug so I don't forget?

Comment 7

[Reed Loden [:reed] (use needinfo?)]

Note that HTTP_X_FORWARDED_FOR is an array! You'll need to pop() until you get the right value. We have some PHP code somewhere that does the right thing...

Comment 8

[Wil Clouser [:clouserw]]

(In reply to comment #7)
> Note that HTTP_X_FORWARDED_FOR is an array! You'll need to pop() until you get
> the right value. We have some PHP code somewhere that does the right thing...

Yeah, that's true.  You can use the code from AMO if you want: http://viewvc.svn.mozilla.org/vc/addons/trunk/site/app/controllers/components/recaptcha.php?revision=17457&view=markup#l94

Comment 9

[Chris Ilias [:cilias]]

I've filed bug 506547 to specifically address the "requested password reset" emails. Fixing this bug would fix that bug as well, but if this can't be done any time soon, we need an interim fix to stop account holders from getting false password reset notifications.

Comment 10

[Reed Loden [:reed] (use needinfo?)]

(In reply to comment #9)
> I've filed bug 506547 to specifically address the "requested password reset"
> emails. Fixing this bug would fix that bug as well, but if this can't be done
> any time soon, we need an interim fix to stop account holders from getting
> false password reset notifications.

What do you mean by "false" here? Just because the IP address is wrong doesn't mean that the password reset notification is fake. Somebody must have requested that user's password be reset, though it may not be the actual user himself/herself.

Comment 11

[Chris Ilias [:cilias]]

Sorry Reed. My mistake. I guess if not all users are getting these emails, than someone must have tried to reset the password on those specific accounts.

Comment 12

[Paul Craciunoiu [:paulc]]

Isn't this a dupe of bug 512841?

Comment 13

[Paul Craciunoiu [:paulc]]

I guess not. But if bug 512841 works, a similar fix would apply here. Adding dep.

Comment 14

[James Socol [:jsocol, :james]]

I can fix the page history issue, but we'll have to look into the banning system to find where it's enforcing the IPs and modify that as well. That seems like it's worthy of its own bug.

Comment 15

[James Socol [:jsocol, :james]]

Attachment: patch, v1

This adds a new global (yes, bad me) named $remote_addr which should be set to the correct remote address, even behind NetScaler (or other well-behaved proxies).

Then it uses that global in tiki-editpage.php to replace $_SERVER["REMOTE_ADDR"]. This was the only place I found that was relevant, but it's a bit hard to test this locally without Squid or something running.

/me goes to set up Squid.

Comment 16

[James Socol [:jsocol, :james]]

I think I have Squid up and running correctly as a reverse proxy accelerator. Here's the relevant line from the Apache access_log:

127.0.0.1 - - [12/Oct/2009:17:18:30 -0700] "POST /tiki-editpage.php?locale=en-US&page=Forum%20link%20test HTTP/1.0" 302 - "https://10.250.3.206/tiki-editpage.php?locale=en-US&page=Forum%20link%20test" "Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.3) Gecko/20090824 Firefox/3.5.3"

And the matching line from the page history:

Mon 12 of Oct, 2009 16:18 PST 	jsocol	10.250.3.77	3/current

Comment 17

[Paul Craciunoiu [:paulc]]

Comment on attachment 405950 [details] [diff] [review]
patch, v1

WFM, I still get the right IP set in the db. Definitely prefer this less intrusive approach to the old.
To avoid a global, would it be better to define a function instead and calling it in tiki-editpage?

Comment 18

[James Socol [:jsocol, :james]]

Global function vs global variable? At least the global variable doesn't have the overhead of a function call, I suppose.

Comment 19

[Paul Craciunoiu [:paulc]]

A function would be better for security and safety considerations :), but here comes the "we already have 10,000 other globals" counter-argument.

Comment 20

[James Socol [:jsocol, :james]]

Attachment: super-patch, v2

Paul is half-right in comment 17, adding another global is bad practice and I will recite 10 Our Compilers in penance.

This patch introduces a constant, REMOTE_ADDR, with the correct value, and then attempts to replace all instances of $_SERVER['REMOTE_ADDR'] with the constant. It fixes both this bug and bug 512841, and hopefully a few other strange behaviours. (Certain hashes, for example, were built from UA and IP. They should be more unique now.)

Unfortunately, this touches 16 files and is not small. But it's all for the best.

I also rewrote the internals of wikiplugin_agentinfo.php to clean up some code smell.

Comment 21

[Michael Morgan [:morgamic]]

Comment on attachment 408132 [details] [diff] [review]
super-patch, v2

This looks sane to me.  Some of the address assignment seems to be duped in the adodblib part, but not a big deal.  Replacements for $_SERVER['REMOTE_ADDR'] look correct.

Comment 22

[James Socol [:jsocol, :james]]

r54274

This also fixes bug 512841, and hopefully any potential future proxy-safe IP address bugs.

Comment 23

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED on https://support-stage.mozilla.org/tiki-pagehistory.php?locale=en-US&page=test%20articles.

Comment 24

[James Socol [:jsocol, :james]]

r54586 (prod)