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

[DRAFT] Add a new sandbox module to provide system index protection from the core #16695

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

cwperks
Copy link
Member

@cwperks cwperks commented Nov 20, 2024

Description

Opening this PR to demonstrate the challenges of porting system index protection to the core repo. System index protection is provided by the security plugin and protects system indices in the following 2 ways:

  1. Admin operations on the indices are forbidden
    • These indices cannot be deleted
    • These indices cannot be written to with a REST Request, they can only be programmatically accessed
      • For cluster operators needing direct system index access they can present the admin certificate (security plugin concept w/o core analog).
  2. Search requests on system indices get their results scrubbed.
  • Rationale is that system indices can contain sensitive data so while search would give a 200 response, the security plugin clears the result set (search is possible if done with the admin certificate)

This PR provides a crude implementation of system index protection in the core for #1. This PR does not include a core analog of the admin certificate so it would only permit programmatic access to system indices.

One of the biggest challenges implementing system index protection as an Action Filter, is resolving a generic ActionRequest to a list of concrete indices. This PR borrows the IndexResolverReplacer from the security plugin to resolve a generic ActionRequest to a resolved request that contains a list of concrete indices that the request resolves to.

Related Issues

Related to discussion in this thread: #15778 (comment)

Check List

  • Functionality includes testing.
  • API changes companion pull request created, if applicable.
  • Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@cwperks cwperks changed the title [DRAFT] A new sandbox module to provide system index protection from the core [DRAFT] Add a new sandbox module to provide system index protection from the core Nov 20, 2024
Copy link
Contributor

❌ Gradle check result for dc75724: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Signed-off-by: Craig Perkins <[email protected]>
Copy link
Contributor

❌ Gradle check result for fb2cbb0: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Copy link
Contributor

❌ Gradle check result for fb2cbb0: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Copy link
Contributor

❌ Gradle check result for d48afcf: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Copy link
Contributor

❌ Gradle check result for 83896cd: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Signed-off-by: Craig Perkins <[email protected]>
Copy link
Contributor

❌ Gradle check result for 0ef49bf: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Copy link
Contributor

❌ Gradle check result for c25f1d7: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Copy link
Contributor

❕ Gradle check result for c25f1d7: UNSTABLE

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

Copy link

codecov bot commented Nov 22, 2024

Codecov Report

Attention: Patch coverage is 9.06516% with 642 lines in your changes missing coverage. Please review.

Project coverage is 71.85%. Comparing base (9388217) to head (8a2a8a4).
Report is 47 commits behind head on main.

Files with missing lines Patch % Lines
...opensearch/index/filter/IndexResolverReplacer.java 0.00% 374 Missing ⚠️
...a/org/opensearch/index/filter/WildcardMatcher.java 34.42% 110 Missing and 10 partials ⚠️
...lugin/systemindex/SystemIndexProtectionPlugin.java 0.00% 36 Missing ⚠️
...org/opensearch/index/filter/SystemIndexFilter.java 0.00% 31 Missing ⚠️
...opensearch/index/filter/SnapshotRestoreHelper.java 0.00% 29 Missing ⚠️
...org/opensearch/index/filter/ClusterInfoHolder.java 0.00% 26 Missing ⚠️
...arch/plugin/systemindex/EmptyFilterLeafReader.java 0.00% 14 Missing ⚠️
...plugin/systemindex/SystemIndexSearcherWrapper.java 0.00% 12 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main   #16695      +/-   ##
============================================
- Coverage     72.12%   71.85%   -0.27%     
+ Complexity    65230    65168      -62     
============================================
  Files          5318     5326       +8     
  Lines        303940   304646     +706     
  Branches      43976    44156     +180     
============================================
- Hits         219207   218901     -306     
- Misses        66803    67800     +997     
- Partials      17930    17945      +15     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Craig Perkins <[email protected]>
Copy link
Contributor

✅ Gradle check result for 8a2a8a4: SUCCESS

@opensearch-trigger-bot
Copy link
Contributor

This PR is stalled because it has been open for 30 days with no activity.

@opensearch-trigger-bot opensearch-trigger-bot bot added stalled Issues that have stalled and removed stalled Issues that have stalled labels Dec 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant