add SSL support

[oranagra]

Adding SSL support.
Tested against stunnel on the server side.

[dbrgn]

It would be great if this pull request would get merged soon, as it will make it possible for all RedisLabs users to add SSL support to their connections: http://redislabs.com/blog/secure-redis-ssl-added-to-redsmin-and-clients

[andymccurdy]

I've merged this with a few caveats:

• I created a separate SSLConnection subclass that holds all the SSL logic.
• I removed the ability to construct an SSL connection directly from the client class. Instead, users can create a connection pool specifying connection_class=SSLConnection along with the SSL specific parameters, then pass that connection pool to the client instance. I'm not keen on adding 5 new arguments on StrictRedis.__init__ that most people won't ever use.
• I removed the ability to construct an SSL connection from an URL. It seems like you need extra information, such as keyfile, ca_cert, etc., but those options aren't present in the URL. I'm fine adding this back if these arguments are optional.

Part of the reason this took so long and part of the reason I removed some of the proposed functionality is that I don't have a setup to test any of this against. It'd be great to figure something out to fix that.

[oranagra]

Hi,
Thanks for taking the time to merge my changes.

I want to argue, that i think you should reinstate the simpler API that allow using the client class directly.
I believe that the SSL use case should be as common as unix-domain socket use case, and if the simple API supports one, it should support the other.

I think users should be able to change their simple code into using SSL by adding just one boolean parameter (use_ssl=true).
All the other SSL parameters are optional. Just like connecting to a website using https://, all you need to do is mention you want to use SSL, and in most cases that should be enough.

There are some servers or users that will need / want to pass the other optional parameters (like special verification demands and specific certificates), but since these are named optional parameters there are no down sides in exposing them from the client class too (i think it will not hurt the user in any way, and the disruption to the library code is not that bad IMO).

So, to sum up, please reconsider allowing both the URL scheme and the client class parameters.

regarding testing, i'll post a quick step by step instructions (in a separate post) of how to set up stunnel's server side on localhost, maybe it will help you or anyone else to testing it.

[itamarhaber]

@andymccurdy if you need an SSL testing environment, we'd be happy to provision it to you (for free of course).

[oranagra]

In order to test the SSL capabilities on a localhost redis-server, please try the following setup:

sudo apt-get install stunnel
sudo vi /etc/stunnel/stunnel.conf
modify:

cert = /etc/stunnel/redis.pem

comment out pop3s and other protocols and add:

[redis]
accept = 6378
connect = 6379

sudo vi /etc/default/stunnel4

ENABLED=1

generate a self-signed certificate, see https://help.ubuntu.com/12.04/serverguide/certificates-and-security.html

openssl genrsa -des3 -out server.key 2048
openssl rsa -in server.key -out server.key.insecure
mv server.key server.key.secure
mv server.key.insecure server.key
openssl req -new -key server.key -out server.csr
openssl x509 -req -days 365 -in server.csr -signkey server.key -out server.crt

sudo cp server.crt /etc/stunnel/redis.pem
sudo /etc/init.d/stunnel4 start

copy server.crt to your source code folder, and test with:

# original pull request client class parameters:
r = redis.Redis(host='localhost', port=6378, use_ssl=True)
r.set('foo','bar')

# new / merged connection pool API.
pool = redis.ConnectionPool(host='localhost', port=6378, db=0,
                            connection_class=redis.SSLConnection,
                            ca_certs='server.crt', cert_reqs=ssl.CERT_REQUIRED)
r = redis.Redis(connection_pool=pool)
r.set('foo', 'bar')

[dbrgn]

I also think that enabling SSL on a connection should be as easy as possible, on order to encourage people to use it.

[itamarhaber]

Also, you can refer to http://redislabs.com/blog/using-stunnel-to-secure-redis for a slightly longer and more detailed howto stunnel

[andymccurdy]

Alright, I went ahead and added this stuff back. I prefixed all arguments on the client class with ssl_ to make it clear to users.

The URL stuff works too, but there's a slight weirdness with passing cert_reqs as a querystring argument because you obviously don't have access to the constants defined in the ssl module (CERT_NONE, etc.) in the URL string. Users could pass the integer values, but I'm unfamiliar with how portable that is. I could also look for well-named values in the url parsing code and convert them to their respective Python constant, e.g: rediss://localhost?cert_reqs=none would get converted to ssl.CERT_NONE.

Thoughts?

[andymccurdy]

I added the ability to pass strings to the cert_reqs argument here: 6103020

It can be configured from URLs now.

[oranagra]

very nice work on the URL...
one thing that is a bit odd:
in the normal client calls you you prefixed the ssl arguments with ssl_, but in the connection pool class you didn't.
maybe you should consider prefixing them there too.

e.g:
r = redis.Redis(host='localhost', port=6378, ssl=True,
ssl_ca_certs='server.crt',
ssl_cert_reqs=ssl.CERT_REQUIRED)

pool = redis.ConnectionPool(connection_class=redis.SSLConnection, host='localhost', port=6378
                            ca_certs='server.crt',
                            cert_reqs=ssl.CERT_REQUIRED)
r = redis.Redis(connection_pool=pool)

thanks for everything.
Oran.

[andymccurdy]

@oranagra agreed, I initially didn't add the prefix because it's already in the SSLConnection class. But making it so that the same argument names can be used on the client class, the connection pool and the URL seems right.

Thanks again.

[dbrgn]

Great!

Any plans regarding the next PyPI release?

[andymccurdy]

Very soon. I have 2 more issues I want to get resolved before 2.10 gets rolled out. I'm hoping Friday or over the weekend.