#1 2016-11-16 12:42:50

mpv
Member
From: Ukraine
Registered: 2012-03-24
Posts: 1,571
Website

CRITICAL - Load balancer & client IP address problem

Because our service has reached a critical load we put a Load balancer  (nginx with realip_module) and run a several instances of application (HTTP.SYS based server). Some part of our application (encryption, user rights management, logging) require a knowledge about real client IP. BUT current THttpApiServer implementation (and all other HTTP servers) determinate a client IP address from a socket. In case of load balance the preferred way is to determinate a Client IP from a HTTP header, ballancer will add for us. In most case this is a X-Real-IP or X-Forwarded-For

I think we should add a property THttpServerGeneric.RealClientIPHeader: SockString and in case it set to something <> '' retrieve a real client IP address from value of RealClientIPHeader header.

In this case if I know I deploy my service behind the load balance, a will set a RealClientIPHeader property to the value, specified by my balancer and got a real client IP as expected. If not - the old realisation must work (to prevent a attack when hacker add a X-Forwarded-For by hand)

For a while I patch a HTTP API server RetrieveHeaders implementation (as a temporary solution) as such:

begin
  assert(low(KNOWNHEADERS)=low(Request.Headers.KnownHeaders));
  assert(high(KNOWNHEADERS)=high(Request.Headers.KnownHeaders));
  // MPV start X-Forwarded-For hack
  P := Request.Headers.pUnknownHeaders;
  RemoteIP := '';
  if P<>nil then
    for i := 1 to Request.Headers.UnknownHeaderCount do begin
      if (P^.NameLength = 15) and (StrLComp(P^.pName, 'X-Forwarded-For', 15) = 0) then begin
        SetString(RemoteIP, P^.pRawValue, P^.RawValueLength);
        break;
      end;
    end;
  if (RemoteIP = '') and (Request.Address.pRemoteAddress<>nil) then
 //MPV End X-Forwarded-For hack
    GetSinIPFromCache(PVarSin(Request.Address.pRemoteAddress)^,RemoteIP);
  // compute headers length 

May be anybody know better solution? AB - can I add a THttpServerGeneric.RealClientIPHeader to the current trunc?

Last edited by mpv (2016-11-16 13:04:08)

Offline

#2 2017-11-23 16:54:43

mpv
Member
From: Ukraine
Registered: 2012-03-24
Posts: 1,571
Website

Re: CRITICAL - Load balancer & client IP address problem

@AB - I see in latest commit you improve a client IP detection. But usage of a constant HEADER_REMOTEIP_UPPER = 'REMOTEIP: ' is invalid in case servers are behind a reverse proxy. Is it possible to add a property THttpServerGeneric.RealClientIPHeaderUpper: SockString and in case it set to something <> '' retrieve a real client IP address from value of RealClientIPHeader header? This feature is critical for me.....

Offline

#3 2017-11-23 21:54:09

ab
Administrator
From: France
Registered: 2010-06-21
Posts: 14,686
Website

Re: CRITICAL - Load balancer & client IP address problem

Please check https://synopse.info/fossil/info/398681f7ca

It should work with http.sys and also with the socket server.

Offline

#4 2017-11-24 13:28:13

mpv
Member
From: Ukraine
Registered: 2012-03-24
Posts: 1,571
Website

Re: CRITICAL - Load balancer & client IP address problem

Thanks a lot!

Offline

#5 2021-05-20 09:38:39

edwinsn
Member
Registered: 2010-07-02
Posts: 1,218

Re: CRITICAL - Load balancer & client IP address problem

I guess the value of TSQLHttpServer.HttpServer.RemoteIPHeader should be set to 'X-Forwarded-For' by default?
Because according to the Mozilla docs:

The X-Forwarded-For (XFF) header is a de-facto standard header for identifying the originating IP address of a client connecting to a web server through an HTTP proxy or a load balancer.


Delphi XE4 Pro on Windows 7 64bit.
Lazarus trunk built with fpcupdelux on Windows with cross-compile for Linux 64bit.

Offline

#6 2021-05-20 12:06:08

ab
Administrator
From: France
Registered: 2010-06-21
Posts: 14,686
Website

Re: CRITICAL - Load balancer & client IP address problem

The default is to retrieve the info from the socket itself, i.e. it is ''.

Idea is to change it only if a reverse proxy is used, to avoid an unneeded search via FindNameValue(headers).

Offline

#7 2021-05-21 03:26:39

Chaa
Member
Registered: 2011-03-26
Posts: 249

Re: CRITICAL - Load balancer & client IP address problem

And in case if reverse proxy is not used, it is mandatory that RemoteIPHeader to be empty.
In other case client can arbitrarily change RemoteIP, that is possible security risk.

Offline

#8 2021-05-21 06:31:17

edwinsn
Member
Registered: 2010-07-02
Posts: 1,218

Re: CRITICAL - Load balancer & client IP address problem

@ab and @chaa, Thanks for the explanation.

Last edited by edwinsn (2021-05-21 14:37:54)


Delphi XE4 Pro on Windows 7 64bit.
Lazarus trunk built with fpcupdelux on Windows with cross-compile for Linux 64bit.

Offline

#9 2021-05-21 12:47:42

ab
Administrator
From: France
Registered: 2010-06-21
Posts: 14,686
Website

Re: CRITICAL - Load balancer & client IP address problem

@Chaa
The security argument is the main one, for sure!

Offline

#10 2021-05-23 08:28:17

mpv
Member
From: Ukraine
Registered: 2012-03-24
Posts: 1,571
Website

Re: CRITICAL - Load balancer & client IP address problem

As noted above dafault value is '', and this is OK.
Be careful with  X-Forwarded-For - it can contains several IP addresses in case of several reverse proxies, so better to use X-Real-IP and calculate it on the last reverse proxy - most of them know how to work with multiple IPs in X-Forvarded-For.

In case of nginx valid address can de retrieved using $remote_addr variable (and a proxies chain builds using  $proxy_add_x_forwarded_for)

 
proxy_set_header    X-Real-IP  $remote_addr;
proxy_set_header    X-Forwarded-For $proxy_add_x_forwarded_for;

and in case of many proxies additional configuration is required to get a real IP form intermediate proxy we trust

  # use client real IP as reported by intermediate proxy
  real_ip_header    X-Forwarded-For;
  # A proxy we trust (address, hostname or mask)
  set_real_ip_from  b.b.b.b;

Offline

Board footer

Powered by FluxBB