Opened 10 years ago

Closed 10 years ago

#539 closed defect (fixed)

Implement CLIENTIP and CLIENTAGENT tracking in the Server's Access.log

Reported by: tonyfang Owned by: tonyfang
Priority: medium Milestone: 2.1
Component: Web API Version: 2.0.0
Severity: minor Keywords:
Cc: External ID:

Description

The CLIENTIP and CLIENTAGENT parameters must be sent to the server in order for the Access.log to record them. Currently, only the Map Admin php application is doing so.

The following changes are needed:

  • The Apache/ISAPI/CGI agent needs to figure out the IP address from the REMOTE_ADDR CGI Server variable and send it to the server. This takes care of the IP address for http server requests.
  • When a web application (like the Map Admin, Ajax Viewer, Fusion Viewer, Schema Report) creates an MgSiteConnection?, the MgUserInformation? should also set the Client IP and Client Agent. Currently, this was only happening for the Map Admin.
  • When a web application (Ajax Viewer, Fusion Viewer) makes an http server request, it needs to set the CLIENTAGENT parameter.
  • The web tier test pages need to also send the CLIENTAGENT parameter with its server requests. We will add editable textboxes for this parameter to be consistent with the other parameters.

Notes:

  • The Fusion Viewer changes will be made to the Fusion vault.

Change History (10)

comment:1 Changed 10 years ago by tonyfang

Here are the CLIENTAGENT names that will be used:

  • Ajax Viewer
  • Fusion Viewer
  • Map Admin
  • MapGuide Developer

Note: "MapGuide Developer" will be used for the Web Tier test pages and the Schema Report.

comment:2 Changed 10 years ago by tonyfang

Resolution: fixed
Status: newclosed

Add CLIENTAGENT parameter to Web Tier test pages: http://trac.osgeo.org/mapguide/changeset/3147

CLIENTIP processing for the Apache/ISAPI/CGI Agents: http://trac.osgeo.org/mapguide/changeset/3148 http://trac.osgeo.org/mapguide/changeset/3150

CLIENTIP and CLIENTAGENT changes for Schema Report and Map Admin: http://trac.osgeo.org/mapguide/changeset/3149

CLIENTIP and CLIENTAGENT changes for Ajax: http://trac.osgeo.org/mapguide/changeset/3151

Note: Fusion Viewer changes will be made in the Fusion vault.

comment:3 Changed 10 years ago by jbirch

What is this being used for (other than logging)?

If it's important to know the end user's IP address, then maybe it's worthwhile determining whether they're behind a proxy and recording the HTTP_CLIENT_IP or X_FORWARDED_FOR values.

These can be problematic though; sometimes not containing a single IP, sometimes with junk embedded. And they come from the client, so can't be trusted the same way as REMOTE_ADDR.

comment:4 Changed 10 years ago by tonyfang

Just for logging. As it says in the Description, the Map Admin was already sending these parameters to the server and getting them logged in the Access.log.

The submissions associated with this ticket complete the work for the rest of the applications (Ajax, Web Tier test pages, Schema Report). And also implements the REMOTE_ADDR IP checking in the Apache/ISAPI/CGI agents for HTTP requests.

I figured REMOTE_ADDR is the most useful IP information for the Access.log. If the user is behind a proxy, the IP behind the proxy may not be very meaningful to the Access.log.

comment:5 Changed 10 years ago by jbirch

Yes, if you have to choose one, REMOTE_ADDR is best.

On one of my hobby sites, I see a large number of requests coming from transparent proxies such as Shaw Cable's. The users behind these proxies have publicly reachable IP addresses, but their web requests are being stuffed through a caching proxy. I've found it valuable to have X_FORWARDED_FOR when tracking down abusive users.

Not a big deal, just thought I'd comment.

comment:6 Changed 10 years ago by tonyfang

Resolution: fixed
Status: closedreopened

Jason: Thanks for the tip!

I looked a little more into HTTP_CLIENT_IP and XFF -- that seems much better than simply using REMOTE_ADDR. So the code will be changed to the following:

function GetClientIp()
{
    $clientIp = '';
    if (array_key_exists('HTTP_CLIENT_IP', $_SERVER)
        && strcasecmp($_SERVER['HTTP_CLIENT_IP'], 'unknown') != 0)
    {
        $clientIp = $_SERVER['HTTP_CLIENT_IP'];
    }
    else if (array_key_exists('HTTP_X_FORWARDED_FOR', $_SERVER)
        && strcasecmp($_SERVER['HTTP_X_FORWARDED_FOR'], 'unknown') != 0)
    {
        $clientIp = $_SERVER['HTTP_X_FORWARDED_FOR'];
    }
    else if (array_key_exists('REMOTE_ADDR', $_SERVER))
    {
        $clientIp = $_SERVER['REMOTE_ADDR'];
    }
    return $clientIp;
}

comment:7 Changed 10 years ago by jbirch

I'm probably being pedantic here and drawing out a really basic change, but I would suggest a regex match on the field to ensure it contains an IP address (or something vaguely resembling one). I see a number of these coming through with junk strings in them. An inaccurate but fairly quick one would be something like:

/^(\d{1,3}\.){3}\d{1,3}$/

This is assuming that the above is PHP code (sure looks like it) and you can use preg_match()

comment:8 Changed 10 years ago by tonyfang

Thanks, I will look into that.

Yes, the above is PHP. But we will need to do the same regex match for JSP, ASP, and C++. JSP and ASP is for the Java and .NET APIs, C++ is for the Apache/CGI/ISAPI agents.

comment:9 Changed 10 years ago by tonyfang

I forgot to adda couple files for the Schema Report last week (so it actually wasn't logging): http://trac.osgeo.org/mapguide/changeset/3155

comment:10 Changed 10 years ago by tonyfang

Resolution: fixed
Status: reopenedclosed

HTTP_CLIENT_IP and HTTP_X_FORWARDED_FOR changes for trunk: http://trac.osgeo.org/mapguide/changeset/3171

All CLIENTIP/CLIENTAGENT changes (including HTTP_CLIENT_IP and HTTP_X_FORWARDED_FOR changes) for 2.0.x branch: http://trac.osgeo.org/mapguide/changeset/3172

Adding CLIENTAGENT to Web Tier test pages in 2.0.x branch: http://trac.osgeo.org/mapguide/changeset/3173

Note: See TracTickets for help on using tickets.