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

Proposal: manage secrets via OpenFaaS API #807

Open
alexellis opened this Issue Aug 7, 2018 · 34 comments

Comments

Projects
None yet
6 participants
@alexellis
Copy link
Member

alexellis commented Aug 7, 2018

Feature

This feature would allow secrets to be created in the backing provider's credential store using a single CLI command regardless of what the backing orchestrator is.

faas-cli secret create --name secret-name --value secret-value

It could also be used with reading from a file with --from-file for instance.

Why is this needed?

Each tutorial for OpenFaaS has to list Docker Swarm and Kubernetes secret creation commands. This is becoming hard to manage and duplicates effort. It'd be better if we could abstract this away.

A secrets API would also be useful for automating OpenFaaS.

DELETE / POST / GET / PUT -> /system/secrets

I would suggest that fetching a secret value would be not allowed, but getting a list of secrets, deleting or updating should be.

Edit: GET is the same as a list operation, it is not meant to get the plain-text value of a secret, that is not what this feature is about (this was misinterpreted in the comments)

Possible Solution

Add support in OpenFaaS:

  • Create REST API extensions
  • Update Swarm, K8s providers
  • Update CLI

Steps to Reproduce (for bugs)

Example: https://docs.openfaas.com/reference/secrets/#use-the-secret-in-your-function

@LucasRoesler

This comment has been minimized.

Copy link
Member

LucasRoesler commented Aug 7, 2018

I think this is a good idea, I like that it will streamline the documentation. I don't see any major gaps at the moment.

There might be some questions around access controls, but the OpenFaaS system wouldn't be able to support those right now anyway. The system would not be any more leaky than it currently is.

@rgee0

This comment has been minimized.

Copy link
Member

rgee0 commented Aug 7, 2018

I like the idea, and like Lucas my first thought was around controls. There would be a shift here in managing secrets which would need some thought, particularly in the context of dev teams sharing a platform. It feels like this is something which could do with multi-tenancy support in order to work effectively.

@alexellis

This comment has been minimized.

Copy link
Member

alexellis commented Aug 7, 2018

I think I'm hearing: exposing a secrets creation endpoint feels like opening up more access, but if you have access to the API you can do whatever you want already. TLS + auth should be a must for using this - it's one of the reasons I think we should have list, but not "get value"

I'd also like @stefanprodan's input.

@stefanprodan

This comment has been minimized.

Copy link
Member

stefanprodan commented Aug 8, 2018

If we want to offer secrets management with OpenFaaS I would consider the following:

Swarm doesn't support secret updates so we need to generate a unique name for it, detect what functions are using the secret, create a new function definition with the new secret name, change the mount, delete the current Docker service and create a new one. I see no way to avoid downtime for this, also the code will break since the secret name changes so does the path, an env var is needed for the function code to find the secret on disk.

On Kubernetes the secret can be updated but on functions that use of-watchdog we need to trigger a rolling update (by changing a label or something in the deployment pod spec). If the old watchdog is used we don't need to do anything since the function reads the secret from disk on every invocation.

@alexellis

This comment has been minimized.

Copy link
Member

alexellis commented Aug 8, 2018

I did say CRUD, but in effect what I want is for us to provide a way to make our tutorials more concise by having one CLI command for secrets. Given that Swarm can't update a secret, and can't delete one which is in-use with a function, maybe create/delete/list would solve most of the problem?

@stefanprodan

This comment has been minimized.

Copy link
Member

stefanprodan commented Aug 8, 2018

Why limit the capabilities just because Swarm doesn't supports it? I would do create/update/delete/list and for update/delete just error out on Swarm with a message telling the user about the Swarm restrictions.

@alexellis alexellis changed the title Proposal: support secret CRUD via OpenFaaS API Proposal: manage secrets via OpenFaaS API Aug 8, 2018

@alexellis

This comment has been minimized.

Copy link
Member

alexellis commented Aug 8, 2018

I'd consider the live-reloading a non-goal for this item.

@stefanprodan

This comment has been minimized.

Copy link
Member

stefanprodan commented Aug 8, 2018

On Kubernetes the live-reloading can be done in the function code, we can advise our users to read the secret file on every function call even if they use of-watchdog.

@LucasRoesler

This comment has been minimized.

Copy link
Member

LucasRoesler commented Aug 8, 2018

I like the idea of returning an error for update endpoint in Swarm

@LucasRoesler

This comment has been minimized.

Copy link
Member

LucasRoesler commented Aug 8, 2018

for live-reloading couldn't of-watchdog watch the secrets folder for changes and reload itself?

@stefanprodan

This comment has been minimized.

Copy link
Member

stefanprodan commented Aug 8, 2018

Yes it can, all it needs is to detect changes in the ..data symlink, here is an example https://github.com/stefanprodan/k8s-podinfo/blob/master/pkg/fscache/fscache.go

@stefanprodan

This comment has been minimized.

Copy link
Member

stefanprodan commented Aug 8, 2018

If we would have a context object in the handler to wrap the HTTP headers, query strings, env vars, etc we could implement the secrets management in the watchdog and make things easier for our users. For example, the watchdog would read/reload all secrets from disk and expose them as a map attached to the context object.

func Handle(req []byte, context openfaas.context) string {
  twitterKey := context.Secrets["twitter-key"]
  twitterTag := context.Environment["twitter-tag"]
  twitterUser := context.URLQuery["twitter-user"]
}
@alexellis

This comment has been minimized.

Copy link
Member

alexellis commented Aug 8, 2018

This is going a bit off-topic, perhaps open a new issue to suggest live-reloaded of secrets?

The other issue is that we are assuming the underlying code or function can automatically reset itself and that's rarely the case.

Please can we keep this feature discussion focused on how to enable a single UX for creating, deleting and listing secrets? Updating I'd say is out of scope, but if we want to include it then it needs to come with a caveat that the updated value won't be used unless the deployment is updated.

Alex

@rgee0

This comment has been minimized.

Copy link
Member

rgee0 commented Aug 8, 2018

Can we elaborate on why reading was omitted from the initial idea? I think that is alluding to something which needs exploring further. If we were to omit reading but allowed listing then isn't trivial to create a function to dump out all the secrets?

Earlier @alexellis mentioned that if you have API access then you already have free rein. What about situations where that (API access) isn't the case?

@stefanprodan

This comment has been minimized.

Copy link
Member

stefanprodan commented Aug 9, 2018

@rgee0 If I can gain control of an OF Gateway, let's say a weak password is used, I can use the list API to find all the secrets name and using those I could deploy my own function that maps and reads all the secrets. So in a way, not implementing secret get in the API doesn't make your secrets safer.

@LucasRoesler

This comment has been minimized.

Copy link
Member

LucasRoesler commented Aug 9, 2018

Since you can't read the secret value, what purpose does a GET endpoint for a specific secret serve? what would it return?

WRT to not having API access, if you don't have API access, then you can't have CLI access either, right?

@alexellis

This comment has been minimized.

Copy link
Member

alexellis commented Aug 9, 2018

I see no value in adding a GET that would give the plaintext value of the secret from a vault. I do see value in having one single CLI command to create, delete and list them. The commands are there to service the cluster and developers - GET serves no value in my opinion.

@rgee0

This comment has been minimized.

Copy link
Member

rgee0 commented Aug 9, 2018

@stefanprodan I agree

@lucas - it depends which API we are referring to. I took the original reference to mean the orchestrator API, since this is about adding secret support to the OF API gateway. There may be situations within enterprises where the platform is managed by a separate team / org and so adding something to the CLI would open up more access to the developer than they would be used to. Specifically we would be allowing any developer to read secrets across the platform, which may not be desirable.

@alexellis the question wasn't about whether GET should be added, but rather whether the reasoning behind disallowing GET may then be negated by the subsequent inclusion of list.

@alexellis

This comment has been minimized.

Copy link
Member

alexellis commented Aug 9, 2018

Not implementing list does not constitute a security feature. If CREATE/DELETE/UPDATE are available then LIST is going to be needed with it for the user-experience.

If someone has access the to secrets API, they would indeed be able to list the secrets (that is the feature) then request them for their function. At that point they already have administrative access on the whole OpenFaaS API anyway.

GET should not be implemented because it's not needed.

In a multi-user environment like OpenFaaS Cloud - access is blocked to the API from functions using NetworkPolicy. The functions which deploy the functions prefix every secret with your username, thus scoping the functions to only the ones you're allowed to use.

While this is very interesting discussion, are there any objections to the original proposal?

@LucasRoesler

This comment has been minimized.

Copy link
Member

LucasRoesler commented Aug 9, 2018

Is it worth pointing out a potential client only implemention that leverages the users credentials already configured in docker/kubeconf? The CLI could talk to the underlying orchestration directly and leverage what ever credentials and ACL already implemented through those systems?

Ultimately, either solution still relies on security through obscurity, there is nothing stopping someone from guessing the name of a secret and deploying a function. If someone is using common names for secrets, they will leak.

@alexellis

This comment has been minimized.

Copy link
Member

alexellis commented Aug 16, 2018

OpenFaaS users may not have administrative access to the cluster (or in the case of Docker Swarm, may not be logged into the server directly)

Another place where this would be needed is OpenFaaS Cloud for Swarm, where the SealedSecret definitions could be created by the import-secrets function.

@alexellis

This comment has been minimized.

Copy link
Member

alexellis commented Sep 25, 2018

@ivanayov

This comment has been minimized.

Copy link
Member

ivanayov commented Oct 8, 2018

I'd like to work on this

@alexellis

This comment has been minimized.

Copy link
Member

alexellis commented Oct 8, 2018

We are still elaborating on design. Please don't work on this yet.

@alexellis

This comment has been minimized.

Copy link
Member

alexellis commented Nov 2, 2018

Do you have any input on the discussion @ ivanayov?

@alexellis

This comment has been minimized.

Copy link
Member

alexellis commented Nov 3, 2018

In effect I'm suggesting only implementing CLUD (List will be a HTTP GET but won't contain secret values, just the key names)

@ivanayov

This comment has been minimized.

Copy link
Member

ivanayov commented Nov 8, 2018

If this is a general feature of the faas-cli, how would one decide whether the secret should be in openfaas or openfaas-fn namespace (or whatever namespaces used in case of multi-tenancy) using the faas-cli secret create/list/delete ?
This adds a gap between Swarm and K8s implementation as well.

I agree there's no need to GET secret value, as it's never used within a function.

For delete would be good to add some verification with warning listing which functions are using it, but this seems hard to implement.

Off-topic, but I like @stefanprodan's suggestion for context. Would be really nice to use from developer's perspective.

@LucasRoesler

This comment has been minimized.

Copy link
Member

LucasRoesler commented Nov 9, 2018

@ivanayov the provider should always know which namespace the functions are in and that is where the secrets should go. So while the implementation is slightly different, I dont see any complications if we pass the secrets request to the provider

@alexellis

This comment has been minimized.

Copy link
Member

alexellis commented Nov 9, 2018

For Kubernetes the secrets should always be within openfaas-fn or the equivalent namespace - Lucas is right about that point. There's no difference that I can see between Swarm/K8s.

On Stefan's point - changing the function API for every template is a huge piece of work and a breaking change. It's definitely out of scope for this task in my opinion.

FYI: deleting secrets which are in use is not possible in Swarm, but this generates an error and can be returned to the user. Secrets can also not be updated in Swarm, but again we can throw an error.

I've tested this with Kubernetes - and Kubernetes will live-reload secrets into the container - I used it in my Monzo demo.

There are sometimes secrets we want to create in the openfaas namespace during instructions such as how to setup the gateway's basic auth - I think this is out of scope and we'll still need to give dual-instructions - however when it comes to managing functions (which is done far more often) we can reduce the amount of "For Swarm do X" "For K8s do Y".

@ivanayov

This comment has been minimized.

Copy link
Member

ivanayov commented Nov 9, 2018

This makes sense, if we're going to restrict to functions only.

@acornies

This comment has been minimized.

Copy link
Member

acornies commented Nov 9, 2018

Just so I'm up-to-speed, we're talking something more along the lines of:

faas-cli secret create --name secret-name --value secret-value --function figlet and equivalent API call?

I can't speak to K8s or Swarm (yet), but with faas-nomad, each secret is managed in Vault under a function-specific key path like /secret/openfaas/{function}/{name} aligning it to function-only secret management.

What about the notion of secret provider overrides?

faas-cli secret create --provider vault|other

Since K8s and Nomad can work directly with Vault secrets, it could open the door to interacting with secret backends directly. I would need to do this regardless for faas-nomad since Nomad doesn't manage secrets at all, but pulls from Vault as a 1st-class integration.

@alexellis

This comment has been minimized.

Copy link
Member

alexellis commented Nov 9, 2018

No, there's no concept of a secret belonging to a function in the original Issue:

faas-cli secret create --name secret-name --value secret-value / --from-file --value-stdin are the options I'd expect to see. These would go to Kubernetes' API or the Swarm API and create the secret that way.

In addition we may want to apply an annotation or label for filtering so that at least with Swarm we can scope the tool to only the secrets created by this process rather than any secret in the cluster. With OpenFaaS on Kubernetes the secrets are scoped to the openfaas-fn namespace only.

The code to bind and associate secrets is already present in the project, the aim of this feature is to unify it under one CLI and enable automation of "CLUD" on secrets.

@alexellis

This comment has been minimized.

Copy link
Member

alexellis commented Nov 9, 2018

@acornies

This comment has been minimized.

Copy link
Member

acornies commented Nov 15, 2018

Understood. This design is in-line with how Swarm, K8s do secrets natively. I'll start there and we'll see what the implications are when we get to the Nomad/Vault ecosystem (since there's no Nomad API for writing secrets, only Vault API).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment