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

[Feature Request] Explicitly declare what transport actions a plugin will perform and prompt on plugin install #15958

Open
cwperks opened this issue Sep 17, 2024 · 5 comments
Labels
enhancement Enhancement or improvement to existing feature or request Plugins untriaged

Comments

@cwperks
Copy link
Member

cwperks commented Sep 17, 2024

Is your feature request related to a problem? Please describe

This issue is coming from a conversation from here

In some cases, plugins need to perform transport actions and have a guarantee that the operation will succeed. When the security plugin is installed, transport actions will be authorized as the authenticated user[1]. In order to perform transport actions outside of the authenticated user context, a plugin will temporarily stash the ThreadContext and perform actions like interacting with a system index. In #14630 and opensearch-project/security#4665, a general method was added for system index access where a plugin is assigned a pluginSubject that they can use to directly replace instances of try (ThreadContext.StoredContext ctx = threadContext.stashContext()) { ... } calls with pluginSubject.runAs(() -> { ...}). The advantage of the latter is that it allows the security plugin to enforce that plugins only interact with their own system indices and not system indices registered by other plugins.

In opensearch-project/opensearch-plugins#238, it was pointed out that there may be instances other than system index access, that a plugin wants to perform an operation outside of an authenticated user context. I am opening this issue to discuss how plugins can declare what these actions will be and to prompt a cluster administrator at installation time to accept that a plugin will perform the requested actions.

The 2 instances pointed out are:

  • In the security plugin, its possible to use an OpenSearch index for audit logging (internal_opensearch). In this case, security needs a guarantee that it can write to the index even though it is not a system index.
  • @sohami pointed out another use case where node stats action needs to be called outside of authenticated user context

Describe the solution you'd like

I would like something analogous to plugin-security.policy that can define what transport actions a plugin will perform. Minimally, it needs to be separated into 2 sections: 1) Cluster Actions and 2) Index Actions. Index Actions will need index patterns associated with the actions.

One strategy is to define the actions in code and load a plugin at installation time to get its requested permissions. A POC for this strategy was done in this PR: #15778

Another strategy could be to have a yml file on the same level as plugin-security.policy and define the requested permissions in the yml file.

Related component

Plugins

Describe alternatives you've considered

Status quo where plugins can perform any transport actions within a try (ThreadContext.StoredContext ctx = threadContext.stashContext()) { ... } block.

Additional context

[1] When handling an API Request the security plugin will authenticate the request before delegating to the original handler. When an API Request is authenticated the security plugin will hydrate the threadContext with info about the authenticated user.

@cwperks
Copy link
Member Author

cwperks commented Sep 17, 2024

In the case of the audit log index of the security plugin, the name of the audit log index could change based on cluster settings.

plugins.security.audit.config.index: 'security-auditlog-'YYYY.MM.dd <- default value

The settings may not be fully available at installation time.

@reta
Copy link
Collaborator

reta commented Sep 17, 2024

Another strategy could be to have a yml file on the same level as plugin-security.policy and define the requested permissions in the yml file.

I believe the permissions should be declarative and non-dynamic (which programmatic implementation brings with it), plugin-permissions.policy or plugin-permissions.yml would be a reasonable approach to me.

@cwperks
Copy link
Member Author

cwperks commented Sep 17, 2024

@reta One of the challenges that I faced with the security plugin, for the audit log as an opensearch index use-case, is that the final index that security needs permission to is only known after the settings are read.

My solution around this problem was to show the default value on the installation prompt and leave a message:

Plugin requests permission to perform the following transport actions. Any index pattern that appears below is a default value and may change depending on plugin settings.

Does that sound reasonable to you?

Section of the installation prompt around requesting explicit permission to perform cluster actions and index actions:

...

Plugin requests permission to perform the following transport actions. Any index
pattern that appears below is a default value and may change depending on plugin settings.

Cluster Actions
---------------

* cluster:monitor/health

Index Actions
-------------

Index Pattern: security-auditlog-*

* indices:data/write/index

Continue with installation? [y/N]

@reta
Copy link
Collaborator

reta commented Sep 17, 2024

@reta One of the challenges that I faced with the security plugin, for the audit log as an opensearch index use-case, is that the final index that security needs permission to is only known after the settings are read.

Thanks @cwperks , so could we refer to the setting name instead?

@cwperks
Copy link
Member Author

cwperks commented Sep 17, 2024

Yes, that's a good idea. Maybe something like:

Index Actions
-------------

Index Pattern: Setting[key=plugins.security.audit.config.index, default_value='security-auditlog-'YYYY.MM.dd]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement or improvement to existing feature or request Plugins untriaged
Projects
None yet
Development

No branches or pull requests

2 participants