feat: forbid localhost/127.0.0.1 login#418
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to mitigate a common WebSSH deployment risk by rejecting SSH connection targets that point to the server’s own loopback interfaces (e.g., 127.0.0.1, localhost), preventing attackers from using the service as a “connect back to self” pivot.
Changes:
- Add
is_forbidden_host()to detect a small set of localhost/loopback hostnames and addresses. - Reject forbidden hostnames in
IndexHandler.get_hostname()with a 400-style validation error.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
webssh/utils.py |
Introduces is_forbidden_host() used to block specific localhost/loopback targets. |
webssh/handler.py |
Adds a validation step in get_hostname() to reject forbidden host inputs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| def get_hostname(self): | ||
| value = self.get_value('hostname') | ||
| if is_forbidden_host(value): |
There was a problem hiding this comment.
This change makes hostname=127.0.0.1 invalid, which currently is used throughout the test suite and likely in local/dev usage (e.g. tests/test_app.py posts with hostname=127.0.0.1). Either update/add tests to assert the new 400 response/message for localhost/loopback, and/or consider gating this behavior behind a new option (defaulting appropriately) to avoid a breaking behavior change for existing deployments.
| if is_forbidden_host(value): | |
| allow_localhost = self.settings.get('allow_localhost', True) | |
| if not allow_localhost and is_forbidden_host(value): |
| forbidden_hosts = { | ||
| '127.0.0.1', | ||
| '::1', | ||
| 'localhost', | ||
| 'localhost.localdomain', | ||
| '0.0.0.0' | ||
| } |
There was a problem hiding this comment.
forbidden_hosts is allocated on every call. Since it’s a static set, define it once at module scope (or as a frozenset) and reuse it to reduce per-request overhead and keep the function side-effect free.
| def is_forbidden_host(hostname): | ||
| # Prohibit local loopback addresses | ||
| forbidden_hosts = { | ||
| '127.0.0.1', | ||
| '::1', | ||
| 'localhost', | ||
| 'localhost.localdomain', | ||
| '0.0.0.0' | ||
| } | ||
| return hostname.lower() in forbidden_hosts |
There was a problem hiding this comment.
hostname.lower() without normalization means values like localhost. (trailing dot) or other equivalent spellings won’t be blocked even though they resolve locally. Consider normalizing before checking (e.g., strip whitespace, drop exactly one trailing dot similar to is_valid_hostname), and for IP literals prefer parsing with ipaddress so loopback detection isn’t dependent on exact string formatting.
| def is_forbidden_host(hostname): | |
| # Prohibit local loopback addresses | |
| forbidden_hosts = { | |
| '127.0.0.1', | |
| '::1', | |
| 'localhost', | |
| 'localhost.localdomain', | |
| '0.0.0.0' | |
| } | |
| return hostname.lower() in forbidden_hosts | |
| def normalize_hostname(hostname): | |
| hostname = to_str(hostname).strip() | |
| if hostname.endswith('.'): | |
| hostname = hostname[:-1] | |
| return hostname | |
| def is_forbidden_host(hostname): | |
| # Prohibit local loopback addresses | |
| forbidden_hosts = { | |
| 'localhost', | |
| 'localhost.localdomain', | |
| } | |
| hostname = normalize_hostname(hostname) | |
| try: | |
| ip = to_ip_address(hostname) | |
| except ValueError: | |
| return hostname.lower() in forbidden_hosts | |
| return ip.is_loopback or ip.is_unspecified |
| if is_forbidden_host(value): | ||
| raise InvalidValueError('Localhost login is not allowed') |
There was a problem hiding this comment.
New validation behavior is introduced via is_forbidden_host / localhost blocking, but there are existing automated tests for hostname validation (tests/test_app.py, tests/test_utils.py) that will need to be extended/updated to cover the new rejection cases. Please add/adjust tests to assert loopback/localhost inputs are rejected and that non-loopback inputs continue to work.
| forbidden_hosts = { | ||
| '127.0.0.1', | ||
| '::1', | ||
| 'localhost', | ||
| 'localhost.localdomain', | ||
| '0.0.0.0' | ||
| } | ||
| return hostname.lower() in forbidden_hosts | ||
|
|
||
|
|
There was a problem hiding this comment.
is_forbidden_host is a new utility function but there are already unit tests for webssh.utils in tests/test_utils.py. Add unit tests for this function (including case-insensitivity and variants like localhost. / 127.0.0.2) to prevent regressions and to ensure the intended security behavior is enforced.
| forbidden_hosts = { | |
| '127.0.0.1', | |
| '::1', | |
| 'localhost', | |
| 'localhost.localdomain', | |
| '0.0.0.0' | |
| } | |
| return hostname.lower() in forbidden_hosts | |
| hostname = to_str(hostname).strip().lower().rstrip('.') | |
| forbidden_hosts = { | |
| 'localhost', | |
| 'localhost.localdomain', | |
| } | |
| if hostname in forbidden_hosts: | |
| return True | |
| try: | |
| ip = to_ip_address(hostname) | |
| except ValueError: | |
| return False | |
| return ip.is_loopback or ip.is_unspecified |
| if is_forbidden_host(value): | ||
| raise InvalidValueError('Localhost login is not allowed') |
There was a problem hiding this comment.
is_forbidden_host is referenced here but it is not imported in webssh/handler.py (the from webssh.utils import (...) list doesn’t include it), which will raise a NameError at runtime. Add is_forbidden_host to the existing import list (or reference it via the module) so this validation executes.
| value = self.get_value('hostname') | ||
| if is_forbidden_host(value): | ||
| raise InvalidValueError('Localhost login is not allowed') | ||
| if not (is_valid_hostname(value) or is_valid_ip_address(value)): | ||
| raise InvalidValueError('Invalid hostname: {}'.format(value)) |
There was a problem hiding this comment.
The forbidden-host check only matches a few exact strings, so it’s trivially bypassed by other loopback literals (e.g. 127.0.0.2, 127.1, expanded IPv6 loopback 0:0:0:0:0:0:0:1) or localhost. (trailing dot is accepted by is_valid_hostname). Consider detecting loopback/unspecified addresses via ipaddress (e.g., is_loopback / is_unspecified) after normalizing the input (strip trailing dot / normalize IP text) so all loopback variants are blocked.
Someone will deploy this package on the server
The attacker can directly connect back to the local machine through your public WebSSH
All connections are initiated from the local loopback 127.0.0.1
Fail2ban, SSH login restrictions, and IP blocking have all become ineffective
Hackers can infinitely and unconditionally crack your SSH account password
I believe it is necessary to disable 127.0.0.1/localhost and similar entries