Bring back protected mode

[itamarhaber]

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]

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:

• https://github.com/docker-library/redis/pull/57 - The introduction of Redis 3.2, where protected mode was first disabled (because without disabling protected mode there, Redis was completely inaccessible except from within the same container, which in a container only running Redis itself, is arguably pretty useless).

• https://github.com/docker-library/redis/pull/59 - Move --protected-mode no into the entrypoint instead (of the default command)

• https://github.com/docker-library/redis/pull/61 - Fix support for configuration files, especially with relation to --protected-mode

• https://github.com/docker-library/redis/pull/71 - Redo "protected-mode" enablement in a way that preserves "save on SIGTERM" (IMO, this is where things start to get interesting thanks to https://github.com/docker-library/redis/issues/46#issuecomment-582600597, but I'll elaborate more below.)

• https://github.com/docker-library/redis/pull/212 - Redis 6.0-rc1 (the Redis code was modified WRT how protected mode is defined to be cleaner and clearer)

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]

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]

... 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]

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