Celery: Unable to create SSL broker/backend connection to redis

Created on 6 Mar 2019  Â·  26Comments  Â·  Source: celery/celery

I'm having some issues making an SSL/TLS connection to redis for the Celery broker and backend.

I'm using:
Celery: 4.3.0rc2 (master)
Python: 3.6
OS: Mac OS X.
Related dependency versions: kombu (4.4.0), redis-py (3.2.0)

Celery works fine using a non-SSL connection to redis. My redis/SSL setup uses redis behind stunnel and I can successfully connect to and use this deployment via py-redis directly and via other libraries.

It looks like this is related to #3830, however that issue seemed to have been resolved by adding the broker_use_ssl option and adding this option doesn't appear to be having any effect here.

To setup SSL, I'm adding the broker_use_ssl attribute to configuration parameters as described in the docs. I'm also adding the same set of parameters for the redis_backend_use_ssl option. I notice there is some information here about adding the parameters in the query string but I've opted to stick with providing the dict of parameters.

broker_url = 'rediss://localhost:6380'
key_file = '/path/to/client.key' 
cert_file = '/path/to/client.crt' 
ca_file = '/path/to/CAcert.pem'

app = Celery('app', broker=broker_url, backend=broker_url,
             broker_use_ssl = {
                 'keyfile': key_file, 'certfile': cert_file, 
                 'ca_certs': ca_file,  
                 'cert_reqs': ssl.CERT_REQUIRED 
            }, 
             redis_backend_use_ssl = { 
                 'keyfile': key_file, 'certfile': cert_file,
                 'ca_certs': ca_file,  
                 'cert_reqs': ssl.CERT_REQUIRED
            })
app.connection().connect()
app.send_task('a_task')

When I attempt to make and use the connection as shown above, I get an error:

ValueError: A rediss:// URL must have parameter ssl_cert_reqs be CERT_REQUIRED, CERT_OPTIONAL, or CERT_NONE

It looks like the rediss URL scheme is being picked up but the other parameters are not. Apologies if I'm missing something in the documentation or elsewhere.

Some investigation suggests that the broker_use_ssl and redis_backend_use_ssl options are not being carried through to the connection settings and, indeed, looking at the __init__ function for the Celery object, I can't see that these are being used anywhere. Stepping through the code, they don't seem to be present by the time the connection function is reached and ssl in the input parameters to the connection function is set to None.

As a test, I've tried making a couple of additions to the Celery __init__ function to add the broker_use_ssl and redis_backed_use_ssl to the configuration:

        self.__autoset('broker_use_ssl', 
                       (kwargs['broker_use_ssl'] if 'broker_use_ssl' 
                        in kwargs else None))
        self.__autoset('redis_backend_use_ssl', 
                       (kwargs['redis_backend_use_ssl'] if  
                        'redis_backend_use_ssl' in kwargs else None))

This adds the provided options to the Celery conf. It flags up another issue which is that the SSL parameters used for the backend connection use different names to those used for the broker connection - the documentation says the values are the same as broker_use_ssl (although the correct values are shown in the example of using query string parameters). I assume this is one of the things being handled under issue #4812.

A further point that I'm unclear about is that in the _connection function, there is a call to get the broker_use_ssl parameters ssl=self.either('broker_use_ssl', ssl) and this returns None. However, if I manually create an app object and then call either, it returns the broker_use_ssl parameters correctly (parameters as per example above):

app = Celery('app', broker=broker_url, backend=broker_url,
             broker_use_ssl = {
                 'keyfile': key_file, 'certfile': cert_file, 
                 'ca_certs': ca_file,  
                 'cert_reqs': ssl.CERT_REQUIRED 
            }, 
             redis_backend_use_ssl = { 
                 'keyfile': key_file, 'certfile': cert_file,
                 'ca_certs': ca_file,  
                 'cert_reqs': ssl.CERT_REQUIRED
            })

app.either('broker_use_ssl', None)  

# Returns:
# {'keyfile': '/path/to/client.key',
#  'certfile': '/path/to/client.crt',
#  'ca_certs': '/path/to/CAcert.pem',
#  'cert_reqs': <VerifyMode.CERT_REQUIRED: 2>}

So, I'm unclear if I'm missing something and taking completely the wrong approach here or whether there are some issues with the SSL implementation in 4.3.0rc2.

Thanks.

Redis Broker Redis Results Backend Critical Major Confirmed ✔ Has Testcase ✔

Most helpful comment

@xirdneh @jcohen02 the following patch seems to fix the issue:

diff --git a/celery/backends/redis.py b/celery/backends/redis.py
index 9954498..fe55bae 100644
--- a/celery/backends/redis.py
+++ b/celery/backends/redis.py
@@ -234,11 +234,13 @@ class RedisBackend(BaseKeyValueStoreBackend, AsyncBackendMixin):
             connparams['connection_class'] = redis.SSLConnection
             # The following parameters, if present in the URL, are encoded. We
             # must add the decoded values to connparams.
-            for ssl_setting in ['ssl_ca_certs', 'ssl_certfile', 'ssl_keyfile']:
+            for ssl_setting in ['ssl_ca_certs', 'ssl_certfile',
+                                'ssl_keyfile', 'ssl_cert_reqs']:
                 ssl_val = query.pop(ssl_setting, None)
                 if ssl_val:
                     connparams[ssl_setting] = unquote(ssl_val)
-            ssl_cert_reqs = query.pop('ssl_cert_reqs', 'MISSING')
+
+            ssl_cert_reqs = connparams.pop('ssl_cert_reqs', 'MISSING')
             if ssl_cert_reqs == 'CERT_REQUIRED':
                 connparams['ssl_cert_reqs'] = CERT_REQUIRED
             elif ssl_cert_reqs == 'CERT_OPTIONAL':

The problem is that we relied on the connection URL query parameters, instead of the more broad dictionary connparams, which contains the configuration settings for the backend as well.

Additionally, configuration must also be passed through app.conf in order to be resolved properly:

app.conf.redis_backend_use_ssl = {
                 'ssl_keyfile': key_file, 'ssl_certfile': cert_file,
                 'ssl_ca_certs': ca_file,
                 'ssl_cert_reqs': 'CERT_REQUIRED'
            }

All 26 comments

Shouldn't it be ssl_cert_reqs instead of cert_reqs? The other options have the same ssl_ prefix for redis.

The linked documentation ~might be wrong/outdated.~ mentions cert_reqs for pyamqp, but ssl_cert_reqs for redis. They are not equally highlighted, which makes it hard to get it right:

screenshot_2019-03-06 configuration and defaults celery 4 3 0rc2 documentation

@michael-k thanks for pointing out that the dictionary parameter names for broker_use_ssl (and redis_backend_use_ssl) should be prefixed with ssl_. I hadn't read this correctly initially. I did eventually get to naming the parameters correctly but this doesn't resolve the issue.

If I take a clean install of celery 4.3.0rc2 and then create a Celery object, (with the revised parameter names):

app = Celery('app', broker=broker_url, backend=broker_url,
             broker_use_ssl = {
                 'ssl_keyfile': key_file, 'ssl_certfile': cert_file, 
                 'ssl_ca_certs': ca_file,  
                 'ssl_cert_reqs': ssl.CERT_REQUIRED 
            }, 
             redis_backend_use_ssl = { 
                 'ssl_keyfile': key_file, 'ssl_certfile': cert_file,
                 'ssl_ca_certs': ca_file,  
                 'ssl_cert_reqs': ssl.CERT_REQUIRED
            })

and then try and initiate the connection to the redis server, I still get the error:

ValueError: A rediss:// URL must have parameter ssl_cert_reqs be CERT_REQUIRED, CERT_OPTIONAL, or CERT_NONE

There's also an error from kombu (kombu/utils/objects.py:42) that appears prior to the other error:

KeyError: 'backend'

but I'm assuming this is probably because the backend object hasn't been instantiated due to the certificate parameter error.

I'm investigating and will aim to provide some further detail on this.

I realised that if I switch to using purely URL based configuration for the SSL options (as shown in the box just above the redis_backend_use_ssl details in the docs), then the connection works.

It looks as though the broker_use_ssl and redis_backend_use_ssl options, when provided using the dictionary format as documented here, are not added into the configuration parameters in the Celery __init__ function.

I've added a suggested modification to celery/app/base.py to address this.

In the redis backend code, where the rediss:// URL scheme is handled, it's only looking in the query string for the ssl_cert_reqs parameter so I've added an update to celery/backends/redis.py to also look in the connection parameters if this parameter is not found in the query string. This update is in the same commit mentioned above.

I guess an alternative option would be to remove support for the broker_use_ssl and redis_backend_use_ssl configuration options, remove them from the docs and only support query string-based SSL configuration but I can raise a PR with my changes if you'd like to retain support for the config options and my changes look reasonable.

@jcohen02 at least for the Redis backend, you should be passing a string, instead of the ssl constants:

Would you mind trying this:

app = Celery('app', broker=broker_url, backend=broker_url,
             broker_use_ssl = {
                 'ssl_keyfile': key_file, 'ssl_certfile': cert_file, 
                 'ssl_ca_certs': ca_file,  
                 'ssl_cert_reqs': 'CERT_REQUIRED'
            }, 
             redis_backend_use_ssl = { 
                 'ssl_keyfile': key_file, 'ssl_certfile': cert_file,
                 'ssl_ca_certs': ca_file,  
                 'ssl_cert_reqs': 'CERT_REQUIRED'
            })

Let us know if this works!

@georgepsarakis, thanks for the suggestion. Testing this against master results in exactly the same behaviour as I was seeing originally. I get the error:

ValueError: A rediss:// URL must have parameter ssl_cert_reqs be CERT_REQUIRED, CERT_OPTIONAL, or CERT_NONE

The only thing that works for me against master is to put all the parameters into the query string, so this works ok:

query_string_params = {'ssl_cert_reqs':'CERT_REQUIRED', 'ssl_keyfile':key_file, 
                       'ssl_certfile':cert_file, 'ssl_ca_certs':ca_file }
broker_url = ('rediss://localhost:6380/0?%s' % 
                  urllib.parse.urlencode(query_string_params))
app = Celery('app', broker=broker_url, backend=broker_url)

In reference to passing a string instead of the ssl constants, I addressed this here, hence my modified version of the code working with either ssl.CERT_REQUIRED or 'CERT_REQUIRED'.

If you have access to an SSL-enabled Redis deployment to test on, the issue I'm seeing should be reproducible with the following code:

from celery import Celery

key_file = '/tmp/keyfile.key'
cert_file = '/tmp/certfile.crt'
ca_file = '/tmp/CAtmp.pem'

broker_url = 'rediss://localhost:6380'

app = Celery('app', broker=broker_url, backend=broker_url, 
               broker_use_ssl = { 
                   'ssl_keyfile': key_file, 'ssl_certfile': cert_file,  
                   'ssl_ca_certs': ca_file,   
                   'ssl_cert_reqs': 'CERT_REQUIRED'  
              },  
               redis_backend_use_ssl = {  
                   'ssl_keyfile': key_file, 'ssl_certfile': cert_file, 
                   'ssl_ca_certs': ca_file,   
                   'ssl_cert_reqs': 'CERT_REQUIRED' 
              })

app.connection().connect() 
app.send_task('test')

Hmm, this might sound weird, but try redis:// instead of rediss://. This hunch is purely based on this unit test:

https://github.com/celery/celery/blob/eef03a3d299a2e4c1a69926be56ca521f0d16779/t/unit/backends/test_redis.py#L273-L299

Thanks @lithammer, I've given this a go. With the current master, switching rediss:// to redis:// results in this error:

ConnectionError: Error while reading from socket: (54, 'Connection reset by peer')

I guess this suggests it's trying to make a standard connection to an SSL endpoint and the host is disconnecting?

However, with my modified code in jcohen02/celery@45cf3e6d4523e407581106360b7091dc1652cb4b, it doesn't seem to make a difference whether the URL scheme is specified as redis:// or rediss://, if the SSL options are provided then an SSL connection is initiated. It looks like this might be a result of how the SSL connection is configured in kombu where the connection_class is set to redis.SSLConnection if the ssl option contains a dict of parameters, as a result, I'm assuming the URL scheme is not significant in this case.

I'm still of the impression that the broker_use_ssl and redis_backend_use_ssl are not being included in the application configuration object although I would agree that this seems strange. However, the test_backend_ssl test highlighted in the previous message explicitly adds the broker_use_ssl dictionary to app.conf which would explain why this test runs successfully.

Thanks for the help with this and hope this info is useful.

@thedrow Testing this locally to see if @jcohen02's suggestion is correct.

Wonderful. This needs to be resolved before 4.3 GA.

I think that what is happening is that we only support SSL configuration from the URI and broker_use_ssl.

I haven't checked though.

@xirdneh @jcohen02 the following patch seems to fix the issue:

diff --git a/celery/backends/redis.py b/celery/backends/redis.py
index 9954498..fe55bae 100644
--- a/celery/backends/redis.py
+++ b/celery/backends/redis.py
@@ -234,11 +234,13 @@ class RedisBackend(BaseKeyValueStoreBackend, AsyncBackendMixin):
             connparams['connection_class'] = redis.SSLConnection
             # The following parameters, if present in the URL, are encoded. We
             # must add the decoded values to connparams.
-            for ssl_setting in ['ssl_ca_certs', 'ssl_certfile', 'ssl_keyfile']:
+            for ssl_setting in ['ssl_ca_certs', 'ssl_certfile',
+                                'ssl_keyfile', 'ssl_cert_reqs']:
                 ssl_val = query.pop(ssl_setting, None)
                 if ssl_val:
                     connparams[ssl_setting] = unquote(ssl_val)
-            ssl_cert_reqs = query.pop('ssl_cert_reqs', 'MISSING')
+
+            ssl_cert_reqs = connparams.pop('ssl_cert_reqs', 'MISSING')
             if ssl_cert_reqs == 'CERT_REQUIRED':
                 connparams['ssl_cert_reqs'] = CERT_REQUIRED
             elif ssl_cert_reqs == 'CERT_OPTIONAL':

The problem is that we relied on the connection URL query parameters, instead of the more broad dictionary connparams, which contains the configuration settings for the backend as well.

Additionally, configuration must also be passed through app.conf in order to be resolved properly:

app.conf.redis_backend_use_ssl = {
                 'ssl_keyfile': key_file, 'ssl_certfile': cert_file,
                 'ssl_ca_certs': ca_file,
                 'ssl_cert_reqs': 'CERT_REQUIRED'
            }

Thanks @georgepsarakis. I included support in my modification so that either a string or the ssl module constant can be used for ssl_cert_reqs since the docs use the ssl.CERT_REQUIRED constant:

https://github.com/jcohen02/celery/blob/45cf3e6d4523e407581106360b7091dc1652cb4b/celery/backends/redis.py#L241-L251

I suppose this adds complexity and maybe the approach of sticking with a string value for ssl_cert_reqs as you've shown above is preferable, however, just highlighting that the documentation will need updating if this is the case.

@jcohen02 although having a unified type between URL query parameters and dictionary configuration could be more clear (and less complicated for some users), I think your modification is quite valid.

Can anyone issue a PR and extend the tests to cover these cases?

@thedrow I'm happy to do that.

@jcohen02 Let me know when the PR is ready.

@thedrow PR created - #5395

Must I explicitly provide the cert and key files if I am using rediss (redis + ssl)?

I have run a test and was able to set and get keys without any extra configuration using PyPi's redis redis.Redis.set and redis.Redist.get

[2019-09-04 22:04:42,950: WARNING/MainProcess] :
[2019-09-04 22:04:42,950: WARNING/MainProcess] A rediss:// URL must have parameter ssl_cert_reqs and this must be set to CERT_REQUIRED, CERT_OPTIONAL, or CERT_NONE

Still getting this issue though

Thanks in advance @georgepsarakis @jcohen02

@BeOleg yes these options are mandatory. Please check the configuration documentation.

Hello,
I'm trying to get this working with Heroku, Django, Redis, and Celery and am getting this error.
A rediss:// URL must have parameter ssl_cert_reqs and this must be set to CERT_REQUIRED, CERT_OPTIONAL, or CERT_NONE.

Has anyone done this? I need some guidance on how to setup and reference the key, cert, and ca_file.

key_file = '/tmp/keyfile.key'
cert_file = '/tmp/certfile.crt'
ca_file = '/tmp/CAtmp.pem'

Hi @jgarzautexas, how are you creating your connection to redis? Are you using redis for both the broker and the backend? I have to say that I've not worked with this for a while but as a test, if I simply create a connection to an SSL redis endpoint without specifying any SSL parameters, I can reproduce the error you're seeing, e.g.

app = Celery('tasks', backend='rediss://localhost:<redis SSL port>', broker='rediss://localhost:<redis SSL port>')

However, if I add the various SSL configuration options, as follows, I get a successful SSL connection to redis:

app = Celery('tasks', broker='rediss://localhost:<redis SSL port>',
                      broker_use_ssl={'ssl_cert_reqs': ssl.CERT_REQUIRED,
                                      'ssl_ca_certs': '/tmp/CAtmp.pem',
                                      'ssl_certfile': '/tmp/certfile.crt',
                                      'ssl_keyfile': '/tmp/keyfile.key'},
                      backend='rediss://localhost:<redis SSL port>',
                      redis_backend_use_ssl={'ssl_cert_reqs': ssl.CERT_REQUIRED,
                                             'ssl_ca_certs': '/tmp/CAtmp.pem',
                                             'ssl_certfile': '/tmp/certfile.crt',
                                             'ssl_keyfile': '/tmp/keyfile.key'})

Hope this helps. It looks like you may just be missing the 'ssl_cert_reqs': ssl.CERT_REQUIRED option.

Thanks @jcohen02. For my staging app my REDIS_URL is redis://so it doesn't require SSL, however on production Heroku it is configured with rediss://.

If I configure with ssl_cert_reqs': ssl.CERT_NONE it works, but I would rather use SSL if possible.
I think my confusion is If I add 'ssl_cert_reqs': ssl.CERT_REQUIRED, and on Heroku, how do I add the .pem, .key, and .crt files? I'm not sure how to do this part on Heroku.

Appreciate your help.

No problem @jgarzautexas, unfortunately I'm not familiar with deployment on Heroku so I'm really not sure how you would handle the keys - I think I see the issue but not sure how you would handle this.

Please move this discussion elsewhere. The mailing list is a good place to have these discussions.
All their maintainers get pinged every time someone comments on a closed issue.

Thanks @jcohen02. For my staging app my REDIS_URL is redis://so it doesn't require SSL, however on production Heroku it is configured with rediss://.

If I configure with ssl_cert_reqs': ssl.CERT_NONE it works, but I would rather use SSL if possible.
I think my confusion is If I add 'ssl_cert_reqs': ssl.CERT_REQUIRED, and on Heroku, how do I add the .pem, .key, and .crt files? I'm not sure how to do this part on Heroku.

Appreciate your help.

Did you ever figure this out? I'm running into the same issues on heroku.

@AustinStehling, I haven't found a solution to this myself. I've moved this discussion to the mailing list, if you'd like to follow-up there with some further information on the issue, hopefully someone on the list can provide assistance.

Was this page helpful?
0 / 5 - 0 ratings