falcosecurity/falcoctl

Refactor and improve registry auth logic

max-frank opened this issue · 7 comments

max-frank commented

What would you like to be added:

Refactor of the registry auth related code to make it easier to handle multiple authentication types.

Why is this needed:

Right now there are only two authentication types Basic and OAuth, but a third one is already planned to be added. Following the discussion in #277 the code base currently is not suited to add more authentication types and therefore should be refactored a bit.

max-frank commented

As mentioned in the linked issue I am gonna try to have a proposal PR for this ready in the next days.

max-frank commented

@loresuso I am working on my refactoring PR and I reached the part for supporting login and I am slightly confused about how I should handle saving credentials. It seems that credentials are always saved in two locations

For the two examples above it seems for the credential file the oauth scopes are saved, but they are not saved in the config file.

Saved information is (almost) identical as far as i can tell. Is there a reason for this and required?

max-frank commented

Additionally I noticed that when running the other commands e.g., pull, push the auth logins for both oauth and basic are executed for all saved auths regardless of the registry targeted by the command

Does this need to stay like this or can this be changed to only do it for the targeted registry?

I am asking about both of these things (credential config double save and logins) since the reasoning behind those would affect the way I re-write the code. Right now it seems that falcoctl is half mirroring credential management of docker-cli, but at the same time keeping its own (arguably less secure due just being backed by a config file while docker credential store can use native secret storage) credential config as well.

alacuku commented

Additionally I noticed that when running the other commands e.g., pull, push the auth logins for both oauth and basic are executed for all saved auths regardless of the registry targeted by the command

* https://github.com/falcosecurity/falcoctl/blob/main/cmd/registry/pull/pull.go#L106

* https://github.com/falcosecurity/falcoctl/blob/main/internal/login/login.go#L45

Does this need to stay like this or can this be changed to only do it for the targeted registry?

I think we should perform the authentication only for the registries targeted by the command.

I am asking about both of these things (credential config double save and logins) since the reasoning behind those would affect the way I re-write the code. Right now it seems that falcoctl is half mirroring credential management of docker-cli, but at the same time keeping its own (arguably less secure due just being backed by a config file while docker credential store can use native secret storage) credential config as well.

For credential management I would suggest having a look at this project: https://github.com/oras-project/oras-credentials-go. It satisfies all of our use cases such as secure stores and credential helpers.

alacuku commented

@loresuso I am working on my refactoring PR and I reached the part for supporting login and I am slightly confused about how I should handle saving credentials. It seems that credentials are always saved in two locations

* the config file e.g., https://github.com/falcosecurity/falcoctl/blob/main/cmd/registry/auth/oauth/oauth.go#L122

* some credentials file e.g., https://github.com/falcosecurity/falcoctl/blob/main/cmd/registry/auth/oauth/oauth.go#L99

For the two examples above it seems for the credential file the oauth scopes are saved, but they are not saved in the config file.

Saved information is (almost) identical as far as i can tell. Is there a reason for this and required?

Our config file is used as a workaround to provide credentials to falcoctl when running in unattended mode(k8s, or systemd service). Those credentials can be used as is, eg. user/password scenario, or used to retrieve a bear token/ identity token which are then saved in the credential store.
In my opinion, we should still have a way to provide an "initial" credential configuration to falcoctl through the config file or interactively and then store those credentials using in a secure store.

max-frank commented

In my opinion, we should still have a way to provide an "initial" credential configuration to falcoctl through the config file or interactively and then store those credentials using in a secure store.

I see two options for this while avoiding redundancy and both could be implemented/kept at the same time.

  1. For unattended mode you could pre-load credentials into whatever credstore that is configured using the auth/login commands. In k8s this could be done via an init container and systemd via startup script.
  2. Place a filestore credential file and set falcoctl to use this file for the registry. The file can also be dropped into whatever system via secret mount.

Also for more advanced unattended environments e.g., GKE, GCE, AWS EKS, etc. proper credential store support will make it possible to use their respective credential helpers. For envs that do not have credhelpers yet they could simply be added which would help not only falcoctl but everything using the credentials store.

I'll prepare a proposal PR using https://github.com/oras-project/oras-credentials-go that will hopefully cover all your concerns.

max-frank commented

Ok I have an initial crude code base that should make things a bit more flexible.

Basically the modification while stretched over multiple files are doing two things.

  1. Only initialize one client that can handle all registries and auth types
  2. Auth credentials are only written to credentialstore (basic auth) and credentials file (oauth) and only those sources are used for authentication in the client. For connivence and backward compatibility though there still exists login logic that reads the creds from the falcoctl config and writes them to their respective credential storage locations.

For the client I moved the registry specific credential retrieval logic into the auth.Client using the pre-existing Credential hook function func(ctx contex.Contex, reg string)
Its possible to register multiple credential sources in the form of Credential hook functions for a client. I've implemented the basic auth credential store and oauth client credentials version that were supported by the previous logic.
The client will resolve credentials for a registry using the following steps

  1. Check the credential function cache, if we have an entry for this registry use it
  2. Iterate all configured credential functions save the first one that gives us a non empty credential in the cache and return that credential
  3. If all credential functions give empty credentials save a special empty cred function in the cache.

With these steps only the first registry lookup will require a full lookup and all subsequent ones will read from the cache.

In addition the client can also support auto login based on the falcoctl config (i.e., registry.auth.basic/oauth). With this setting enabled the client will read the config and try to execute the login logic for basic auth/oauth, if it can find an entry in the config for the registry.