redis / docker-library-redis

Docker Official Image packaging for Redis
http://redis.io
BSD 3-Clause "New" or "Revised" License
1.12k stars 564 forks source link

Bring back protected mode #232

Open itamarhaber opened 4 years ago

itamarhaber commented 4 years ago

Hi all,

Currently, we're disabling protected mode (in a very brutal way) for this image. I would like to argue that despite the protection that Docker's networking layer provides, protected mode should be enabled if only to block access to exposed instances (i.e. without a password). For example, #217 would probably not have been an issue if we had protected mode enabled as a default.

Thoughts?

tianon commented 4 years ago

Before providing my thoughts, I want to summarize the context of where this started and the steps it's seen in the interim as a basic level-set:

To be clear, I absolutely agree with the idea of Protected Mode and the goal to make Redis more secure by default. However, the way Protected Mode is implemented (only available on localhost/loopback), operationally it renders Redis inaccessible in any container context other than containers sharing a network namespace (which is not even common in Kubernetes anymore, where one-container-per-pod is the most common deployment mode).

If we were to overcome that and somehow change Protected Mode to only enforce setting a password, we'll immediately run into what I think is best described by my example in https://github.com/docker-library/redis/issues/46#issuecomment-582600597, namely that adding a flag to redis-server changes the default values (specifically, the value of the save configuration option), which is why we've been recommending folks use full-blown configuration files to adjust settings instead.

So I'm certainly open to reverting this if we can overcome the "loopback is the only acceptable connection method" requirement and/or make it simpler for users to actually specify a password without having to supply a full configuration file (or just know which settings have defaults that will suddenly change when they do so).

Maybe a compromise on the "loopback" issue would be to have a "container" variant of protected mode where the allowable external connections extend out to private IP ranges (127.0.0.0/8, 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16)? That would at least block direct internet accesses while still allowing most inter-container communication.

itamarhaber commented 4 years ago

Thanks for the exhaustive analysis @tianon!

The compromise you're suggesting - extend the range or even disable the IP check when in a container - is what I had in mind as well. I guess it could be done as a patch during build process, but if there's a way to have Redis discover whether it is containerized or not, we can consider adding it upstream.

As for the comment in #46, well, here's an issue which I hope to resolve "soon(tm)" :) https://github.com/antirez/redis/issues/5629

tianon commented 4 years ago

... a way to have Redis discover whether it is containerized or not ...

The most reliable solution to this I've seen other products use is to have an environment variable that gets set in the Docker image to identify that it should have Docker behavior -- trying to detect whether we're a container gets kind of hairy (and either way, it would be a good idea to have an escape hatch for users to explicitly move the needle back one way or the other based on their own circumstances).

tianon commented 2 years ago

Friendly poke @itamarhaber -- any progress on a more nuanced protected mode that we could be using? :smile: