Attacker can populate ip block rules and block any address

[vankoven]

Scope

The vulnerability was added in #1178 by using X-forwarded-for header for client accounting. It's not very ease to reproduce but still possible though. The root of the issue - naive client blocking in http_limits.c.

First, we try to get more accurate client description and current limits by X-forwarded-for header:

tempesta/tempesta_fw/http_limits.c

Lines 1109 to 1110 in 8d90875

	if (req->peer)
	ra = FRANG_CLI2ACC(req->peer);

Then if security event happens we block that client by ip address:

tempesta/tempesta_fw/http_limits.c

Lines 1118 to 1120 in 8d90875

	r = frang_http_req_process(ra, conn, data, dvh);
	if (r == TFW_BLOCK && dvh->frang_gconf->ip_block)
	tfw_filter_block_ip(&FRANG_ACC2CLI(ra)->addr);

The address used to block client comes from X-forwarded-for header, which has no relation to connection ip address. And can be forged by attacker.

The X-forwarded-for header tracking was implemented to better track users behind proxies and
apply Frang limits not only for connections, but also for clients behind that connections. E.g. if requests comes from CDN server. The feature is really required, but due to naive implementation we have added several problems, some can be caused by attacker, some by legitimate users.

First issue is unintentional and caused by legitimate users behind corporate proxies. The x-forwarded-for will contain local non-public ip address, e.g. 192.168.0.5. We definitely don't want to treat all users with local scope addresses as the came clients.

But attacker can do much more harm to us. First, he can forge X-forwarded-for header to point at specific client and make him blocked. Can be very harmful if majority of your clients comes from corporate networks with known addresses.

Next attacker can generate anything he wants in X-forwarded-for header and populate ip block table. The bigger the table, the severe performance impact.

It seems like req->peer must be depended on req->conn->peer,

We need to severely patch client guessing by x-forwarded-for header, my fault that I missed that security considerations in the original PR. To avoid my mistake always think about attackers possibilities when developing and reviewing a new security feature.

Related issues: #1350 #1045 #934

Although issues above tells about changes in the blocking layer, none of them highlights ip address forging. So this security considerations must be taken into account in all that issues.

The issue more likely, if messages from attacker if full message from attacker fits single skb. In that case exactly one Frang call happens after a request is fully assembled and a new TfwClient is bound with req->peer. If a request is split into several skb, Frang will fire multiple times with higher possibility of blocking entire connection in the middle of request processing.

Testing

This section is mandatory. Please, specify name of the test revealing the problem or at least generic considerations how to make a test for our QA. A link to a new issue in https://github.com/tempesta-tech/tempesta-test/issues would be just perfect.

[krizhanovsky]

The blocking behaviour heavily depends on the installation scenario, so we must provide a configuration settings how to treat client IPs (from the network layer and HTTP headers).

[vankoven]

After discussion with @avbelov23 I figured out that most of he issues is not applicable for Tempesta. Actually I missed somehow that the 'detailed' client IS linked with generic client description, i.e. req->peer is already depended on req->conn->peer, and req->peer->addr is the same address req->conn->peer has. Don't understand how could I missed that.

So there is no threat that malicious client can ban legitimate clients. Everything's here. But there is still an issue, that client behind proxy may populate client database without any control. I moved that threat description to #598 since it actually will provide a mitigation for free.

Since there no more other requirements in this issue, I close it.