Avoid redundant authz indices contains check for wildcard expansion #76540
Conversation
albertzaharovits
added
>non-issue
:Security/Authorization
|
Pinging @elastic/es-security (Team:Security) |
| @@ -103,7 +103,7 @@ public IndexAbstractionResolver(IndexNameExpressionResolver indexNameExpressionR | |||
| } else if (dateMathName.equals(indexAbstraction)) { | |||
There was a problem hiding this comment.
Note that the current behavior in Security is different the "core" behavior.
In core, excluding missing index names (ie "-nonexistent") is an error, but not in this case.
Most likely, the fix for it is to adopt the same behavior from the date math expression evaluation from a couple of lines above.
But, I'll stay away from it for now.
The robust, non-patchy fix, is to inject into the index name resolver from core the authorized indices view.
The proposed fix in this PR maintains the current (buggy) behavior, but avoids redundant availableIndexAbstractions.contains checks, hopefully having a small positive performance impact.
|
@elasticmachine test this |
|
@elasticmachine test this please |
There was a problem hiding this comment.
LGTM from a security point of view, but I think we need someone from Core/Infra (or Data Mgt) to review the change to IndexAbstractionResolver
It worries me that we're making this sort of change to a class in core, but didn't need to change any tests - that is, there are no existing tests for how that class should deal with missing concrete indices.
This method is ultimately only used in the I had to triple check. |
|
@elasticmachine update branch |
|
@elasticmachine update branch |
albertzaharovits
added
the
auto-backport
💚 Backport successful
|
* upstream/master: (24 commits) Implement framework for migrating system indices (elastic#78951) Improve transient settings deprecation message (elastic#79504) Remove getValue and getValues from Field (elastic#79516) Store Template's mappings as bytes for disk serialization (elastic#78746) [ML] Add queue_capacity setting to start deployment API (elastic#79433) [ML] muting rest compat test issue elastic#79518 (elastic#79519) Avoid redundant available indices check (elastic#76540) Re-enable BWC tests TEST Ensure password 14 chars length on Kerberos FIPS tests (elastic#79496) [DOCS] Temporarily remove APM links (elastic#79411) Fix CCSDuelIT for skipped shards (elastic#79490) Add other time accounting in HotThreads (elastic#79392) Add deprecation info API entries for deprecated monitoring settings (elastic#78799) Add note in breaking changes for nameid_format (elastic#77785) Use 'migration' instead of 'upgrade' in GET system feature migration status responses (elastic#79302) Upgrade lucene version 8b68bf60c98 (elastic#79461) Use Strings#EMPTY_ARRAY (elastic#79452) Quicker shared cache file preallocation (elastic#79447) [ML] Removing some code that's obsolete for 8.0 (elastic#79444) Ensure indexing_data CCR requests are compressed (elastic#79413) ...
The resolved indices list contains name elements that either: - are concrete names that been explicitly mentioned in the original request, or - are authorized indices that are referred to by a date math expression from the request, or - are authorized indices that are covered by a wildcard from the request If the request option `ignoreUnavailable` is set to true the resolved indices list overall must silently ignore the names that are not in the authorized set. Consequently, only the names in the first category mentioned above must be verified; the check for the other names is redundant.
The resolved indices list contains name elements that either:
If the request option
ignoredUnavailableis set totruethe resolved indices list overallmust silently ignore the names that are not in the authorized set.
Consequently, only the names in the first category mentioned above must be verified;
the check for the other names is redundant.
Related #76481