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

Authorizing User against requested extension endpoint #2622

Closed
Tracked by #2590
DarshitChanpura opened this issue Mar 31, 2023 · 7 comments
Closed
Tracked by #2590

Authorizing User against requested extension endpoint #2622

DarshitChanpura opened this issue Mar 31, 2023 · 7 comments
Assignees
Labels
triaged Issues labeled as 'Triaged' have been reviewed and are deemed actionable.

Comments

@DarshitChanpura
Copy link
Member

DarshitChanpura commented Mar 31, 2023

[WIP] #2601

We need to create a new check to confirm if a user is authorized to make a request to a specific endpoint. This is not currently part of our security model, but we want to add it to enhance security and efficiency by preventing any unauthorized requests to the extension. This will improve network bandwidth by discarding unauthorized requests since all communication with extensions would happen at REST Layer.

---
title: Authorize User against requested extension endpoint
---
flowchart TB
    Request((Request)) --> |An API Request received in OpenSearch <br/> is intercepted by rest wrapper| Wrapper
    Wrapper --> |After authn is successful, Authorize user against requested endpoint| Check{AuthZ check}
    Check --> |Failure, 403 Unauthorized| Response((Response))
    Check -->|Success, proceed with original rest handler| OriginalRestHandler
    OriginalRestHandler --> |Process the request and form the response| OriginalRestHandler
    OriginalRestHandler --> |Return the response, 200 OK| Response
Loading
@github-actions github-actions bot added the untriaged Require the attention of the repository maintainers and may need to be prioritized label Mar 31, 2023
@stephen-crawford stephen-crawford added triaged Issues labeled as 'Triaged' have been reviewed and are deemed actionable. and removed untriaged Require the attention of the repository maintainers and may need to be prioritized labels Apr 3, 2023
@stephen-crawford
Copy link
Contributor

[Triage] This is part of the Extensions project.

@samuelcostae
Copy link
Contributor

@scrawfor99 commenting so you're able to assign me the issue.

I started looking into during downtime from 2553

@samuelcostae
Copy link
Contributor

samuelcostae commented Apr 22, 2023

@scrawfor99 @cwperks Hi, I was working on this, but then while looking for more info I came across this branch/PR which at first sight seem to me to be aiming at the same thing.

https://github.com/opensearch-project/security/pull/2601/files#diff-ae9baf16a1b48d45664aa53173cf7a11f190d949b3aa1b351ae6aba7550d49ee

Is that the case?

@cwperks
Copy link
Member

cwperks commented Apr 22, 2023

Hi @samuelcostae, that PR is related but there are still items related to figure out. I was hoping to get extension TLS related changes merged earlier which helps with testing REST-layer authz since you can spin up an extension and an opensearch node with the security plugin installed and test the change end-to-end. Eventually, all of that manual testing will be automated where we can test an opensearch node with the security plugin installed operating with extensions.

Related to that PR there are 2 immediate follow-up problems to figure out:

  1. Allow extensions to register roles #2595 - This change is taking roles defined in the security plugin's roles.yml and instead sourcing them to the security index from an extension configuration file
  2. The PR works to migrate any plugin that defines a cluster:* action, but does not work with indices:* such as:
index_management_full_access:
  reserved: true
  cluster_permissions:
    - "cluster:admin/opendistro/ism/*"
    - "cluster:admin/opendistro/rollup/*"
    - "cluster:admin/opendistro/transform/*"
    - "cluster:admin/opensearch/notifications/feature/publish"
  index_permissions:
    - index_patterns:
        - '*'
      allowed_actions:
        - 'indices:admin/opensearch/ism/*'

There also needs to be a way to permission based on index patterns that are supplied with a request too. This second item isn't captured in an issue yet, but I will create one for it on Monday.

@samuelcostae
Copy link
Contributor

samuelcostae commented Apr 26, 2023

@cwperks Thanks!

I pulled your branch locally to avoid future conflicts, but then i got some missing references to OpenSearch packages, specifically

 org.opensearch.rest.extensions.RestSendToExtensionAction;
 org.opensearch.rest.PermissibleRoute;

I couln't find in which branch of opensearch those have been added to. I had assumed 'Features/extensions', but that doesnt seem to be the case. 

Can you point in which branch/build those were added

Edit:
Strike that, just noticed that you linked it in the PR description. https://github.com/cwperks/OpenSearch/tree/extension-handshake

@cwperks
Copy link
Member

cwperks commented Apr 26, 2023

@samuelcostae Yes that's the one. I was hoping to have some of these merged/closer to merging by now but these are currently blocked by this PR in the SDK repo: opensearch-project/opensearch-sdk-java#619

When checking out that branch in opensearch core do:

./gradlew publishToMavenLocal

to publish artifacts to your local maven repository which will be picked up first during gradle dependency resolution if artifacts exist in your local maven repo.

@davidlago
Copy link

Addressed in the REST AuthZ META feature, closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triaged Issues labeled as 'Triaged' have been reviewed and are deemed actionable.
Projects
None yet
Development

No branches or pull requests

5 participants