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

[Python] Cmd line parameter to log proxied clients #351

Closed
wants to merge 1 commit into from

Conversation

praimmugen
Copy link

@praimmugen praimmugen commented May 3, 2018

When running behind a reverse proxy, websockify doesn't currently read and log anything that could provide information on the originating client.

This code adds a startup flag (--log-proxied-client) to enable logging of originating client plus intermediate proxies. Information is retrieved from the "X-Forwarded-For" header (if present). The immediate client would be logged too at the end of the content of the header, separated by a comma.

Eg:

  • default message: 127.0.0.1: Token 'abcd' not found
  • using the --log-proxied-client option: 192.168.0.51,127.0.0.1: Token 'abcd' not found

See also #334

* Added a command line parameter, '--log-proxied-client' to
  enable logging of intermediate proxies;
* only supports X-Forwarded-For header.
@@ -17,6 +17,7 @@

from websockify.websocket import WebSocket, WebSocketWantReadError, WebSocketWantWriteError


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noise. :)

immediate_client = super(WebSocketRequestHandler, self).address_string()
fwd_for = self.headers.get("X-Forwarded-For", None)
if show_proxied and fwd_for:
return "{},{}".format(fwd_for, immediate_client)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This format string isn't compatible with Python 2.6, which is our current baseline.

@@ -88,24 +88,29 @@ def __init__(self, req, addr, server):
self.daemon = getattr(server, "daemon", False)
self.record = getattr(server, "record", False)
self.run_once = getattr(server, "run_once", False)
self.rec = None
self.rec = None
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More noise

_, exc, _ = sys.exc_info()
# Connection was not a WebSockets connection
if exc.args[0]:
self.msg("%s: %s" % (address[0], exc.args[0]))
orig_client = getattr(e, 'orig_client', None)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should start piggy-backing extra information on the exception object. Better to add this information to the connection object somewhere.

@CendioOssman
Copy link
Member

Ping @praimmugen.

@samhed samhed added the python label Jul 22, 2018
@praimmugen
Copy link
Author

Sorry, had to divert my attention to something else. Will get back to this on the weekend - I see there are some conflicts, too, so better not to wait too much.

@samhed samhed removed the python label Sep 26, 2019
@samhed
Copy link
Member

samhed commented Nov 12, 2020

No response.

@samhed samhed closed this Nov 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants