From 5fe16cd487c2acb3b390cfa4e508b9bcf1ca7c0d Mon Sep 17 00:00:00 2001
From: Vishal Choudhary <sendtovishalchoudhary@gmail.com>
Date: Tue, 21 Nov 2023 15:31:51 +0530
Subject: [PATCH] feat: add checks for max response size in API Call (#8957)
* feat: add checks for max response size in API Call GET request
Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>
* fix: tests
Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>
* fix: added changes suggested by jim
Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>
* cleanup
Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>
---------
Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>
Co-authored-by: Jim Bugwadia <jim@nirmata.com>
---
cmd/background-controller/main.go | 10 +++++---
cmd/internal/engine.go | 4 ++-
cmd/kyverno/main.go | 4 +++
cmd/reports-controller/main.go | 4 +++
pkg/engine/apicall/apiCall.go | 27 ++++++++++++++++++--
pkg/engine/apicall/apiCall_test.go | 19 +++++++++-----
pkg/engine/context/loaders/apicall.go | 5 +++-
pkg/engine/factories/contextloaderfactory.go | 16 +++++++++---
8 files changed, 71 insertions(+), 18 deletions(-)
diff --git a/cmd/background-controller/main.go b/cmd/background-controller/main.go
index e4f482244f..ca86556a5c 100644
--- a/cmd/background-controller/main.go
+++ b/cmd/background-controller/main.go
@@ -17,6 +17,7 @@ import (
"github.com/kyverno/kyverno/pkg/config"
policymetricscontroller "github.com/kyverno/kyverno/pkg/controllers/metrics/policy"
engineapi "github.com/kyverno/kyverno/pkg/engine/api"
+ "github.com/kyverno/kyverno/pkg/engine/apicall"
"github.com/kyverno/kyverno/pkg/engine/jmespath"
"github.com/kyverno/kyverno/pkg/event"
"github.com/kyverno/kyverno/pkg/leaderelection"
@@ -82,14 +83,16 @@ func createrLeaderControllers(
func main() {
var (
- genWorkers int
- maxQueuedEvents int
- omitEvents string
+ genWorkers int
+ maxQueuedEvents int
+ omitEvents string
+ maxAPICallResponseLength int64
)
flagset := flag.NewFlagSet("updaterequest-controller", flag.ExitOnError)
flagset.IntVar(&genWorkers, "genWorkers", 10, "Workers for the background controller.")
flagset.IntVar(&maxQueuedEvents, "maxQueuedEvents", 1000, "Maximum events to be queued.")
flagset.StringVar(&omitEvents, "omit-events", "", "Set this flag to a comma sperated list of PolicyViolation, PolicyApplied, PolicyError, PolicySkipped to disable events, e.g. --omit-events=PolicyApplied,PolicyViolation")
+ flagset.Int64Var(&maxAPICallResponseLength, "maxAPICallResponseLength", 2*1000*1000, "Maximum allowed response size from API Calls. A value of 0 bypasses checks (not recommended).")
// config
appConfig := internal.NewConfiguration(
@@ -161,6 +164,7 @@ func main() {
setup.KubeClient,
setup.KyvernoClient,
setup.RegistrySecretLister,
+ apicall.NewAPICallConfiguration(maxAPICallResponseLength),
)
// start informers and wait for cache sync
if !internal.StartInformersAndWaitForCacheSync(signalCtx, setup.Logger, kyvernoInformer) {
diff --git a/cmd/internal/engine.go b/cmd/internal/engine.go
index 417188dfb8..0e451e606b 100644
--- a/cmd/internal/engine.go
+++ b/cmd/internal/engine.go
@@ -13,6 +13,7 @@ import (
"github.com/kyverno/kyverno/pkg/engine"
"github.com/kyverno/kyverno/pkg/engine/adapters"
engineapi "github.com/kyverno/kyverno/pkg/engine/api"
+ "github.com/kyverno/kyverno/pkg/engine/apicall"
"github.com/kyverno/kyverno/pkg/engine/context/resolvers"
"github.com/kyverno/kyverno/pkg/engine/factories"
"github.com/kyverno/kyverno/pkg/engine/jmespath"
@@ -34,6 +35,7 @@ func NewEngine(
kubeClient kubernetes.Interface,
kyvernoClient versioned.Interface,
secretLister corev1listers.SecretNamespaceLister,
+ apiCallConfig apicall.APICallConfiguration,
) engineapi.Engine {
configMapResolver := NewConfigMapResolver(ctx, logger, kubeClient, 15*time.Minute)
exceptionsSelector := NewExceptionSelector(ctx, logger, kyvernoClient, 15*time.Minute)
@@ -46,7 +48,7 @@ func NewEngine(
adapters.Client(client),
factories.DefaultRegistryClientFactory(adapters.RegistryClient(rclient), secretLister),
ivCache,
- factories.DefaultContextLoaderFactory(configMapResolver),
+ factories.DefaultContextLoaderFactory(configMapResolver, factories.WithAPICallConfig(apiCallConfig)),
exceptionsSelector,
imageSignatureRepository,
)
diff --git a/cmd/kyverno/main.go b/cmd/kyverno/main.go
index f3b3ee9ef0..d588a68eea 100644
--- a/cmd/kyverno/main.go
+++ b/cmd/kyverno/main.go
@@ -25,6 +25,7 @@ import (
vapcontroller "github.com/kyverno/kyverno/pkg/controllers/validatingadmissionpolicy-generate"
webhookcontroller "github.com/kyverno/kyverno/pkg/controllers/webhook"
engineapi "github.com/kyverno/kyverno/pkg/engine/api"
+ "github.com/kyverno/kyverno/pkg/engine/apicall"
"github.com/kyverno/kyverno/pkg/event"
"github.com/kyverno/kyverno/pkg/informers"
"github.com/kyverno/kyverno/pkg/leaderelection"
@@ -217,6 +218,7 @@ func main() {
servicePort int
webhookServerPort int
backgroundServiceAccountName string
+ maxAPICallResponseLength int64
)
flagset := flag.NewFlagSet("kyverno", flag.ExitOnError)
flagset.BoolVar(&dumpPayload, "dumpPayload", false, "Set this flag to activate/deactivate debug mode.")
@@ -235,6 +237,7 @@ func main() {
flagset.StringVar(&backgroundServiceAccountName, "backgroundServiceAccountName", "", "Background service account name.")
flagset.StringVar(&caSecretName, "caSecretName", "", "Name of the secret containing CA.")
flagset.StringVar(&tlsSecretName, "tlsSecretName", "", "Name of the secret containing TLS pair.")
+ flagset.Int64Var(&maxAPICallResponseLength, "maxAPICallResponseLength", 10*1000*1000, "Configure the value of maximum allowed GET response size from API Calls")
// config
appConfig := internal.NewConfiguration(
internal.WithProfiling(),
@@ -361,6 +364,7 @@ func main() {
setup.KubeClient,
setup.KyvernoClient,
setup.RegistrySecretLister,
+ apicall.NewAPICallConfiguration(maxAPICallResponseLength),
)
// create non leader controllers
nonLeaderControllers, nonLeaderBootstrap := createNonLeaderControllers(
diff --git a/cmd/reports-controller/main.go b/cmd/reports-controller/main.go
index ba947c9e31..6b3ca7c7d9 100644
--- a/cmd/reports-controller/main.go
+++ b/cmd/reports-controller/main.go
@@ -19,6 +19,7 @@ import (
backgroundscancontroller "github.com/kyverno/kyverno/pkg/controllers/report/background"
resourcereportcontroller "github.com/kyverno/kyverno/pkg/controllers/report/resource"
engineapi "github.com/kyverno/kyverno/pkg/engine/api"
+ "github.com/kyverno/kyverno/pkg/engine/apicall"
"github.com/kyverno/kyverno/pkg/engine/jmespath"
"github.com/kyverno/kyverno/pkg/event"
"github.com/kyverno/kyverno/pkg/leaderelection"
@@ -191,6 +192,7 @@ func main() {
maxQueuedEvents int
omitEvents string
skipResourceFilters bool
+ maxAPICallResponseLength int64
)
flagset := flag.NewFlagSet("reports-controller", flag.ExitOnError)
flagset.BoolVar(&backgroundScan, "backgroundScan", true, "Enable or disable background scan.")
@@ -204,6 +206,7 @@ func main() {
flagset.IntVar(&maxQueuedEvents, "maxQueuedEvents", 1000, "Maximum events to be queued.")
flagset.StringVar(&omitEvents, "omit-events", "", "Set this flag to a comma separated list of PolicyViolation, PolicyApplied, PolicyError, PolicySkipped to disable events, e.g. --omit-events=PolicyApplied,PolicyViolation")
flagset.BoolVar(&skipResourceFilters, "skipResourceFilters", true, "If true, resource filters wont be considered.")
+ flagset.Int64Var(&maxAPICallResponseLength, "maxAPICallResponseLength", 2*1000*1000, "Maximum allowed response size from API Calls. A value of 0 bypasses checks (not recommended).")
// config
appConfig := internal.NewConfiguration(
internal.WithProfiling(),
@@ -271,6 +274,7 @@ func main() {
setup.KubeClient,
setup.KyvernoClient,
setup.RegistrySecretLister,
+ apicall.NewAPICallConfiguration(maxAPICallResponseLength),
)
// start informers and wait for cache sync
if !internal.StartInformersAndWaitForCacheSync(ctx, setup.Logger, kyvernoInformer) {
diff --git a/pkg/engine/apicall/apiCall.go b/pkg/engine/apicall/apiCall.go
index 2a13c0d4e3..288d10059a 100644
--- a/pkg/engine/apicall/apiCall.go
+++ b/pkg/engine/apicall/apiCall.go
@@ -26,6 +26,17 @@ type apiCall struct {
entry kyvernov1.ContextEntry
jsonCtx enginecontext.Interface
client ClientInterface
+ config APICallConfiguration
+}
+
+type APICallConfiguration struct {
+ maxAPICallResponseLength int64
+}
+
+func NewAPICallConfiguration(maxLen int64) APICallConfiguration {
+ return APICallConfiguration{
+ maxAPICallResponseLength: maxLen,
+ }
}
type ClientInterface interface {
@@ -38,6 +49,7 @@ func New(
entry kyvernov1.ContextEntry,
jsonCtx enginecontext.Interface,
client ClientInterface,
+ apiCallConfig APICallConfiguration,
) (*apiCall, error) {
if entry.APICall == nil {
return nil, fmt.Errorf("missing APICall in context entry %v", entry)
@@ -48,6 +60,7 @@ func New(
entry: entry,
jsonCtx: jsonCtx,
client: client,
+ config: apiCallConfig,
}, nil
}
@@ -129,8 +142,18 @@ func (a *apiCall) executeServiceCall(ctx context.Context, apiCall *kyvernov1.API
}
defer resp.Body.Close()
+ if a.config.maxAPICallResponseLength != 0 {
+ if resp.ContentLength <= 0 {
+ return nil, fmt.Errorf("content length header must be present.")
+ }
+ if resp.ContentLength > a.config.maxAPICallResponseLength {
+ return nil, fmt.Errorf("content length must be less than max response length of %d.", a.config.maxAPICallResponseLength)
+ }
+ }
+
+ reader := io.LimitReader(resp.Body, max(a.config.maxAPICallResponseLength, resp.ContentLength))
if resp.StatusCode < 200 || resp.StatusCode >= 300 {
- b, err := io.ReadAll(resp.Body)
+ b, err := io.ReadAll(reader)
if err == nil {
return nil, fmt.Errorf("HTTP %s: %s", resp.Status, string(b))
}
@@ -138,7 +161,7 @@ func (a *apiCall) executeServiceCall(ctx context.Context, apiCall *kyvernov1.API
return nil, fmt.Errorf("HTTP %s", resp.Status)
}
- body, err := io.ReadAll(resp.Body)
+ body, err := io.ReadAll(reader)
if err != nil {
return nil, fmt.Errorf("failed to read data from APICall %s: %w", a.entry.Name, err)
}
diff --git a/pkg/engine/apicall/apiCall_test.go b/pkg/engine/apicall/apiCall_test.go
index ba0f8f40f1..d9cc6a457c 100644
--- a/pkg/engine/apicall/apiCall_test.go
+++ b/pkg/engine/apicall/apiCall_test.go
@@ -16,7 +16,12 @@ import (
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
)
-var jp = jmespath.New(config.NewDefaultConfiguration(false))
+var (
+ jp = jmespath.New(config.NewDefaultConfiguration(false))
+ apiConfig = APICallConfiguration{
+ maxAPICallResponseLength: 1 * 1000 * 1000,
+ }
+)
func buildTestServer(responseData []byte) *httptest.Server {
mux := http.NewServeMux()
@@ -44,7 +49,7 @@ func Test_serviceGetRequest(t *testing.T) {
entry := kyvernov1.ContextEntry{}
ctx := enginecontext.NewContext(jp)
- _, err := New(logr.Discard(), jp, entry, ctx, nil)
+ _, err := New(logr.Discard(), jp, entry, ctx, nil, apiConfig)
assert.ErrorContains(t, err, "missing APICall")
entry.Name = "test"
@@ -54,19 +59,19 @@ func Test_serviceGetRequest(t *testing.T) {
},
}
- call, err := New(logr.Discard(), jp, entry, ctx, nil)
+ call, err := New(logr.Discard(), jp, entry, ctx, nil, apiConfig)
assert.NilError(t, err)
_, err = call.FetchAndLoad(context.TODO())
assert.ErrorContains(t, err, "invalid request type")
entry.APICall.Method = "GET"
- call, err = New(logr.Discard(), jp, entry, ctx, nil)
+ call, err = New(logr.Discard(), jp, entry, ctx, nil, apiConfig)
assert.NilError(t, err)
_, err = call.FetchAndLoad(context.TODO())
assert.ErrorContains(t, err, "HTTP 404")
entry.APICall.Service.URL = s.URL + "/resource"
- call, err = New(logr.Discard(), jp, entry, ctx, nil)
+ call, err = New(logr.Discard(), jp, entry, ctx, nil, apiConfig)
assert.NilError(t, err)
data, err := call.FetchAndLoad(context.TODO())
@@ -91,7 +96,7 @@ func Test_servicePostRequest(t *testing.T) {
}
ctx := enginecontext.NewContext(jp)
- call, err := New(logr.Discard(), jp, entry, ctx, nil)
+ call, err := New(logr.Discard(), jp, entry, ctx, nil, apiConfig)
assert.NilError(t, err)
data, err := call.FetchAndLoad(context.TODO())
assert.NilError(t, err)
@@ -139,7 +144,7 @@ func Test_servicePostRequest(t *testing.T) {
},
}
- call, err = New(logr.Discard(), jp, entry, ctx, nil)
+ call, err = New(logr.Discard(), jp, entry, ctx, nil, apiConfig)
assert.NilError(t, err)
data, err = call.FetchAndLoad(context.TODO())
assert.NilError(t, err)
diff --git a/pkg/engine/context/loaders/apicall.go b/pkg/engine/context/loaders/apicall.go
index 03fbfe9dfe..6f09a22bc9 100644
--- a/pkg/engine/context/loaders/apicall.go
+++ b/pkg/engine/context/loaders/apicall.go
@@ -19,6 +19,7 @@ type apiLoader struct {
enginectx enginecontext.Interface
jp jmespath.Interface
client engineapi.RawClient
+ config apicall.APICallConfiguration
data []byte
}
@@ -29,6 +30,7 @@ func NewAPILoader(
enginectx enginecontext.Interface,
jp jmespath.Interface,
client engineapi.RawClient,
+ apiCallConfig apicall.APICallConfiguration,
) enginecontext.Loader {
return &apiLoader{
ctx: ctx,
@@ -37,6 +39,7 @@ func NewAPILoader(
enginectx: enginectx,
jp: jp,
client: client,
+ config: apiCallConfig,
}
}
@@ -45,7 +48,7 @@ func (a *apiLoader) HasLoaded() bool {
}
func (a *apiLoader) LoadData() error {
- executor, err := apicall.New(a.logger, a.jp, a.entry, a.enginectx, a.client)
+ executor, err := apicall.New(a.logger, a.jp, a.entry, a.enginectx, a.client, a.config)
if err != nil {
return fmt.Errorf("failed to initiaize APICal: %w", err)
}
diff --git a/pkg/engine/factories/contextloaderfactory.go b/pkg/engine/factories/contextloaderfactory.go
index 74a1ead540..539f092f72 100644
--- a/pkg/engine/factories/contextloaderfactory.go
+++ b/pkg/engine/factories/contextloaderfactory.go
@@ -7,6 +7,7 @@ import (
"github.com/go-logr/logr"
kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1"
engineapi "github.com/kyverno/kyverno/pkg/engine/api"
+ "github.com/kyverno/kyverno/pkg/engine/apicall"
enginecontext "github.com/kyverno/kyverno/pkg/engine/context"
"github.com/kyverno/kyverno/pkg/engine/context/loaders"
"github.com/kyverno/kyverno/pkg/engine/jmespath"
@@ -35,10 +36,17 @@ func WithInitializer(initializer engineapi.Initializer) ContextLoaderFactoryOpti
}
}
+func WithAPICallConfig(config apicall.APICallConfiguration) ContextLoaderFactoryOptions {
+ return func(cl *contextLoader) {
+ cl.apiCallConfig = config
+ }
+}
+
type contextLoader struct {
- logger logr.Logger
- cmResolver engineapi.ConfigmapResolver
- initializers []engineapi.Initializer
+ logger logr.Logger
+ cmResolver engineapi.ConfigmapResolver
+ initializers []engineapi.Initializer
+ apiCallConfig apicall.APICallConfiguration
}
func (l *contextLoader) Load(
@@ -92,7 +100,7 @@ func (l *contextLoader) newLoader(
}
} else if entry.APICall != nil {
if client != nil {
- ldr := loaders.NewAPILoader(ctx, l.logger, entry, jsonContext, jp, client)
+ ldr := loaders.NewAPILoader(ctx, l.logger, entry, jsonContext, jp, client, l.apiCallConfig)
return enginecontext.NewDeferredLoader(entry.Name, ldr, l.logger)
} else {
l.logger.Info("disabled loading of APICall context entry", "name", entry.Name)