Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clair wrongly injects Authorization headers when fetching layers #1283

Closed
majewsky opened this issue Jun 3, 2021 · 1 comment · Fixed by #1284
Closed

Clair wrongly injects Authorization headers when fetching layers #1283

majewsky opened this issue Jun 3, 2021 · 1 comment · Fixed by #1284

Comments

@majewsky
Copy link

majewsky commented Jun 3, 2021

tl;dr: When sending HTTP requests to fetch layers, Clair injects an extraneous Authorization header which may confuse the object store containing the layers.


Our internal registry uses OpenStack Swift as a backing storage, and Clair for scanning vulnerabilities. We have Clair set up with the auth.psk config section, and our registry issues tokens to itself to talk to the Clair API. After upgrading Clair from 4.0.2 to 4.1.0, we saw indexing in Clair fail with:

failed to fetch layers: encountered error while fetching a layer: fetcher: unexpected status code: 403 403 Forbidden

After some digging, the problem turned out to be that Clair adds an Authorization header to each HTTP request, containing a Bearer token derived from the auth.psk key:

if cs.Signer != nil {
// TODO(hank) Make this mint longer-lived tokens and re-use them, only
// refreshing when needed. Like a resettable sync.Once.
now := time.Now()
cl := cs.base
cl.IssuedAt = jwt.NewNumericDate(now)
cl.NotBefore = jwt.NewNumericDate(now.Add(-jwt.DefaultLeeway))
cl.Expiry = jwt.NewNumericDate(now.Add(jwt.DefaultLeeway))
h, err := jwt.Signed(cs).Claims(&cl).CompactSerialize()
if err != nil {
return nil, err
}
r.Header.Add("authorization", "Bearer "+h)
}

This Authorization header trips up Swift, since the issued token is only valid inside of Clair itself. The Swift authentication is supposed to be handled by query parameters in the layer URL as provided by in the index request, but the extraneous Authorization header makes Swift ignore all that.

This code has not been touched in a while, so I'm not entirely sure why this did not happen with Clair 4.0.2, but it's very reproducible on Clair 4.1.0. The correct behavior here would be to honor the index request and fetch the layer only with the headers included therein.

Environment

  • Clair version/image: 4.1.0
  • Clair client name/version: N/A
  • Host OS: Linux
  • Kernel (e.g. uname -a): N/A
  • Kubernetes version (use kubectl version): N/A
  • Network/Firewall setup: N/A
@majewsky
Copy link
Author

majewsky commented Jun 3, 2021

I have patched Clair like this and that fixes the issue for us, but that patch is incredibly environment-specific and cannot be the general solution. I'd be willing to work on a PR to fix this issue in the general case, but I'd like to hear opinions first on how to approach it. I can see two options from a cursory read of the Clair and claircore code.

Option 1

The http.Client that is given to libindex.New() only seems to be used for fetching layers, so it may be possible to just pass an http.Client that fills only the User-Agent header, not the Authorization header.

Option 2

If having multiple http.Client instances is not desired, the fetcher could do some signalling inside the http.Request (e.g. via context.WithValue) to indicate that no Authorization header shall be added to the request.

hdonnay added a commit to hdonnay/clair that referenced this issue Jun 3, 2021
This change makes the HTTP client configuration method accept a Claims
pointer, and if `nil` is passed, omits the automatic signing that would
happen if a PSK was configured.

Fixes quay#1283.

Signed-off-by: Hank Donnay <hdonnay@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

1 participant