Skip to content

Commit

Permalink
Merge pull request rancher#27336 from prachidamle/fwd_port_cis_svc_accnt
Browse files Browse the repository at this point in the history
[Forwardport] Default serviceaccounts of system namespaces should not be used
  • Loading branch information
prachidamle committed May 29, 2020
2 parents 54ff40f + 2022b26 commit e3e7caa
Show file tree
Hide file tree
Showing 4 changed files with 111 additions and 2 deletions.
2 changes: 2 additions & 0 deletions pkg/controllers/user/controllers.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"github.com/rancher/rancher/pkg/controllers/user/noderemove"
"github.com/rancher/rancher/pkg/controllers/user/nodesyncer"
"github.com/rancher/rancher/pkg/controllers/user/nslabels"
"github.com/rancher/rancher/pkg/controllers/user/nsserviceaccount"
"github.com/rancher/rancher/pkg/controllers/user/pipeline"
"github.com/rancher/rancher/pkg/controllers/user/rbac"
"github.com/rancher/rancher/pkg/controllers/user/rbac/podsecuritypolicy"
Expand Down Expand Up @@ -73,6 +74,7 @@ func Register(ctx context.Context, cluster *config.UserContext, clusterRec *mana
certsexpiration.Register(ctx, cluster)
ingresshostgen.Register(ctx, cluster.UserOnlyContext())
windows.Register(ctx, clusterRec, cluster)
nsserviceaccount.Register(ctx, cluster)

// register controller for API
cluster.APIAggregation.APIServices("").Controller()
Expand Down
71 changes: 71 additions & 0 deletions pkg/controllers/user/nsserviceaccount/nssvcaccnt.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
package nsserviceaccount

import (
"context"
"strings"

"github.com/rancher/norman/types/slice"
"github.com/rancher/rancher/pkg/settings"
rv1 "github.com/rancher/types/apis/core/v1"
"github.com/rancher/types/config"
"github.com/sirupsen/logrus"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/runtime"
)

type defaultSvcAccountHandler struct {
serviceAccounts rv1.ServiceAccountInterface
}

func Register(ctx context.Context, cluster *config.UserContext) {
logrus.Debugf("Registering defaultSvcAccountHandler for checking default service account of system namespaces")
nsh := &defaultSvcAccountHandler{
serviceAccounts: cluster.Core.ServiceAccounts(""),
}
cluster.Core.ServiceAccounts("").AddHandler(ctx, "defaultSvcAccountHandler", nsh.Sync)
}

func (nsh *defaultSvcAccountHandler) Sync(key string, sa *corev1.ServiceAccount) (runtime.Object, error) {
if sa == nil || sa.DeletionTimestamp != nil {
return nil, nil
}
logrus.Debugf("defaultSvcAccountHandler: Sync service account: key=%v", key)
//handle default svcAccount of system namespaces only
if err := nsh.handleIfSystemNSDefaultSA(sa); err != nil {
logrus.Errorf("defaultSvcAccountHandler: Sync: error handling default ServiceAccount of namespace key=%v, err=%v", key, err)
}
return nil, nil
}

func (nsh *defaultSvcAccountHandler) handleIfSystemNSDefaultSA(defSvcAccnt *corev1.ServiceAccount) error {
//check if sa is "default"
if defSvcAccnt.Name != "default" {
return nil
}
//check if ns is a system-ns
namespace := defSvcAccnt.Namespace
if namespace == "kube-system" || namespace == "default" || !nsh.isSystemNS(namespace) {
return nil
}
if defSvcAccnt.AutomountServiceAccountToken != nil && *defSvcAccnt.AutomountServiceAccountToken == false {
return nil
}
automountServiceAccountToken := false
defSvcAccnt.AutomountServiceAccountToken = &automountServiceAccountToken
logrus.Debugf("defaultSvcAccountHandler: updating default service account key=%v", defSvcAccnt)
_, err := nsh.serviceAccounts.Update(defSvcAccnt)
if err != nil {
logrus.Errorf("defaultSvcAccountHandler: error updating default service account flag for namespace: %v, err=%+v", namespace, err)
return err
}
return nil
}

func (nsh *defaultSvcAccountHandler) isSystemNS(namespace string) bool {
systemNamespacesStr := settings.SystemNamespaces.Get()
if systemNamespacesStr == "" {
return false
}
systemNamespaces := strings.Split(systemNamespacesStr, ",")
return slice.ContainsString(systemNamespaces, namespace)
}
2 changes: 1 addition & 1 deletion pkg/settings/setting.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ var (
ServerURL = NewSetting("server-url", "")
ServerVersion = NewSetting("server-version", "dev")
SystemDefaultRegistry = NewSetting("system-default-registry", "")
SystemNamespaces = NewSetting("system-namespaces", "kube-system,kube-public,cattle-system,cattle-alerting,cattle-logging,cattle-pipeline,cattle-prometheus,ingress-nginx,cattle-global-data,cattle-istio,kube-node-lease,cert-manager")
SystemNamespaces = NewSetting("system-namespaces", "kube-system,kube-public,cattle-system,cattle-alerting,cattle-logging,cattle-pipeline,cattle-prometheus,ingress-nginx,cattle-global-data,cattle-istio,kube-node-lease,cert-manager,cattle-global-nt,security-scan")
TelemetryOpt = NewSetting("telemetry-opt", "prompt")
TLSMinVersion = NewSetting("tls-min-version", "1.2")
TLSCiphers = NewSetting("tls-ciphers", "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305,TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305")
Expand Down
38 changes: 37 additions & 1 deletion tests/integration/suite/test_system_project.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
import pytest
from rancher import ApiError
from kubernetes.client import CoreV1Api
from .conftest import wait_for

systemProjectLabel = "authz.management.cattle.io/system-project"
defaultProjectLabel = "authz.management.cattle.io/default-project"
initial_system_namespaces = set(["kube-node-lease",
"kube-system",
"cattle-system",
"kube-public",
"cattle-global-data"])
"cattle-global-data",
"cattle-global-nt"])
loggingNamespace = "cattle-logging"


Expand Down Expand Up @@ -71,3 +74,36 @@ def test_system_project_cant_be_deleted(admin_mc, admin_cc):
admin_mc.client.delete(system_project)
assert e.value.error.status == 405
assert e.value.error.message == 'System Project cannot be deleted'


def test_system_namespaces_default_svc_account(admin_mc):
system_namespaces_setting = admin_mc.client.by_id_setting(
"system-namespaces")
system_namespaces = system_namespaces_setting["value"].split(",")
k8sclient = CoreV1Api(admin_mc.k8s_client)
def_saccnts = k8sclient.list_service_account_for_all_namespaces(
field_selector='metadata.name=default')
for sa in def_saccnts.items:
ns = sa.metadata.namespace

def _check_system_sa_flag():
if ns in system_namespaces and ns != "kube-system":
if sa.automount_service_account_token is False:
return True
else:
return False
else:
if sa.automount_service_account_token is None:
return True
elif sa.automount_service_account_token is True:
return True
else:
return False

def _sa_update_fail():
name = sa.metadata.name
flag = sa.automount_service_account_token
return 'Service account {} in namespace {} does not have correct \
automount_service_account_token flag: {}'.format(name, ns, flag)

wait_for(_check_system_sa_flag, fail_handler=_sa_update_fail)

0 comments on commit e3e7caa

Please sign in to comment.