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

fix(*): localhost exposed application shouldn't be reachable #4750

Merged
merged 21 commits into from
Aug 12, 2022
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions UPGRADE.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ changed to agree with the other `image` values.
* The `/versions` endpoint was removed. This is not something that was reliable enough and version compatibility
is checked inside the DP
* We are deprecating `kuma.io/builtindns` and `kuma.io/builtindnsport` annotations in favour of the clearer `kuma.io/builtin-dns` and `kuma.io/builtin-dns-port`. The behavior of the new annotations is unchanged but you should migrate (a warning is present on the log if you are using the deprecated version).
* Applications that are binding to `localhost` won't be reachable anymore. We are changing the default inbound cluster that was always pointing to `localhost` to `DataplaneIP`. Before upgrade check if your applications are listening on `localhost` and should be exposed. In this case change address on which application listen to `0.0.0.0`. Other option is to define `dataplane.networking.inbound[].serviceAddress` to the address which service is binding. Another way is to disable the new behavior by setting `kuma-cp` configuration `KUMA_DEFAULTS_ENABLE_LOCALHOST_INBOUND_CLUSTERS` to `true` or `defaults.enableLocalhostInboundClusters` to `true`. The last option is going to be removed in further versions.

## Upgrade to `1.7.x`

Expand Down
11 changes: 10 additions & 1 deletion api/mesh/v1alpha1/dataplane_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ import (
"github.com/pkg/errors"
)

// To remove in the future.
// TODO: https://github.com/kumahq/kuma/issues/4772
var EnableLocalhostInboundClusters bool
lukidzi marked this conversation as resolved.
Show resolved Hide resolved

const (
// Mandatory tag that has a reserved meaning in Kuma.
ServiceTag = "kuma.io/service"
Expand Down Expand Up @@ -158,7 +162,12 @@ func (n *Dataplane_Networking) ToInboundInterface(inbound *Dataplane_Networking_
if inbound.ServiceAddress != "" {
iface.WorkloadIP = inbound.ServiceAddress
} else {
iface.WorkloadIP = "127.0.0.1"
switch EnableLocalhostInboundClusters {
case true:
iface.WorkloadIP = "127.0.0.1"
default:
iface.WorkloadIP = iface.DataplaneIP
}
}
if inbound.ServicePort != 0 {
iface.WorkloadPort = inbound.ServicePort
Expand Down
2 changes: 1 addition & 1 deletion api/mesh/v1alpha1/dataplane_helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ var _ = Describe("Dataplane_Networking", func() {
},
},
expected: []InboundInterface{
{DataplaneAdvertisedIP: "192.168.0.1", DataplaneIP: "192.168.0.1", DataplanePort: 80, WorkloadIP: "127.0.0.1", WorkloadPort: 80},
{DataplaneAdvertisedIP: "192.168.0.1", DataplaneIP: "192.168.0.1", DataplanePort: 80, WorkloadIP: "192.168.0.1", WorkloadPort: 80},
{DataplaneAdvertisedIP: "192.168.0.2", DataplaneIP: "192.168.0.2", DataplanePort: 443, WorkloadIP: "192.168.0.3", WorkloadPort: 8443},
},
}),
Expand Down
2 changes: 2 additions & 0 deletions app/kuma-dp/cmd/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,7 @@ func getApplicationsToScrape(kumaSidecarConfiguration *types.KumaSidecarConfigur
if kumaSidecarConfiguration != nil {
for _, item := range kumaSidecarConfiguration.Metrics.Aggregate {
applicationsToScrape = append(applicationsToScrape, metrics.ApplicationToScrape{
Address: item.Address,
Name: item.Name,
Path: item.Path,
Port: item.Port,
Expand All @@ -306,6 +307,7 @@ func getApplicationsToScrape(kumaSidecarConfiguration *types.KumaSidecarConfigur
applicationsToScrape = append(applicationsToScrape, metrics.ApplicationToScrape{
Name: "envoy",
Path: "/stats",
Address: "127.0.0.1",
Port: envoyAdminPort,
QueryModifier: metrics.AddPrometheusFormat,
Mutator: metrics.MergeClusters,
Expand Down
28 changes: 16 additions & 12 deletions app/kuma-dp/pkg/dataplane/envoy/remote_bootstrap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -300,14 +300,16 @@ var _ = Describe("Remote Bootstrap", func() {
Metrics: types.MetricsConfiguration{
Aggregate: []types.Aggregate{
{
Name: "my-app",
Port: 123,
Path: "/stats",
Address: "127.0.0.1",
Name: "my-app",
Port: 123,
Path: "/stats",
},
{
Name: "my-app-2",
Port: 12345,
Path: "/stats/2",
Address: "1.2.3.4",
Name: "my-app-2",
Port: 12345,
Path: "/stats/2",
},
},
},
Expand Down Expand Up @@ -343,13 +345,15 @@ var _ = Describe("Remote Bootstrap", func() {
Expect(bootstrap).ToNot(BeNil())
Expect(len(kumaSidecarConfiguration.Metrics.Aggregate)).To(Equal(2))
Expect(kumaSidecarConfiguration.Metrics.Aggregate).To(ContainElements(types.Aggregate{
Name: "my-app",
Port: 123,
Path: "/stats",
Address: "127.0.0.1",
Name: "my-app",
Port: 123,
Path: "/stats",
}, types.Aggregate{
Name: "my-app-2",
Port: 12345,
Path: "/stats/2",
Address: "1.2.3.4",
Name: "my-app-2",
Port: 12345,
Path: "/stats/2",
}))

})
Expand Down
39 changes: 30 additions & 9 deletions app/kuma-dp/pkg/dataplane/metrics/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"net/http"
"net/url"
"os"
"strconv"
"sync"

"github.com/pkg/errors"
Expand All @@ -20,6 +21,10 @@ import (
"github.com/kumahq/kuma/pkg/xds/envoy"
)

var upstreamLocalAddress = &net.TCPAddr{IP: net.ParseIP("127.0.0.6")}

const IPv4Loopback = "127.0.0.1"

var prometheusRequestHeaders = []string{"accept", "accept-encoding", "user-agent", "x-prometheus-scrape-timeout-seconds"}
var logger = core.Log.WithName("metrics-hijacker")

Expand All @@ -40,22 +45,33 @@ func AddPrometheusFormat(queryParameters url.Values) string {

type ApplicationToScrape struct {
Name string
Address string
Path string
Port uint32
QueryModifier QueryParametersModifier
Mutator MetricsMutator
}

type Hijacker struct {
socketPath string
httpClient http.Client
applicationsToScrape []ApplicationToScrape
socketPath string
httpClient http.Client
upstreamOverrideHttpClient http.Client
applicationsToScrape []ApplicationToScrape
}

func New(dataplane kumadp.Dataplane, applicationsToScrape []ApplicationToScrape) *Hijacker {
// we need this in case of not localhost requests, it returns fast in iptabels
d := &net.Dialer{
LocalAddr: upstreamLocalAddress,
}
return &Hijacker{
socketPath: envoy.MetricsHijackerSocketName(dataplane.Name, dataplane.Mesh),
httpClient: http.Client{},
socketPath: envoy.MetricsHijackerSocketName(dataplane.Name, dataplane.Mesh),
httpClient: http.Client{},
upstreamOverrideHttpClient: http.Client{
Transport: &http.Transport{
DialContext: d.DialContext,
},
},
applicationsToScrape: applicationsToScrape,
}
}
Expand Down Expand Up @@ -110,10 +126,10 @@ func (s *Hijacker) Start(stop <-chan struct{}) error {

// We pass QueryParameters only for the specific application.
// Currently, we only support QueryParameters for Envoy metrics.
func rewriteMetricsURL(path string, port uint32, queryModifier QueryParametersModifier, in *url.URL) string {
func rewriteMetricsURL(address string, port uint32, path string, queryModifier QueryParametersModifier, in *url.URL) string {
u := url.URL{
Scheme: "http",
Host: fmt.Sprintf("127.0.0.1:%d", port),
Host: net.JoinHostPort(address, strconv.FormatUint(uint64(port), 10)),
Path: path,
RawQuery: queryModifier(in.Query()),
}
Expand Down Expand Up @@ -157,14 +173,19 @@ func (s *Hijacker) ServeHTTP(writer http.ResponseWriter, req *http.Request) {
}

func (s *Hijacker) getStats(ctx context.Context, initReq *http.Request, app ApplicationToScrape) []byte {
req, err := http.NewRequest("GET", rewriteMetricsURL(app.Path, app.Port, app.QueryModifier, initReq.URL), nil)
req, err := http.NewRequest("GET", rewriteMetricsURL(app.Address, app.Port, app.Path, app.QueryModifier, initReq.URL), nil)
if err != nil {
logger.Error(err, "failed to create request")
return nil
}
s.passRequestHeaders(req.Header, initReq.Header)
req = req.WithContext(ctx)
resp, err := s.httpClient.Do(req)
var resp *http.Response
if app.Address != IPv4Loopback {
resp, err = s.upstreamOverrideHttpClient.Do(req)
} else {
resp, err = s.httpClient.Do(req)
}
if err != nil {
logger.Error(err, "failed call", "name", app.Name, "path", app.Path, "port", app.Port)
return nil
Expand Down
10 changes: 7 additions & 3 deletions app/kuma-dp/pkg/dataplane/metrics/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
var _ = Describe("Rewriting the metrics URL", func() {
type testCase struct {
input string
address string
adminPort uint32
expected string
queryModifier QueryParametersModifier
Expand All @@ -18,21 +19,24 @@ var _ = Describe("Rewriting the metrics URL", func() {
func(given testCase) {
u, err := url.Parse(given.input)
Expect(err).ToNot(HaveOccurred())
Expect(rewriteMetricsURL("/stats", given.adminPort, given.queryModifier, u)).Should(Equal(given.expected))
Expect(rewriteMetricsURL(given.address, given.adminPort, "/stats", given.queryModifier, u)).Should(Equal(given.expected))
},
Entry("use the admin port", testCase{
address: "1.2.3.4",
input: "http://foo/bar",
adminPort: 99,
expected: "http://127.0.0.1:99/stats?format=prometheus",
expected: "http://1.2.3.4:99/stats?format=prometheus",
queryModifier: AddPrometheusFormat,
}),
Entry("preserve query parameters", testCase{
address: "1.2.3.4",
input: "http://foo/bar?one=two&three=four&filter=test_.*&usedonly",
adminPort: 80,
expected: "http://127.0.0.1:80/stats?filter=test_.%2A&format=prometheus&one=two&three=four&usedonly=",
expected: "http://1.2.3.4:80/stats?filter=test_.%2A&format=prometheus&one=two&three=four&usedonly=",
queryModifier: AddPrometheusFormat,
}),
Entry("remove query parameters", testCase{
address: "127.0.0.1",
input: "http://foo/bar?one=two&three=four",
adminPort: 80,
expected: "http://127.0.0.1:80/stats",
Expand Down
8 changes: 7 additions & 1 deletion pkg/config/app/kuma-cp/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ var _ config.Config = &Defaults{}

type Defaults struct {
SkipMeshCreation bool `yaml:"skipMeshCreation" envconfig:"kuma_defaults_skip_mesh_creation"`
// If true, instead of providing inbound clusters with address of dataplane, generates cluster with localhost.
// Enabled can cause security threat by exposing application listing on localhost. This configuration is going to
// removed.
// TODO: https://github.com/kumahq/kuma/issues/4772
EnableLocalhostInboundClusters bool `yaml:"enableLocalhostInboundClusters" envconfig:"kuma_defaults_enable_localhost_inbound_clusters"`
}

func (d *Defaults) Sanitize() {
Expand Down Expand Up @@ -172,7 +177,8 @@ var DefaultConfig = func() Config {
BootstrapServer: bootstrap.DefaultBootstrapServerConfig(),
Runtime: runtime.DefaultRuntimeConfig(),
Defaults: &Defaults{
SkipMeshCreation: false,
SkipMeshCreation: false,
EnableLocalhostInboundClusters: false,
},
Metrics: &Metrics{
Dataplane: &DataplaneMetrics{
Expand Down
1 change: 1 addition & 0 deletions pkg/config/app/kuma-cp/kuma-cp.defaults.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,7 @@ runtime:
# Default Kuma entities configuration
defaults:
skipMeshCreation: false # ENV: KUMA_DEFAULTS_SKIP_MESH_CREATION
enableLocalhostInboundClusters: false #ENV: KUMA_DEFAULTS_ENABLE_LOCALHOST_INBOUND_CLUSTERS

# Metrics configuration
metrics:
Expand Down
3 changes: 3 additions & 0 deletions pkg/config/loader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,7 @@ var _ = Describe("Config loader", func() {
Expect(cfg.Multizone.Zone.KDS.MaxMsgSize).To(Equal(uint32(2)))

Expect(cfg.Defaults.SkipMeshCreation).To(BeTrue())
Expect(cfg.Defaults.EnableLocalhostInboundClusters).To(BeTrue())

Expect(cfg.Diagnostics.ServerPort).To(Equal(uint32(5003)))
Expect(cfg.Diagnostics.DebugEndpoints).To(BeTrue())
Expand Down Expand Up @@ -431,6 +432,7 @@ dnsServer:
serviceVipEnabled: false
defaults:
skipMeshCreation: true
enableLocalhostInboundClusters: true
diagnostics:
serverPort: 5003
debugEndpoints: true
Expand Down Expand Up @@ -611,6 +613,7 @@ experimental:
"KUMA_MULTIZONE_ZONE_KDS_MAX_MSG_SIZE": "2",
"KUMA_MULTIZONE_GLOBAL_KDS_ZONE_INSIGHT_FLUSH_INTERVAL": "5s",
"KUMA_DEFAULTS_SKIP_MESH_CREATION": "true",
"KUMA_DEFAULTS_ENABLE_LOCALHOST_INBOUND_CLUSTERS": "true",
"KUMA_DIAGNOSTICS_SERVER_PORT": "5003",
"KUMA_DIAGNOSTICS_DEBUG_ENDPOINTS": "true",
"KUMA_XDS_SERVER_DATAPLANE_STATUS_FLUSH_INTERVAL": "7s",
Expand Down
4 changes: 4 additions & 0 deletions pkg/core/bootstrap/bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

"github.com/pkg/errors"

mesh_proto "github.com/kumahq/kuma/api/mesh/v1alpha1"
"github.com/kumahq/kuma/pkg/api-server/customization"
kuma_cp "github.com/kumahq/kuma/pkg/config/app/kuma-cp"
config_core "github.com/kumahq/kuma/pkg/config/core"
Expand Down Expand Up @@ -125,6 +126,9 @@ func buildRuntime(appCtx context.Context, cfg kuma_cp.Config) (core_runtime.Runt
))
}

// The setting should be removed, and there is no easy way to set it without breaking most of the code
mesh_proto.EnableLocalhostInboundClusters = builder.Config().Defaults.EnableLocalhostInboundClusters
michaelbeaumont marked this conversation as resolved.
Show resolved Hide resolved

builder.WithAccess(core_runtime.Access{
ResourceAccess: resources_access.NewAdminResourceAccess(builder.Config().Access.Static.AdminResources),
DataplaneTokenAccess: tokens_access.NewStaticGenerateDataplaneTokenAccess(builder.Config().Access.Static.GenerateDPToken),
Expand Down
18 changes: 11 additions & 7 deletions pkg/core/faultinjections/matcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,8 @@ var _ = Describe("Match", func() {
Entry("1 inbound dataplane, 2 policies", testCase{
dataplane: dataplaneWithInboundsFunc([]*mesh_proto.Dataplane_Networking_Inbound{
{
ServicePort: 8080,
ServiceAddress: "1.2.3.4",
ServicePort: 8080,
Tags: map[string]string{
"service": "web",
"version": "0.1",
Expand Down Expand Up @@ -128,7 +129,7 @@ var _ = Describe("Match", func() {
},
expected: core_xds.FaultInjectionMap{
mesh_proto.InboundInterface{
WorkloadIP: "127.0.0.1",
WorkloadIP: "1.2.3.4",
WorkloadPort: 8080,
}: {
policyWithDestinationsFunc("fi2", time.Unix(1, 0), []*mesh_proto.Selector{
Expand All @@ -144,7 +145,8 @@ var _ = Describe("Match", func() {
Entry("should apply policy only to the first inbound", testCase{
dataplane: dataplaneWithInboundsFunc([]*mesh_proto.Dataplane_Networking_Inbound{
{
ServicePort: 8080,
ServiceAddress: "1.2.3.4",
ServicePort: 8080,
Tags: map[string]string{
"service": "web",
"version": "0.1",
Expand All @@ -153,7 +155,8 @@ var _ = Describe("Match", func() {
},
},
{
ServicePort: 8081,
ServiceAddress: "1.2.3.4",
ServicePort: 8081,
Tags: map[string]string{
"service": "web-api",
"version": "0.1.2",
Expand All @@ -174,7 +177,7 @@ var _ = Describe("Match", func() {
},
expected: core_xds.FaultInjectionMap{
mesh_proto.InboundInterface{
WorkloadIP: "127.0.0.1",
WorkloadIP: "1.2.3.4",
WorkloadPort: 8081,
}: {
policyWithDestinationsFunc("fi1", time.Unix(1, 0), []*mesh_proto.Selector{
Expand All @@ -191,7 +194,8 @@ var _ = Describe("Match", func() {
Entry("should select all policies matched for the inbound", testCase{
dataplane: dataplaneWithInboundsFunc([]*mesh_proto.Dataplane_Networking_Inbound{
{
ServicePort: 8080,
ServiceAddress: "1.2.3.4",
ServicePort: 8080,
Tags: map[string]string{
"service": "web",
"version": "0.1",
Expand Down Expand Up @@ -237,7 +241,7 @@ var _ = Describe("Match", func() {
},
expected: core_xds.FaultInjectionMap{
mesh_proto.InboundInterface{
WorkloadIP: "127.0.0.1",
WorkloadIP: "1.2.3.4",
WorkloadPort: 8080,
}: {
policyWithDestinationsFunc("fi1", time.Unix(1, 0), []*mesh_proto.Selector{
Expand Down
4 changes: 2 additions & 2 deletions pkg/core/permissions/matcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,8 @@ var _ = Describe("Match", func() {
},
},
expected: map[mesh_proto.InboundInterface]string{
{DataplaneAdvertisedIP: "192.168.0.1", DataplaneIP: "192.168.0.1", WorkloadIP: "127.0.0.1", WorkloadPort: 8081, DataplanePort: 8080}: "more-specific-kong-to-web",
{DataplaneAdvertisedIP: "192.168.0.1", DataplaneIP: "192.168.0.1", WorkloadIP: "127.0.0.1", WorkloadPort: 1234, DataplanePort: 1234}: "metrics",
{DataplaneAdvertisedIP: "192.168.0.1", DataplaneIP: "192.168.0.1", WorkloadIP: "192.168.0.1", WorkloadPort: 8081, DataplanePort: 8080}: "more-specific-kong-to-web",
{DataplaneAdvertisedIP: "192.168.0.1", DataplaneIP: "192.168.0.1", WorkloadIP: "192.168.0.1", WorkloadPort: 1234, DataplanePort: 1234}: "metrics",
},
}),
)
Expand Down
Loading