From 380abbc8d7328f5ba17a3fbeb44b262480c24f92 Mon Sep 17 00:00:00 2001 From: evilgensec Date: Tue, 12 May 2026 22:11:41 +0545 Subject: [PATCH] dockerutil: block SSRF in remote.Image via private-IP dialer The pin of github.com/google/go-containerregistry is from 2022-02 and predates the upstream Bearer-realm SSRF fix (validateRealmURL, google/go-containerregistry#2243). Pre-fix go-containerregistry follows the realm URL from a WWW-Authenticate Bearer challenge verbatim and sends Basic auth from the keychain to it, so a tenant who points an App/Build/SourcePackage at an attacker-controlled registry can drive the kf controller to fetch the cluster's GCE/EC2 metadata endpoint and leak the workload identity token to the attacker on the next request. A clean dep bump pulls invasive OpenTelemetry v0.20 to v1.x, k8s.io, knative.dev, and tekton updates and currently fails `go mod tidy` on removed-package paths. This change adds an SSRF-protected http.Transport in pkg/dockerutil and wires it into every remote.Image call site: - cmd/build-helpers/extract.go - cmd/setup-buildpack-build/main.go - pkg/sourceimage/download.go - pkg/reconciler/task/reconciler.go - pkg/reconciler/appstartcommand/reconciler.go The protection runs as a net.Dialer Control hook after DNS resolution, so it is DNS-rebinding safe. Blocked ranges: loopback, link-local (unicast and multicast), interface-local multicast, all multicast, unspecified, RFC1918, ULA, and RFC6598 CGNAT (100.64.0.0/10). Public addresses pass through. A follow-up PR can still bump go-containerregistry once the rest of the dep tree is ready; this transport remains useful as defense in depth because it also covers the Basic-auth-to-attacker leak when only the initial registry hostname is attacker-controlled (the upstream fix only validates the realm URL, not the registry itself). --- cmd/build-helpers/extract.go | 2 +- cmd/setup-buildpack-build/main.go | 2 +- pkg/dockerutil/ssrfprotect.go | 102 ++++++++++++++++ pkg/dockerutil/ssrfprotect_test.go | 116 +++++++++++++++++++ pkg/reconciler/appstartcommand/reconciler.go | 2 +- pkg/reconciler/task/reconciler.go | 2 +- pkg/sourceimage/download.go | 3 +- 7 files changed, 224 insertions(+), 5 deletions(-) create mode 100644 pkg/dockerutil/ssrfprotect.go create mode 100644 pkg/dockerutil/ssrfprotect_test.go diff --git a/cmd/build-helpers/extract.go b/cmd/build-helpers/extract.go index b5c22f6d4..e3b5e1ed3 100644 --- a/cmd/build-helpers/extract.go +++ b/cmd/build-helpers/extract.go @@ -168,7 +168,7 @@ func extractSourceImage(sourceImage, sourcePath, targetPath string) error { return err } - image, err := remote.Image(imageRef, dockerutil.GetAuthKeyChain()) + image, err := remote.Image(imageRef, dockerutil.GetAuthKeyChain(), dockerutil.SafeRemoteTransport()) if err != nil { return err } diff --git a/cmd/setup-buildpack-build/main.go b/cmd/setup-buildpack-build/main.go index 5bfbd3d22..f13ff8ed8 100644 --- a/cmd/setup-buildpack-build/main.go +++ b/cmd/setup-buildpack-build/main.go @@ -139,7 +139,7 @@ func getBuildUser(builderImage string) (uid, gid int, err error) { return 0, 0, err } - image, err := remote.Image(imageRef, remote.WithAuthFromKeychain(authn.DefaultKeychain)) + image, err := remote.Image(imageRef, remote.WithAuthFromKeychain(authn.DefaultKeychain), dockerutil.SafeRemoteTransport()) if err != nil { return 0, 0, err } diff --git a/pkg/dockerutil/ssrfprotect.go b/pkg/dockerutil/ssrfprotect.go new file mode 100644 index 000000000..bfb5f047e --- /dev/null +++ b/pkg/dockerutil/ssrfprotect.go @@ -0,0 +1,102 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package dockerutil + +import ( + "fmt" + "net" + "net/http" + "syscall" + "time" + + "github.com/google/go-containerregistry/pkg/v1/remote" +) + +// SafeRemoteTransport returns a remote.Option that installs an HTTP transport +// which refuses to dial private, loopback, link-local, multicast, unspecified, +// or RFC6598 (CGNAT) addresses. It is intended to be passed to every +// remote.Image / remote.Get / remote.Index call that operates on a +// tenant-controlled image reference. +// +// The transport blocks SSRF via attacker-controlled WWW-Authenticate Bearer +// realm URLs (the same class of bug fixed upstream in +// google/go-containerregistry#2243) and via DNS rebinding, because the address +// check runs in the dialer Control hook after DNS resolution and before the +// kernel connects. +// +// The kf pin of go-containerregistry predates the upstream fix by ~4 years and +// a clean dep bump pulls invasive OpenTelemetry / k8s.io / knative / tekton +// updates. This file provides defense-in-depth without that upgrade. +func SafeRemoteTransport() remote.Option { + return remote.WithTransport(SafeTransport()) +} + +// SafeTransport returns an *http.Transport identical to http.DefaultTransport +// except that its Dialer rejects connections to non-public IP addresses. +func SafeTransport() *http.Transport { + dialer := &net.Dialer{ + Timeout: 30 * time.Second, + KeepAlive: 30 * time.Second, + Control: rejectInternalAddress, + } + return &http.Transport{ + Proxy: http.ProxyFromEnvironment, + DialContext: dialer.DialContext, + ForceAttemptHTTP2: true, + MaxIdleConns: 100, + IdleConnTimeout: 90 * time.Second, + TLSHandshakeTimeout: 10 * time.Second, + ExpectContinueTimeout: 1 * time.Second, + } +} + +// rejectInternalAddress is a Dialer.Control hook. The kernel has already +// performed DNS resolution by this point, so the address argument contains an +// IP literal (not a hostname). This makes the check DNS-rebinding safe. +func rejectInternalAddress(network, address string, _ syscall.RawConn) error { + host, _, err := net.SplitHostPort(address) + if err != nil { + return fmt.Errorf("kf: cannot parse dial address %q: %w", address, err) + } + ip := net.ParseIP(host) + if ip == nil { + return fmt.Errorf("kf: dial address %q is not an IP literal", host) + } + if isBlockedIP(ip) { + return fmt.Errorf("kf: refused to dial blocked address %s (%s/%s)", ip, network, address) + } + return nil +} + +// isBlockedIP reports whether ip is in a range that should never be the target +// of a registry HTTP request driven by a tenant-supplied image reference. +func isBlockedIP(ip net.IP) bool { + if ip.IsLoopback() || + ip.IsLinkLocalUnicast() || + ip.IsLinkLocalMulticast() || + ip.IsInterfaceLocalMulticast() || + ip.IsMulticast() || + ip.IsUnspecified() || + ip.IsPrivate() { + return true + } + // RFC6598 CGNAT 100.64.0.0/10. Not covered by IsPrivate(). + if v4 := ip.To4(); v4 != nil { + if v4[0] == 100 && v4[1] >= 64 && v4[1] <= 127 { + return true + } + } + return false +} diff --git a/pkg/dockerutil/ssrfprotect_test.go b/pkg/dockerutil/ssrfprotect_test.go new file mode 100644 index 000000000..3a4336df1 --- /dev/null +++ b/pkg/dockerutil/ssrfprotect_test.go @@ -0,0 +1,116 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package dockerutil + +import ( + "context" + "net" + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +func TestIsBlockedIP(t *testing.T) { + cases := []struct { + ip string + blocked bool + }{ + {"127.0.0.1", true}, + {"127.5.5.5", true}, + {"169.254.169.254", true}, // GCE / EC2 / Azure metadata + {"169.254.170.2", true}, // ECS metadata + {"fe80::1", true}, // IPv6 link-local + {"::1", true}, // IPv6 loopback + {"::", true}, // unspecified + {"0.0.0.0", true}, // unspecified + {"10.0.0.1", true}, // RFC1918 + {"10.255.255.254", true}, + {"172.16.0.1", true}, // RFC1918 + {"172.31.255.254", true}, + {"192.168.0.1", true}, // RFC1918 + {"fc00::1", true}, // ULA + {"100.64.0.1", true}, // CGNAT (RFC6598) + {"100.127.255.254", true}, + {"224.0.0.1", true}, // multicast + {"ff02::1", true}, // IPv6 multicast + {"1.1.1.1", false}, + {"8.8.8.8", false}, + {"172.15.0.1", false}, // just outside RFC1918 lower bound + {"172.32.0.1", false}, // just outside RFC1918 upper bound + {"100.63.255.254", false}, // just outside CGNAT lower bound + {"100.128.0.1", false}, // just outside CGNAT upper bound + {"2001:4860:4860::8888", false}, // Google Public DNS v6 + } + for _, tc := range cases { + ip := net.ParseIP(tc.ip) + if ip == nil { + t.Errorf("net.ParseIP(%q) returned nil", tc.ip) + continue + } + if got := isBlockedIP(ip); got != tc.blocked { + t.Errorf("isBlockedIP(%s) = %v, want %v", tc.ip, got, tc.blocked) + } + } +} + +func TestSafeTransport_BlocksLoopback(t *testing.T) { + // Spin up a localhost server. SafeTransport should refuse to connect to it. + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Fatalf("SafeTransport must not reach loopback handler, got request: %s %s", r.Method, r.URL) + })) + defer srv.Close() + + client := &http.Client{Transport: SafeTransport()} + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, srv.URL, nil) + if err != nil { + t.Fatalf("NewRequest: %v", err) + } + resp, err := client.Do(req) + if err == nil { + resp.Body.Close() + t.Fatalf("expected SafeTransport to refuse loopback dial, got status %s", resp.Status) + } + if !strings.Contains(err.Error(), "refused to dial blocked address") { + t.Errorf("expected blocked-address error, got: %v", err) + } +} + +func TestSafeTransport_AllowsPublicHost(t *testing.T) { + // Synthesize a request whose host resolves to a public IP. We cannot + // actually call out from a hermetic test environment, so we verify only + // that the dialer Control hook accepts the post-resolution address. + cases := []string{"1.1.1.1:443", "8.8.8.8:53", "[2001:4860:4860::8888]:53"} + for _, addr := range cases { + if err := rejectInternalAddress("tcp", addr, nil); err != nil { + t.Errorf("rejectInternalAddress(tcp, %q) = %v, want nil", addr, err) + } + } +} + +func TestRejectInternalAddress_MalformedAddress(t *testing.T) { + if err := rejectInternalAddress("tcp", "not-an-addr", nil); err == nil { + t.Errorf("expected error for malformed address, got nil") + } +} + +func TestRejectInternalAddress_HostnameInsteadOfIP(t *testing.T) { + // The kernel resolves DNS before calling the Control hook, so the address + // should always be an IP literal. If for some reason it is not, reject it + // rather than allow an unverified connection. + if err := rejectInternalAddress("tcp", "evil.example.com:443", nil); err == nil { + t.Errorf("expected error for hostname-in-address, got nil") + } +} diff --git a/pkg/reconciler/appstartcommand/reconciler.go b/pkg/reconciler/appstartcommand/reconciler.go index 4ffcf0100..88c9f9ed7 100644 --- a/pkg/reconciler/appstartcommand/reconciler.go +++ b/pkg/reconciler/appstartcommand/reconciler.go @@ -168,7 +168,7 @@ func fetchContainerCommand(image string) (*containerregistryv1.ConfigFile, error return nil, err } - img, err := remote.Image(imageRef, dockerutil.GetAuthKeyChain()) + img, err := remote.Image(imageRef, dockerutil.GetAuthKeyChain(), dockerutil.SafeRemoteTransport()) if err != nil { return nil, err } diff --git a/pkg/reconciler/task/reconciler.go b/pkg/reconciler/task/reconciler.go index 72a922c43..6ca5d96bc 100644 --- a/pkg/reconciler/task/reconciler.go +++ b/pkg/reconciler/task/reconciler.go @@ -302,7 +302,7 @@ func (r *Reconciler) fetchContainerCommand(app *v1alpha1.App) ([]string, error) return nil, err } - img, err := remote.Image(imageRef, dockerutil.GetAuthKeyChain()) + img, err := remote.Image(imageRef, dockerutil.GetAuthKeyChain(), dockerutil.SafeRemoteTransport()) if err != nil { return nil, err } diff --git a/pkg/sourceimage/download.go b/pkg/sourceimage/download.go index 4940cb2ce..1002e4845 100644 --- a/pkg/sourceimage/download.go +++ b/pkg/sourceimage/download.go @@ -23,6 +23,7 @@ import ( "github.com/google/go-containerregistry/pkg/v1/remote" "github.com/google/kf/v2/pkg/apis/kf/v1alpha1" v1alpha1lister "github.com/google/kf/v2/pkg/client/kf/listers/kf/v1alpha1" + "github.com/google/kf/v2/pkg/dockerutil" "github.com/pkg/errors" ) @@ -43,7 +44,7 @@ func Download(sourcePackageLister v1alpha1lister.SourcePackageLister, namespace, if err != nil { return nil, errors.Wrap(err, fmt.Sprintf("failed to parse image ref %q", imageName)) } - image, err := remote.Image(imageRef, remote.WithAuthFromKeychain(Keychain())) + image, err := remote.Image(imageRef, remote.WithAuthFromKeychain(Keychain()), dockerutil.SafeRemoteTransport()) if err != nil { return nil, errors.Wrap(err, "failed to get image") }