Skip to content
This repository has been archived by the owner on Dec 10, 2018. It is now read-only.

TSSLSocket does not support connection without client key #186

Closed
bquinart opened this issue Mar 13, 2016 · 1 comment
Closed

TSSLSocket does not support connection without client key #186

bquinart opened this issue Mar 13, 2016 · 1 comment

Comments

@bquinart
Copy link

Looks like the support for SSL added in #136 does not cover some cases.
As soon as you do not specify the optional client key (keyfile and certfile), the initialisation of the socket fails.

TSSLSocket("localhost", "21050", validate=False)**
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/.../lib/python3.5/site-packages/thriftpy/transport/sslsocket.py", line 50, in __init__
    if not os.access(c_file, os.R_OK):
TypeError: access: can't specify None for path argument
TSSLSocket("localhost", "21050", validate=True, cafile='/.../CA.pem')**
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/.../lib/python3.5/site-packages/thriftpy/transport/sslsocket.py", line 50, in __init__
    if not os.access(c_file, os.R_OK):
TypeError: access: can't specify None for path argument

I understand thriftpy wants to be compatible with thrift. However I have no idea if you want to follow as much as possible the thrift api's. I notice the api for TSSLSocket that was implemented was very similar except for the argument name for the Certificate Authority pem file: cafile vs ca_certs. Is there a particular reason?
Finally note that thrift has changed its api for TSSLSocket in december. Would it make sense to align it?

Thanks for your help.
Bruno

@lxyu lxyu added the Scheduled label Mar 24, 2016
@lxyu
Copy link
Contributor

lxyu commented Mar 24, 2016

The optional arguments should be fixed in da2e933.

cafile, capath is the default name for SSLContext used in python std lib. The SSL implementation used by thriftpy is all about SSLContext cause it have a much better defaults. You may customized it and send it to constructor or thriftpy will try build one internally, so the API was adapted around the SSLContext.

I just checked and find apache thrift now accept ssl context as optional argument too, but if it's not provided, it will not build one internally, and use the legacy way to verify ssl. I think that's the major difference.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants