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

[BUG] Requests from extensions should not trust incoming data #7467

Open
peternied opened this issue May 8, 2023 · 2 comments
Open

[BUG] Requests from extensions should not trust incoming data #7467

peternied opened this issue May 8, 2023 · 2 comments
Labels
bug Something isn't working extensions

Comments

@peternied
Copy link
Member

Describe the bug
In the extensions manager there are several handlers that accept data from extensions. This handlers accept as part of the request the extension id that the request is to apply to. The request from an extension should not be trusted, as it can impersonate any other extension that it guesses the extension id of.

By my inspecting of opensearch-sdk-java the following actions need modification of some kind:

  • AddSettingsUpdateConsumerRequest
  • ExtensionRequest
  • RegisterTransportActionsRequest
  • TransportActionRequestFromExtension
  • RegisterRestActionsRequest
  • AddSettingsUpdateConsumerRequest

Expected behavior
When the request is received an authority within OpenSearch should be referenced to resolve the extension id. This could be by checking against the registered transport address, or by embedding a tamperproof identifier that is expected on receipt of the request.

@peternied peternied added bug Something isn't working untriaged labels May 8, 2023
@peternied
Copy link
Member Author

FYI - @dbwiddis

@mch2 mch2 added the extensions label May 9, 2023
@dbwiddis
Copy link
Member

dbwiddis commented Jun 7, 2023

Describe the bug In the extensions manager there are several handlers that accept data from extensions. This handlers accept as part of the request the extension id that the request is to apply to. The request from an extension should not be trusted, as it can impersonate any other extension that it guesses the extension id of.

Fair call-out as the extension is only providing its unique ID as its identity.

By my inspecting of opensearch-sdk-java the following actions need modification of some kind:

Please see opensearch-project/opensearch-sdk-java#767 prompted in part by this issue, but also by multiple other related issues (most significant, the inability of an extension outside a VPC to directly connect to an OpenSearch node inside a private subnet without extra network configuration steps)

@owaiskazi19 is actively working on changing the initialization order as part of opensearch-project/opensearch-sdk-java#782 which will remove the following requests and their handlers, replacing them with a response to the OpenSearch-initiated request:

  • RegisterTransportActionsRequest
  • RegisterRestActionsRequest

For the others:

  • ExtensionRequest

The version needing the unique ID is just a request for extension dependencies which isn't actually implemented or triggered in the SDK right now; when implemented it can probably be removed and be part of the initialization sequence similar to the above two.

  • AddSettingsUpdateConsumerRequest

This must be of serious concern as it's listed twice ;)

It can/should be replaced by a REST API call per opensearch-project/opensearch-sdk-java#767

  • TransportActionRequestFromExtension

I think this one can be replaced by a REST API call as well, although it's not trivial.

Expected behavior When the request is received an authority within OpenSearch should be referenced to resolve the extension id. This could be by checking against the registered transport address, or by embedding a tamperproof identifier that is expected on receipt of the request.

One other potential alternative we've thought about is having a REST API "I have a message for you" request from an extension, which OpenSearch would respond to over it's own self-initiated transport connection.

But the TLDR of all of these is that all handlers using Unique ID should be removed from the Extensions Manager at a minimum, and if I get my way, all handlers will be completely removed anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working extensions
Projects
None yet
Development

No branches or pull requests

4 participants