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

trust_real_ip only sets host, not IP. #168

Closed
ghost opened this issue Sep 1, 2014 · 2 comments
Closed

trust_real_ip only sets host, not IP. #168

ghost opened this issue Sep 1, 2014 · 2 comments

Comments

@ghost
Copy link

ghost commented Sep 1, 2014

When using a reverse proxy, such as Apache with mod_proxy or nginx and setting trust_real_ip=1 in miniserv.conf, $acptip is not set. Only $acpthost and $loghost get set to reflect whatever X-Forwarded-For or X-Real-IP is.

Therefore when an incorrect username/password is supplied, the "block host" is incremented against the IP that the front-end server is proxying to. For example if the front-end server is proxying to https://localhost:20000, the bad login is seen to come from 127.0.0.1 even though trust_real_ip=1 in miniserv.conf.

In this scenario someone can effectively DoS the service by trying an incorrect username/password combination which will eventually block 127.0.0.1, which will prevent anyone from logging in since requests will always come from 127.0.0.1 due to the reverse proxying being used.

The fix is to correctly set $acptip along with $acpthost and $loghost:

If a remote IP is given in a header (such as via a proxy), only use it

for logging unless trust_real_ip is set

local $headerhost = $header{'x-forwarded-for'} ||
$header{'x-real-ip'};
if ($config{'trust_real_ip'}) {
$acptip = $headerhost || $acptip; # <-- Set IP too
$acpthost = $headerhost || $acpthost;
$loghost = $acpthost;
}
else {
$loghost = $headerhost || $loghost;
}

@ghost
Copy link
Author

ghost commented Sep 1, 2014

btw, not setting $acptip when trust_real_ip=1 also causes PAM authentication messages to syslog to use 127.0.0.1 (or whatever the reverse proxy is proxying to). This makes it impossible to use things like fail2ban.

@jcameron
Copy link
Collaborator

jcameron commented Sep 1, 2014

Good point - this will be fixed in the next Webmin release.

@jcameron jcameron closed this as completed Sep 1, 2014
vsilvar added a commit to vsilvar/webmin that referenced this issue Nov 12, 2018
The fix for webmin#168 was not complete, as the last suggested change was never pulled.
This fixes the problem that IP checks are done before $acptip is updated with the remote IP, by re-checking when set.

Another possible fix would be to only check IP's after reading the headers, but imo it is best to deny access as soon as possible.
jcameron added a commit that referenced this issue Nov 13, 2018
Re-check remote IP if trusted, fixes #168
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant