Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changes in Apache 2.4 / mod_perl (remote IP) #214

Closed
sebastfr opened this issue Mar 12, 2014 · 18 comments
Closed

Changes in Apache 2.4 / mod_perl (remote IP) #214

sebastfr opened this issue Mar 12, 2014 · 18 comments

Comments

@sebastfr
Copy link
Contributor

@sebastfr sebastfr commented Mar 12, 2014

Reported by David McElroy on ep-tech.

See http://www.marshut.com/ippzhs/problem-with-apache2-connection-remote-ip.html and http://httpd.apache.org/docs/2.4/developer/new_api_2_4.html (search for "remote_ip").

conn_rec->remote_ip and conn_rec->remote_addr
These fields have been renamed in order to distinguish between the client IP address of the connection and the useragent IP address of the request (potentially overridden by a load balancer or proxy). References to either of these fields must be updated with one of the following options, as appropriate for the module:

    When you require the IP address of the user agent, which might be connected directly to the server, or might optionally be separated from the server by a transparent load balancer or proxy, use request_rec->useragent_ip and request_rec->useragent_addr.
    When you require the IP address of the client that is connected directly to the server, which might be the useragent or might be the load balancer or proxy itself, use conn_rec->client_ip and conn_rec->client_addr.
@davidmce
Copy link

@davidmce davidmce commented Mar 12, 2014

At the moment I'll stick with client_ip as this works. useragent_ip seems to require more than just the two lines:

Can't locate object method "useragent_ip" via package "Apache2::Connection" at /usr/share/eprints3/p
erl_lib/EPrints/DataObj/LoginTicket.pm line 145.\n

@sebastfr
Copy link
Contributor Author

@sebastfr sebastfr commented Mar 12, 2014

You're calling useragent_ip on the wrong object...

@davidmce
Copy link

@davidmce davidmce commented Mar 12, 2014

ah.. yeah.. works better now..

@sebastfr
Copy link
Contributor Author

@sebastfr sebastfr commented Mar 12, 2014

Nice! :-) thanks for testing (I don't have apache 2.4 handy).

@sebastfr
Copy link
Contributor Author

@sebastfr sebastfr commented Mar 13, 2014

ep-tech conversation on that topic:

Yes, this true but if we are under reverse proxy apache (balanced) that set 'X-Forwarded-For' and a remote connection is from a proxy like 'squid' that in http set
'X-Forwarded-For' and in https is DIRECT (tunnel a connection) we must take the ip of 'squid' and not the client's ip.
Otherwise when the client is in http we take the client's ip and when in https we take the proxy ip (so the login cookie authenticate only https request)
In https 'squid' is in tunnel mode so it can not set 'X-Forwarded-For'.

In this scenario i use:

X-Forwarded-For: client, proxy1, proxy2

the last value isn't the original client's IP but the who connect with us

my $ip = $ENV{'HTTP_X_FORWARDED_FOR'} || $r->connection->remote_ip;
$ip=~s/^.*, *//;

Enio
Il 13/03/2014 09:40, John Salter ha scritto:

Just an addition to this, under Apache 2.[something less than 4?], if you’ve got proxy servers in the routing, you might need to do something like this:

#Check if the remote IP is in our known proxy IPs, and it’s got a forwarded-for header.

#Only trust the proxies under your control!

if ( ($r->connection->remote_ip =~ /$self->{'_proxy_ips'}/) && ( $r->header_in('X-Forwarded-For') ) ){

# Select last value in the chain -- original client's IP

if (my ($ip) = $r->headers_in->{'X-Forwarded-For'} =~ /([^,\s]+)$/) {

    $self->log->error('message' => "Incoming IP: $ip  is proxied.");

    #set the remote_ip to the real remote IP sop other things can use it sensibly

    $r->connection->remote_ip($ip);

}

}

Under 2.4 it looks like this is all handled in a different (better) way.

Cheers,

John

@jesusbagpuss
Copy link
Contributor

@jesusbagpuss jesusbagpuss commented Apr 8, 2014

Looks like Apache2::ServerUtil::get_server_version() should provide a way to switch between behaviours for 2.(<4) and 2.4.
I get something like:
Apache/2.2.15
from the above function.

@sebastfr
Copy link
Contributor Author

@sebastfr sebastfr commented Apr 8, 2014

also UNIVERSAL::can might help.

if( $r->connection->can( 'remote_ip' ) ) ....

@sebastfr
Copy link
Contributor Author

@sebastfr sebastfr commented May 30, 2014

A collection of patches which fixes issues with using EPrints over Apache v2.4.

For the 3.3 branch, here are the patches you should use (if you had to):

88567f9
34f85e9
26e97fc
67986a0

This basically wraps the various ways to get the "real" client IP (by opposition to the IP of a machine in-between such as a proxy or load-balancer) into a single method. Other patches make use of the new API method $repository->remote_ip.

@sebastfr sebastfr closed this May 30, 2014
@eniocarboni
Copy link
Contributor

@eniocarboni eniocarboni commented Oct 22, 2014

In Repository.pm, X-Forwarded-For is taken as is without any parsing but in proxy environment it may include multiples ip if there are more proxy.
An example:
eprint installation behind a reverse proxy (apache) and a client browser behind a proxy (squid)
In this scenario X-Forwarded-For is "ip_client, ip_squid_proxy"

You can see http://en.wikipedia.org/wiki/X-Forwarded-For#Format

So i propose a patch:

*** pp/perl_lib/EPrints/Repository.pm 2014-10-22 14:18:33.278788495 +0200
--- pp/perl_lib/EPrints/Repository1.pm 2014-10-22 14:54:05.330188714 +0200


*** 431,436 ****
--- 431,437 ----

      # Proxy has set the "X-Forwarded-For" HTTP header?
      my $ip = $r->headers_in->{"X-Forwarded-For"};
  •   $ip=~s/^.*, *//;
    
      # Apache v2.4+ (http://httpd.apache.org/docs/trunk/developer/new_api_2_4.html)
      if( !EPrints::Utils::is_set( $ip ) && $r->can( "useragent_ip" ) )
    
@phluid61
Copy link
Contributor

@phluid61 phluid61 commented Oct 22, 2014

Doesn't this proposal leave you with the last (proxy), rather than the first (client) address?

Rather you'd want something like $ip =~ s/\s*,.*$//;

@phluid61
Copy link
Contributor

@phluid61 phluid61 commented Oct 22, 2014

Actually, even that's imperfect, if the XFF header is malformed with a leading comma. To work in most real-world eventualities you'd need something more complex, like:

# sanitise: remove leading commas and whitespace
$ip =~ s/^\s*,+|\s+//g;
# slice: take only first address from the list
$ip =~ s/,.*//;
@eniocarboni
Copy link
Contributor

@eniocarboni eniocarboni commented Oct 23, 2014

Yes, this leave the last proxy (who connect whith me) but why?
in my scenario:
eprint server <---> apache rev proxy <--- the net ---> squid proxy <---> client browser
when in non secure protocol, http, the squid proxy can add XFF and so i can have the ip client but when in secure protocol, https, squid proxy work in tunnel mode (open a connection to a server eprint and leave the comunication to the cient browser) so it can't add XFF.
So in your case the Apache::LogHandle for the same ip clent in http log the ip client and in https the squid proxy. In my case i log who connect with me, and in this scenario the squid proxy.
And i think (but I'm not sure) this is a problem also for EPrints::DataObj::LoginTicket if $c->{ignore_login_ip} is not set,
Sanitize is always the welcome

@eniocarboni
Copy link
Contributor

@eniocarboni eniocarboni commented Oct 23, 2014

... and if the client browser is in a nat office (network address translation), the ip of the client will surely be a private IP (in XFF)

@phluid61
Copy link
Contributor

@phluid61 phluid61 commented Oct 24, 2014

Usually a NAT proxy knows not to expose private IPs to the public internet.

But back to your use-case; you say you want to log the first hop outside your gateway/reverse-proxy. Note Seb's comment, "the various ways to get the "real" client IP (by opposition to the IP of a machine in-between such as a proxy or load-balancer)" Our goal with this method is to get the client IP, what you want is something different. I feel that's something you negotiate between your eprints server and your gateway/RP.

phluid61 added a commit to QUTlib/eprints that referenced this issue Oct 24, 2014
Discussion in eprints#214 highlighted that XFF may contain multiple forwarded
IP address, separated by commas. This patch takes only the first such
address from the list (i.e. the original client's address).
phluid61 added a commit to QUTlib/eprints that referenced this issue Oct 24, 2014
Discussion in eprints#214 highlighted that XFF may contain multiple forwarded
IP address, separated by commas. This patch takes only the first such
address from the list (i.e. the original client's address).
@eniocarboni
Copy link
Contributor

@eniocarboni eniocarboni commented Oct 24, 2014

In my use-case I would like to take the ip that connects to the RP apache, since it is, in my opinion, the only valid and that it has a value always true.
X-Forwarded-For is an environment variable and as such is not safe
In fact, someone could run the simple command:
wget -S --header="X-Forwarded-For: a-very-false-ip" http://demoprints.eprints.org/
(if demoprints can->issue #214)
So for LoginTicket, for logging , for Recaptcha, cfg.d/security.pl and so on you use 'a-very-false-ip' as if it were that valid.
I take the last in X-Forwarded-For since I consider valid and what my RP sets in X-Forwarded-For

@phluid61
Copy link
Contributor

@phluid61 phluid61 commented Oct 24, 2014

As I said, you're wanting to do something different than what this method provides. We're giving the equivalent of the new useragent_ip, but what you want is the new client_ip (which really should have been called peer_ip but we won't go into that).

If you want to use something for security purposes (LoginTicket, Recaptcha, cfg.d/security.pl) you should not use any user-supplied value. If you really trust your reverse proxy, then I suppose it's safe to use a value that you are certain it provided. However most of us don't have that guarantee -- we don't trust any node in the path, we just want to have EPrints make the best effort to tell us the IP address of the actual web browser or robot accessing our site. For the common case, the IP of some random proxy out in the web is just as useless as the (rare) XFF-spoofed address, but for the most part the values we're going to get back are what we're after.

For logging, there are other options, for example logging at the reverse proxy, or including the entire XFF header in the log.

@Hariatma17
Copy link

@Hariatma17 Hariatma17 commented Nov 27, 2014

hello, can anyone help me, complete guide how to install eprints in my localhost.

@denics
Copy link
Contributor

@denics denics commented Apr 15, 2015

Short answer tested on eprints 3.3.12:
Edit file

/perl_lib/EPrints/DataObj/LoginTicket.pm

change

143         my $ip = $r->connection->remote_ip;

to

143         my $ip = $r->connection->client_ip;

and

118                 $data->{ip} = $repo->get_request->connection->remote_ip;

to

118                 $data->{ip} = $repo->get_request->connection->client_ip;
jesusbagpuss added a commit that referenced this issue Apr 17, 2019
This was the approximate solution I was thinking of - but it mat not be perfect - and would need validation under Apache 2.2 and Apache2.4, for code that does, and does not use the `connection->remote_ip`  method.

The deparsed code block doesn't not contain comments (so wouldn't flag commented-out code - which I think is correct).
It may be that a link to a wiki page relating to the issue would be helpful to output - or alink to #214?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
7 participants
You can’t perform that action at this time.