record security exceptions in resolved index expressions #135630

richard-dennehy · 2025-09-29T14:18:29Z

Record security exception for resolved index expressions with CONCRETE_RESOURCE_UNAUTHORIZED results

elasticsearchmachine · 2025-09-29T16:15:24Z

Pinging @elastic/es-security (Team:Security)

n1v0lg · 2025-10-07T11:02:36Z

...ugin/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java

                                if (e instanceof IndexNotFoundException) {
                                    listener.onFailure(e);
                                } else {
-                                    listener.onFailure(actionDenied(authentication, authzInfo, action, request, e));


We need to set security exceptions in the case where the action actually succeeded (due to ignore_unavailable=true) instead of when it failed. When it failed, the recorded expressions are irrelevant since we will throw directly. So we need to take the request post resolution and see if:

it has resolved expressions recorded

if any of those have CONCRETE_RESOURCE_UNAUTHORIZED set

If the above is true, we need to set an actionDenied exception as you do below

n1v0lg · 2025-10-07T16:11:58Z

Sorry, only got to this late!

@richard-dennehy could you update the branch, address this bit, and the req a review from @ywangd when it's ready? If my comment doesn't make sense I hope Yang can provide guidance. The Jira, for reference is ES-13005.

ywangd · 2025-10-08T02:54:29Z

I am bit unsure about the need of this change. Is there a JIRA ticket for it that might provide some more context.

We record CONCRETE_RESOURCE_UNAUTHORIZED with null exception in IndexAbstractionResolver when the index is unauthorized. In CrossProjectIndexResolutionValidator, we check and create a security exception for them. So it already works as is today. Is the problem that this exception does not have sufficient information to be user-friendly and we want something like the exception created by AuthorizationService#actionDenied?

richard-dennehy · 2025-10-08T08:52:38Z

Nikolaj mentioned the Jira ticket in his last comment: ES-13005

But there's not a great deal of context provided there, tbh

ywangd · 2025-10-09T04:03:20Z

OK thanks for the clarification. I think I understand it better now. In CPS, we defer security exception for unauthorized concrete index so that when AuthorizationResultListener#onResponse is called, it is possible that the final validation after CPS fanout resolution still fails even when the result.isGranted() == true.

elasticsearch/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java

Lines 1101 to 1102 in 18f38f9

    
           public void onResponse(T result) { 
        
               if (result.isGranted()) {

Therefore, in addition to call handleFailure in the else branch, we want to iterate ResolvedIndexExpressions to check for any local expressions recorded with CONCRETE_RESOURCE_UNAUTHORIZED and set an exception, generated by calling accessDenied, for them. These exceptions are then used by CrossProjectIndexResolutionValidator#securityException for error reporting. I hope this makes sense.

PS: There is a question about how auditing should behave in this case. But we can defer it for this work.

ywangd · 2025-10-10T04:10:19Z

...security/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java

+            equalTo(
+                "action [indices:data/read/search] is unauthorized for user [user] with effective roles [partial-access-role], "
+                    + "this action is granted by the index privileges [read,all]"
+            )


We should assert the index name is included in the error message

…ession-record-exceptions

ywangd

I have a few comments. Could you also raise a linked serverless PR to update CrossProjectSearchIT with error message assertions for 403 cases in the section of "missing and unauthorized concrete resources -- strict indices options -- errors"?

ywangd · 2025-10-16T03:03:44Z

server/src/main/java/org/elasticsearch/action/ResolvedIndexExpression.java

+                : "If the local resolution result is SUCCESS, exception must be null";
+            Objects.requireNonNull(exception);
+
+            this.exception = exception;


We should also assert this.exception is null before set it

This is causing the CPS Rest IT to fail - I need to figure out why that's happening

Ah OK. I think this is the same double security filtering issue which required the method setResolvedIndexExpressionsIfUnset.

We can do something similar, i.e. set the exception when this.exception is null. If this.exception is already set, assert the two exceptions have the same type and error message (stacktraces are not necessarily the same).

...ugin/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java

ywangd · 2025-10-16T05:27:35Z

...security/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java

+        when(crossProjectModeDecider.crossProjectEnabled()).thenReturn(true);
+        when(crossProjectModeDecider.resolvesCrossProject(any())).thenReturn(true);
+
+        mockMetadataWithIndex("available-index");


The deciding factor here is that the user has no access to the index regardless whether it exists or not. So I'd suggest:

rename available-index and not-available-index to accessible-index and not-accessible-index.

Randomly also add not-accessible-index to the project metadata. The behaviour should not change whether this index exists or not

random between indices request between accessible-index,not-accessible-index and just not-accessible-index. The behaviour should be the same.

ywangd · 2025-10-20T07:38:44Z

Btw, updating CrossProjectSearchIT as suggested above will require updating CrossProjectIndexResolutionValidator as well. I would be OK if you prefer to have both changes as a follow-up.

ywangd

Only a small comment. Could you please make sure CI is green? The serverless side PR is having formatting issue.

ywangd · 2025-10-28T04:17:38Z

server/src/main/java/org/elasticsearch/action/ResolvedIndexExpression.java

+            } else {
+                this.exception = exception;
+            }


This violates what the method name says, i.e. set if unset, since this can happen when it is already set but the newer exception has identical message.

ywangd

LGTM

) * record security exceptions in resolved index expressions * [CI] Auto commit changes from spotless * override equality in LocalExpressions * [CI] Auto commit changes from spotless * record exception for unresolved index when ignore_unavailable is true * clean up * spotless * enable CPS mode in test * include index name in exception message * [CI] Auto commit changes from spotless * address review comments * [CI] Auto commit changes from spotless * assert exception is not set twice * use recorded exception in CrossProjectIndexResolutionValidator * [CI] Auto commit changes from spotless * actually only set exception if unset --------- Co-authored-by: elasticsearchmachine <[email protected]>

record security exceptions in resolved index expressions

eba4f92

richard-dennehy added >non-issue :Security/Security Security issues without another label Team:Security Meta label for security team labels Sep 29, 2025

elasticsearchmachine added the v9.2.0 label Sep 29, 2025

elasticsearchmachine and others added 2 commits September 29, 2025 14:27

[CI] Auto commit changes from spotless

e5e3e12

override equality in LocalExpressions

d2f5563

richard-dennehy marked this pull request as ready for review September 29, 2025 16:15

elasticsearchmachine added v9.3.0 and removed v9.2.0 labels Oct 2, 2025

n1v0lg reviewed Oct 7, 2025

View reviewed changes

richard-dennehy and others added 2 commits October 7, 2025 15:57

merge main

c236591

[CI] Auto commit changes from spotless

e7e8c16

n1v0lg requested review from ywangd and removed request for ywangd October 7, 2025 16:04

richard-dennehy added 3 commits October 9, 2025 15:31

record exception for unresolved index when ignore_unavailable is true

6c302bd

clean up

359fca7

spotless

1fdb0a8

ywangd reviewed Oct 10, 2025

View reviewed changes

richard-dennehy and others added 5 commits October 10, 2025 12:04

Merge remote-tracking branch 'upstream/main' into resolved-index-expr…

4a3ffd4

…ession-record-exceptions

enable CPS mode in test

98a50b9

Merge remote-tracking branch 'upstream/main' into resolved-index-expr…

9d89502

…ession-record-exceptions

include index name in exception message

f143685

[CI] Auto commit changes from spotless

6109ecb

richard-dennehy requested a review from ywangd October 15, 2025 08:14

ywangd reviewed Oct 16, 2025

View reviewed changes

richard-dennehy requested review from a team as code owners October 24, 2025 13:27

merge main

b81d263

richard-dennehy force-pushed the resolved-index-expression-record-exceptions branch from 3753285 to b81d263 Compare October 24, 2025 13:31

richard-dennehy and others added 5 commits October 24, 2025 14:42

address review comments

69e46f3

[CI] Auto commit changes from spotless

a0e594f

assert exception is not set twice

d99f487

use recorded exception in CrossProjectIndexResolutionValidator

dfb3081

[CI] Auto commit changes from spotless

38115ad

elasticsearchmachine added the serverless-linked Added by automation, don't add manually label Oct 27, 2025

richard-dennehy requested a review from ywangd October 27, 2025 16:20

ywangd reviewed Oct 28, 2025

View reviewed changes

richard-dennehy added 2 commits October 28, 2025 09:42

actually only set exception if unset

88f63a3

Merge branch 'main' into resolved-index-expression-record-exceptions

9aef86f

richard-dennehy requested a review from ywangd October 28, 2025 15:51

ywangd approved these changes Oct 29, 2025

View reviewed changes

richard-dennehy merged commit 7edc047 into elastic:main Oct 29, 2025
40 checks passed

richard-dennehy deleted the resolved-index-expression-record-exceptions branch October 29, 2025 09:02

record security exceptions in resolved index expressions #135630

record security exceptions in resolved index expressions #135630

Uh oh!

Conversation

richard-dennehy commented Sep 29, 2025

Uh oh!

elasticsearchmachine commented Sep 29, 2025

Uh oh!

n1v0lg Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

n1v0lg commented Oct 7, 2025

Uh oh!

ywangd commented Oct 8, 2025

Uh oh!

richard-dennehy commented Oct 8, 2025

Uh oh!

ywangd commented Oct 9, 2025

Uh oh!

ywangd Oct 10, 2025

Choose a reason for hiding this comment

Uh oh!

ywangd left a comment

Choose a reason for hiding this comment

Uh oh!

ywangd Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

richard-dennehy Oct 24, 2025

Choose a reason for hiding this comment

Uh oh!

ywangd Oct 27, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ywangd Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

ywangd commented Oct 20, 2025

Uh oh!

ywangd left a comment

Choose a reason for hiding this comment

Uh oh!

ywangd Oct 28, 2025

Choose a reason for hiding this comment

Uh oh!

ywangd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

n1v0lg Oct 7, 2025 •

edited

Loading