Replaced default auth configuration from 'none' to 'htpasswd'.

[liron-l]

Replaced the default registry auth configuration from 'none' to
'htpasswd'.
Following the change in distribution/distribution#2362.

Signed-off-by: Liron Levin [email protected]

[liron-l]

CC @dmcgowan

[dmcgowan]

My main concern with changing the default like this is our potential to break production deployments behind load balancers who are making use of the latest tag. I am OK with introducing this for newly created tags, but if we are going to have a set of tags with both configurations we may need 2 configuration files.

Ping @tianon @yosifkit for any guidance on making major changes to the latest and point release tags before getting this change in.

[yosifkit]

We've had a few instances when updating the Dockerization or configuration of an existing image:tag has broken some users, so we try to limit it. But sometimes we have to break things. My opinion on latest is to break it all you want, since users should really be specifying the version or even the sha256 for a production deployment.

We have https://github.com/docker-library/repo-info which helps when we do break users. They can find a point-in-time sha256 of the version that works.

[liron-l]

Thanks @yosifkit and @dmcgowan, so this means we can keep this configuration as is?

[liron-l]

@dmcgowan PTAL

[liron-l]

@yosifkit, @tianon, @dmcgowan PTAL

[endophage]

@tianon @yosifkit @dmcgowan what's required to get this merged? This is an important security feature as it has been determined that a lot of users are unfortunately creating insecure registries on the public internet.

[stevvooe]

What is the user story for disabling this if they are running default config behind a proxy?

[endophage]

@stevvooe the user sets their own config file that removes the auth section.

[endophage]

Note, we expect to release this as a minor version bump too. Unless distribution operates differently to other projects, a minor version bump is generally expected to change some behaviours and possibly break some users.

Edit: just confirmed specifics on semantic version numbers. Minor bump should be backwards compatible. This change is. Changing default behavior and offering a new feature (auto generation of htpasswd when not explicitly set as part of auth: htpasswd config) is entirely backwards compatible.

[liron-l]

@endophage @stevvooe a user can set REGISTRY_AUTH= to disable basic authorization.

[stevvooe]

Most users don't replace the configuration and just use the default. This was surprising to me, as well, but we got a lot of early complaints about this when we first released. Most users either don't want to ship a registry image with the baked configuration or don't want to ship the configuration to the host for bind mounting.

In practice, they either replace the storage by bind/volume mounting /var/lib/registry or by using environment variables to configure some other backend storage. It would be good if we could understand the depth of change required and ensure that we have appropriate documentation to cover that problem. I think you can just include the env variable that configures none for auth, but it would be good to see some upgrade tests to confirm this is all that it takes.

As far as versioning is concerned, this is clearly a compatibility break if existing user deployments need a change for an upgrade. I am not sure everyone sees it that way, but those running infrastructure will. Technically, this requires a major bump to version 3, if we are following semver but I am not necessarily opposed to doing this in a minor bump.

Whatever escape hatch is provided, it should be on the distribution readme, the image readme, tweeted, blogged and a part of the release notes, at least.

Re-iterating, we should have the following:

1. Clearly documented numbers with reasoning as to why this is necessary that users' can be pointed to when they ask why this change was made.
2. Clearly documented and tested escape hatch for upgrades from existing registries to this.
3. A plan for announcing, disseminating and supporting users for the foreseeable future.

[liron-l]

Thanks @stevvooe, just so it's clear, a simple way to disable auth after this change:

 docker run -d   -p 5000:5000 -v /var/lib/registry:/var/lib/registry   -e "REGISTRY_AUTH="    registry:2

How do you suggest we processed (in terms of code and documentation updates)?

[stevvooe]

@liron-l

How do you suggest we processed (in terms of code and documentation updates)?

I think this would be the plan going forward:

1. Clearly documented numbers with reasoning as to why this is necessary that users' can be pointed to when they ask why this change was made.
2. Clearly documented and tested escape hatch for upgrades from existing registries to this.
3. A plan for announcing, disseminating and supporting users for the foreseeable future.

[liron-l]

Thanks @stevvooe. Few questions:
Re 1. Where do you think is the appropriate place for such documentation.
Is it ok to add it as part of distribution/distribution#2362?
Re 2. Given the simple workaround of setting the REGISTRY_AUTH environment variable to none, Where do you suggest to add the workaround documentation?
Re 3. Who owns this?

[tianon]

Does a more obvious value for "REGISTRY_AUTH" like "no" or "none" also work?

[liron-l]

@tianon, no, only empty string.

[tianon]

That's kind of a poor user experience. 😞

Any chance that could be updated to also allow no or none? IMO, -e REGISTRY_AUTH=none is a lot easier to read and understand at a glance (and looks appropriately scarier from a security perspective, which is the goal of this whole change as I've understood it) than -e REGISTRY_AUTH= (which frankly looks like a typo).

[liron-l]

@stevvooe, @dmcgowan, @endophage

I've created the following script to verify that the override parameter works. Note that the validation below is actually more strict since it verifies that the AUTH environment variable overrides even when the registry is configured with authentication.

Let me know where to put this script, and any further steps necessary.
Note that I've used set -e so any sub-command that return non zero status will cause the script to exit immediately with error.

#!/bin/bash

set -e

# Validate ne registry settings are backward competitble
# Run baseline registry (latest) ith htpasswd enabled (with persistent volume for storage)
# Push sample image
# Re-run with new image with REGISTRY_AUTH set to empty (no htpasswd)
# Pull image (to validate data is persistent)
# Push image
# Re-run with default settings (with htpasswd)
# Ensure valid credential works, and push image

registry_baseline=registry
registry_new=registry:new
persistent_storage=/tmp/registry # Store all data in persistent location

mkdir -p /tmp/auth

# Cleanup previous runs
docker rm -vf registry || true

# Generate password
docker run --rm --entrypoint htpasswd registry:2 -Bbn testuser testpassword > /tmp/auth/htpasswd

# Run the baseline registry with authorization
docker run -d -p 5000:5000 --name registry -v ${persistent_storage}:/var/lib/registry -v /tmp/auth/htpasswd:/etc/registry -e "REGISTRY_AUTH=htpasswd" \
  -e "REGISTRY_AUTH_HTPASSWD_REALM=Registry Realm" \
  -e REGISTRY_AUTH_HTPASSWD_PATH=/etc/registry \
  ${registry_baseline}

# Pull some image
docker pull --disable-content-trust=false hello-world:latest

# Tag it
image_tag=localhost:5000/hello-world:latest
docker tag hello-world:latest ${image_tag}

# Push image to registry
docker login --username testuser --password testpassword localhost:5000
docker push ${image_tag}

# Build the new registry
docker build . -t ${registry_new}

# Remove old registry
docker rm -vf registry

# Run new registry
docker run -d -p 5000:5000 --name registry -v ${persistent_storage}:/var/lib/registry -v `pwd`/auth/htpasswd:/etc/registry -e "REGISTRY_AUTH="  ${registry_new}

# Sleep to ensure registry starts
sleep 1
# Login with fake password (no auth)
docker login --username fakeuser --password fakepassword localhost:5000
# Ensure data is persistent (pull existing image)
docker pull ${image_tag}
docker push ${image_tag}

# Remove old registry and
docker rm -vf registry

# Run with default htpasswd
docker run -d -p 5000:5000 --name registry -v ${persistent_storage}:/var/lib/registry -v `pwd`/auth/htpasswd:/etc/registry  ${registry_new}
set +e
# Ensure registry authenticate user
docker login --username fakeuser --password fakepassword localhost:5000
if [[ $? == 0 ]];then
    exit 1
fi
set -e
docker login --username testuser --password testpassword localhost:5000
docker push ${image_tag}
rm -rf /tmp/auth

[stevvooe]

@liron-l I agree with @tianon. Can you alias none to empty in the registry PR?

[liron-l]

NP, @stevvooe updated distribution/distribution#2362.
Thanks.

[dmp42]

@tianon wdyt at this point?

[tianon]

If this is something y'all want to move forward with, it should be reasonably safe to do so (assuming the docs are also updated, which currently assume the default will be authentication-less).

Given that the implementation supports REGISTRY_AUTH=none as a value, I'm +1 -- it'll force folks who want that to be more explicit, which will probably better highlight that it's a security issue.