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

cdp: added resources and verbs for the cluster role #614

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions api/falcon/v1alpha1/falconnodesensor_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,11 @@ type FalconNodeSensorConfig struct {
// For more information, please see https://github.com/CrowdStrike/falcon-operator/blob/main/docs/ADVANCED.md.
// +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="DaemonSet Advanced Settings"
Advanced FalconAdvanced `json:"advanced,omitempty"`

// Enable cluster roles for Cloud Data Protection module
// +kubebuilder:default=true
// +operator-sdk:csv:customresourcedefinitions:type=spec,order=13
CdpRolesEnabled *bool `json:"cdpRolesEnabled,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

This shouldn't be added because clusterroles should NOT be reconciled because of the auditablity and permission visibility requirement and best practices of operator permissions that many review before install.

}

type PriorityClassConfig struct {
Expand Down
5 changes: 5 additions & 0 deletions api/falcon/v1alpha1/zz_generated.deepcopy.go
Original file line number Diff line number Diff line change
Expand Up @@ -1088,6 +1088,11 @@ func (in *FalconNodeSensorConfig) DeepCopyInto(out *FalconNodeSensorConfig) {
**out = **in
}
in.Advanced.DeepCopyInto(&out.Advanced)
if in.CdpRolesEnabled != nil {
in, out := &in.CdpRolesEnabled, &out.CdpRolesEnabled
*out = new(bool)
**out = **in
}
}

// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new FalconNodeSensorConfig.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,10 @@ spec:
- kernel
- bpf
type: string
cdpRolesEnabled:
default: true
description: Enable cluster roles for Cloud Data Protection module
type: boolean
disableCleanup:
default: false
description: Disables the cleanup of the sensor through DaemonSet
Expand Down
17 changes: 17 additions & 0 deletions config/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,23 @@ rules:
- list
- update
- watch
- apiGroups:
- ""
resources:
- cronjobs
- daemonsets
- deployments
- ingresses
- jobs
- nodes
- persistentvolumes
- pods
- replicasets
- services
verbs:
- get
- list
- watch
- apiGroups:
- ""
resources:
Expand Down
21 changes: 21 additions & 0 deletions deploy/falcon-operator.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3259,6 +3259,10 @@ spec:
- kernel
- bpf
type: string
cdpRolesEnabled:
default: true
description: Enable cluster roles for Cloud Data Protection module
type: boolean
disableCleanup:
default: false
description: Disables the cleanup of the sensor through DaemonSet
Expand Down Expand Up @@ -3961,6 +3965,23 @@ rules:
- list
- update
- watch
- apiGroups:
- ""
resources:
- cronjobs
- daemonsets
- deployments
- ingresses
- jobs
- nodes
- persistentvolumes
- pods
- replicasets
- services
verbs:
- get
- list
- watch
- apiGroups:
- ""
resources:
Expand Down
4 changes: 4 additions & 0 deletions docs/deployment/openshift/resources/node/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,10 @@ spec:
| node.backend | (optional) Configure the backend mode for Falcon Sensor (allowed values: kernel, bpf) |
| node.disableCleanup | (optional) Cleans up `/opt/CrowdStrike` on the nodes by deleting the files and directory. |
| node.version | (optional) Enforce particular Falcon Sensor version to be installed (example: "6.35", "6.35.0-13207") |
| node.cdpRolesEnabled | (optional) Enable cluster roles for Cloud Data Protection module |

> [!IMPORTANT]
> node.tolerations will be appended to the existing tolerations for the daemonset due to GKE Autopilot allowing users to manage Tolerations directly in the console. See documentation here: https://cloud.google.com/kubernetes-engine/docs/how-to/workload-separation. Removing Tolerations from an existing daemonset requires a redeploy of the FalconNodeSensor manifest.

#### Falcon Sensor Settings
| Spec | Description |
Expand Down
4 changes: 4 additions & 0 deletions docs/resources/node/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,10 @@ spec:
| node.backend | (optional) Configure the backend mode for Falcon Sensor (allowed values: kernel, bpf) |
| node.disableCleanup | (optional) Cleans up `/opt/CrowdStrike` on the nodes by deleting the files and directory. |
| node.version | (optional) Enforce particular Falcon Sensor version to be installed (example: "6.35", "6.35.0-13207") |
| node.cdpRolesEnabled | (optional) Enable cluster roles for Cloud Data Protection module |

> [!IMPORTANT]
Copy link
Author

Choose a reason for hiding this comment

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

make docs generated this

> node.tolerations will be appended to the existing tolerations for the daemonset due to GKE Autopilot allowing users to manage Tolerations directly in the console. See documentation here: https://cloud.google.com/kubernetes-engine/docs/how-to/workload-separation. Removing Tolerations from an existing daemonset requires a redeploy of the FalconNodeSensor manifest.

#### Falcon Sensor Settings
| Spec | Description |
Expand Down
1 change: 1 addition & 0 deletions docs/src/resources/node.md.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ spec:
| node.backend | (optional) Configure the backend mode for Falcon Sensor (allowed values: kernel, bpf) |
| node.disableCleanup | (optional) Cleans up `/opt/CrowdStrike` on the nodes by deleting the files and directory. |
| node.version | (optional) Enforce particular Falcon Sensor version to be installed (example: "6.35", "6.35.0-13207") |
| node.cdpRolesEnabled | (optional) Enable cluster roles for Cloud Data Protection module |

> [!IMPORTANT]
> node.tolerations will be appended to the existing tolerations for the daemonset due to GKE Autopilot allowing users to manage Tolerations directly in the console. See documentation here: https://cloud.google.com/kubernetes-engine/docs/how-to/workload-separation. Removing Tolerations from an existing daemonset requires a redeploy of the FalconNodeSensor manifest.
Expand Down
108 changes: 106 additions & 2 deletions internal/controller/falcon_node/falconnodesensor_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package falcon

import (
"context"
goerr "errors"
"reflect"

falconv1alpha1 "github.com/crowdstrike/falcon-operator/api/falcon/v1alpha1"
Expand All @@ -15,6 +16,7 @@ import (
"github.com/crowdstrike/gofalcon/falcon"
"github.com/go-logr/logr"
"github.com/operator-framework/operator-lib/proxy"
"golang.org/x/exp/slices"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
rbacv1 "k8s.io/api/rbac/v1"
Expand All @@ -33,6 +35,16 @@ import (
clog "sigs.k8s.io/controller-runtime/pkg/log"
)

var (
cdpRoleEnabledNil = goerr.New("CdpRolesEnabled must be defined")

cdpRoles = rbacv1.PolicyRule{
APIGroups: []string{""},
Verbs: []string{"get", "watch", "list"},
Resources: []string{"pods", "services", "nodes", "daemonsets", "replicasets", "deployments", "jobs", "ingresses", "cronjobs", "persistentvolumes"},
}
)

Copy link
Contributor

@redhatrises redhatrises Dec 25, 2024

Choose a reason for hiding this comment

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

These permissions need to be added to https://github.com/CrowdStrike/falcon-operator/blob/main/config/rbac/falconnodesensor_role.yaml and https://github.com/CrowdStrike/falcon-operator/blob/main/config/non-olm/patches/falconnodesensor_role.yaml and are autogenerated through kustomize. Use make manifest && make generate && make non-olm

// FalconNodeSensorReconciler reconciles a FalconNodeSensor object
type FalconNodeSensorReconciler struct {
client.Client
Expand Down Expand Up @@ -76,6 +88,7 @@ func (r *FalconNodeSensorReconciler) SetupWithManager(mgr ctrl.Manager, tracker
//+kubebuilder:rbac:groups="rbac.authorization.k8s.io",resources=clusterrolebindings,verbs=get;list;watch;create
//+kubebuilder:rbac:groups="security.openshift.io",resources=securitycontextconstraints,resourceNames=privileged,verbs=use
//+kubebuilder:rbac:groups="scheduling.k8s.io",resources=priorityclasses,verbs=get;list;watch;create;delete;update
//+kubebuilder:rbac:groups="",resources=pods;services;nodes;daemonsets;replicasets;deployments;jobs;ingresses;cronjobs;persistentvolumes,verbs=get;watch;list

// Reconcile is part of the main kubernetes reconciliation loop which aims to
// move the current state of the cluster closer to the desired state.
Expand Down Expand Up @@ -790,6 +803,11 @@ func (r *FalconNodeSensorReconciler) handlePermissions(ctx context.Context, node
return created, err
}

created, err = r.handleClusterRole(ctx, nodesensor, logger)
Copy link
Contributor

@redhatrises redhatrises Dec 25, 2024

Choose a reason for hiding this comment

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

Clusterroles for operators should not reconcile permissions. The cluster roles are required to be auditable BEFORE install by cluster admins, security architects, and auditors. For operator security design and best practices, these permissions need to be added to https://github.com/CrowdStrike/falcon-operator/blob/main/config/rbac/falconnodesensor_role.yaml and https://github.com/CrowdStrike/falcon-operator/blob/main/config/non-olm/patches/falconnodesensor_role.yaml and are autogenerated through kustomize. Use make manifest && make generate && make non-olm

if created || err != nil {
return created, err
}

return r.handleClusterRoleBinding(ctx, nodesensor, logger)
}

Expand All @@ -810,7 +828,7 @@ func (r *FalconNodeSensorReconciler) handleClusterRoleBinding(ctx context.Contex
RoleRef: rbacv1.RoleRef{
APIGroup: "rbac.authorization.k8s.io",
Kind: "ClusterRole",
Name: "falcon-operator-node-sensor-role",
Name: common.NodeClusterRoleName,
},
Subjects: []rbacv1.Subject{
{
Expand All @@ -829,7 +847,7 @@ func (r *FalconNodeSensorReconciler) handleClusterRoleBinding(ctx context.Contex
logger.Info("Creating FalconNodeSensor ClusterRoleBinding")
err = r.Create(ctx, &binding)
if err != nil && !errors.IsAlreadyExists(err) {
logger.Error(err, "Failed to create new ClusterRoleBinding", "ClusteRoleBinding.Name", common.NodeClusterRoleBindingName)
logger.Error(err, "Failed to create new ClusterRoleBinding", "ClusterRoleBinding.Name", common.NodeClusterRoleBindingName)
return false, err
}

Expand Down Expand Up @@ -880,6 +898,43 @@ func (r *FalconNodeSensorReconciler) handleServiceAccount(ctx context.Context, n
return false, nil
}

// handleClusterRole updates the cluster role and grants necessary permissions to it
func (r *FalconNodeSensorReconciler) handleClusterRole(ctx context.Context, nodesensor *falconv1alpha1.FalconNodeSensor, logger logr.Logger) (bool, error) {
if nodesensor.Spec.Node.CdpRolesEnabled == nil {
return false, cdpRoleEnabledNil
}

if !*nodesensor.Spec.Node.CdpRolesEnabled {
return false, nil
}
clusterRole := rbacv1.ClusterRole{}
err := r.Get(ctx, types.NamespacedName{Name: common.NodeClusterRoleName}, &clusterRole)
if err != nil {
logger.Error(err, "Failed to get FalconNodeSensor ClusterRole")
return false, err
}

// check if CDP cluster role was already set
for _, rule := range clusterRole.Rules {
if slices.Equal(rule.Resources, cdpRoles.Resources) &&
slices.Equal(rule.Verbs, cdpRoles.Verbs) &&
slices.Equal(rule.APIGroups, cdpRoles.APIGroups) {
return false, nil
}
}

clusterRole.Rules = append(clusterRole.Rules, cdpRoles)

err = r.Update(ctx, &clusterRole)
if err != nil {
logger.Error(err, "Failed to update ClusterRole", "Namespace.Name", nodesensor.Spec.InstallNamespace, "ClusterRole.Name", common.NodeClusterRoleName)
return false, err
}
logger.Info("Updated FalconNodeSensor ClusterRole")
return true, nil

}

// handleServiceAccount creates and updates the service account and grants necessary permissions to it
func (r *FalconNodeSensorReconciler) handleSAAnnotations(ctx context.Context, nodesensor *falconv1alpha1.FalconNodeSensor, logger logr.Logger) error {
sa := corev1.ServiceAccount{}
Expand Down Expand Up @@ -1030,6 +1085,11 @@ func (r *FalconNodeSensorReconciler) finalizeDaemonset(ctx context.Context, imag
return err
}

if err := r.cleanupClusterRole(ctx, nodesensor, logger); err != nil {
logger.Error(err, "Failed to cleanup Falcon sensor cluster role")
return err
}

// If we have gotten here, the cleanup should be successful
logger.Info("Successfully deleted node directory", "Path", common.FalconDataDir)
} else if err != nil {
Expand All @@ -1041,6 +1101,50 @@ func (r *FalconNodeSensorReconciler) finalizeDaemonset(ctx context.Context, imag
return nil
}

// cleanupClusterRole cleanup the cluster role from permissions granted in runtime
func (r *FalconNodeSensorReconciler) cleanupClusterRole(ctx context.Context, nodesensor *falconv1alpha1.FalconNodeSensor, logger logr.Logger) error {
if nodesensor.Spec.Node.CdpRolesEnabled == nil {
return cdpRoleEnabledNil
}

if !*nodesensor.Spec.Node.CdpRolesEnabled {
return nil
}
clusterRole := rbacv1.ClusterRole{}
err := r.Get(ctx, types.NamespacedName{Name: common.NodeClusterRoleName}, &clusterRole)
if err != nil {
logger.Error(err, "Failed to get FalconNodeSensor ClusterRole")
return err
}

indexToRemove := 0
roleToRemoveFound := false
var rule rbacv1.PolicyRule
// check if CDP cluster role was set
for indexToRemove, rule = range clusterRole.Rules {
if slices.Equal(rule.Resources, cdpRoles.Resources) &&
slices.Equal(rule.Verbs, cdpRoles.Verbs) &&
slices.Equal(rule.APIGroups, cdpRoles.APIGroups) {
roleToRemoveFound = true
break
}
}
// continue as role to remove wasn't found
if !roleToRemoveFound {
return nil
}

clusterRole.Rules = append(clusterRole.Rules[:indexToRemove], clusterRole.Rules[indexToRemove+1:]...)
err = r.Update(ctx, &clusterRole)
if err != nil {
logger.Error(err, "Failed to update ClusterRole", "Namespace.Name", nodesensor.Spec.InstallNamespace, "ClusterRole.Name", common.NodeClusterRoleName)
return err
}
logger.Info("Removed FalconNodeSensor ClusterRole runtime granted permissions")
return nil

}

func (r *FalconNodeSensorReconciler) reconcileObjectWithName(ctx context.Context, name types.NamespacedName) error {
obj := &falconv1alpha1.FalconNodeSensor{}
err := r.Get(ctx, name, obj)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,13 @@ import (

falconv1alpha1 "github.com/crowdstrike/falcon-operator/api/falcon/v1alpha1"
"github.com/crowdstrike/falcon-operator/internal/controller/common/sensorversion"
"github.com/crowdstrike/falcon-operator/pkg/common"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"golang.org/x/exp/slices"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
rbacv1 "k8s.io/api/rbac/v1"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
Expand All @@ -34,18 +37,30 @@ var _ = Describe("FalconNodeSensor controller", func() {
}

typeNamespaceName := types.NamespacedName{Name: NodeSensorName, Namespace: NodeSensorName}
clusterRole := rbacv1.ClusterRole{
ObjectMeta: metav1.ObjectMeta{
Namespace: NodeSensorName,
Name: common.NodeClusterRoleName,
Labels: common.CRLabels("serviceaccount", common.NodeServiceAccountName, common.FalconKernelSensor),
}, Rules: []rbacv1.PolicyRule{}}

BeforeEach(func() {
By("Creating the Namespace to perform the tests")
err := k8sClient.Create(ctx, namespace)
Expect(err).To(Not(HaveOccurred()))

By("Creating the Namespace to perform the tests")
err = k8sClient.Create(ctx, &clusterRole)
Expect(err).To(Not(HaveOccurred()))
})

AfterEach(func() {
// TODO(user): Attention if you improve this code by adding other context test you MUST
// be aware of the current delete namespace limitations. More info: https://book.kubebuilder.io/reference/envtest.html#testing-considerations
By("Deleting the Namespace to perform the tests")
_ = k8sClient.Delete(ctx, namespace)

_ = k8sClient.Delete(ctx, &clusterRole)
})

It("should successfully reconcile a custom resource for FalconNodeSensor", func() {
Expand Down Expand Up @@ -106,6 +121,31 @@ var _ = Describe("FalconNodeSensor controller", func() {
})
Expect(err).To(Not(HaveOccurred()))

// ClusterRole reconcile
_, err = falconNodeReconciler.Reconcile(ctx, reconcile.Request{
NamespacedName: typeNamespaceName,
})
Expect(err).To(Not(HaveOccurred()))

By("Checking if the cluster role permissions were set")
Eventually(func() error {
clusterRole := rbacv1.ClusterRole{}
err := falconNodeReconciler.Get(ctx, types.NamespacedName{Name: common.NodeClusterRoleName}, &clusterRole)
if err != nil {
return fmt.Errorf("clusterrole doesn't exist")
}

// check if CDP cluster role was correctly set
for _, rule := range clusterRole.Rules {
if slices.Equal(rule.Resources, cdpRoles.Resources) &&
slices.Equal(rule.Verbs, cdpRoles.Verbs) &&
slices.Equal(rule.APIGroups, cdpRoles.APIGroups) {
return nil
}
}
return fmt.Errorf("clusterrole doesn't have the correct permissions")
}, time.Minute, time.Second).Should(Succeed())

// TODO: clusterRoleBinding reconciliation might be removed in the future
_, err = falconNodeReconciler.Reconcile(ctx, reconcile.Request{
NamespacedName: typeNamespaceName,
Expand Down
1 change: 1 addition & 0 deletions pkg/common/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,5 @@ const (
AdmissionServiceAccountName = "falcon-operator-admission-controller"
NodeClusterRoleBindingName = "falcon-operator-node-sensor-rolebinding"
ImageServiceAccountName = "falcon-operator-image-analyzer"
NodeClusterRoleName = "falcon-operator-node-sensor-role"
)
Loading