Skip to content

Process execution checks and IT tests #119010

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

Merged
merged 3 commits into from
Dec 18, 2024
Merged

Conversation

prdoyle
Copy link
Contributor

@prdoyle prdoyle commented Dec 18, 2024

This implements checks for the process-creation methods listed under our Process Execution tab.

Supersedes #118796.

Limitation

I'm unable to test the sensitive instance methods in the entitlement-denied plugin because I'm not sure how to create an instance.

One idea was to create them in the entitlement-allowed plugin, which does have permission, and then pass them to entitlement-denied by createComponents and dependency injection, but when I started to code that, it seemed like a lot of complexity, and I wanted to check that's how we wanted to proceed before putting the effort into getting it working.

@prdoyle prdoyle added >non-issue :Core/Infra/Core Core issues without another label auto-backport Automatically create backport pull requests when merged v9.0.0 v8.18.0 labels Dec 18, 2024
@prdoyle prdoyle self-assigned this Dec 18, 2024
@elasticsearchmachine elasticsearchmachine added the Team:Core/Infra Meta label for core/infra team label Dec 18, 2024
@elasticsearchmachine
Copy link
Collaborator

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

@prdoyle prdoyle requested review from rjernst and jdconrad December 18, 2024 19:15
@prdoyle prdoyle enabled auto-merge (squash) December 18, 2024 22:04
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 once the missing test is addressed.

try (var classLoader = new URLClassLoader("test", new URL[0], RestEntitlementsCheckAction.class.getClassLoader())) {
logger.info("Created URLClassLoader [{}]", classLoader.getName());
} catch (IOException e) {
throw new UncheckedIOException(e);
}
}

private static void processBuilder_start() {
// TODO: processBuilder().start();
Copy link
Member

Choose a reason for hiding this comment

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

We can still construct a process builder, right? I suggest trying to start a jvm, since you can find the executable for the current jvm through java.home sysprop.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok let me add that in a follow-on PR.

I've been passing an empty string for the command on the theory that the command doesn't matter, since whatever happens, it won't throw NotEntitledException and is therefore distinguishable from the correct behaviour.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Follow-on PR is #119100.

@prdoyle prdoyle merged commit c3a59bb into elastic:main Dec 18, 2024
15 of 16 checks passed
@elasticsearchmachine
Copy link
Collaborator

💔 Backport failed

Status Branch Result
8.x Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 119010

prdoyle added a commit to prdoyle/elasticsearch that referenced this pull request Dec 19, 2024
* Process creation checks and IT tests

* Remove process queries; only forbid execution
elasticsearchmachine pushed a commit that referenced this pull request Dec 19, 2024
* Process creation checks and IT tests

* Remove process queries; only forbid execution
navarone-feekery pushed a commit to navarone-feekery/elasticsearch that referenced this pull request Dec 26, 2024
* Process creation checks and IT tests

* Remove process queries; only forbid execution
@prdoyle
Copy link
Contributor Author

prdoyle commented Dec 30, 2024

I did the backport manually in #119106

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/Core Core issues without another label >non-issue Team:Core/Infra Meta label for core/infra team v8.18.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants