Skip to content

Conversation

@ldematte
Copy link
Contributor

@ldematte ldematte commented Feb 28, 2025

NIO Files function accept an arbitrary Path object; this objects may be of different classes/come from FileSystems different from the default one, which is the one that provide access to the file systems accessible to the Java virtual machine, and which we are trying to protect.

For example: you can pass a ZipPath (created by a ZipFileSystem) to Files.exists(Path), to check for contents of a Zip file. This works, but check for entitlements on that ZipPath is not correct.

While checking for Files entitlements, we should skip checks on Paths coming from different FileSystems, as they do not intersect with the Paths we are protecting. This PR adds a condition to do that.

@ldematte ldematte added >non-issue auto-backport Automatically create backport pull requests when merged v8.18.1 v8.19.0 v9.0.1 v9.1.0 :Core/Infra/Entitlements Entitlements infrastructure labels Feb 28, 2025
@ldematte ldematte requested a review from a team as a code owner February 28, 2025 18:14
@ldematte ldematte requested a review from rjernst February 28, 2025 18:14
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

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

LGTM

}

private boolean checkPath(String path, String[] paths) {
logger.debug(() -> Strings.format("checking [%s] against [%s]", path, String.join(",", paths)));
Copy link
Member

Choose a reason for hiding this comment

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

This seems more trace level? It could be extremely verbose?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I always mix them up! I will fix all 3 of them

}

private static boolean isParent(String maybeParent, String path) {
logger.debug(() -> Strings.format("checking isParent [%s] for [%s]", maybeParent, path));
Copy link
Member

Choose a reason for hiding this comment

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

Same here, this could log a lot.

@ldematte ldematte enabled auto-merge (squash) February 28, 2025 21:11
@ldematte ldematte added the test-windows Trigger CI checks on Windows label Feb 28, 2025
@ldematte ldematte removed the test-windows Trigger CI checks on Windows label Feb 28, 2025
@ldematte
Copy link
Contributor Author

Removing test-windows as it still fails until we merge #123689 (and we need this to merge it)

@ldematte ldematte merged commit b346427 into elastic:main Feb 28, 2025
16 of 17 checks passed
ldematte added a commit to ldematte/elasticsearch that referenced this pull request Feb 28, 2025
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.18
8.x
9.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-backport Automatically create backport pull requests when merged :Core/Infra/Entitlements Entitlements infrastructure >non-issue Team:Core/Infra Meta label for core/infra team v8.18.1 v8.19.0 v9.0.1 v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants