Skip to content
This repository has been archived by the owner on Jun 26, 2023. It is now read-only.

[MTB] added block other tenant resources benchmark #1318

Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 13 additions & 0 deletions benchmarks/kubectl-mtb/internal/kubectl-mtb/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,16 @@ func initConfig() error {
return err
}

if benchmarkRunOptions.OtherNamespace != "" && benchmarkRunOptions.OtherTenant != "" {
otherTenantConfig := config
otherTenantConfig.Impersonate.UserName = benchmarkRunOptions.OtherTenant

benchmarkRunOptions.OClient, err = kubernetes.NewForConfig(tenantConfig)
if err != nil {
return err
}
}

return err
}

Expand Down Expand Up @@ -130,6 +140,9 @@ func validateFlags(cmd *cobra.Command) error {
return fmt.Errorf("tenant namespace must be set via --namespace or -n")
}

benchmarkRunOptions.OtherNamespace, _ = cmd.Flags().GetString("other-namespace")
benchmarkRunOptions.OtherTenant, _ = cmd.Flags().GetString("other-tenant-admin")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can add the arguments in a list itself like -n test test2 --as bob-k8s-access bob2-k8s-access?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a great idea! @phoenixking25 what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the Cobra CLI package allows this:

cmd --slice a --slice b --slice c,d,e

spf13/cobra#661


err := initConfig()
if err != nil {
return err
Expand Down
3 changes: 2 additions & 1 deletion benchmarks/kubectl-mtb/internal/mtb_builder/tpl.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,14 @@ func BenchmarkFileTemplate() []byte {

import (
"fmt"
"sigs.k8s.io/multi-tenancy/benchmarks/kubectl-mtb/types"

"k8s.io/client-go/kubernetes"
"sigs.k8s.io/multi-tenancy/benchmarks/kubectl-mtb/bundle/box"
"sigs.k8s.io/multi-tenancy/benchmarks/kubectl-mtb/pkg/benchmark"
"sigs.k8s.io/multi-tenancy/benchmarks/kubectl-mtb/test"
)


var b = &benchmark.Benchmark{

PreRun: func(options types.RunOptions) error {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
# Block access to other tenant resources <small>[MTB-PL1-CC-TI-2] </small>

**Profile Applicability:**

2

**Type:**

Configuration

**Category:**

Tenant Protection

**Description:**

Access controls should be configured so that a tenant cannot view, edit, create, or delete namespaced resources belonging to another tenant.

**Rationale:**

Tenant resources should be isolated from other tenants.


**Audit:**

Run the following commands to retrieve the list of namespaced resources available in Tenant B
```bash
kubectl --kubeconfig tenant-b api-resources --namespaced=true
```
For each namespaced resource, and each verb (get, list, create, update, patch, watch, delete, and deletecollection) issue the following command
```bash
kubectl --kubeconfig tenant-a -n b1 &lt;verb&gt; &lt;resource&gt;
```
Each command must return &#39;no&#39;


**namespaceRequired:**

2

Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
package blockothertenantresources

import (
"fmt"
"k8s.io/apimachinery/pkg/runtime/schema"
"sigs.k8s.io/multi-tenancy/benchmarks/kubectl-mtb/test/utils"
"sigs.k8s.io/multi-tenancy/benchmarks/kubectl-mtb/types"

"sigs.k8s.io/multi-tenancy/benchmarks/kubectl-mtb/bundle/box"
"sigs.k8s.io/multi-tenancy/benchmarks/kubectl-mtb/pkg/benchmark"
"sigs.k8s.io/multi-tenancy/benchmarks/kubectl-mtb/test"
)

var verbs = []string{"get", "update"}

var b = &benchmark.Benchmark{

PreRun: func(options types.RunOptions) error {

return nil
},

Run: func(options types.RunOptions) error {
var primaryNamespaceResources []utils.GroupResource

lists, err := options.KClient.Discovery().ServerPreferredResources()
if err != nil {
options.Logger.Debug(err.Error())
return err
}

for _, list := range lists {
if len(list.APIResources) == 0 {
continue
}
gv, err := schema.ParseGroupVersion(list.GroupVersion)
if err != nil {
continue
}
for _, resource := range list.APIResources {
if len(resource.Verbs) == 0 {
continue
}

if resource.Namespaced {
continue
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if len(resource.Verbs) == 0 {
continue
}
if resource.Namespaced {
continue
}
if len(resource.Verbs) == 0 || resource.Namespaced {
continue
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please refactor as suggested.

primaryNamespaceResources = append(primaryNamespaceResources, utils.GroupResource{
APIGroup: gv.Group,
APIResource: resource,
})
}
}

for _, resource := range primaryNamespaceResources {
for _, verb := range verbs {
access, msg, err := utils.RunAccessCheck(options.OClient, options.TenantNamespace, resource, verb)
if err != nil {
options.Logger.Debug(err.Error())
return err
}
if access {
return fmt.Errorf(msg)
}
}
}

for _, resource := range primaryNamespaceResources {
for _, verb := range verbs {
access, msg, err := utils.RunAccessCheck(options.TClient, options.OtherNamespace, resource, verb)
if err != nil {
options.Logger.Debug(err.Error())
return err
}
if access {
return fmt.Errorf(msg)
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these two for loops can be clubbed into a single for loop.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah this can be made a separate function, right now this particular piece of duplicated at three places

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please refactor as suggested.


return nil
},
}

func init() {
// Get the []byte representation of a file, or an error if it doesn't exist:
err := b.ReadConfig(box.Get("block_other_tenant_resources/config.yaml"))
if err != nil {
fmt.Println(err)
}

test.BenchmarkSuite.Add(b);
}

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
package blockothertenantresources
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
id: MTB-PL1-CC-TI-2
title: Block access to other tenant resources
benchmarkType: Configuration
category: Tenant Protection
description: Access controls should be configured so that a tenant cannot view, edit, create, or delete namespaced resources belonging to another tenant.
remediation:
profileLevel: 2
namespaceRequired: 2
rationale: Tenant resources should be isolated from other tenants.
Audit: |
Run the following commands to retrieve the list of namespaced resources available in Tenant B
```bash
kubectl --kubeconfig tenant-b api-resources --namespaced=true
```
For each namespaced resource, and each verb (get, list, create, update, patch, watch, delete, and deletecollection) issue the following command
```bash
kubectl --kubeconfig tenant-a -n b1 <verb> <resource>
```
Each command must return 'no'
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,5 @@ Set `imagePullPolicy` to `always` for the container.

**namespaceRequired:**

2
1

1 change: 1 addition & 0 deletions benchmarks/kubectl-mtb/types/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,6 @@ type RunOptions struct {
Label string
KClient *kubernetes.Clientset
TClient *kubernetes.Clientset
OClient *kubernetes.Clientset
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we be a little bit more descriptive while naming different clients?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, agreed. What would be good names here? Something like:

  • ClusterAdminClient
  • Tenant1Client
  • Tenant2Client

Logger *zap.SugaredLogger
}