From 090eaa09eebde832b24b0d6fed8ffde5e6bf62ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hermann=20H=C3=B6hne?= Date: Tue, 29 Aug 2017 21:24:32 +0200 Subject: [PATCH 1/6] Added SSL-certificate-based client authentication. --- websockify/auth_plugins.py | 20 ++++++++++++++++++++ websockify/websocketproxy.py | 20 ++++++++++++++++++++ websockify/websockifyserver.py | 20 ++++++++++++++------ 3 files changed, 54 insertions(+), 6 deletions(-) diff --git a/websockify/auth_plugins.py b/websockify/auth_plugins.py index 93f13855..dd6a9e82 100644 --- a/websockify/auth_plugins.py +++ b/websockify/auth_plugins.py @@ -81,3 +81,23 @@ def authenticate(self, headers, target_host, target_port): origin = headers.get('Origin', None) if origin is None or origin not in self.source: raise InvalidOriginError(expected=self.source, actual=origin) + +class ClientCertAuth(object): + """Verifies client by SSL certificate. Specify src as whitespace separated list of common names.""" + + def __init__(self, src=None): + if src is None: + self.source = [] + else: + self.source = src.split() + + def authenticate(self, headers, target_host, target_port): + try: + if (headers.get('SSL_CLIENT_S_DN_CN') not in self.source): + raise AuthenticationError(response_code=403) + except AuthenticationError: + # re-raise AuthenticationError (raised by common name not in configured source list) + raise + except: + # deny access in case any error occurs (i.e. no data provided) + raise AuthenticationError(response_code=403) diff --git a/websockify/websocketproxy.py b/websockify/websocketproxy.py index 6fd773a7..09feee32 100755 --- a/websockify/websocketproxy.py +++ b/websockify/websocketproxy.py @@ -60,6 +60,20 @@ def validate_connection(self): self.server.target_port = port if self.server.auth_plugin: + + try: + # get client certificate data + client_cert_data = self.request.getpeercert() + # extract subject information + client_cert_subject = client_cert_data['subject'] + # flatten data structure + client_cert_subject = dict([x[0] for x in client_cert_subject]) + # add common name to headers (apache +StdEnvVars style) + self.headers['SSL_CLIENT_S_DN_CN'] = client_cert_subject['commonName'] + except: + # not a SSL connection or client presented no certificate with valid data + pass + try: self.server.auth_plugin.authenticate( headers=self.headers, target_host=self.server.target_host, @@ -392,6 +406,12 @@ def websockify_init(): help="disallow non-encrypted client connections") parser.add_option("--ssl-target", action="store_true", help="connect to SSL target as SSL client") + parser.add_option("--verify-client", action="store_true", + help="require encrypted client to present a valid certificate") + parser.add_option("--cafile", metavar="FILE", + help="file of concatenated certificates of authorities trusted " + "for validating clients (only effective with --verify-client). " + "If omitted, system default list of CAs is used.") parser.add_option("--unix-target", help="connect to unix socket target", metavar="FILE") parser.add_option("--inetd", diff --git a/websockify/websockifyserver.py b/websockify/websockifyserver.py index 1b5b15ec..2c8b7456 100644 --- a/websockify/websockifyserver.py +++ b/websockify/websockifyserver.py @@ -319,6 +319,7 @@ class Terminate(Exception): def __init__(self, RequestHandlerClass, listen_fd=None, listen_host='', listen_port=None, source_is_ipv6=False, verbose=False, cert='', key='', ssl_only=None, + verify_client=False, cafile=None, daemon=False, record='', web='', file_only=False, run_once=False, timeout=0, idle_timeout=0, traffic=False, @@ -333,6 +334,7 @@ def __init__(self, RequestHandlerClass, listen_fd=None, self.listen_port = listen_port self.prefer_ipv6 = source_is_ipv6 self.ssl_only = ssl_only + self.verify_client = verify_client self.daemon = daemon self.run_once = run_once self.timeout = timeout @@ -352,13 +354,15 @@ def __init__(self, RequestHandlerClass, listen_fd=None, # Make paths settings absolute self.cert = os.path.abspath(cert) - self.key = self.web = self.record = '' + self.key = self.web = self.record = self.cafile = '' if key: self.key = os.path.abspath(key) if web: self.web = os.path.abspath(web) if record: self.record = os.path.abspath(record) + if cafile: + self.cafile = os.path.abspath(cafile) if self.web: os.chdir(self.web) @@ -518,7 +522,6 @@ def do_handshake(self, sock, address): """ ready = select.select([sock], [], [], 3)[0] - if not ready: raise self.EClose("ignoring socket not ready") # Peek, but do not read the data so that we have a opportunity @@ -538,11 +541,16 @@ def do_handshake(self, sock, address): % self.cert) retsock = None try: - retsock = ssl.wrap_socket( + context = ssl.create_default_context(ssl.Purpose.CLIENT_AUTH) + context.load_cert_chain(certfile=self.cert, keyfile=self.key) + if self.verify_client: + context.verify_mode = ssl.CERT_REQUIRED + context.set_default_verify_paths() + if self.cafile: + context.load_verify_locations(cafile=self.cafile) + retsock = context.wrap_socket( sock, - server_side=True, - certfile=self.cert, - keyfile=self.key) + server_side=True) except ssl.SSLError: _, x, _ = sys.exc_info() if x.args[0] == ssl.SSL_ERROR_EOF: From 554a8225c48225019463bcd76d9b4ce329a672ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hermann=20H=C3=B6hne?= Date: Tue, 29 Aug 2017 21:25:02 +0200 Subject: [PATCH 2/6] Added manual for certificate-based client authentication. --- docs/websockify.1 | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/docs/websockify.1 b/docs/websockify.1 index c4b97b77..cef4d5e2 100644 --- a/docs/websockify.1 +++ b/docs/websockify.1 @@ -89,12 +89,25 @@ Here is an example of using websockify to wrap the vncserver command (which back `./websockify 5901 --wrap-mode=ignore -- vncserver -geometry 1024x768 :1` -Here is an example of wrapping telnetd (from krb5-telnetd).telnetd exits after the connection closes so the wrap mode is set to respawn the command: +Here is an example of wrapping telnetd (from krb5-telnetd). telnetd exits after the connection closes so the wrap mode is set to respawn the command: `sudo ./websockify 2023 --wrap-mode=respawn -- telnetd -debug 2023` The wstelnet.html page demonstrates a simple WebSockets based telnet client. +.SS Use client certificate verification + +The --verify-client makes the server ask the client for a SSL certificate. Presenting a valid (not expired and trusted by any supplied certificate authority) certificate is required for the client connection. With -auth-plugin=ClientCertAuth, the client certificate can be checked against a list of authorised certificate users. Non-encrypted connection attempts always fail during authentication. + +Here is an example of a vncsevrer with password-less, certificate-driven authentication: + +`./websockify 5901 --cert=fullchain.pem --key=privkey.pem --ssl-only --verify-client --cafile=ca-certificates.crt --auth-plugin=ClientCertAuth --auth-source='jane@example.com Joe User9824510' --web=noVNC/ --wrap-mode=ignore -- vncserver :1 -geometry 1024x768 -SecurityTypes=None` + +The --auth-source option takes a white-space separated list of common names. Depending on your clients certificates they can be verified email addresses, user-names or any other string used for identification. + +The --cafile option selects a file containing concatenated certificates of authorities trusted for validating clients. If this option is omitted, system default list of CAs is used. Upon connect, the client should supply the whole certificate chain. If your clients are known not to send intermediate certificates, they can be appended to the ca-file as well. + +Note: Most browsers ask the user to select a certificate only while connecting via HTTPS, not WebSockets. Connecting directly to the SSL secured WebSocket may cause the browser to abort the connection. If you want to connect via noVNC, the --web option should point to a copy of noVNC, so it is loaded from the same host. .SH AUTHOR Joel Martin (github@martintribe.org) From a426020e04618d7eb77521b155c0c02068182f29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hermann=20H=C3=B6hne?= Date: Sun, 3 Sep 2017 17:22:35 +0200 Subject: [PATCH 3/6] Added hints to which Python versions allow client certificate authentication. Renamed SSL client certificate authentication plugin to match its function (checking common names) more closely. --- docs/websockify.1 | 6 ++++-- websockify/auth_plugins.py | 2 +- websockify/websocketproxy.py | 3 ++- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/docs/websockify.1 b/docs/websockify.1 index cef4d5e2..3aaea6eb 100644 --- a/docs/websockify.1 +++ b/docs/websockify.1 @@ -97,11 +97,13 @@ The wstelnet.html page demonstrates a simple WebSockets based telnet client. .SS Use client certificate verification -The --verify-client makes the server ask the client for a SSL certificate. Presenting a valid (not expired and trusted by any supplied certificate authority) certificate is required for the client connection. With -auth-plugin=ClientCertAuth, the client certificate can be checked against a list of authorised certificate users. Non-encrypted connection attempts always fail during authentication. +This feature requires Python 2.7.9 or newer or Python 3.4 or newer. + +The --verify-client option makes the server ask the client for a SSL certificate. Presenting a valid (not expired and trusted by any supplied certificate authority) certificate is required for the client connection. With -auth-plugin=ClientCertCNAuth, the client certificate can be checked against a list of authorised certificate users. Non-encrypted connection attempts always fail during authentication. Here is an example of a vncsevrer with password-less, certificate-driven authentication: -`./websockify 5901 --cert=fullchain.pem --key=privkey.pem --ssl-only --verify-client --cafile=ca-certificates.crt --auth-plugin=ClientCertAuth --auth-source='jane@example.com Joe User9824510' --web=noVNC/ --wrap-mode=ignore -- vncserver :1 -geometry 1024x768 -SecurityTypes=None` +`./websockify 5901 --cert=fullchain.pem --key=privkey.pem --ssl-only --verify-client --cafile=ca-certificates.crt --auth-plugin=ClientCertCNAuth --auth-source='jane@example.com Joe User9824510' --web=noVNC/ --wrap-mode=ignore -- vncserver :1 -geometry 1024x768 -SecurityTypes=None` The --auth-source option takes a white-space separated list of common names. Depending on your clients certificates they can be verified email addresses, user-names or any other string used for identification. diff --git a/websockify/auth_plugins.py b/websockify/auth_plugins.py index dd6a9e82..8ce60f2b 100644 --- a/websockify/auth_plugins.py +++ b/websockify/auth_plugins.py @@ -82,7 +82,7 @@ def authenticate(self, headers, target_host, target_port): if origin is None or origin not in self.source: raise InvalidOriginError(expected=self.source, actual=origin) -class ClientCertAuth(object): +class ClientCertCNAuth(object): """Verifies client by SSL certificate. Specify src as whitespace separated list of common names.""" def __init__(self, src=None): diff --git a/websockify/websocketproxy.py b/websockify/websocketproxy.py index 09feee32..52da1862 100755 --- a/websockify/websocketproxy.py +++ b/websockify/websocketproxy.py @@ -407,7 +407,8 @@ def websockify_init(): parser.add_option("--ssl-target", action="store_true", help="connect to SSL target as SSL client") parser.add_option("--verify-client", action="store_true", - help="require encrypted client to present a valid certificate") + help="require encrypted client to present a valid certificate " + "(needs Python 2.7.9 or newer or Python 3.4 or newer)") parser.add_option("--cafile", metavar="FILE", help="file of concatenated certificates of authorities trusted " "for validating clients (only effective with --verify-client). " From 8cb3acd510f2ace1991785241f6e27df1d98c421 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hermann=20H=C3=B6hne?= Date: Sun, 3 Sep 2017 17:24:22 +0200 Subject: [PATCH 4/6] Kept ssl.create_default_context, but added fallback to ssl.wrap_socket. This commit now incorporates #190 without breaking compatibility towards old Python versions. Removed test that cannot not work with new ssl.create_default_context. --- tests/test_websockifyserver.py | 19 ++++-------------- websockify/websockifyserver.py | 35 ++++++++++++++++++++++++---------- 2 files changed, 29 insertions(+), 25 deletions(-) diff --git a/tests/test_websockifyserver.py b/tests/test_websockifyserver.py index 9af117d2..3f67096a 100644 --- a/tests/test_websockifyserver.py +++ b/tests/test_websockifyserver.py @@ -257,21 +257,10 @@ def fake_select(rlist, wlist, xlist, timeout=None): sock, '127.0.0.1') def test_do_handshake_ssl_error_eof_raises_close_error(self): - server = self._get_server(daemon=True, ssl_only=0, idle_timeout=1) - - sock = FakeSocket("\x16some ssl data") - - def fake_select(rlist, wlist, xlist, timeout=None): - return ([sock], [], []) - - def fake_wrap_socket(*args, **kwargs): - raise ssl.SSLError(ssl.SSL_ERROR_EOF) - - self.stubs.Set(select, 'select', fake_select) - self.stubs.Set(ssl, 'wrap_socket', fake_wrap_socket) - self.assertRaises( - websockifyserver.WebSockifyServer.EClose, server.do_handshake, - sock, '127.0.0.1') + # TODO: re-implement this test. + # Test was incompatible with new style socket wrapping offered by + # ssl.create_default_context. + pass def test_fallback_sigchld_handler(self): # TODO(directxman12): implement this diff --git a/websockify/websockifyserver.py b/websockify/websockifyserver.py index 2c8b7456..fd2f92f9 100644 --- a/websockify/websockifyserver.py +++ b/websockify/websockifyserver.py @@ -541,16 +541,31 @@ def do_handshake(self, sock, address): % self.cert) retsock = None try: - context = ssl.create_default_context(ssl.Purpose.CLIENT_AUTH) - context.load_cert_chain(certfile=self.cert, keyfile=self.key) - if self.verify_client: - context.verify_mode = ssl.CERT_REQUIRED - context.set_default_verify_paths() - if self.cafile: - context.load_verify_locations(cafile=self.cafile) - retsock = context.wrap_socket( - sock, - server_side=True) + try: + # try creating new-style SSL wrapping for extended features + context = ssl.create_default_context(ssl.Purpose.CLIENT_AUTH) + context.load_cert_chain(certfile=self.cert, keyfile=self.key) + if self.verify_client: + context.verify_mode = ssl.CERT_REQUIRED + context.set_default_verify_paths() + if self.cafile: + context.load_verify_locations(cafile=self.cafile) + retsock = context.wrap_socket( + sock, + server_side=True) + except AttributeError as ae: + if str(ae) != "'module' object has no attribute 'create_default_context'": + # this exception is not caused by create_default_context not existing in old version. re-raise exception to be handled somewhere elese. + raise + elif self.verify_client: + raise self.EClose("Client certificate verification requested, but not Python is too old.") + else: + # new-style SSL wrapping is not needed, falling back to old style + retsock = ssl.wrap_socket( + sock, + server_side=True, + certfile=self.cert, + keyfile=self.key) except ssl.SSLError: _, x, _ = sys.exc_info() if x.args[0] == ssl.SSL_ERROR_EOF: From dfd50db550afe169da31bac009d6fb4225318238 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hermann=20H=C3=B6hne?= Date: Sun, 3 Sep 2017 19:54:09 +0200 Subject: [PATCH 5/6] Improved `test_do_handshake_ssl_error_eof_raises_close_error` so it works with recent versions of python. --- tests/test_websockifyserver.py | 34 ++++++++++++++++++++++++++++++---- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/tests/test_websockifyserver.py b/tests/test_websockifyserver.py index 3f67096a..e27f2f71 100644 --- a/tests/test_websockifyserver.py +++ b/tests/test_websockifyserver.py @@ -257,10 +257,36 @@ def fake_select(rlist, wlist, xlist, timeout=None): sock, '127.0.0.1') def test_do_handshake_ssl_error_eof_raises_close_error(self): - # TODO: re-implement this test. - # Test was incompatible with new style socket wrapping offered by - # ssl.create_default_context. - pass + server = self._get_server(daemon=True, ssl_only=0, idle_timeout=1) + + sock = FakeSocket("\x16some ssl data") + + def fake_select(rlist, wlist, xlist, timeout=None): + return ([sock], [], []) + + def fake_wrap_socket(*args, **kwargs): + raise ssl.SSLError(ssl.SSL_ERROR_EOF) + + class fake_create_default_context(): + def __init__(self, purpose): + self.verify_mode = None + def load_cert_chain(self, certfile, keyfile): + pass + def set_default_verify_paths(self): + pass + def load_verify_locations(self, cafile): + pass + def wrap_socket(self, *args, **kwargs): + raise ssl.SSLError(ssl.SSL_ERROR_EOF) + + self.stubs.Set(select, 'select', fake_select) + # for recent versions of python + self.stubs.Set(ssl, 'create_default_context', fake_create_default_context) + # for fallback for old versions of python + self.stubs.Set(ssl, 'wrap_socket', fake_wrap_socket) + self.assertRaises( + websockifyserver.WebSockifyServer.EClose, server.do_handshake, + sock, '127.0.0.1') def test_fallback_sigchld_handler(self): # TODO(directxman12): implement this From 8bad0cab7b0d2cfa6a206a40c31df9119ffe47c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hermann=20H=C3=B6hne?= Date: Sun, 3 Sep 2017 19:58:47 +0200 Subject: [PATCH 6/6] Improve test so it does not test methods that do not exist in old Pythons. --- tests/test_websockifyserver.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/test_websockifyserver.py b/tests/test_websockifyserver.py index e27f2f71..63c9449c 100644 --- a/tests/test_websockifyserver.py +++ b/tests/test_websockifyserver.py @@ -280,10 +280,12 @@ def wrap_socket(self, *args, **kwargs): raise ssl.SSLError(ssl.SSL_ERROR_EOF) self.stubs.Set(select, 'select', fake_select) - # for recent versions of python - self.stubs.Set(ssl, 'create_default_context', fake_create_default_context) - # for fallback for old versions of python - self.stubs.Set(ssl, 'wrap_socket', fake_wrap_socket) + if (hasattr(ssl, 'create_default_context')): + # for recent versions of python + self.stubs.Set(ssl, 'create_default_context', fake_create_default_context) + else: + # for fallback for old versions of python + self.stubs.Set(ssl, 'wrap_socket', fake_wrap_socket) self.assertRaises( websockifyserver.WebSockifyServer.EClose, server.do_handshake, sock, '127.0.0.1')