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

[Clone] [Security Manager Replacement] Native Java Agent #17517

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

kumargu
Copy link
Contributor

@kumargu kumargu commented Mar 5, 2025

Clone of original implementation of Java agent

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.

Copy link
Contributor

github-actions bot commented Mar 5, 2025

❌ Gradle check result for 1b6692e: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Copy link
Contributor

github-actions bot commented Mar 5, 2025

❌ Gradle check result for 2fcdd44: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@kumargu kumargu force-pushed the kumargu_java_agent_v1 branch from 2fcdd44 to 21e708c Compare March 5, 2025 09:09
Copy link
Contributor

github-actions bot commented Mar 5, 2025

❌ Gradle check result for 21e708c: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@kumargu kumargu force-pushed the kumargu_java_agent_v1 branch from 21e708c to e8aab30 Compare March 5, 2025 15:00
Copy link
Contributor

github-actions bot commented Mar 5, 2025

❌ Gradle check result for e8aab30: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@kumargu kumargu force-pushed the kumargu_java_agent_v1 branch from e8aab30 to dbbef0e Compare March 5, 2025 15:50
Copy link
Contributor

github-actions bot commented Mar 5, 2025

❌ Gradle check result for dbbef0e: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@kumargu kumargu force-pushed the kumargu_java_agent_v1 branch from dbbef0e to 1923e2d Compare March 5, 2025 18:13
Copy link
Contributor

github-actions bot commented Mar 5, 2025

❌ Gradle check result for 1923e2d: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@kumargu kumargu force-pushed the kumargu_java_agent_v1 branch from 1923e2d to 5cb1c07 Compare March 5, 2025 18:42
Copy link
Contributor

github-actions bot commented Mar 5, 2025

❌ Gradle check result for 5cb1c07: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@kumargu kumargu force-pushed the kumargu_java_agent_v1 branch from 5cb1c07 to 51328a0 Compare March 5, 2025 19:08
Copy link
Contributor

github-actions bot commented Mar 5, 2025

❌ Gradle check result for 51328a0: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@kumargu kumargu force-pushed the kumargu_java_agent_v1 branch from 51328a0 to 447c4a8 Compare March 6, 2025 08:23
Copy link
Contributor

github-actions bot commented Mar 6, 2025

❌ Gradle check result for 447c4a8: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@kumargu kumargu force-pushed the kumargu_java_agent_v1 branch from 447c4a8 to 88f3d6f Compare March 6, 2025 09:01
Copy link
Contributor

github-actions bot commented Mar 6, 2025

❌ Gradle check result for 88f3d6f: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@kumargu kumargu force-pushed the kumargu_java_agent_v1 branch 3 times, most recently from 12d3cc3 to ae3e3c2 Compare March 6, 2025 10:36
Copy link
Contributor

github-actions bot commented Mar 6, 2025

❌ Gradle check result for ae3e3c2: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@kumargu kumargu force-pushed the kumargu_java_agent_v1 branch from ae3e3c2 to a5c15b4 Compare March 6, 2025 11:35
Copy link
Contributor

github-actions bot commented Mar 6, 2025

❌ Gradle check result for a5c15b4: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@kumargu kumargu force-pushed the kumargu_java_agent_v1 branch from a5c15b4 to 5a51029 Compare March 6, 2025 13:44
Copy link
Contributor

github-actions bot commented Mar 6, 2025

❌ Gradle check result for 5a51029: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@kumargu kumargu force-pushed the kumargu_java_agent_v1 branch from 5a51029 to bfe165f Compare March 6, 2025 14:16
Copy link
Contributor

github-actions bot commented Mar 6, 2025

❌ Gradle check result for bfe165f: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@kumargu
Copy link
Contributor Author

kumargu commented Mar 6, 2025

❌ Gradle check result for bfe165f: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@peterzhuamazon would you know if I can run this check locally and fix it faster? thanks!

@kumargu kumargu force-pushed the kumargu_java_agent_v1 branch from bfe165f to 1bc341c Compare March 6, 2025 17:56
Copy link
Contributor

❌ Gradle check result for c4db178: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@kumargu kumargu force-pushed the kumargu_java_agent_v1 branch from c4db178 to aa61d93 Compare March 12, 2025 18:52
Copy link
Contributor

❌ Gradle check result for 03761a6: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@kumargu kumargu force-pushed the kumargu_java_agent_v1 branch 2 times, most recently from c8805a6 to f832563 Compare March 12, 2025 20:01
Copy link
Contributor

❌ Gradle check result for f832563: null

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@@ -52,19 +52,23 @@ public static void intercept(@Advice.AllArguments Object[] args, @Origin Method
if (args[0] instanceof InetSocketAddress address) {
if (!AgentPolicy.isTrustedHost(address.getHostString())) {
final String host = address.getHostString() + ":" + address.getPort();

final SocketPermission permission = new SocketPermission(host, "connect,resolve");
final SocketPermission connectResolve = new SocketPermission("*", "connect,resolve");
Copy link
Collaborator

Choose a reason for hiding this comment

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

@kumargu I think this is not the right fix: the host comes from policy and has to be matched.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you are right. I overlooked it while copying it from the policy.

@kumargu kumargu closed this Mar 13, 2025
@kumargu kumargu deleted the kumargu_java_agent_v1 branch March 13, 2025 05:02
@kumargu kumargu reopened this Mar 13, 2025
@kumargu kumargu force-pushed the kumargu_java_agent_v1 branch from f832563 to c772cc5 Compare March 13, 2025 06:30
Copy link
Contributor

✅ Gradle check result for c772cc5: SUCCESS

Copy link

codecov bot commented Mar 13, 2025

Codecov Report

Attention: Patch coverage is 21.01785% with 1195 lines in your changes missing coverage. Please review.

Project coverage is 72.30%. Comparing base (2ee8660) to head (1483b8b).
Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
...va/org/opensearch/secure_sm/policy/PolicyFile.java 24.82% 405 Missing and 28 partials ⚠️
.../org/opensearch/secure_sm/policy/PolicyParser.java 21.00% 350 Missing and 26 partials ⚠️
...ava/org/opensearch/secure_sm/policy/ParseUtil.java 7.14% 189 Missing and 6 partials ⚠️
...java/org/opensearch/secure_sm/policy/Password.java 0.00% 60 Missing ⚠️
...va/org/opensearch/secure_sm/policy/PolicyUtil.java 15.78% 31 Missing and 1 partial ⚠️
.../src/main/java/org/opensearch/javaagent/Agent.java 0.00% 25 Missing ⚠️
...opensearch/javaagent/SocketChannelInterceptor.java 0.00% 24 Missing ⚠️
...rg/opensearch/javaagent/bootstrap/AgentPolicy.java 0.00% 19 Missing ⚠️
.../opensearch/secure_sm/policy/PropertyExpander.java 60.86% 12 Missing and 6 partials ⚠️
...pensearch/javaagent/StackCallerChainExtractor.java 0.00% 6 Missing ⚠️
... and 4 more
Additional details and impacted files
@@             Coverage Diff              @@
##               main   #17517      +/-   ##
============================================
- Coverage     72.43%   72.30%   -0.13%     
- Complexity    65694    65877     +183     
============================================
  Files          5311     5322      +11     
  Lines        304937   306408    +1471     
  Branches      44226    44574     +348     
============================================
+ Hits         220872   221555     +683     
- Misses        65912    66717     +805     
+ Partials      18153    18136      -17     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@kumargu
Copy link
Contributor Author

kumargu commented Mar 13, 2025

@reta thank-you! for pointing out the issue. looks like the all-the-gradle-check passed.

@kumargu kumargu force-pushed the kumargu_java_agent_v1 branch 3 times, most recently from d5e1c00 to 1483b8b Compare March 13, 2025 09:49
Copy link
Contributor

❕ Gradle check result for 1483b8b: UNSTABLE

  • TEST FAILURES:
      4 org.opensearch.repositories.s3.S3BlobStoreRepositoryTests.classMethod
      2 org.opensearch.snapshots.DedicatedClusterSnapshotRestoreIT.testSnapshotWithStuckNode
      1 org.opensearch.action.admin.indices.create.CreateIndexIT.testCreateAndDeleteIndexConcurrently
      1 org.opensearch.action.admin.indices.create.CreateIndexIT.classMethod

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

Signed-off-by: Gulshan Kumar <[email protected]>
@kumargu kumargu force-pushed the kumargu_java_agent_v1 branch from 1483b8b to e36c1e5 Compare March 13, 2025 11:17
@peterzhuamazon
Copy link
Member

peterzhuamazon commented Mar 13, 2025

Commit sha: e36c1e5, Author: Gulshan, Committer: Gulshan; Expected "Gulshan [email protected]", but got "Gulshan Kumar [email protected]".

Copy link
Contributor

❌ Gradle check result for e36c1e5: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@@ -25,7 +25,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- Add filter function for AbstractQueryBuilder, BoolQueryBuilder, ConstantScoreQueryBuilder([#17409](https://github.com/opensearch-project/OpenSearch/pull/17409))
- [Star Tree] [Search] Resolving keyword & numeric bucket aggregation with metric aggregation using star-tree ([#17165](https://github.com/opensearch-project/OpenSearch/pull/17165))
- Added error handling support for the pull-based ingestion ([#17427](https://github.com/opensearch-project/OpenSearch/pull/17427))

- A Java Agent for intercepting socket operation and enforcing a security policy to it.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- A Java Agent for intercepting socket operation and enforcing a security policy to it.
- A Java Agent for intercepting socket operation and enforcing a security policy to it ([#17517](https://github.com/opensearch-project/OpenSearch/pull/17517))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ack


final SocketPermission permission = new SocketPermission(host, "connect,resolve");
final SocketPermission connectResolve = new SocketPermission(host, "connect,resolve");
final SocketPermission listenResolve = new SocketPermission(host, "listen,resolve");
Copy link
Collaborator

Choose a reason for hiding this comment

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

@kumargu I am not sure what exactly you are trying to fix here: we are intercepting SocketChannel:connect only which connects to remote address. The listen is not applicable here, we are not be listening on a socket (this is ServerSocketChannel responsibility). Could you please clarify this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was trying to fix this test failure. Looking at the trace, I think it fails with while attempting "listen & resolve" from localhost:0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had a similar assumption that we are only dealing with SocketChannel:connect but all my efforts to just play around with the policy file didn't work-out and this particular does seem to work.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks @kumargu , I will take a look shortly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants