Uploaded image for project: 'OpenAM'
  1. OpenAM
  2. OPENAM-12380

client ip audit logging is not storing as IP but a list of IPs

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 13.5.0, 13.5.1, 13.5.2, 14.0.0, 14.1.0, 14.1.1, 14.5.0, 14.5.1
    • Fix Version/s: 13.5.3, 6.0.0, 14.1.2, 5.5.2
    • Component/s: audit logging, rest
    • Labels:
    • Target Version/s:
    • Sprint:
      AM Sustaining Sprint 47
    • Story Points:
      1
    • Needs backport:
      No
    • Support Ticket IDs:
    • Needs QA verification:
      Yes
    • Functional tests:
      No
    • Are the reproduction steps defined?:
      Yes and I used the same an in the description

      Description

      Bug description

      When configuring client ip to take form the header using com.sun.identity.authentication.client.ipAddressHeader=X-Forwarded-For ,
      some of the audit logs show multiple IPs. The issue is that client-ip field in the CVS should be an IP and not the full list of IPs (from XFF)

      Some of the access URL (like /json/authenticate) works but the CREST side logs this bad client-ip.

      How to reproduce the issue

      1. Add com.sun.identity.authentication.client.ipAddressHeader=X-Forwarded-For to the Server default Advance properties
      2. Restart
      3. Now do a REST json authenticate (using curl) to /json/authenticate
       curl -s -X POST -H 'X-Forwarded-For: 1.1.1.1, 2.2.2.2, 3.3.3.3' -H 'Content-Type: application/json' -H 'X-OpenAM-Username: amadmin' -H 'X-OpenAM-Password: password' -H 'X-NoSession: true' -H 'X-Password: anonymous' -H 'X-Requested-With: XMLHttpRequest' 'http://openamexample.com:8080/openam/json/authenticate?realm=/'
      
      1. Check the access.csv and see
      ....,"AM-ACCESS-OUTCOME","97d7d5f6-edd0-45f8-a800-31584706b147-121","id=amadmin,ou=user,dc=openam,dc=forgerock,dc=org","[""5b999e4aab71252c01""]",,,"1.1.1.1",,,,,"false","POST","http://openam.example.com:8080/openam/json/authenticate","{""realm"":[""/""]}","{""accept"":[""*/*""],""host"":[""openam.example.com:8080""],""user-agent"":[""curl/7.29.0""],""x-forwarded-for"":[""1.1.1.1, 2.2.2.2, 3.3.3.3""],""x-nosession"":[""true""],""x-openam-username"":[""amadmin""],""x-password"":[""anonymous""],""x-requested-with"":[""XMLHttpRequest""]}","{}",,"SUCCESSFUL",,,"38","MILLISECONDS","Authentication","/"
      
      • Notice the client.ip field is fine with value "1.1.1.1"
      1. Now access using the above obtained SSOToken
      curl -s -k -X POST -H 'X-Forwarded-For: 1.1.1.1, 2.2.2.2, 3.3.3.3' -H 'Cookie: iPlanetDirectoryPro=AQIC5wM2LY4SfcwBQw3SN2hyLgTUHcm7l424hvUTc6MOeuE.*AAJTSQACMDEAAlNLABQtNzAwMDM4Nzk4MTQ2MTM4MDkxMwACUzEAAA..*' 'http://openam.example.com:8080/openam/json/sessions?_action=validate'
      

      and check the logs:

      "97d7d5f6-edd0-45f8-a800-31584706b147-118","2018-01-30T05:45:57.693Z","AM-ACCESS-OUTCOME","97d7d5f6-edd0-45f8-a800-31584706b147-116","id=amadmin,ou=user,dc=openam,dc=forgerock,dc=org","[""2b58f1337644c3ce01""]","172.28.1.228","8080","1.1.1.1, 2.2.2.2, 3.3.3.3",,"CREST","ACTION","{""action"":""validate""}","false","POST","http://openam.example.com:8080/openam/json/sessions","{""_action"":[""validate""]}","{""accept"":[""*/*""],""Accept-API-Version"":[""protocol=1.0""],""host"":["openam.example.com:8080""],""user-agent"":[""curl/7.29.0""],""x-forwarded-for"":[""1.1.1.1, 2.2.2.2, 3.3.3.3""]}","{}",,"SUCCESSFUL",,,"3","MILLISECONDS","Session","/"
      
      

      Notice the client.ip is is the same as X-forwarded-for

      Expected behaviour
      The audit client.ip field should only be 1 values
      
      Current behaviour
      The value of XFF is stored in client.ip whole-sale
      

       Impact

      When logging goes to some audit system, the extra long client-ip from XFF may cause AM audit system failure and the event may not be audited.

      Work around

      -

      Code analysis

      org.forgerock.openam.rest.fluent.CrestAuditor.java
          private void setClientFromHttpContextHeaderIfExists(AMAccessAuditEventBuilder builder, Context context) {
              if (context.containsContext(HttpContext.class)) {
                  HttpContext httpContext = context.asContext(HttpContext.class);
                  List<String> xForwardedFor = httpContext.getHeader(
                          SystemPropertiesManager.get(Constants.CLIENT_IP_ADDR_HEADER));
                  if (xForwardedFor != null && xForwardedFor.size() > 0) {
                      builder.client(xForwardedFor.get(0));
                  }
              }
          }
      

      Need to do same filtering as previous work

        Attachments

          Activity

            People

            • Assignee:
              chee-weng.chea C-Weng C
              Reporter:
              chee-weng.chea C-Weng C
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: