Clear separate requirement for making sure that application gets actual user IP (move V1.14.7 away from V1)

[elarlang]

The main goal is, that logs must contain actual user IP, not some firewall, load-balancer, proxy etc IP.

and the other part of it, that users should not be able to fake their IP addresses using X-Forwarded-For or similar request headers.

To what category to put this requirement - I'm not sure. Most important it is for logs as it is the corner-stone for every investigation.

[jsulinski]

Also important for effective IP-based rate limiting.

[jsulinski]

8.1.x might be a good place for this.

[elarlang]

8.1 is valid when we like to protect the client ip from leaking (covered it as sensitive data anyway).

I think first choice should be current 14.5, as it is delivered via HTTP request header and in general is configuration issue.

[elarlang]

But with requirement text, in valid and correct English, I need help :)

[jsulinski]

Good call, I agree that 14.5 is a solid place for this. Perhaps:

14.5.6 "Verify that headers containing the user's IP address, such as X-Forwarded-For and/or X-Real-IP, include the true IP address."

Could also consider adding rationale: "to prevent spoofing in logs and rate limits."

[elarlang]

Final pieces:

• Level - proposal for level 1, 2, 3 - IP is the main corner-stone for investigations, if you don't have this data, there is nothing to investigate.
• CWE - CWE-348: Use of Less Trusted Source

[elarlang]

Analyzing this proposal text and maybe it needs still some finetuning - what if headers contain the faked IP, but application just ignores them? The point is - application should use only trusted one.

I throw some pieces here / potential points to cover (at the moment overlapped in wording):

• Verify that application uses only user's real IP
• accross all applicaiton components (load balanceres, firewalls, apis, application, etc)
• which is not spoofed by headers like X-Forwarded-For or X-Real-IP
• to provide data integrity related to user IP like to prevent spoofing in logs, rate limits

[jsulinski]

what if headers contain the faked IP, but application just ignores them? The point is - application should use only trusted one.

I throw some pieces here / potential points to cover (at the moment overlapped in wording):

* Verify that application uses only user's real IP

* accross all applicaiton components (load balanceres, firewalls, apis, application, etc)

The last point would be difficult to achieve on some cloud vendors/devices, for example: Google Cloud Load Balancers.

Aside from this point, the wording could be:

"Verify that the application is able to discern and utilizes the user's true, non-spoofed IP address from a header, for example X-Forwarded-For or X-Real-IP, and that rate limiting and logging use this true IP."

The non-spoofed part here is probably also superfluous as true IP suggests as much, so could probably be cut.

[elarlang]

Thank you for brainstorming. Part of making requirement texts good, is to "attacking" them.

Verify that the application is able to discern and utilizes the user's true, non-spoofed IP address from a header, for example X-Forwarded-For or X-Real-IP, and that rate limiting and logging use this true IP.

IP can come also from environment variable like REMOTE_ADDR, it's not always read from header.

Is this the material we can improve?
Verify that application uses only user's real IP which is not spoofed by headers like X-Forwarded-For or X-Real-IP to provide data integrity related to user IP like to prevent spoofing in logs, rate limits

[jmanico]

Well said. And IP addresses are not just sensitive data, they are also GDPR/Privacy protected data.

[jsulinski]

Here's another draft:

Verify that the application is able to discern and utilizes the user's true IP address to provide data integrity and that rate limiting and logging use this true IP.

[tghosth]

Verify that the application is able to discern and utilizes the user's true IP address to provide data integrity and that rate limiting and logging use this true IP.

@elarlang what do you think about this suggestion from @jsulinski?

[elarlang]

This issue is in a way related to #1697

Need to think should we merge them or not. Those are like 2 sides of the same problem.

[tghosth]

Waiting for response to #1697 (comment)

[jsulinski]

These two issues can likely be merged as this one is part of #1697 with a bit more detail. I'm not sure that detail is necessary, so could probably drop this one and accept #1697 (after adding X-Real-IP).

[elarlang]

It makes sense to keep them separately. One (#1697) is for avoiding end-users to manipulate or override expected headers and other is just being sure one of the most important piece of information for investigations - IP - is the correct one, not some waf, or proxy IP. In some cases they overlap, but at the moment it seems good for me to keep them separately.

So the reason and problem to solve is - IP addresses in application data and logs are often incorrect "by design" (without attackers attacking or spoofing)

[jsulinski]

Good point and agreed.

[elarlang]

From @jsulinski

Verify that the application is able to discern and utilizes the user's true IP address to provide data integrity and that rate limiting and logging use this true IP.

Previously I left it on hold as there was something to improve in my opinion - "now it is fixed" as I don't remember, what it was :) So I think we can put this requirement in and if needed, improve it later.

[elarlang]

IP spoofing vector is / will be covered now via #1697, so this requirement focus should be to avoid misconfiguration on the application side. Which rises questions like:

• category?
• cwe?
• level?

[elarlang]

Reopen for moving the requirement away from V1.14

[tghosth]

I think I am less opinionated about this right now, I think it could be considered to be V7 logging or V11 business logic so maybe V14 is a good compromise. Any further thoughts @elarlang ?

[elarlang]

This is something that program code does and that most likely you need to check from the program code, so from the paragraph choice is V10, but there is no suitable section there at the moment.

[tghosth]

Hmm, not sure about that, this doesn't feel that simple as it probably involves various components including program code, might also involve configuration of front end devices such as waf or proxy or cloud component or something. That's why I like v14 as more general than just code or v7 as less code connected

[elarlang]

See my previous comment #1389 (comment) and your own comment here #1389 (comment).

In case there are WAF or whatever component in place to "mess" with the IP address, it's the application's decision to use the correct one - that is why the requirement exists.

For "do not mess with IP address" we have:

V13.6.4 Verify that any HTTP headers used by the application and defined by intermediary devices like load balancers or proxies, such as X-Real-IP and X-Forwarded-*, cannot be overridden by the end-user.

(for other topic discussion, V13.6.4 may suit to configuration better)

[tghosth]

OK having discussed with Elar, ultimately the buck stops with the program code to be able to correctly interpret this so I would support this going into V10.

@elarlang please PR it in