diff --git a/internal/handler/container.go b/internal/handler/container.go index d9360fbd..c9240dde 100644 --- a/internal/handler/container.go +++ b/internal/handler/container.go @@ -454,6 +454,13 @@ func (h *ContainerHandler) handleBlobDownload(w http.ResponseWriter, r *http.Req if writeUpstreamHeld(w, err) { return } + // A 403 from the upstream is a policy denial, not a proxy failure: + // report it the way an OCI registry reports a refused blob. + if status, _, ok := upstreamStatus(err); ok && status == http.StatusForbidden { + h.proxy.Logger.Warn("blob refused by upstream policy", "name", name, "digest", digest) + h.containerError(w, http.StatusForbidden, "DENIED", "blob refused by upstream policy") + return + } h.proxy.Logger.Error("failed to fetch blob", "error", err) h.containerError(w, http.StatusBadGateway, "INTERNAL_ERROR", "failed to fetch blob") return diff --git a/internal/handler/handler.go b/internal/handler/handler.go index 4b7ebe72..9f205d2f 100644 --- a/internal/handler/handler.go +++ b/internal/handler/handler.go @@ -1573,6 +1573,9 @@ func (p *Proxy) serveArtifactError(w http.ResponseWriter, err error, clientMsg s if writeUpstreamHeld(w, err) { return } + if p.serveUpstreamBlock(w, err) { + return + } p.Logger.Error("failed to get artifact", "error", err) http.Error(w, clientMsg, http.StatusBadGateway) } diff --git a/internal/handler/npm.go b/internal/handler/npm.go index 62bf4409..4c023105 100644 --- a/internal/handler/npm.go +++ b/internal/handler/npm.go @@ -585,6 +585,10 @@ func (h *NPMHandler) handleDownload(w http.ResponseWriter, r *http.Request) { if writeUpstreamHeld(w, err) { return } + // A policy denial keeps its own status and JSON report body. + if h.proxy.serveUpstreamBlock(w, err) { + return + } h.proxy.Logger.Error("failed to get artifact", "error", err) JSONError(w, http.StatusBadGateway, "failed to fetch package") } diff --git a/internal/handler/upstream_block.go b/internal/handler/upstream_block.go new file mode 100644 index 00000000..7e24c51d --- /dev/null +++ b/internal/handler/upstream_block.go @@ -0,0 +1,60 @@ +package handler + +import ( + "net/http" + "regexp" + "strconv" + "strings" +) + +// upstreamStatusPattern matches the error the fetch layer returns for an +// upstream response it does not handle itself: fmt.Errorf("unexpected status +// %d: %s", code, body). The fetch layer exposes neither the status nor the +// body any other way, and the body is the only place an upstream's reason for +// refusing an artifact survives, so both are read back out of the message. +var upstreamStatusPattern = regexp.MustCompile(`unexpected status (\d+): `) + +// upstreamStatus returns the status code and body of an upstream response the +// fetch layer rejected, and reports whether err carried one. Transport errors +// and the fetch layer's own sentinels (not found, rate limited, upstream down) +// carry no status and report false. +func upstreamStatus(err error) (code int, body string, ok bool) { + if err == nil { + return 0, "", false + } + msg := err.Error() + loc := upstreamStatusPattern.FindStringSubmatchIndex(msg) + if loc == nil { + return 0, "", false + } + code, convErr := strconv.Atoi(msg[loc[2]:loc[3]]) + if convErr != nil { + return 0, "", false + } + return code, msg[loc[1]:], true +} + +// serveUpstreamBlock answers a request whose upstream refused the artifact on +// policy grounds with that upstream's status and report body, and reports +// whether err was such a refusal. +// +// A firewall in front of the upstream denies with 403 for npm, PyPI, Maven and +// OCI, and with 409 for NuGet, and puts the reason in the body. Answering with +// the real status and body keeps that reason visible to the package manager +// instead of the 502 a failed fetch otherwise produces, which reads as a proxy +// fault and hides why the artifact was refused. +func (p *Proxy) serveUpstreamBlock(w http.ResponseWriter, err error) bool { + status, body, ok := upstreamStatus(err) + if !ok || (status != http.StatusForbidden && status != http.StatusConflict) { + return false + } + p.Logger.Warn("upstream refused artifact on policy grounds", "status", status, "detail", body) + contentType := "text/plain; charset=utf-8" + if strings.HasPrefix(strings.TrimSpace(body), "{") { + contentType = contentTypeJSON + } + w.Header().Set(headerContentType, contentType) + w.WriteHeader(status) + _, _ = w.Write([]byte(body)) + return true +} diff --git a/internal/handler/upstream_block_test.go b/internal/handler/upstream_block_test.go new file mode 100644 index 00000000..f6eecaae --- /dev/null +++ b/internal/handler/upstream_block_test.go @@ -0,0 +1,220 @@ +package handler + +import ( + "errors" + "fmt" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/git-pkgs/registries/fetch" +) + +const firewallReport = `{"status":403,"title":"Sonatype Firewall Report","detail":"Sonatype has identified this component as potentially malicious and blocked the download."}` + +// blockedByUpstream is the error the fetch layer produces for an upstream that +// refuses an artifact with a policy denial. +func blockedByUpstream(status int, body string) error { + return fmt.Errorf("fetching from upstream: %w", fmt.Errorf("unexpected status %d: %s", status, body)) +} + +func TestUpstreamStatus(t *testing.T) { + tests := []struct { + name string + err error + wantCode int + wantBody string + wantOK bool + }{ + { + name: "wrapped policy denial", + err: blockedByUpstream(http.StatusForbidden, firewallReport), + wantCode: http.StatusForbidden, + wantBody: firewallReport, + wantOK: true, + }, + { + name: "unwrapped policy denial", + err: errors.New("unexpected status 409: conflict"), + wantCode: http.StatusConflict, + wantBody: "conflict", + wantOK: true, + }, + { + name: "transport error carries no status", + err: errors.New("dial tcp 127.0.0.1:9: connect: connection refused"), + wantOK: false, + }, + { + name: "fetch layer sentinel carries no status", + err: fetch.ErrUpstreamDown, + wantOK: false, + }, + { + name: "no error", + err: nil, + wantOK: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + code, body, ok := upstreamStatus(tt.err) + if ok != tt.wantOK { + t.Fatalf("ok = %v, want %v", ok, tt.wantOK) + } + if code != tt.wantCode { + t.Errorf("code = %d, want %d", code, tt.wantCode) + } + if body != tt.wantBody { + t.Errorf("body = %q, want %q", body, tt.wantBody) + } + }) + } +} + +func TestServeArtifactErrorForwardsPolicyBlock(t *testing.T) { + tests := []struct { + name string + err error + wantStatus int + wantBody string + wantCT string + }{ + { + name: "403 with a report body is forwarded verbatim", + err: blockedByUpstream(http.StatusForbidden, firewallReport), + wantStatus: http.StatusForbidden, + wantBody: firewallReport, + wantCT: contentTypeJSON, + }, + { + name: "409 with a plain-text body is forwarded verbatim", + err: blockedByUpstream(http.StatusConflict, "blocked by policy"), + wantStatus: http.StatusConflict, + wantBody: "blocked by policy", + wantCT: "text/plain; charset=utf-8", + }, + { + name: "an upstream 5xx is still a proxy failure", + err: blockedByUpstream(http.StatusInternalServerError, "boom"), + wantStatus: http.StatusBadGateway, + wantBody: "failed to fetch package\n", + wantCT: "text/plain; charset=utf-8", + }, + { + name: "a transport error is still a proxy failure", + err: errors.New("dial tcp 127.0.0.1:9: connect: connection refused"), + wantStatus: http.StatusBadGateway, + wantBody: "failed to fetch package\n", + wantCT: "text/plain; charset=utf-8", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + proxy, _, _, _ := setupTestProxy(t) + w := httptest.NewRecorder() + proxy.serveArtifactError(w, tt.err, "failed to fetch package") + + if w.Code != tt.wantStatus { + t.Errorf("status = %d, want %d", w.Code, tt.wantStatus) + } + if got := w.Body.String(); got != tt.wantBody { + t.Errorf("body = %q, want %q", got, tt.wantBody) + } + if got := w.Header().Get(headerContentType); got != tt.wantCT { + t.Errorf("content-type = %q, want %q", got, tt.wantCT) + } + }) + } +} + +// A policy denial from the upstream reaches the Maven client with the +// upstream's own status and report body, through the real fetch layer. +func TestMavenHandler_ForwardsUpstreamPolicyBlock(t *testing.T) { + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set(headerContentType, contentTypeJSON) + w.WriteHeader(http.StatusForbidden) + _, _ = io.WriteString(w, firewallReport) + })) + defer upstream.Close() + + proxy, _, _, _ := setupTestProxy(t) + artifactFetcher := fetch.NewFetcher(fetch.WithHTTPClient(upstream.Client())) + t.Cleanup(func() { _ = artifactFetcher.Close() }) + proxy.Fetcher = artifactFetcher + proxy.HTTPClient = upstream.Client() + + h := NewMavenHandler(proxy, "http://localhost", upstream.URL, upstream.URL) + w := httptest.NewRecorder() + h.Routes().ServeHTTP(w, httptest.NewRequest(http.MethodGet, + "/org/sonatype/maven-policy-demo/1.1.0/maven-policy-demo-1.1.0.jar", nil)) + + if w.Code != http.StatusForbidden { + t.Fatalf("status = %d, want %d; body: %s", w.Code, http.StatusForbidden, w.Body.String()) + } + if got := w.Body.String(); got != firewallReport { + t.Errorf("body = %q, want %q", got, firewallReport) + } +} + +// npm keeps its JSON error shape, but a policy denial is passed through as the +// upstream sent it rather than as a generic 502. +func TestNPMHandler_ForwardsUpstreamPolicyBlock(t *testing.T) { + var upstreamURL string + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set(headerContentType, contentTypeJSON) + _, _ = io.WriteString(w, `{"name":"policy-demo","versions":{"2.1.0":{"dist":{"tarball":"`+ + upstreamURL+`/policy-demo/-/policy-demo-2.1.0.tgz"}}}}`) + })) + defer upstream.Close() + upstreamURL = upstream.URL + + proxy, _, _, fetcher := setupTestProxy(t) + proxy.HTTPClient = upstream.Client() + fetcher.fetchErrByURL = map[string]error{ + upstreamURL + "/policy-demo/-/policy-demo-2.1.0.tgz": blockedByUpstream(http.StatusForbidden, firewallReport), + } + + h := NewNPMHandler(proxy, "http://localhost", upstreamURL) + w := httptest.NewRecorder() + h.Routes().ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/policy-demo/-/policy-demo-2.1.0.tgz", nil)) + + if w.Code != http.StatusForbidden { + t.Fatalf("status = %d, want %d; body: %s", w.Code, http.StatusForbidden, w.Body.String()) + } + if got := w.Body.String(); got != firewallReport { + t.Errorf("body = %q, want %q", got, firewallReport) + } +} + +// A refused blob is reported the way an OCI registry reports a denial, not as +// a proxy failure. +func TestContainerHandler_BlobPolicyBlockIsDenied(t *testing.T) { + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusForbidden) + _, _ = io.WriteString(w, firewallReport) + })) + defer upstream.Close() + + proxy, _, _, _ := setupTestProxy(t) + artifactFetcher := fetch.NewFetcher(fetch.WithHTTPClient(upstream.Client())) + t.Cleanup(func() { _ = artifactFetcher.Close() }) + proxy.Fetcher = artifactFetcher + proxy.HTTPClient = upstream.Client() + + h := &ContainerHandler{proxy: proxy, registryURL: upstream.URL, proxyURL: "http://localhost"} + digest := sha256Digest([]byte("blocked layer")) + w := httptest.NewRecorder() + h.Routes().ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/library/nginx/blobs/"+digest, nil)) + + if w.Code != http.StatusForbidden { + t.Fatalf("status = %d, want %d; body: %s", w.Code, http.StatusForbidden, w.Body.String()) + } + if !strings.Contains(w.Body.String(), "DENIED") { + t.Errorf("body = %q, want an OCI DENIED error", w.Body.String()) + } +}