solver: reuse source policy for proxy network

Route proxy network policy checks through the existing source policy evaluator so
session metadata, deny messages, and URL converts use the same path as LLB
sources. Keep proxy-specific request rewriting in the proxy provider.

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>

solver/llbsolver/network.go

@@ -1,17 +1,11 @@
 package llbsolver
 
 import (
-	"context"
-
-	gatewaypb "github.com/moby/buildkit/frontend/gateway/pb"
-	"github.com/moby/buildkit/session"
 	"github.com/moby/buildkit/solver"
 	"github.com/moby/buildkit/solver/pb"
 	"github.com/moby/buildkit/sourcepolicy"
 	spb "github.com/moby/buildkit/sourcepolicy/pb"
-	"github.com/moby/buildkit/sourcepolicy/policysession"
 	"github.com/moby/buildkit/util/network"
-	"github.com/moby/buildkit/util/urlutil"
 	"github.com/pkg/errors"
 )
 
@@ -39,21 +33,6 @@ func WithProxyNetwork(proxyNetwork bool) LoadOpt {
 	}
 }
 
-func newProxyPolicy(sm *session.Manager, srcPol *spb.Policy, policySession string) network.ProxyPolicy {
-	if srcPol == nil && policySession == "" {
-		return nil
-	}
-	var engine *sourcepolicy.Engine
-	if srcPol != nil {
-		engine = sourcepolicy.NewEngine([]*spb.Policy{srcPol})
-	}
-	return &proxyPolicy{
-		engine:        engine,
-		sm:            sm,
-		policySession: policySession,
-	}
-}
-
 func (b *provenanceBridge) ProxyPolicy() (network.ProxyPolicy, error) {
 	return b.llbBridge.ProxyPolicy()
 }
@@ -67,60 +46,12 @@ func (b *llbBridge) ProxyPolicy() (network.ProxyPolicy, error) {
 	if err != nil {
 		return nil, err
 	}
-	return newProxyPolicy(b.sm, srcPol, policySession), nil
-}
-
-type proxyPolicy struct {
-	engine        *sourcepolicy.Engine
-	sm            *session.Manager
-	policySession string
-}
-
-func (p *proxyPolicy) CheckProxyRequest(ctx context.Context, url string) error {
-	redactedURL := urlutil.RedactCredentials(url)
-	op := &pb.Op{
-		Op: &pb.Op_Source{
-			Source: &pb.SourceOp{
-				Identifier: redactedURL,
-			},
-		},
-	}
-	if p.engine != nil {
-		if _, err := p.engine.Evaluate(ctx, op.GetSource()); err != nil {
-			return err
-		}
-	}
-	if p.policySession == "" {
-		return nil
-	}
-	if p.sm == nil {
-		return errors.Errorf("source policy session %q is set but session manager is unavailable", p.policySession)
-	}
-	caller, err := p.sm.Get(ctx, p.policySession, false)
-	if err != nil {
-		return err
-	}
-	verifier := policysession.NewPolicyVerifierClient(caller.Conn())
-	resp, err := verifier.CheckPolicy(ctx, &policysession.CheckPolicyRequest{
-		Source: &gatewaypb.ResolveSourceMetaResponse{
-			Source: op.GetSource(),
-		},
-	})
-	if err != nil {
-		return err
-	}
-	if resp.GetRequest() != nil {
-		return errors.Errorf("source policy metadata requests are not supported for proxy request %q", redactedURL)
-	}
-	decision := resp.GetDecision()
-	if decision == nil {
-		return errors.Errorf("no decision in policy response")
-	}
-	if decision.Action == spb.PolicyAction_DENY {
-		return errors.Wrapf(sourcepolicy.ErrSourceDenied, "source %q denied by policy", redactedURL)
-	}
-	if decision.Action == spb.PolicyAction_CONVERT {
-		return errors.Errorf("source policy convert action is not supported for proxy request %q", redactedURL)
-	}
-	return nil
+	if (srcPol == nil || len(srcPol.Rules) == 0) && policySession == "" {
+		return nil, nil
+	}
+	var policies []*spb.Policy
+	if srcPol != nil {
+		policies = append(policies, srcPol)
+	}
+	return b.policy(sourcepolicy.NewEngine(policies)), nil
 }

solver/llbsolver/network_test.go

@@ -1,32 +1,5 @@
 package llbsolver
 
-import (
-	"testing"
+import "github.com/moby/buildkit/util/network"
 
-	"github.com/moby/buildkit/sourcepolicy"
-	spb "github.com/moby/buildkit/sourcepolicy/pb"
-	"github.com/stretchr/testify/require"
-)
-
-func TestProxyPolicyRedactsCredentialsInErrors(t *testing.T) {
-	p := &proxyPolicy{
-		engine: sourcepolicy.NewEngine([]*spb.Policy{
-			{
-				Rules: []*spb.Rule{
-					{
-						Action: spb.PolicyAction_DENY,
-						Selector: &spb.Selector{
-							Identifier: "https://*",
-						},
-					},
-				},
-			},
-		}),
-	}
-
-	err := p.CheckProxyRequest(t.Context(), "https://user:pass@example.com/path")
-	require.ErrorIs(t, err, sourcepolicy.ErrSourceDenied)
-	require.NotContains(t, err.Error(), "user")
-	require.NotContains(t, err.Error(), "pass")
-	require.Contains(t, err.Error(), "https://xxxxx:xxxxx@example.com/path")
-}
+var _ network.ProxyPolicy = (*policyEvaluator)(nil)

util/network/proxy.go

@@ -4,12 +4,13 @@ import (
 	"context"
 	"sync"
 
+	"github.com/moby/buildkit/solver/pb"
 	digest "github.com/opencontainers/go-digest"
 )
 
 // ProxyPolicy authorizes requests made through a BuildKit-owned exec proxy.
 type ProxyPolicy interface {
-	CheckProxyRequest(context.Context, string) error
+	Evaluate(context.Context, *pb.Op) (bool, error)
 }
 
 // ProxyNamespace is implemented by network namespaces that expose an internal

util/network/proxyprovider/provider_linux.go

@@ -19,6 +19,7 @@ import (
 	"math/big"
 	"net"
 	"net/http"
+	neturl "net/url"
 	"os"
 	"path/filepath"
 	"runtime"
@@ -32,6 +33,7 @@ import (
 	"github.com/containerd/containerd/v2/pkg/oci"
 	resourcestypes "github.com/moby/buildkit/executor/resources/types"
 	"github.com/moby/buildkit/identity"
+	"github.com/moby/buildkit/solver/pb"
 	"github.com/moby/buildkit/util/network"
 	"github.com/moby/buildkit/util/network/netpool"
 	"github.com/moby/buildkit/util/urlutil"
@@ -427,9 +429,12 @@ func (h *proxyHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		r.URL.Scheme = "http"
 		r.URL.Host = r.Host
 	}
-	if err := h.check(r.Context(), r.URL.String()); err != nil {
+	if target, err := h.check(r.Context(), r.Method, r.URL.String()); err != nil {
 		http.Error(w, "Forbidden", http.StatusForbidden)
 		return
+	} else if target != nil {
+		r.URL = target
+		r.Host = target.Host
 	}
 	resp, err := h.roundTrip(r)
 	if err != nil {
@@ -481,10 +486,13 @@ func (h *proxyHandler) handleConnect(w http.ResponseWriter, r *http.Request) {
 		req.URL.Scheme = "https"
 		req.URL.Host = r.Host
 		req.RequestURI = ""
-		if err := h.check(req.Context(), req.URL.String()); err != nil {
+		if target, err := h.check(req.Context(), req.Method, req.URL.String()); err != nil {
 			_ = req.Body.Close()
 			_, _ = io.WriteString(tlsConn, "HTTP/1.1 403 Forbidden\r\nContent-Length: 10\r\nConnection: close\r\n\r\nForbidden\n")
 			return
+		} else if target != nil {
+			req.URL = target
+			req.Host = target.Host
 		}
 		resp, err := h.roundTrip(req)
 		if err != nil {
@@ -614,11 +622,41 @@ func (h *proxyHandler) roundTrip(r *http.Request) (*http.Response, error) {
 	return h.provider.client.RoundTrip(r)
 }
 
-func (h *proxyHandler) check(ctx context.Context, url string) error {
+func (h *proxyHandler) check(ctx context.Context, method, rawURL string) (*neturl.URL, error) {
 	if h.policy == nil {
-		return nil
+		return nil, nil
 	}
-	return h.policy.CheckProxyRequest(ctx, url)
+	redactedURL := redactURL(rawURL)
+	op := &pb.Op{
+		Op: &pb.Op_Source{
+			Source: &pb.SourceOp{
+				Identifier: redactedURL,
+			},
+		},
+	}
+	if _, err := h.policy.Evaluate(ctx, op); err != nil {
+		return nil, err
+	}
+	source := op.GetSource()
+	target := source.Identifier
+	converted := target != redactedURL || len(source.Attrs) != 0
+	if !converted {
+		return nil, nil
+	}
+	if method != http.MethodGet {
+		return nil, errors.Errorf("source policy converted proxy request %q, but conversion is only supported for GET", redactedURL)
+	}
+	if len(source.Attrs) != 0 {
+		return nil, errors.Errorf("source policy converted proxy request %q with attrs, but proxy conversion only supports URL updates", redactedURL)
+	}
+	u, err := neturl.Parse(target)
+	if err != nil {
+		return nil, errors.Wrapf(err, "error parsing converted proxy request URL %q", redactURL(target))
+	}
+	if !u.IsAbs() || (u.Scheme != "http" && u.Scheme != "https") {
+		return nil, errors.Errorf("source policy converted proxy request to unsupported URL %q", redactURL(target))
+	}
+	return u, nil
 }
 
 func newCA() ([]byte, *x509.Certificate, *rsa.PrivateKey, error) {

util/network/proxyprovider/provider_linux_test.go

@@ -3,11 +3,15 @@
 package proxyprovider
 
 import (
+	"context"
 	"net/http"
 	"net/http/httptest"
 	"strings"
 	"testing"
 
+	"github.com/moby/buildkit/solver/pb"
+	"github.com/moby/buildkit/sourcepolicy"
+	spb "github.com/moby/buildkit/sourcepolicy/pb"
 	"github.com/moby/buildkit/util/network"
 	"github.com/stretchr/testify/require"
 )
@@ -101,6 +105,102 @@ func TestProxyHandlerRedactsCapturedCredentials(t *testing.T) {
 	require.Contains(t, materials[0].URL, "xxxxx:xxxxx@")
 }
 
+func TestProxyHandlerAppliesPolicyConvert(t *testing.T) {
+	original := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		t.Error("original upstream should not receive converted request")
+	}))
+	t.Cleanup(original.Close)
+	mirrorMethodCh := make(chan string, 1)
+	mirror := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		mirrorMethodCh <- r.Method
+		_, _ = w.Write([]byte("mirror material"))
+	}))
+	t.Cleanup(mirror.Close)
+
+	capture := network.NewProxyCapture()
+	handler := newTestProxyHandler(t, capture)
+	handler.policy = proxyPolicyFunc(func(_ context.Context, op *pb.Op) (bool, error) {
+		require.Equal(t, original.URL+"/file", op.GetSource().Identifier)
+		op.GetSource().Identifier = mirror.URL + "/file"
+		return true, nil
+	})
+	resp := httptest.NewRecorder()
+	req := httptest.NewRequest(http.MethodGet, original.URL+"/file", nil)
+
+	handler.ServeHTTP(resp, req)
+
+	require.Equal(t, http.StatusOK, resp.Code)
+	require.Equal(t, "mirror material", resp.Body.String())
+	require.Equal(t, http.MethodGet, <-mirrorMethodCh)
+	materials := capture.Materials()
+	require.Len(t, materials, 1)
+	require.Equal(t, mirror.URL+"/file", materials[0].URL)
+	require.Empty(t, capture.Incomplete())
+}
+
+func TestProxyHandlerPolicyRedactsCredentialsInErrors(t *testing.T) {
+	handler := newTestProxyHandler(t, nil)
+	handler.policy = enginePolicyEvaluator{engine: sourcepolicy.NewEngine([]*spb.Policy{
+		{
+			Rules: []*spb.Rule{
+				{
+					Action: spb.PolicyAction_DENY,
+					Selector: &spb.Selector{
+						Identifier: "https://*",
+					},
+				},
+			},
+		},
+	})}
+
+	_, err := handler.check(t.Context(), http.MethodGet, "https://user:pass@example.com/path")
+	require.ErrorIs(t, err, sourcepolicy.ErrSourceDenied)
+	require.NotContains(t, err.Error(), "user")
+	require.NotContains(t, err.Error(), "pass")
+	require.Contains(t, err.Error(), "https://xxxxx:xxxxx@example.com/path")
+}
+
+func TestProxyHandlerRejectsConvertedNonGetRequest(t *testing.T) {
+	handler := newTestProxyHandler(t, nil)
+	handler.policy = proxyPolicyFunc(func(_ context.Context, op *pb.Op) (bool, error) {
+		op.GetSource().Identifier = "https://mirror.example.com/file"
+		return true, nil
+	})
+
+	_, err := handler.check(t.Context(), http.MethodPost, "https://example.com/file")
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "conversion is only supported for GET")
+}
+
+func TestProxyHandlerRejectsConvertedAttrs(t *testing.T) {
+	handler := newTestProxyHandler(t, nil)
+	handler.policy = proxyPolicyFunc(func(_ context.Context, op *pb.Op) (bool, error) {
+		op.GetSource().Identifier = "https://mirror.example.com/file"
+		op.GetSource().Attrs = map[string]string{
+			pb.AttrHTTPChecksum: "sha256:6e4b94fc270e708e1068be28bd3551dc6917a4fc5a61293d51bb36e6b75c4b53",
+		}
+		return true, nil
+	})
+
+	_, err := handler.check(t.Context(), http.MethodGet, "https://example.com/file")
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "proxy conversion only supports URL updates")
+}
+
+type proxyPolicyFunc func(context.Context, *pb.Op) (bool, error)
+
+func (f proxyPolicyFunc) Evaluate(ctx context.Context, op *pb.Op) (bool, error) {
+	return f(ctx, op)
+}
+
+type enginePolicyEvaluator struct {
+	engine *sourcepolicy.Engine
+}
+
+func (e enginePolicyEvaluator) Evaluate(ctx context.Context, op *pb.Op) (bool, error) {
+	return e.engine.Evaluate(ctx, op.GetSource())
+}
+
 func newTestProxyHandler(t *testing.T, capture *network.ProxyCapture) *proxyHandler {
 	t.Helper()
 	tr := http.DefaultTransport.(*http.Transport).Clone()