ASVS
ASVS copied to clipboard
OWASP
Clear separate requirement for making sure that application gets actual user IP
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.
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.
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."
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
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
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.
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
Well said. And IP addresses are not just sensitive data, they are also GDPR/Privacy protected data.
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.
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?
This issue is in a way related to https://github.com/OWASP/ASVS/issues/1697
Need to think should we merge them or not. Those are like 2 sides of the same problem.
Waiting for response to https://github.com/OWASP/ASVS/issues/1697#issuecomment-1715772670
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).
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)
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.
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?
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.
What do you mean by "data integrity" in this case @jsulinski? Do you think you could be more specific?
What do you mean by "data integrity" in this case @jsulinski? Do you think you could be more specific?
This seems superfluous and was from previous copy that I was editing.
Perhaps:
Verify that the application is able to discern and utilizes the user's true IP address to provide for sensitive functions, including rate limiting, logging, and/or as an aspect of authentication.
CWE: 348 Category: 14.4 or 10.3 Level: 2
I added the part about authentication because IP is sometimes used in the decision to require additional authentication factors (along with other information) but this can be removed if the requirement is becoming too unclear or unfocused.
I added the part about authentication because IP is sometimes used in the decision to require additional authentication factors (along with other information) but this can be removed if the requirement is becoming too unclear or unfocused.
Yes, I would skip this authentication part - if authentication is limited by IP, probably they use actual IP. But geolocation or whatever can be topic.
Category - actually I don't know where it fits. In big picture V14 configuration seems to be correct place.
- 14.4 are response headers for browser, so this does not seems to be correct. 14.5 is better, but this part is covered via #1697 .
- 10.3 - V10 in general wait rebuild and proper cleanup.
@elarlang what do you think about chapter 11.2 ("Anti-automation" within "Business Logic") as this new requirement mostly seems to be related to this.
V11.2 uses it. So do entire logging V7 and whatever uses IP as input data.
By v4.0.3 logic suitable category is V1.14, but it requires cleanup itself. As a temporary solution to just get the requirement in, we can use this maybe.
Requirement got in and at the moment is in V1 category as there was not any better one at this moment.
V1.14 Configuration Architecture
| # | Description | L1 | L2 | L3 | CWE |
|---|---|---|---|---|---|
| 1.14.8 | [ADDED] Verify that the application is able to discern and utilizes the user's true IP address to provide for sensitive functions, including rate limiting and logging. | ✓ | ✓ | 348 |
Via #1324 / PR #1343 we may get new subcategory to V14 Configuration category named "V14.7 Web Application Server Configuration" , and I think it makes to move this requirement to V14.7
I disagree that this is server configuration because it is the application itself that needs a clear way of doing this.
I think I would still support being in the business logic section as it is kinda a business logic thing