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

Deployment created by Function CRD with service account annotation does not automount the token #892

Open
1 of 2 tasks
kevin-lindsay-1 opened this issue Dec 17, 2021 · 6 comments

Comments

@kevin-lindsay-1
Copy link
Contributor

kevin-lindsay-1 commented Dec 17, 2021

In kubernetes, a service account with automountServiceAccountToken: false does not automatically mount the token into pods created by the deployment.

In order to accommodate this, you need to create a service account which has automountServiceAccountToken: true if you'd like a pod to be able to use it. Being set to true is the default behavior for a service account, which allows any pod in that namespace which requests that service account to automount.

Instead, I would think it's probably a more advisable security practice to instead create a service account with automountServiceAccountToken: false and then in the deployment created by OpenFaaS, set automountServiceAccountToken: true for the pod.

This way, regardless of whether or not you want a service account to amount on pods by default, OF will automount regardless. Right now, if you don't automount by default OF doesn't let you specify that you want the pod to automount at all.

Expected Behaviour

Normally when I create service accounts I set automount to false, and then set automount to true on the pods that will need it.

Current Behaviour

Right now pods do not specify that they would like a token automounted, so if you don't make the service account automount itself, the pod will not attempt to mount it.

Truth table

SA Type SA automount Has Annotation? Pod Automount Is Automounted?
default default (true) false inherited true
custom true false inherited true
custom true true inherited true
custom false false inherited false
custom false true inherited false

Are you a GitHub Sponsor (Yes/No?)

Check at: https://github.com/sponsors/openfaas

  • Yes
  • No

List All Possible Solutions and Workarounds

  • You could just ignore it, as a service account should generally be on a per-function basis. But, I'd say automounting off is a perfectly reasonable setting.
  • You could make a function set automountServiceAccountToken to true when there is a value for com.openfaas.serviceaccount.

Whether or not functions should even have a default service account, and that it should be automounted by default is a different question.

Which Solution Do You Recommend?

The latter. Automount being off by default for a service account, and then opted-in specifically by pods seems easier to reason about, as well as my gut tells me there's possible security smells from having automount on all service accounts in a namespace by default.

Truth table

SA Type SA automount Has Annotation? Pod Automount Is Automounted?
default default (true) false inherited true
custom true false inherited true
custom true true true true
custom false false inherited false
custom false true true true

Steps to Reproduce (for bugs)

  1. Create a service account with automount set to false
  2. There is no way to configure the pod to use the service account

Context

Trying to make a function that uses the k8s api.

Your Environment

  • FaaS-CLI version ( Full output from: faas-cli version ):

  • Docker version docker version (e.g. Docker 17.0.05 ):

  • What version and distriubtion of Kubernetes are you using? kubectl version

  • Operating System and version (e.g. Linux, Windows, MacOS):

  • Link to your project or a code example to reproduce issue:

  • What network driver are you using and what CIDR? i.e. Weave net / Flannel

@LucasRoesler
Copy link
Member

@kevin-lindsay-1 i am a little lost on the motivation and impact here.

Your current proposal is focused on the implementation proposal, but I don't see an explicit statement of why the current default behavior (which matches k8s current default) is broken or insecure.

Currently, with OpenFaaS you can control the ServiceAccount via an annotation com.openfaas.serviceaccount. If the annotation is omitted, then OpenFaaS allows the cluster to do its default action. Generally this is just using the default ServiceAccount in the namespace, whose token should not have any API access unless an admin has assigned a role to it. So this default behavior does not seem very dangerous. When the annotation value is set, then we do pass it to the deployment spec and the pod will use that instead. Both of which behave exactly as you describe, with respect to automount, meaning it will just respect the configuration on the ServiceAccount spec.

I know of two use cases for assigning the ServiceAccount:

  1. You want to give the Pod access to the k8s API
  2. Setting the ImagePullSecrets

It is pretty clear what automountServiceAccountToken does in case (1). But as far as I can tell, it has no impact on case (2). I also think that for OpenFaaS, case (2) is likely the most common use case. But we certainly want to support both.

@kevin-lindsay-1 can you think of any other use cases / stories for how the ServiceAccount would be used?

With the two cases above in mind:

In a production environment, in which someone was to ensure that the cluster follow the principle of least privilage, I would actually recommend that all ServiceAccounts set automountServiceAccountToken: False. And the functions must individually specify automountServiceAccountToken: True. In this situation, I don't think OpenFaaS should ever automatically set automountServiceAccountToken: True. Instead it should be explicitly decided on a case by case basis per function. Meaning, we probably need another annotation for controlling this.

This will support the common use case for imagePullSecrets and seems more secure by default. Making this annotation explicit means that any cluster admin should be able to easily audit which functions and who is potentially requesting access to the k8s API.

Proposal 0

I think we can actually choose to do nothing, instead focusing on documentation and explaining how to use multiple ServiceAccounts with various configurations to control this. The advice would be something like

In a production environment:

  1. The default ServiceAccount should set automountServiceAccountToken: False and should not be given any RBAC roles to allow it to access the k8s API.
    • I think the default SeviceAccount does not have any API access unless you take an active step to add it. But we would still want to advice people to not ever assign permissions unless they are sure and know what they are doing.
  2. Don't reuse ServiceAccounts between functions/applications. Aside from the namespaces default, you should make distinct ServiceAccounts with limited RBAC per your use case.
  3. Only set automountServiceAccountToken: True if the functions you plan on using this ServiceAccount require access to the k8s API.
  4. If you set automountServiceAccountToken: True on a ServiceAccount, we advise being very explicit about which APIs and permissions you allow it to access (this is controlled via RBAC).

The result from this advice is

  • functions are not given tokens by default
  • if you want a function to have a token and access to the API, create a ServiceAccount for it and set automountServiceAccountToken: True

This is, in effect, implicitly setting the automountServiceAccountToken because the pod will just always inherit the value from the ServiceAccount and the function deployer has no ability to override this.

I actually think one could argue that cluster admins would infact prefer this, because you then know that the Pods are using the configuration that they originall created.

Proposal 1

I think the most predictable solution is to add support for a new annotation com.openfaas.serviceaccount.mountToken with the following behavior

  1. if this annotation exists, then we set it on the deployment spec
  2. if the annotation does not exist, we do nothing, we allow the cluster to perform it's default logic

This solution is backwards compatible and allows anyone to opt in to making the cluster more secure by default by simply managing their ServiceAccounts.

This still requires the cluster admin to configure the ServiceAccounts correctly. But it also makes it very explicit about which functions are actually requesting/require a token because it would be encoded in the stack.yml

Proposal 1b (breaking alternative)

If we want to be very strict with "secure by default and PoLP", then the annotation should behave like

  1. if this annotation exists, then we set it on the deployment spec
  2. if the annotation does not exist, we set automountServiceAccountToken: False
    However, someone needs to experiment and confirm if we can set automountServiceAccountToken: False without specifying the serviceAccountName

But, I would consider this behavior a breaking change, but it would be the most secure in the sense that a function is only given a token if it explicitly asks for it.

@kevin-lindsay-1
Copy link
Contributor Author

kevin-lindsay-1 commented Feb 17, 2022

@LucasRoesler I merely propose that the deployment created should at a pod level set automount on if there's a service account associated with it.

Right now, you have to create the service account with automount enabled for all pods that reference it.

There's 4 levels:

  1. default sa auto mount
  2. sa auto mount
  3. pod auto mount
  4. pod manual mount

Pod manual mount requires application-level code to do something that's a 1-line code change in k8s to enable, which is inconvenient and possibly error-prone (and kind of unnecessary in the first place), so I suggest targeting level 3 and leave levels 1 and 2 turned off, if possible. This way service accounts don't control automounting themselves, and instead the pod requests it to be automounted.

Right now, you have to enable automount at level 2 if you don't want to deal with the hassle of doing it at level 4, even though you're really setting the token at level 3. The deployment generated just doesn't do that, probably just because it was a forgotten implementation detail.

@LucasRoesler
Copy link
Member

@kevin-lindsay-1 my understanding of your level (3) is that the OpenFaaS deployment should always enable automount on the Pod level. I propose that this is actually undesirable. This would result in the pod always mounting the token regardless of the ServiceAccount configuration. I don't think we can justify OpenFaaS overriding the configuration that the cluster admin set when they created the ServiceAccount, especailly because we have no sense of the level of cluster authorization the deployer has.

My other proposal is that with options (1) and (2) you can achieve all of the desired capabilities for a function and respect cluster level RBAC permissions. You want to operate at layer (3), but to me that looks equivalent to layer (2) in which you copy or create a new ServiceACcount and toggle the automount value to the desired state (on or off).

Especially because the use case we are discussing already has a non-default ServiceAccount, deploying the function already assumes that someone has created or can create the custom ServiceAccount. Hence they can also create this ServiceAccount with the correct automount value.

I still don't understand the context to what capability or feature is blocked by this with the existing workarounds (copy and toggle at layer 2)? Especially when we consider the use case of unprivileged users being allowed to deploy via OpenFaaS, why should it override the ServiceAccount config, especially on an object that is not owned and managed by OpenFaaS? With a good story for why it is needed and when it should be used, i would be pretty happy to support a new annotation.

I am very curious to hear @alexellis's ideas as well.

@kevin-lindsay-1
Copy link
Contributor Author

kevin-lindsay-1 commented Feb 17, 2022

my understanding of your level (3) is that the OpenFaaS deployment should always enable automount on the Pod level

no, just the ability to specify. just asking for an API, not an opinionated solution.

I still don't understand the context to what capability or feature is blocked by this with the existing workarounds (copy and toggle at layer 2)?

Nothing is blocked, this is just a best practice and exposition of pretty basic k8s functionality that admins are used to.

why should it override the ServiceAccount config

Because otherwise anything that references the service account at all can mount the token.


As far as I'm concerned, level 2 is already implemented. If I automount the token and the pod references it, the pod automounts the token 100% of the time.

I was referring to the pod specifically opting into automounting it, rather than automount by reference alone. That, and if you want a pod to be able to automount a service account at level 2 there's no way to have another pod manual mount (for whatever reason). Since you can't opt-in right now you also can't opt-out.

Most people recommend turning automount off at the sa level and instead turning it on at the pod/deployment level. That's what I'm suggesting. Whether or not openfaas uses a boolean to say whether or not it would like a pod to automount isn't really an issue.

The issue was that if I made a sa I had to enable automount at the sa level, because you can't enable it at the pod level right now. I was merely proposing the ability to opt-in on a pod level, as is the best practice from what I can tell.

So I'm merely asking for the ability to flip a switch that tells the pod to automount or not. The config of the SA is completely out of scope for this IMO, as you've said, OF doesn't manage.

@alexellis
Copy link
Member

Is there any further interest in this @kevin-lindsay-1 ?

Could you summarise why this is needed, if so?

@kevin-lindsay-1
Copy link
Contributor Author

kevin-lindsay-1 commented Mar 1, 2023

It's probably a good idea, yes.

In the truth table provided, the only way to mount said SA token with OF is to use case 2, where all pods automount a SA token. In reality, the best practice is likely case 5, where only the pods that need them automount.

You would probably be able to simply specify a SA for a function via an annotation and it automounts that token, rather than needing an annotation to specify the SA and another to enable it. I imagine this is the most desirable behavior for both security and simplicity's sake.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants