SSL client validation (certificate-based authentication) #295
Conversation
|
The python requirement comment doesn't mean it's impossible. Just make sure that it doesn't break on systems with old python. You can add an 'if' that checks the version and just return if its too old. |
…ication. Renamed SSL client certificate authentication plugin to match its function (checking common names) more closely.
…t. This commit now incorporates #190 without breaking compatibility towards old Python versions. Removed test that cannot not work with new ssl.create_default_context.
|
Fallback is added in 8cb3acd. Please note that I cheated by removing |
…orks with recent versions of python.
websockify/websocketproxy.py
Outdated
| 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.") |
There was a problem hiding this comment.
This isn't completely true, is it? The code suggests that system CAs will always be used.
There was a problem hiding this comment.
Indeed. I should move the call to context.set_default_verify_paths() into the else alternative.
websockify/auth_plugins.py
Outdated
| @@ -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): | |||
There was a problem hiding this comment.
Please merge this commit with the earlier ones.
| sock, | ||
| server_side=True) | ||
| except AttributeError as ae: | ||
| if str(ae) != "'module' object has no attribute 'create_default_context'": |
There was a problem hiding this comment.
This can be done a lot cleaner. E.g.
if hasattr(ssl, 'create_default_context'):| sock, | ||
| server_side=True) | ||
| except AttributeError as ae: | ||
| if str(ae) != "'module' object has no attribute 'create_default_context'": |
There was a problem hiding this comment.
When fixed this should also be merged in to the original commit.
tests/test_websockifyserver.py
Outdated
| self.stubs.Set(ssl, 'wrap_socket', fake_wrap_socket) | ||
| self.assertRaises( | ||
| websockifyserver.WebSockifyServer.EClose, server.do_handshake, | ||
| sock, '127.0.0.1') |
There was a problem hiding this comment.
Merge this in to the first commit as well.
|
Requested changes made in new pull request #308. |
I don't like passwords. But I love public key authentication. That is why I added SSL client validation to websockify (specifically for use with noVNC). I know this won't make it into upstream due to compatibility issues as discussed in #190, but other people might want to know this feature now exists in at least one fork of Python websockify. For maximum ease of use, I recommend certificates by letsencrypt as mentioned in #207.
The test
test_do_handshake_ssl_error_eof_raises_close_errorfails since the ssl module does not actually accept the fake socket of the FakeSocket type (there is an explicit type check).