DO NOT MERGE: fix: allow empty scope on /v2

[sagikazarmark]

What type of PR is this?

bug

Which issue does this PR fix:

#1125

What does this PR do / Why do we need it:

This PR fixes the bearer auth implementation allowing registry clients to authenticate with an empty scope when calling /v2.

As said in this comment, according to the distribution specification that most registry clients implement, any empty scope in the token is valid when a non-resource URL is called (essentially /v2).

However, the current implementation (that relies on chartmuseum/auth) always requires a scope to be present in order to accept a token. (Under the hood the library tries to compare the empty access request contained in the token to a bogus repository::pull scope. The repository name is empty and Zot falls back to pull)

After reading through the Chartmuseum auth spec here, I came to the conclusion that it's fundamentally incompatible with the Docker auth spec.

The chartmuseum spec says the first step is always pulling or pushing a specific artifact, whereas the Docker spec does not require that. The detailed nature of that spec also suggests that it may build on the original docker spec, it's actually a different one.

This PR is just a demonstration that shows how Zot should behave correctly.

There are several potential solutions:

• If chartmuseum is willing to "fix" their spec/library to accept empty access list, we can easily fix the problem. However, I'm not sure that's in the best interest on Chartmuseum if they ever plan to use their auth spec (but it essentially seems unmaintained to me)
• Copy the relevant code from the distribution code base
• Write a whole new library

If an issue # is not available please add repro steps and logs showing the issue:

Testing done on this change:

Automation added to e2e:

Will this break upgrades or downgrades?

NO

Does this PR introduce any user-facing change?:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[seniorquico]

I encountered this testing zot with skopeo, and I agree- it seems the chartmuseum/auth library is broken (although, maybe as you suggest, it was simply designed for a different spec). Another callout- The chartmuseum/auth library only supports RSA keys, which is small bummer for our use case (we've fully transitioned to ECC keys).

Taking inspiration from your comment, "copy the relevant code from the distribution code base", I went spelunking... It doesn't seem very complicated (although I'm sure dragons may lurk somewhere in there). The auth context is setup and the auth handler invoked here (which is invoked by their HTTP request middleware):

func (app *App) authorized(w http.ResponseWriter, r *http.Request, context *Context) error ...

The function above calls two simple functions to generate the auth requirements from the URL:

1. func appendAccessRecords(records []auth.Access, method string, repo string) []auth.Access ...
2. func appendCatalogAccessRecord(accessRecords []auth.Access, r *http.Request) []auth.Access ...

Notably, when the request path is the base /v2/, there are no auth requirements added to the slice.

The bearer auth handler is contained in the registry/auth/token directory. When there is no scope, you can see the parameter is completely omitted from the challenge (ref):

if scope := ac.accessSet.scopeParam(); scope != "" {
	str = fmt.Sprintf("%s,scope=%q", str, scope)
}

I probably need to spend some more time reading through the Token Authentication Specification, but nothing jumps out to me to suggest the chartmuseum/auth library can't be replaced by the distribution implementation. There would be behavioral changes for consumers, and that might be considered a breaking change. But given how the chartmuseum/auth library's implementation seems to be broken for modern distribution clients... 🤔

After some more reading, I might try to take a stab at pulling in the distribution implementation and run some tests. I wonder if the conformance workflow in this repo with the bearer auth configured would detect these issues?

[seniorquico]

Before diving in to copying bits from the distribution code base, I thought I'd go searching for an alternative library (as an alternative to the "write a whole new library" suggestion)...

https://github.com/portward/registry-auth/

😉👍

OCI research notes (for posterity)

For those following along that aren't in-sync with all of the OCI happenings...

The OCI Distribution Spec does not address authentication/authorization. This is a known omission:

• Feature: Authentication and authorization support opencontainers/distribution-spec#240
• AutH/AuthZ flows for registries opencontainers/distribution-spec#338

A working group was formed to attempt to define a standard (and a zot maintainer is a member):

• Working group proposal for auth opencontainers/tob#119

The meeting notes are available in a dedicated repo:

• opencontainers/wg-auth

Today, the Docker v2 Token Authentication Specification is the de facto standard, and the working group appears to be starting with it as the base. The working group appears to be exploring use cases/limitations and might expand on the Docker v2 Token Authentication Specification accordingly.

Notably, in the last meeting dated 2023-08-08, the question of creating a reference authentication library was raised. That would be cool. 👍

A question... Why "DO NOT MERGE" on this PR? If the OCI working group needs time to develop the long-term solution, couldn't the workaround proposed with this PR be sufficient to temporarily get zot working again?

[sagikazarmark]

@seniorquico a couple thoughts:

This PR serves two purposes: it allowed me to try Zot with Portward (which is a project I created actually) and it also allowed to demonstrate what's wrong with the current implementation.

I suspect the solution either lies in "fixing" chartmuseum or not using it at all, but this PR is probably not the right answer (ie. I made this change in roughly 5 minutes without thinking about potential security issues it may introduce).

WG auth: it's just been formed and although there is progress, it's gonna take some time to come up with a proposal that addresses all the issues and requirements, gain feedback from the wider community and get it accepted as a formal spec. Even then, it's gonna take years to adopt it. So yes, a future OCI auth spec is the answer, but it's not going to happen over night.

Portward (the registry-auth lib) sits primarily on the other side of the aisle, service as a tool for authorization services. Although it may be used in a registry as well, I haven't considered that use case when I wrote the library and it may not be the best fit for this use case. There may be bits and pieces though that could be reused. Just know that it's primarily targeted at authorization services and NOT at registries (which generally have different requirements, like verifying tokens instead of issuing them)

@rchincha might be able to provide some guidance here on what we should do next.

Should we attempt "fixing" the chartmuseum implementation? It's certainly worth a try talking to them, but my gut tells me (as I said before) that it's actually a different spec and therefore not required to be compatible with the registry auth spec.

Admittedly, this would result in a simpler resolution for Zot itself.

The alternative (as I said above) is replacing the implementation with a different one.

[andaaron]

An updated version of this fix was merged in #2176.
You should be able to us the latest zot release.