Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[cfg] Make sure Tempesta doesn't specify itself as a backend #774

Closed
keshonok opened this issue Jul 26, 2017 · 5 comments
Closed

[cfg] Make sure Tempesta doesn't specify itself as a backend #774

keshonok opened this issue Jul 26, 2017 · 5 comments
Assignees
Milestone

Comments

@keshonok
Copy link
Contributor

keshonok commented Jul 26, 2017

A backend server (an IP address and port number) specified in Tempesta configuration may in fact turn out to be the same as an address where Tempesta listens to incoming requests from clients. The mistake is very easy to make. There should be a protection against that.

The problem was recognized before and seemingly fixed in 83c987f. However, the fix has issues of its own.

  • A listening address and port and backend address and port may be specified in different forms. One may be specified as an IPv4 address, while the other may be specified as an IPv6 address. The function tfw_addr_ifmatch() that compares the addresses doesn't seem to handle this case. See issue Unify the representation of IP addresses in Tempesta #775.
  • The check to see if the addresses of backend servers and Tempesta listeners match is performed after the configuration is processed by each module. However, the implementation details of configuration processing are that servers specified outside of any group are grouped together in the group named default, and that is done after complete configuration has been processed. So at the time the check is performed these servers are not in the configuration yet, so they are missed by the check.

The listener's module is called and initialized before the module that handles backends. Yet the check is implemented in the listener's module. Perhaps it makes more sense to move it to the module for backends.

@keshonok keshonok added the bug label Jul 26, 2017
@krizhanovsky krizhanovsky added this to the 1.0 WebOS milestone Jul 26, 2017
@krizhanovsky krizhanovsky modified the milestones: backlog, 0.6 KTLS Jan 9, 2018
@krizhanovsky krizhanovsky modified the milestones: 0.6 KTLS, 0.7 HTTP/2 Jan 9, 2018
@krizhanovsky
Copy link
Contributor

krizhanovsky commented Jan 9, 2018

Most of Web accelearators don't care abut such kind of fool protection, so low priority. Just add the config file checks to tempesta.sh.

@krizhanovsky krizhanovsky modified the milestones: 0.7 HTTP/2, 0.8 TDB v0.2 Jan 9, 2018
@krizhanovsky krizhanovsky modified the milestones: 1.2 TDB v0.2, 1.1 QUIC Aug 8, 2018
@krizhanovsky krizhanovsky changed the title Make sure Tempesta doesn't specify itself as a backend [cfg] Make sure Tempesta doesn't specify itself as a backend Feb 2, 2019
kevgs added a commit that referenced this issue Jun 6, 2019
As I understand both claims from issue are satisfied now:
* mix of v4 and v6 addresses is handled
* check is performed at the right time

So there is basically nothing to fix.

Remove misleading TODO: v6 case is alredy the same as v4 one.

Remove redundant port check from tfw_addr_ifmatch() which happens in
tfw_addr_eq() anyways.

Replace direct sockaddr_in::sin6_port access with accessor
function tfw_addr_port().
@i-rinat
Copy link
Contributor

i-rinat commented Jun 6, 2019

As mentioned in the description, the issue consists of two parts.

First is that the same address may be specified as IPv4 or as IPv4-mapped IPv6. That was described in issue #775, and was fixed by #1081. Now IPv6 is used to represent all addresses, thus normalizing them during configuration stage.

Last is about checking for matches at the very end of the configuration stage. That was fixed in somewhere near 6ff9bba which was a part of #851. That moved checking to the new stage which was called after all configuration was parsed.

With all of the above, I believe the issue is fixed already.

@krizhanovsky
Copy link
Contributor

If Tempesta doesn't start (or at least prints warnings) with configuration having the same address as listening and a upstream, then we can just close the issue. It'd be good to have a functional test (or just add the check to some existing test), but with that the issue is low priority it's not necessary.

kevgs added a commit that referenced this issue Jun 7, 2019
As I understand both claims from issue are satisfied now:
* mix of v4 and v6 addresses is handled
* check is performed at the right time

So there is basically nothing to fix.

Remove misleading TODO: v6 case is alredy the same as v4 one.

Replace direct sockaddr_in::sin6_port access with accessor
function tfw_addr_port().
@i-rinat
Copy link
Contributor

i-rinat commented Jun 7, 2019

If Tempesta doesn't start (or at least prints warnings) with configuration having the same address as listening and a upstream

It indeed doesn't start.
./scripts/tempesta.sh --start says:

Starting Tempesta...
...enable RPS on enp1s0
...load Tempesta modules
Loading Tempesta kernel modules...
Loading module tempesta_lib 
Loading module tempesta_tls 
Loading module tempesta_db 
Loading module tempesta_fw tfw_cfg_path=/root/tempesta/.settings/tempesta_fw.conf
...start Tempesta FW
Un-loading Tempesta kernel modules...
ERROR: cannot start Tempesta FW (sysctl code: Invalid argument)

dmesg output:

[ 2408.764955] [tdb] Start Tempesta DB
[ 2408.776415] net_ratelimit: 7 callbacks suppressed
[ 2408.776415] [tempesta fw] Initializing Tempesta FW kernel module...
[ 2408.781713] [tempesta fw] Registering new classifier: frang
[ 2408.784496] [tempesta fw] Registering new scheduler: hash
[ 2408.787215] [tempesta fw] Registering new scheduler: ratio
[ 2408.793992] [tempesta fw] Preparing for the configuration processing.
[ 2408.798905] [tempesta fw] ERROR: One of the backends is Tempesta itself! Please, fix the configuration.
[ 2408.803345] [tempesta fw] ERROR: Unable to complete the configuration of module 'sock_clnt': -22
[ 2408.807852] [tempesta fw] Warning: Configuration parsing has failed. Clean up...
[ 2408.818763] [tempesta fw] New configuration is cleaned.
[ 2408.822562] [tempesta fw] exiting...
[ 2408.824636] [tempesta fw] Un-registering scheduler: ratio
[ 2408.828506] [tempesta fw] Un-registering scheduler: hash
[ 2408.837757] [tempesta fw] Unregistering classifier: frang
[ 2408.866023] [tdb] Shutdown Tempesta DB

That's what happens when configuration below is used:

listen 81;
server 127.1.2.3:81;

@krizhanovsky
Copy link
Contributor

Not so bad, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants