channels_redis icon indicating copy to clipboard operation
channels_redis copied to clipboard

Feature request: configure SSL settings in CHANNEL_LAYERS

Open jray-afk opened this issue 4 years ago • 12 comments

Hi, I posted about my adventure trying to configure SSL for Redis in the suggested Google Groups page which has all the requested information prepopulated in the body of this issue form, haven't gotten a response yet: https://groups.google.com/g/django-users/c/S094vik_gW0

After digging into the channels_redis code in this repo I realized that I probably need to submit a new feature request here in order to configure SSL settings in CHANNEL_LAYERS. (Unless, do you know of another way to configure SSL using channels_redis?)

Thank you!

jray-afk avatar Jan 20 '21 22:01 jray-afk

So... we'd need to by able to provide an SSLContext when creating connections.

See aioredis.create_connection().

Happy to accept input here.

carltongibson avatar Jan 24 '21 19:01 carltongibson

@carltongibson I appreciate the response. Yes that makes sense.

For now I will set up symmetric_encryption_keys to add some level of protection, but ultimately I would prefer setting up SSL if possible.

jray-afk avatar Jan 24 '21 20:01 jray-afk

Super! If you get started and need some input, let me know. Thanks! 👍

carltongibson avatar Jan 25 '21 06:01 carltongibson

I was just running in to this issue myself today, so happy to help test!

skadz avatar Feb 05 '21 22:02 skadz

@skadz glad to hear I'm not the only one! Haven't had a chance to fork off of this project and try to implement the changes, but I may have to go rogue and go off of the version I built my project on, 2.3.2

jray-afk avatar Feb 06 '21 03:02 jray-afk

@skadz glad to hear I'm not the only one! Haven't had a chance to fork off of this project and try to implement the changes, but I may have to go rogue and go off of the version I built my project on, 2.3.2

@jray-afk Does this mean you actually have made the changes and got it working and just need to do the fork/submit PR or that you hacked some sort of other "fix" in? I'd like to get this implemented ASAP and if you have something that is working, I'd love to help you get it in to the main code, but also can help test anything you've done so far (or would like to know about how you hacked it in ;)

skadz avatar Feb 11 '21 17:02 skadz

@jray-afk @skadz any updates here? I'm facing the very same issue and help would be much appreciated!!

nmacianx avatar Mar 09 '21 06:03 nmacianx

Just awaiting someone to pick it up and submit a pull request I think

carltongibson avatar Mar 09 '21 06:03 carltongibson

When configuring the hosts in the Django settings you can use a dictionary and pass the SSL context option amongst other parameters :

# Mute the Heroku Redis error:
# "[SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: self signed certificate in certificate chain"
# by disabling hostname check.

ssl_context = ssl.SSLContext()
ssl_context.check_hostname = False

heroku_redis_ssl_host = {
    'address': 'rediss://:[email protected]:6379/0'  # The 'rediss' schema denotes a SSL connection.
    'ssl': ssl_context
}

CHANNEL_LAYERS = {
    'default': {
        'BACKEND': 'channels_redis.core.RedisChannelLayer',
        'CONFIG': {
            'hosts': (heroku_redis_ssl_host,)
        }
    },
}

dablak avatar Mar 10 '21 14:03 dablak

@dablak Thanks for the comment — should get people going — but if we can ssl_context.check_hostname = False, surely we can configure the SSLContext correctly too? 🤔

Is this just documentation?

carltongibson avatar Mar 10 '21 16:03 carltongibson

I think that the SSLContext can be configured correctly in this way. If you check channels_redis.core.RedisChannelLayer.decode_hosts() you can see that if the hosts in the configuration are a dict then they are used as-is.

for entry in hosts:
    if isinstance(entry, dict):
        result.append(entry)
    else:
        result.append({"address": entry})

The dict items are then used as the parameters passed to aioredis.create_redis() in channels_redis.core.ConnectionPool.pop().

conn = await aioredis.create_redis(**self.host)

This is not in the documentation for channels_redis, but it should be.

dablak avatar Mar 10 '21 17:03 dablak

In case it helps others;

  • I have Redis setup with SSL certs (rediss://...).
  • channels==2.1.2
  • channels-redis==2.4.2

Here is a snippet from my Django config

redis_backend_url = "rediss://%s:%s/5" % (
    os.environ.get("REDIS_HOST", "redis_int"),
    os.environ.get("REDIS_PORT", 6379)
)
ssl_context = ssl.SSLContext()
ssl_context.load_cert_chain(
    certfile="/opt/redis/certs/client.crt",
    keyfile="/opt/redis/certs/client.key",
)
CHANNEL_LAYERS = {
    "default": {
        "BACKEND": "channels_redis.core.RedisChannelLayer",
        "CONFIG": {
            "hosts": ({
                'address': redis_backend_url,
                'ssl': ssl_context
            },)
        },
    }
}

ezeakeal avatar Nov 05 '21 11:11 ezeakeal

Hello,

unfortunately for me it isn't working. I have a frontend on netlify which build a websocket connection to a django channels backend. Everything works on localserver as expected. If i try to connect in production to the channels backend it doesn't work.

in production i'll get the error which was mentioned here. [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: self-signed certificate in certificate chain (_ssl.c:997)

Anyway how i try to set the context, channels doesn't accept the connection on heroku.

ssl_context = ssl.SSLContext()
ssl_context.check_hostname = False

    CHANNEL_LAYERS = {
        'default': {
            'BACKEND': 'channels_redis.core.RedisChannelLayer',
            'CONFIG': {
                'hosts': ({
                    'address': env('REDIS_TLS_URL'),
                    'ssl': ssl_context
                },)
            }
        }
    }

Please help

Amplitude88 avatar Oct 27 '22 13:10 Amplitude88

@Amplitude88 the above solution stopped working for me as well were you able to solve this issue?

geekynasir avatar Oct 31 '22 07:10 geekynasir

@Amplitude88 the above solution stopped working for me as well were you able to solve this issue?

Hey mate, yes i was in able to solve it. I've downgraded to 3.4.1 In Version 4 the problem is not solvable with the provided solution above.

Best regards, Amp

Amplitude88 avatar Oct 31 '22 12:10 Amplitude88

@Amplitude88 Yeah it also stopped working for me after I upgraded to channels v4 but it was working for a few days I think or i might have not noticed it since I don't have unit testing for my project

geekynasir avatar Oct 31 '22 13:10 geekynasir

@carltongibson @ezeakeal @andrewgodwin @adamchainz do you know why it might have stopped working? and how can we fix it ?

geekynasir avatar Oct 31 '22 13:10 geekynasir

I have no info to add, just posted a working config after days of tweaks haha

On Mon 31 Oct 2022, 14:45 Nasir Iqbal, @.***> wrote:

@carltongibson https://github.com/carltongibson @ezeakeal https://github.com/ezeakeal @andrewgodwin https://github.com/andrewgodwin @adamchainz https://github.com/adamchainz do you know why it might have stopped working? and how can we fix it ?

— Reply to this email directly, view it on GitHub https://github.com/django/channels_redis/issues/235#issuecomment-1297114738, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKWEZUAJIBGCWDRBA4WPS3WF7EPRANCNFSM4WLNHFRQ . You are receiving this because you were mentioned.Message ID: @.***>

ezeakeal avatar Oct 31 '22 13:10 ezeakeal

@nasir733 please don't @mention whole lists of people to try and draw attention to your issue. It's just spam.

You need to look at how connection params are passed down to redis-py, and from there your particular redis provider (be it local or hosted or ...)

Once you've worked that out you may be in a position to make a documentation PR to resolve this issue.

carltongibson avatar Oct 31 '22 13:10 carltongibson

@nasir733 please don't @mention whole lists of people to try and draw attention to your issue. It's just spam.

You need to look at how connection params are passed down to redis-py, and from there your particular redis provider (be it local or hosted or ...)

Once you've worked that out you may be in a position to make a documentation PR to resolve this issue.

thanks for the immediate response I will try to fix the problem and make a PR. I might need some help since it will be my first PR to a major opensource project sorry 😔 for tagging so many people

geekynasir avatar Oct 31 '22 13:10 geekynasir

I dug into this a little after having similar issues to Amplitude88 and having to downgrade to 3.4.x

This seems like it might actually be a bug in the redis.asyncio.connection Connection class? in either the redis.asyncio.connection SSLConnection class or the redis.asyncio.connection RedisSSLContext class. There's a slot defined in the redis.asyncio Connection class for ssl_context but no arg for it in the init() so it's always ignored / set to None. Similar story in RedisSSLContext and SSLConnection where there are other SSL args but no arg for ssl_context so it ends up being None or ignored always as well.

Probably missing something somewhere, but initial thoughts are it might be a pretty easy fix upstream in redis.asyncio since they just need to allow for passing in ssl_context in either RedisSSLContext, SSLConnection, or Connection? Currently it seems like if you pass in ssl_context all of the redis.asyncio classes are configured to just drop it on the ground and set it to None which is whats causing people problems.

It seems strange to me that if you specifically pass in the individual SSLContext parameters e.g. ssl_keyfile, ssl_certfile, etc those are configured correctly and get handled by SSLConnection and RedisSSLContext, but the ssl_context specifically even though referenced and used in those classes is just dropped on the ground or built from scratch instead of allowing a passed in ssl_context.

If anyone can sanity check this or has ideas why redis.asyncio might have done it that way would be happy to hear anything on it. If anyone can confirm they think this is an upstream bug in redis.asyncio I can try to make some time to put together a repro for an upstream redis.asyncio issue post.

TLDR: Seems like possible redis.asyncio bug?

edit: clarify redis.asyncio not actual asyncio

aerickson1337 avatar Nov 01 '22 23:11 aerickson1337

I've submitted a PR for a similar issue last week which I think might solve the problem with current versions https://github.com/django/channels_redis/pull/337

marcinowski avatar Nov 06 '22 17:11 marcinowski

Hey marcinowski, that PR looks good and is what I did in my first attempt at getting it working. I think it should solve the problem but it would break the previous channels_redis SSL API from 3.4.x that let you pass in an SSLContext directly. From more digging around in redis-py and their implementation it seems like they don't support passing an SSLContext directly. I did find a work around for people who need to maintain that functionality of passing the SSLContext directly instead of as params, but it also changes the existing channels_redis channel layer API compared to older versions which I wasn't a fan of either.

monkeypatch the SSL classes used in redis-py to allow ssl_context

from redis.asyncio.connection import Connection, RedisSSLContext
from typing import Optional

class CustomSSLConnection(Connection):
    def __init__(
        self,
        ssl_context: Optional[str] = None,
        **kwargs,
    ):
        super().__init__(**kwargs)
        self.ssl_context = RedisSSLContext(ssl_context)

class RedisSSLContext:
    __slots__ = (
        "context",
    )

    def __init__(
        self,
        ssl_context,
    ):
        self.context = ssl_context

    def get(self):
        return self.context

using in channel declaration/connection

import ssl
from NameOfOurMonkeyPatchFile import CustomSSLConnection
context = ssl.create_default_context(purpose=ssl.Purpose.CLIENT_AUTH, cafile='some.crt')
context.load_cert_chain(certfile='some.crt', keyfile='some.key')
channel_settings = {
    'CHANNEL_LAYERS': {
        'default': {
            'BACKEND': 'channels_redis.core.RedisChannelLayer',
            'CONFIG': {
                "hosts": [
                    {
                        'host': f"{os.environ['REDIS']}",
                        'port': 6379,
                        'connection_class': CustomSSLConnection,
                        'ssl_context': context,
                    }
                ],
            },
        },
    }
}

I think the solution marcinowski provided is probably the best solution moving forward if redis-py doesn't want to support passing in an SSLContext directly and instead doing it with configuration options.

Either way seems like 4.0 ends up changing how the redis SSL connection API in the channel_layer declaration works. Not sure what the best path forward is but seems like there needs to be: 3.4.x SSL connection docs, 4.x SSL connection docs, and 3.4.x->4.x SSL changes docs.

I could probably take a stab at updating the docs with that but would probably need to wait and see what the verdict is on macrinowski's PR -> #337 before it would make sense to write those up.

aerickson1337 avatar Nov 09 '22 17:11 aerickson1337

I've spent many days trying to combine all suggestions I found on resolving this issue on Heroku with Channels 4.0.0. What worked for me, is slightly modified version of @aerickson1337 solution.

from redis.asyncio.connection import Connection, RedisSSLContext
from typing import Optional
import ssl
from urllib.parse import urlparse

class CustomSSLConnection(Connection):
    def __init__(
        self,
        ssl_context: Optional[str] = None,
        **kwargs,
    ):
        super().__init__(**kwargs)
        self.ssl_context = RedisSSLContext(ssl_context)

class RedisSSLContext:
    __slots__ = (
        "context",
    )

    def __init__(
        self,
        ssl_context,
    ):
        self.context = ssl_context

    def get(self):
        return self.context


url = urlparse(os.environ.get("REDIS_URL"))

ssl_context = ssl.SSLContext()
ssl_context.check_hostname = False

CHANNEL_LAYERS = {
    'default': {
        'BACKEND': 'channels_redis.core.RedisChannelLayer',
        'CONFIG': {
            'hosts': [
                    {
                        'host': url.hostname,
                        'port': url.port,
                        'username': url.username,
                        'password': url.password,
                        'connection_class': CustomSSLConnection,
                        'ssl_context': ssl_context,
                    }
                ],
        }
    },
}

ah, it feels so good to finally see Django Channels working properly with Heroku Data for Redis. Thank you everyone on this thread.

paulina7m avatar Dec 24 '22 13:12 paulina7m

@paulina7m The urlparse was a crucial part of information for me and a further look at the code confirmed that channels_redis won't allow custom connection parameters such as connection_class if instead a host containing an address is specified (if "address" in host: in create_pool).

That isn't ideal behavior, because having to decode the Redis URL defeats the purpose of having a well formed URL in the first place. I will also try to solve this by parsing the URL and manually passing the components for now.

Of course on the other hand platform providers like Heroku could up their Redis security and not have us disregard invalid or self signed certs so easily. I assumed Let's Encrypt solved this issue, but maybe it hasn't for all purposes?

justuswilhelm avatar Feb 09 '23 09:02 justuswilhelm

Just to add an other info here: redis-py has not yet documented "how to provide kwargs to perform an SSL/TLS connection" (see https://github.com/redis/redis-py/issues/780).

There are already currently an SSLConnection and a RedisSSLContext class in redis-py (which are used by the from_url method). So there is no need to create/use custom classes.

sevdog avatar Jun 07 '23 09:06 sevdog

This should be resolved by https://github.com/django/channels_redis/pull/370.

carltongibson avatar Sep 04 '23 11:09 carltongibson

Copying @vsobotka's https://github.com/django/channels_redis/issues/373#issue-1953892079 for future searchers.

Earlier this year we upgraded redis-py for our app on Heroku from 3.9 to 4.5.4. We immediately ran into https://github.com/django/channels_redis/issues/235 and had to place in a workaround, with the ssl context. Everything went fine, until now, when we upgraded redis-py again, to 5.0.1. We immediately got "Connection reset by peer" error, preventing a stable connection. I was not able to find anything that would help. What helped us was to remove the workaround from https://github.com/django/channels_redis/issues/235 and use the newly supported option ssl_cert_reqs in layer config. So far this looks like it works perfectly well.

carltongibson avatar Jan 12 '24 09:01 carltongibson