rpardini / docker-registry-proxy

An HTTPS Proxy for Docker providing centralized configuration and caching of any registry (quay.io, DockerHub, k8s.gcr.io)
Apache License 2.0
912 stars 170 forks source link

Fix processing of HTTP redirects #120

Closed noseka1 closed 2 years ago

noseka1 commented 2 years ago

For some reason, the nginx proxy decided to cache the HTTP 302 redirect response sent back by the image registry. The reason could have been that the Expires header was not set enough into the past but I didn't investigate in detail.

The caching of the HTTP redirect caused the proxy to break. The proxy cached the HTTP 302 reponse and then in @handle_redirects it found this response in cache (cache HIT in the logs) and returned it to the client.

The error message seen in the logs caused by this issue:

[error] 78#78: *46 invalid URL prefix in ""

Below are relevant parts of the code. If you look at them, the code is using the same proxy_cache_key in both parts. The HTTP 302 body is cached in nginx.manifest.common.conf and found in the cache in nginx.conf.

nginx.manifest.common.conf

    # nginx config fragment included in every manifest-related location{} block.
    add_header X-Docker-Registry-Proxy-Cache-Upstream-Status "$upstream_cache_status";
    add_header X-Docker-Registry-Proxy-Cache-Type "$docker_proxy_request_type";
    proxy_pass https://$targetHost;
    proxy_cache cache;
    proxy_cache_key   $uri;
    proxy_intercept_errors on;
    error_page 301 302 307 = @handle_redirects;

nginx.conf

        location @handle_redirects {
            #store the current state of the world so we can reuse it in a minute
            # We need to capture these values now, because as soon as we invoke
            # the proxy_* directives, these will disappear
            set $original_uri $uri;
            set $orig_loc $upstream_http_location;

            # during this process, nginx will preserve the headers intended for the original destination.
            # in most cases thats okay, but for some (eg: google storage), passing an Authorization
            # header can cause problems. Also, that would leak the credentials for the registry
            # into the storage system (unrelated).
            proxy_set_header      Authorization "";

            # nginx goes to fetch the value from the upstream Location header
            proxy_pass $orig_loc;
            proxy_cache cache;
            # But we store the result with the cache key of the original request URI
            # so that future clients don't need to follow the redirect too
            proxy_cache_key $original_uri;
        }

This pull request fixes the issue by ignoring the cached value in @handle_redirects and just overwriting it.

noseka1 commented 2 years ago

This pull request is invalid. I think I misunderstood how the code should actually work. Caching the HTTP redirect responses should not be an issue. The problem must be somewhere else.