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

[CVE-2022-35980] Ensure concrete indexes included during index resolution for DLS/FLS/Masking checks #1999

Merged

Conversation

peternied
Copy link
Member

Resolving index patterns into concrete indices had some confusing logic
make it hard to verify its behavior. Updated this code to be easier
to read and added test cases to confirm resolution patterns. Also added
unit tests to ensure line/by line coverage was correct.

Issues Resolved

Testing

Check List

  • New functionality includes testing
  • New functionality has been documented
  • Commits are signed per the DCO using --signoff

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.

@peternied peternied requested a review from a team August 5, 2022 21:22
@peternied peternied self-assigned this Aug 5, 2022
Resolving index patterns into concrete indices had some confusing logic
make it hard to verify its behavior.  Updated this code to be easier
to read and added test cases to confirm resolution patterns.  Also added
unit tests to ensure line/by line coverage was correct.

Signed-off-by: Peter Nied <[email protected]>
@peternied peternied force-pushed the resolve-indices-changes branch from 6c000d5 to 107cd4f Compare August 5, 2022 21:24
Copy link
Member

@cwperks cwperks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the additional tests around the index pattern resolution logic. This is a good addition to making it clearer about how the process works.

cwperks
cwperks previously approved these changes Aug 8, 2022
cliu123
cliu123 previously approved these changes Aug 8, 2022
@cliu123
Copy link
Member

cliu123 commented Aug 8, 2022

The fix looks good to me! But the CI failed. I'll redo approval when CI passes.

@peternied peternied dismissed stale reviews from cliu123 and cwperks via ac1acc8 August 8, 2022 18:33
@codecov-commenter
Copy link

Codecov Report

Merging #1999 (ac1acc8) into main (f7b6fe5) will increase coverage by 0.01%.
The diff coverage is 94.11%.

@@             Coverage Diff              @@
##               main    #1999      +/-   ##
============================================
+ Coverage     61.03%   61.04%   +0.01%     
  Complexity     3232     3232              
============================================
  Files           256      256              
  Lines         18085    18084       -1     
  Branches       3222     3221       -1     
============================================
+ Hits          11038    11040       +2     
  Misses         5471     5471              
+ Partials       1576     1573       -3     
Impacted Files Coverage Δ
...pensearch/security/securityconf/ConfigModelV7.java 63.29% <94.11%> (+0.53%) ⬆️
...ecurity/ssl/rest/SecuritySSLReloadCertsAction.java 84.78% <0.00%> (-0.33%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@peternied peternied merged commit 42b936e into opensearch-project:main Aug 8, 2022
@peternied peternied added backport 2.2 backport to 2.2 backport 2.x backport to 2.x branch labels Aug 8, 2022
opensearch-trigger-bot bot pushed a commit that referenced this pull request Aug 8, 2022
opensearch-trigger-bot bot pushed a commit that referenced this pull request Aug 8, 2022
peternied added a commit that referenced this pull request Aug 9, 2022
(cherry picked from commit 42b936e)

Co-authored-by: Peter Nied <[email protected]>
@peternied peternied deleted the resolve-indices-changes branch August 9, 2022 17:27
bharath-techie pushed a commit to bharath-techie/security that referenced this pull request Aug 12, 2022
@peternied peternied changed the title Update indices resolution to be clearer [CVE-2022-35980] Ensure concrete indexes included during indices resolution Aug 12, 2022
@peternied peternied changed the title [CVE-2022-35980] Ensure concrete indexes included during indices resolution [CVE-2022-35980] Ensure concrete indexes included during index resolution for DLS/FLS/Masking checks Aug 12, 2022
peternied added a commit that referenced this pull request Aug 12, 2022
(cherry picked from commit 42b936e)

Co-authored-by: Peter Nied <[email protected]>
peternied added a commit that referenced this pull request Nov 9, 2022
* Update indices resolution to be clearer (#1999)
* Resolving backing indices of data streams when resolving for aliases
* Fixing resolution of indices for non-wild card scenarios / exact matches
* Adding tests for DLS/FLS/Field-Masking on Data Streams

Signed-off-by: Craig Perkins <[email protected]>
Co-authored-by: Sandesh Kumar <[email protected]>
Co-authored-by: Peter Nied <[email protected]>
Co-authored-by: Dave Lago <[email protected]>
Co-authored-by: Sandesh Kumar <[email protected]>
stephen-crawford pushed a commit to stephen-crawford/security that referenced this pull request Nov 10, 2022
wuychn pushed a commit to ochprince/security that referenced this pull request Mar 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x backport to 2.x branch backport 2.2 backport to 2.2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants