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

Security issue with SSL termination. Recommend SSL termination proxy or allow changing uid/gui of process. #79

Open
dol opened this issue Apr 24, 2013 · 5 comments
Labels
bug Something isn't working feature New feature or request patchwelcome

Comments

@dol
Copy link

dol commented Apr 24, 2013

websockify has IMHO a security flaw when it come to the use of SSL.

On any systems, it's recommended, to restrict the access to the SSL key and certificate. On Debian systems, all the keys and certs under /etc/ssl/private/ are only readable by the root user.
First solution:
websockify has to be started as root user to be able to read the certificate information under /etc/ssl/private/. If websockify has a vulnerability (yet or in the future, hope not), it might be possible to gain root access.
The second solution is to create a specific user for the websockify service, similar to the 'www-data' user for Apache. In this case, the websockify user must be able to read the certificate on startup. This can only be achieved by granting 'world' access to the certificate under /etc/ssl/private/. This might break other applications using the same certificate and performing a check if the certificate permissions are correct. Some application don't even startup if the permission are incorrect.

The only solution to this problem is using a SSL termination proxy in front of websockify.
1.) Start websockify under a specific websockify system user listen to localhost:<websockifyPort> without SSL support -> sudo -u websockify /opt/local/websockify localhost:<websockifyPort>
2.) Use a SSL termination proxy like stunnel and setup the port binding setting like accept=<publicPort>, connect=localhost:<websockifyPort>

stunnel is one of the only SSL termination proxy that are capable dealing with websocket upgrades. NGINX has recently added websocket support.

An other possible solution is, to start the server as root, read the certificate in memory and change the uid/gid of the process.
supervisord is doing this here. This can also be added to websockify here with a newly introduced parameter --user/--group.

@kanaka
Copy link
Member

kanaka commented Apr 24, 2013

Yeah, that's a good point. Unfortunately, the ssl implementation in python takes file paths and not string data, so getting the cert data securely from one side of the setuid/fork to the other side will be non-trivial.

I won't have time to do this any time soon, but I'm certainly willing to take a well written/tested patch.

@dol
Copy link
Author

dol commented Apr 25, 2013

Valid point.

Note for the future:
According to this issue/patch this can be achieved in Python 3.2 with the use of SSL Contexts.
For older Python versions, the use of pyOpenSSL might be a solution. pyOpenSSL is shipped as a package in most of the Linux distros.

@kanaka
Copy link
Member

kanaka commented Apr 25, 2013

@dol that's a good reference.

Does pyOpenSSL have a routine to wrap an existing non-SSL socket like the standard ssl module?

@dol
Copy link
Author

dol commented Apr 25, 2013

According to this example and the documentation of Connection class it's possible. Guess this is what you where looking for.
The 'Roundup Issue Tracker' project has a pyOpenSSL Server implementation with a privilege change of the uid/gid.

@markmont
Copy link

Here is a different approach, which uses a setuid wrapper program. I threw this together quickly, but if there is interest I could clean it up and implement it properly:
https://bitbucket.org/umlsait/umich-python-websockify

@samhed samhed removed the python label Sep 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working feature New feature or request patchwelcome
Projects
None yet
Development

No branches or pull requests

4 participants