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

Security Vulnerabilities found in secrets-store-csi-driver-provider-gcp #242

Open
kalpesh6331 opened this issue Apr 5, 2023 · 9 comments
Labels
bug Something isn't working

Comments

@kalpesh6331
Copy link

Description

We are using secrets-store-csi-driver-provider-gcp to fetch sensitive data from GCP secret manager into our application.
This application is mission critical and has been running in production since couple months.
As a part of CIS benchmarks, we need to run different scans on the runtime Prisma Cloud (from PaloAlto networks) has detected following vulnerabilities.

We would like to know if these can be fixed or already fixed in any future releases.
If it is fixed via configuration (changes in helm values), would it affect any functionality or working of csi-driver?

Vulnerabilities:

  • Do not share the host's network namespace
  • Mount container's root filesystem as read only
  • Do not share the host's UTS namespace
  • Do not disable default seccomp profile
  • Use PIDs cgroup limit
  • Container is running as root
  • Restrict container from acquiring additional privileges
  • Do not set mount propagation mode to shared
  • Do not use privileged containers
  • Verify AppArmor profile, if applicable

Specifications

  • KEDA Version: 2.8.2
  • csi_driver: k8s.gcr.io/csi-secrets-store/driver:v1.2.4
  • csi_driver_crds: k8s.gcr.io/csi-secrets-store/driver-crds:v1.2.4
  • csi_node_driver_registrar: k8s.gcr.io/sig-storage/csi-node-driver-registrar:v2.5.1
  • sig_storage_liveness_probe: k8s.gcr.io/sig-storage/livenessprobe:v2.7.0
  • csi_gcp_provider_plugin: us-docker.pkg.dev/secretmanager-csi/secrets-store-csi-driver-provider-gcp/plugin@sha256:cb491d4af4776ac352aac415676918fa7cd4ef1671e71c579175ef3e2820af09
@kalpesh6331 kalpesh6331 added the bug Something isn't working label Apr 5, 2023
@kalpesh6331
Copy link
Author

Could anyone please update on this?

@amitmodak
Copy link
Collaborator

amitmodak commented Jun 22, 2023

Thank you for reporting this issue! We have added this to our roadmap and will look into it.

As you have pointed out, it looks like most of these vulnerabilities can indeed be fixed via configuration changes. However, we will need to do extensive testing of the provider and the driver to make sure there is no impact on functionality due to these changes.

@NaMNDV
Copy link
Collaborator

NaMNDV commented Nov 20, 2023

[Progress Comment]
Hi,

Below are the provider fixes for the configurations:

Do not share the host's network namespace
Fixed #289

Mount container's root filesystem as read only
Fixed #292

Do not share the host's UTS namespace
Fixed #289, #290

Do not disable default seccomp profile
FIxed #331

Use PIDs cgroup limit
NA

Container is running as root
In Progress (driver requirements)

Restrict container from acquiring additional privileges
Fixed #321

Do not set mount propagation mode to shared
Fixed #318

Do not use privileged containers
Fixed #321

Verify AppArmor profile, if applicable
In Progress

A few are in progress and we are constantly looking into that. Till then could you please verify the fixes from your end by running benchmarking tools at your end.

Soon we will discuss vulnerabilities and configuration fixes with our dependencies e.g. driver, livenessprobe ...

@jagtapa
Copy link

jagtapa commented Nov 24, 2023

Hello @NaMNDV

Thank you for the information. Could you please verify the release version in which the mentioned vulnerabilities have been addressed? We will conduct testing based on that information to confirm the resolution.

@NaMNDV
Copy link
Collaborator

NaMNDV commented Nov 30, 2023

Sure, it's 1.4.0

@jagtapa
Copy link

jagtapa commented Feb 6, 2024

Greetings team,

We've noticed that the secrets-store-csi-driver.yaml includes the securityContext option:

      fieldPath: spec.nodeName
      imagePullPolicy: {{ .Values.linux.image.pullPolicy }}
      securityContext:
        privileged: true

However, its default values are not currently present in the values.yaml file for easy overriding. Could you please include it in the default values.yaml file so that it can be overridden externally & we can test it?

And also confirm about this vulnerability "Verify AppArmor profile, if applicable" whether its fixed or not in 1.4.0 release @NaMNDV

@NaMNDV
Copy link
Collaborator

NaMNDV commented Mar 6, 2024

Thanks @jagtapa for pointing this out.

I'll raise a PR to update the driver: https://github.com/kubernetes-sigs/secrets-store-csi-driver/blob/main/charts/secrets-store-csi-driver/templates/secrets-store-csi-driver.yaml

and corresponding values.yaml: https://github.com/kubernetes-sigs/secrets-store-csi-driver/blob/main/charts/secrets-store-csi-driver/values.yaml

also the manifest_staging: https://github.com/kubernetes-sigs/secrets-store-csi-driver/tree/main/manifest_staging charts

Regarding AppArmor profile: The AppArmor profile is not applicable and is not applied in 1.4.0 release

@NaMNDV
Copy link
Collaborator

NaMNDV commented Mar 13, 2024

      fieldPath: spec.nodeName
      imagePullPolicy: {{ .Values.linux.image.pullPolicy }}
      securityContext:
        privileged: true

kubernetes-sigs/secrets-store-csi-driver#1469 (review) the security configuration is a hard-requirement for the driver and can not be changed. The driver won't work by configuring this with other values of securityContext.privilaged . Thus we can not make the security context configurable.

If future has some other configurable fields in the securityContext, we may think of this approach later.

@jagtapa
Copy link

jagtapa commented Apr 8, 2024

@NaMNDV:

I concur with your point that setting this flag to true is necessary. We also need to incorporate additional parameters into the securityContext. However, because the option below

securityContext: privileged: true

is absent in the default values file, we are unable to add or test other security parameters. It's important to note that we should not modify privileged: true. Nevertheless, having this parameter in the default values file would provide us with the flexibility to add other securityContext configurations.

Please share your thoughts on this matter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants