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

please support more parts of securityContext #7924

Closed
FWiesner opened this issue May 12, 2020 · 18 comments · Fixed by #9060
Closed

please support more parts of securityContext #7924

FWiesner opened this issue May 12, 2020 · 18 comments · Fixed by #9060
Labels
area/API API objects and controllers kind/feature Well-understood/specified features, ready for coding.
Milestone

Comments

@FWiesner
Copy link

In what area(s)?

/area API

Describe the feature

https://knative.dev/docs/serving/spec/knative-api-specification-1.0/ allows you using runAsUser from the containers' securityContext, but none of the other fields.

When running Knative services have to access shared volumes, this is not enough. When your shared volumes rely on proper group membership, then runAsGroup and fsGroup would be required as well. I don't know whether you have a specific reason not to support these fields (runAsNonRoot would be another candidate), but we would really love to see support for those extended aspects of securityContext

@FWiesner FWiesner added the kind/feature Well-understood/specified features, ready for coding. label May 12, 2020
@knative-prow-robot knative-prow-robot added the area/API API objects and controllers label May 12, 2020
@dprotaso
Copy link
Member

Hey @FWiesner thanks for the issue

Just to capture more context can you comment on how you plan on using these fields?

For example:

  • are you trying to run certain images that expect certain gid/uids
  • if you're trying to follow a best practice - what is it?
  • etc.

@FWiesner
Copy link
Author

FWiesner commented May 13, 2020

Hey @dprotaso here's my scenario

When you have shared volumes and those volumes need to be accessed by multiple different services in read/write mode, you would normally want to make sure that proper file system permissions are automatically enforced. An example could be a volume to write application audit trails.

When that volume gets mounted, you would then want to tell the service about the groupId that has access to this shared volume, say groupId 7000. That it would be 7000 you don't necessarily know when you delivered the image. On that volume you might want to have directories that belong to different groups and different users. In this case you'd set fsGroup to 7000 to give all pods read/write access to the root of the volume, but you'd run them with different userIds (which you support) and might want to have some userIds in the same groups. Then you'd run several services with the same fsGroup, same runAsGroup and different runAsUser. You certainly could achieve all of that also from modifying all the images that you use, but thats a tough task when you have many delivery units or when you don't trust them all and don't want them to suddenly be able to read from other directories that they should not.

In our specific case we ask our teams to not give use ready to use KSVC descriptors, but just PodTemplate documents and some CRD documents capturing extra information we need. We then synthesize the actual KSVC descriptors via our own operator. That way we'd be easily able to make sure we maintain proper access permissions on shared volumes, given KSVCs would support those securityContext fields

@dprotaso
Copy link
Member

Sorta related question:

At this time Knative OSS only supports configmap, secret & projected volumes. Are you circumventing that?

@dng00
Copy link

dng00 commented Jul 30, 2020

This is our use case:

For function that runs inside EKS that needs to access various AWS resources such as S3 bucket, we want to create separate K8s serviceaccount that maps to specific IAM role. We have done all the steps to create a OIDC provider, IAM role that maps to the desired K8s serviceaccount and add the serviceacount to the service.yaml. It all works well if we running as root user.

However, when we run the docker image using non-root user, we ran into permission issue ( aws/amazon-eks-pod-identity-webhook#8) .

There is a document in AWS that explain how to use fsGroup under securityContext to grant such permission. This works if it is a regular Kubernetes pod.
https://docs.aws.amazon.com/eks/latest/userguide/iam-roles-for-service-accounts-technical-overview.html

However, Knative does not allow fsGroup under securityContext.
https://knative.dev/docs/serving/spec/knative-api-specification-1.0/

This is a blocker for us. Running docker image as a root user is against best practice and risky.

@dprotaso dprotaso added this to the 0.17.x milestone Jul 31, 2020
@dprotaso
Copy link
Member

@dng00 thanks for the input - I'll throw this into the current milestone (Aug 18th release) since it should be easy to add.

I'll bring it up at the API WG next week for input from the community

@mattmoor
Copy link
Member

mattmoor commented Aug 5, 2020

cc @julz @evankanderson

@evankanderson
Copy link
Member

Looking at PodSecurityContext, the following fields seem "harmless":

Field Description
fsGroup A special supplemental group that applies to all containers in a pod. Some volume types allow the Kubelet to change the ownership of that volume to be owned by the pod: 1. The owning GID will be the FSGroup 2. The setgid bit is set (new files created in the volume will be owned by FSGroup) 3. The permission bits are OR'd with rw-rw---- If unset, the Kubelet will not modify the ownership and permissions of any volume.
fsGroupChangePolicy fsGroupChangePolicy defines behavior of changing ownership and permission of the volume before being exposed inside Pod. This field will only apply to volume types which support fsGroup based ownership(and permissions). It will have no effect on ephemeral volume types such as: secret, configmaps and emptydir. Valid values are "OnRootMismatch" and "Always". If not specified defaults to "Always".
runAsGroup The GID to run the entrypoint of the container process. Uses runtime default if unset. May also be set in SecurityContext. If set in both SecurityContext and PodSecurityContext, the value specified in SecurityContext takes precedence for that container.
runAsNonRoot Indicates that the container must run as a non-root user. If true, the Kubelet will validate the image at runtime to ensure that it does not run as UID 0 (root) and fail to start the container if it does. If unset or false, no such validation will be performed. May also be set in SecurityContext. If set in both SecurityContext and PodSecurityContext, the value specified in SecurityContext takes precedence.
runAsUser The UID to run the entrypoint of the container process. Defaults to user specified in image metadata if unspecified. May also be set in SecurityContext. If set in both SecurityContext and PodSecurityContext, the value specified in SecurityContext takes precedence for that container.
supplementalGroups A list of groups applied to the first process run in each container, in addition to the container's primary GID. If unspecified, no groups will be added to any container.

I'm also wondering if the AWS webhook mentioned in #7924 (comment) should be setting spec.securityContext.fsGroup in addition to the volume changes.

I'm leery of adding the following, as they seem to drive through privilege escalation as well as additional constraint:

Field Description
seLinuxOptions The SELinux context to be applied to all containers. If unspecified, the container runtime will allocate a random SELinux context for each container. May also be set in SecurityContext. If set in both SecurityContext and PodSecurityContext, the value specified in SecurityContext takes precedence for that container.
sysctls Sysctls hold a list of namespaced sysctls used for the pod. Pods with unsupported sysctls (by the container runtime) might fail to launch.
windowsOptions The Windows specific settings applied to all containers. If unspecified, the options within a container's SecurityContext will be used. If set in both SecurityContext and PodSecurityContext, the value specified in SecurityContext takes precedence.

@julz
Copy link
Member

julz commented Aug 5, 2020

so: I don't know how much we care, but technically I think this opens up some new security surface area. For example, if we allow runAsUser a user can now run as an arbitrary user id (including 0). Now in a way this isn't a big deal because (a) they could do that before if they edited the image metadata (b) it should probably be blocked by a PodSecurityPolicy anyway, but some users may have locked-down image registries so couldn't previously have changed the image metadata (b) some users may not have enabled PSPs, since they were relying on knative not allowing these values to be set.

Again, it's not clear to me that there are actually any real users in such a situation so I can see an argument for just allowing this, but: I personally feel quite conservative about exposing fields with security implications - even ones that are very likely ok - without a major version bump or a feature flag defaulting the other way.

@dprotaso
Copy link
Member

dprotaso commented Aug 11, 2020

runUser is already allowed by our runtime contract as a MUST requirement:

https://github.com/knative/serving/blob/master/docs/runtime-contract.md#user

There's a conformance test for it here:

// TestMustRunAsUser verifies that a supplied runAsUser through securityContext takes
// effect as declared by "MUST" in the runtime-contract.
func TestMustRunAsUser(t *testing.T) {

@dprotaso
Copy link
Member

since they were relying on knative not allowing these values to be set.

For enforcement that covers multiple products I'd expect cluster operators to use admission webhooks as a gate.

I think the use case we're missing is taking off the self products and running them on your cluster - these images may have a poorer security posture and thus you may want to set the following properties to ensure they are not running as root

fsGroup
runAsGroup
runAsNonRoot

@dprotaso
Copy link
Member

Talked to @julz offline - we'll add this feature but gate it behind a feature flag in the short term.

Long term we'll want to sort out a few things

  1. when to include this by default and remove the flag
  2. when & how to add this as a requirement (level to be determined) to the runtime contract

@dng00
Copy link

dng00 commented Sep 24, 2020

We are still running into the same issue after upgrading to Knative 0.17
When applying Knative service.yaml with securityContext/fsGroup, we see the following error:

‘’’Error from server (BadRequest): error when creating "servicenew.yaml": admission webhook "webhook.serving.knative.dev" denied the request: mutation failed: cannot decode incoming new object: json: unknown field "fsGroup"’’’

Any advice? Thank you

@dprotaso
Copy link
Member

You'll need to set the feature flag here to allowed

# This feature allows end-users to set a subset of fields on the Pod's SecurityContext
# in addition to expanding the allowable fields within a Container's SecurityContext.
#
# When set to "enabled" or "allowed" it allows the following
# PodSecurityContext properties:
# - FSGroup
# - RunAsGroup
# - RunAsNonRoot
# - SupplementalGroups
# - RunAsUser
#
# When set to "enabled" or "allowed" it allows the following
# Container SecurityContext properties:
# - RunAsNonRoot
# - RunAsGroup
# - RunAsUser (already allowed without this flag)
#
# This feature flag should be used with caution as the PodSecurityContext
# properties may have a side-effect on non-user sidecar containers that come
# from Knative or your service mesh
#
# WARNING: Cannot safely be disabled once enabled.
kubernetes.podspec-securitycontext: "disabled"

@dng00
Copy link

dng00 commented Sep 25, 2020

@dprotaso thank you for your quick response. We did a kubectl edit configmap in knative-serving NS on the running Knative env but still get the same error. Is there a way to check if that flag is active ? Do we need to something else?
Thank you.

@dprotaso
Copy link
Member

dprotaso commented Sep 25, 2020

Are you modifying the text I linked? That's under an _example field which trips up a lot of people.

You need to copy it to under data beside _example

data:
_example: |

@dng00
Copy link

dng00 commented Sep 25, 2020

That was it. It is working now. Thank you for pointing that out.

@dng00
Copy link

dng00 commented Sep 29, 2020

@dprotaso Does the following change affect Istio once changed to "allowed"? Can you help us to understand what are the side-effects once enabled? And any security concerns? Thanks

# This feature flag should be used with caution as the PodSecurityContext
# properties may have a side-effect on non-user sidecar containers that come
# from Knative or your service mesh
#
# WARNING: Cannot safely be disabled once enabled.
kubernetes.podspec-securitycontext: "disabled"

@dprotaso
Copy link
Member

The flag doesn't enable the entire pod security context but just the following properties

     # When set to "enabled" or "allowed" it allows the following 
     # PodSecurityContext properties: 
     # - FSGroup 
     # - RunAsGroup 
     # - RunAsNonRoot 
     # - SupplementalGroups 
     # - RunAsUser 
     # 
     # When set to "enabled" or "allowed" it allows the following 
     # Container SecurityContext properties: 
     # - RunAsNonRoot 
     # - RunAsGroup 
     # - RunAsUser (already allowed without this flag) 

So the side-effect of RunAsUser and RunAsGroup is it could change the running uid/gid of all the sidecars. I'm not deeply familiar with Istio so I don't know the implication of that. I think there are general best-practices though that should be followed - ie. don't run things as root in your container.

I think the answer to your question depends on the trust between the platform team and app dev teams. ie. if the app dev team can directly create pods and set those properties then there's no difference setting them on a Knative Service

Ultimately if you're looking to set only a single property and not expose that to app-teams then you'd probably want a custom webhook

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/API API objects and controllers kind/feature Well-understood/specified features, ready for coding.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants