Skip to content
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
5beafae
Create OpenSearch replacements for widely used methods in AccessContr…
cwperks Apr 22, 2025
e8248de
Fix javadoc
cwperks Apr 22, 2025
9afced3
Remove getException
cwperks Apr 22, 2025
a0a0d9f
Remove other instance of apiNote
cwperks Apr 22, 2025
cc5a240
Modify javadoc and restart stuck CI checks
cwperks Apr 22, 2025
a172f67
Remove mistakenly added line
cwperks May 20, 2025
366406f
Add to CHANGELOG
cwperks May 20, 2025
53be672
Address code review feedback
cwperks May 21, 2025
18ccef4
Use callable and runnable
cwperks May 22, 2025
44eb148
Use Callable
cwperks May 22, 2025
00c22c7
Merge branch 'main' into access-controller
cwperks May 22, 2025
3678956
Add checked equivalents to interface
cwperks May 23, 2025
0d6e1b3
Add throws IllegalArgumentException
cwperks May 23, 2025
d79bdc1
Merge branch 'main' into access-controller
cwperks May 23, 2025
71ba997
Fix precommit
cwperks May 23, 2025
9cfa314
Show example of replacement in a module
cwperks May 23, 2025
435fe93
Merge branch 'main' into access-controller
cwperks May 29, 2025
995c66c
Address code review comments
cwperks May 29, 2025
5b04b59
Fix precommit
cwperks May 29, 2025
f054131
Merge branch 'main' into access-controller
cwperks Jun 2, 2025
7e2a98d
Merge branch 'main' into access-controller
cwperks Jun 6, 2025
cca10f5
Merge branch 'main' into access-controller
cwperks Jun 10, 2025
1348eeb
Address code review comments
cwperks Jun 10, 2025
9c44efb
Merge branch 'access-controller' of https://github.com/cwperks/OpenSe…
cwperks Jun 10, 2025
e81fdf5
Merge branch 'main' into access-controller
cwperks Jun 10, 2025
e735773
Merge branch 'main' into access-controller
cwperks Jun 11, 2025
293fd83
Merge branch 'main' into access-controller
cwperks Jun 13, 2025
2c8e511
Merge branch 'main' into access-controller
cwperks Jun 17, 2025
9dc5780
Merge branch 'access-controller' of https://github.com/cwperks/OpenSe…
cwperks Jun 17, 2025
fc6a21b
Merge branch 'main' into access-controller
cwperks Jun 18, 2025
dc7eafd
Create separate agent-api lib and remove compileOnlyApi
cwperks Jun 19, 2025
9a3f3f2
Re-use agent-policy lib
cwperks Jun 19, 2025
8950858
Address review comments
cwperks Jun 20, 2025
c6a61fc
Move to secure_sm package
cwperks Jun 20, 2025
5c32ba2
Merge branch 'main' into access-controller
cwperks Jun 20, 2025
235bd69
Merge branch 'main' into access-controller
cwperks Jun 21, 2025
e7270f7
Fix conflicts in CHANGELOG
cwperks Jun 21, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- Add support for `matched_fields` with the unified highlighter ([#18164](https://github.com/opensearch-project/OpenSearch/issues/18164))
- [repository-s3] Add support for SSE-KMS and S3 bucket owner verification ([#18312](https://github.com/opensearch-project/OpenSearch/pull/18312))
- Added File Cache Stats - Involves Block level as well as full file level stats ([#17538](https://github.com/opensearch-project/OpenSearch/issues/17479))
- Create equivalents of JSM's AccessController in the java agent ([#18346](https://github.com/opensearch-project/OpenSearch/issues/18346))

### Changed
- Create generic DocRequest to better categorize ActionRequests ([#18269](https://github.com/opensearch-project/OpenSearch/pull/18269)))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import java.lang.StackWalker.StackFrame;
import java.security.ProtectionDomain;
import java.util.Collection;
import java.util.Set;
import java.util.function.Function;
import java.util.stream.Collectors;
import java.util.stream.Stream;
Expand All @@ -24,6 +25,14 @@
*/
public static final StackCallerProtectionDomainChainExtractor INSTANCE = new StackCallerProtectionDomainChainExtractor();

private static final StackWalker STACK_WALKER = StackWalker.getInstance(StackWalker.Option.RETAIN_CLASS_REFERENCE);
private static final Set<String> ACCESS_CONTROLLER_CLASSES = Set.of(

Check warning on line 29 in libs/agent-sm/agent/src/main/java/org/opensearch/javaagent/StackCallerProtectionDomainChainExtractor.java

View check run for this annotation

Codecov / codecov/patch

libs/agent-sm/agent/src/main/java/org/opensearch/javaagent/StackCallerProtectionDomainChainExtractor.java#L28-L29

Added lines #L28 - L29 were not covered by tests
"java.security.AccessController",
"org.opensearch.javaagent.bootstrap.AccessController"
);

private static final Set<String> DO_PRIVILEGED_METHODS = Set.of("doPrivileged", "doPrivilegedChecked");

Check warning on line 34 in libs/agent-sm/agent/src/main/java/org/opensearch/javaagent/StackCallerProtectionDomainChainExtractor.java

View check run for this annotation

Codecov / codecov/patch

libs/agent-sm/agent/src/main/java/org/opensearch/javaagent/StackCallerProtectionDomainChainExtractor.java#L34

Added line #L34 was not covered by tests

/**
* Constructor
*/
Expand All @@ -36,7 +45,7 @@
@Override
public Collection<ProtectionDomain> apply(Stream<StackFrame> frames) {
return frames.takeWhile(
frame -> !(frame.getClassName().equals("java.security.AccessController") && frame.getMethodName().equals("doPrivileged"))
frame -> !(ACCESS_CONTROLLER_CLASSES.contains(frame.getClassName()) && DO_PRIVILEGED_METHODS.contains(frame.getMethodName()))
)
.map(StackFrame::getDeclaringClass)
.map(Class::getProtectionDomain)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,16 @@
import java.security.ProtectionDomain;
import java.util.List;
import java.util.Set;
import java.util.concurrent.Callable;
import java.util.function.Supplier;
import java.util.stream.Collectors;

import static org.opensearch.javaagent.bootstrap.AccessController.CheckedRunnable;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.hasItem;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThrows;

public class StackCallerProtectionDomainExtractorTests {

Expand Down Expand Up @@ -115,4 +119,148 @@ public Void run() {
}
});
}

@Test
public void testStackTruncationWithOpenSearchAccessController() {
org.opensearch.javaagent.bootstrap.AccessController.doPrivileged(() -> {
StackCallerProtectionDomainChainExtractor extractor = StackCallerProtectionDomainChainExtractor.INSTANCE;
Set<ProtectionDomain> protectionDomains = (Set<ProtectionDomain>) extractor.apply(captureStackFrames().stream());
assertEquals(1, protectionDomains.size());
List<String> simpleNames = protectionDomains.stream().map(pd -> {
try {
return pd.getCodeSource().getLocation().toURI();
} catch (URISyntaxException e) {
throw new RuntimeException(e);
}
})
.map(URI::getPath)
.map(Paths::get)
.map(Path::getFileName)
.map(Path::toString)
// strip trailing “-VERSION.jar” if present
.map(name -> name.replaceFirst("-\\d[\\d\\.]*\\.jar$", ""))
// otherwise strip “.jar”
.map(name -> name.replaceFirst("\\.jar$", ""))
.toList();
assertThat(
simpleNames,
containsInAnyOrder(
"test" // from the build/classes/java/test directory
)
);
});
}

@Test
public void testStackTruncationWithOpenSearchAccessControllerUsingSupplier() {
org.opensearch.javaagent.bootstrap.AccessController.doPrivileged((Supplier<Void>) () -> {
StackCallerProtectionDomainChainExtractor extractor = StackCallerProtectionDomainChainExtractor.INSTANCE;
Set<ProtectionDomain> protectionDomains = (Set<ProtectionDomain>) extractor.apply(captureStackFrames().stream());
assertEquals(1, protectionDomains.size());
List<String> simpleNames = protectionDomains.stream().map(pd -> {
try {
return pd.getCodeSource().getLocation().toURI();
} catch (URISyntaxException e) {
throw new RuntimeException(e);
}
})
.map(URI::getPath)
.map(Paths::get)
.map(Path::getFileName)
.map(Path::toString)
// strip trailing “-VERSION.jar” if present
.map(name -> name.replaceFirst("-\\d[\\d\\.]*\\.jar$", ""))
// otherwise strip “.jar”
.map(name -> name.replaceFirst("\\.jar$", ""))
.toList();
assertThat(
simpleNames,
containsInAnyOrder(
"test" // from the build/classes/java/test directory
)
);
return null;
});
}

@Test
public void testStackTruncationWithOpenSearchAccessControllerUsingCallable() throws Exception {
org.opensearch.javaagent.bootstrap.AccessController.doPrivilegedChecked((Callable<Void>) () -> {
StackCallerProtectionDomainChainExtractor extractor = StackCallerProtectionDomainChainExtractor.INSTANCE;
Set<ProtectionDomain> protectionDomains = (Set<ProtectionDomain>) extractor.apply(captureStackFrames().stream());
assertEquals(1, protectionDomains.size());
List<String> simpleNames = protectionDomains.stream().map(pd -> {
try {
return pd.getCodeSource().getLocation().toURI();
} catch (URISyntaxException e) {
throw new RuntimeException(e);
}
})
.map(URI::getPath)
.map(Paths::get)
.map(Path::getFileName)
.map(Path::toString)
// strip trailing “-VERSION.jar” if present
.map(name -> name.replaceFirst("-\\d[\\d\\.]*\\.jar$", ""))
// otherwise strip “.jar”
.map(name -> name.replaceFirst("\\.jar$", ""))
.toList();
assertThat(
simpleNames,
containsInAnyOrder(
"test" // from the build/classes/java/test directory
)
);
return null;
});
}

@Test
public void testAccessControllerUsingCallableThrowsException() {
assertThrows(IllegalArgumentException.class, () -> {
org.opensearch.javaagent.bootstrap.AccessController.doPrivilegedChecked((Callable<Void>) () -> {
throw new IllegalArgumentException("Test exception");
});
});
}

@Test
public void testStackTruncationWithOpenSearchAccessControllerUsingCheckedRunnable() throws IllegalArgumentException {
org.opensearch.javaagent.bootstrap.AccessController.doPrivilegedChecked((CheckedRunnable<IllegalArgumentException>) () -> {
StackCallerProtectionDomainChainExtractor extractor = StackCallerProtectionDomainChainExtractor.INSTANCE;
Set<ProtectionDomain> protectionDomains = (Set<ProtectionDomain>) extractor.apply(captureStackFrames().stream());
assertEquals(1, protectionDomains.size());
List<String> simpleNames = protectionDomains.stream().map(pd -> {
try {
return pd.getCodeSource().getLocation().toURI();
} catch (URISyntaxException e) {
throw new RuntimeException(e);
}
})
.map(URI::getPath)
.map(Paths::get)
.map(Path::getFileName)
.map(Path::toString)
// strip trailing “-VERSION.jar” if present
.map(name -> name.replaceFirst("-\\d[\\d\\.]*\\.jar$", ""))
// otherwise strip “.jar”
.map(name -> name.replaceFirst("\\.jar$", ""))
.toList();
assertThat(
simpleNames,
containsInAnyOrder(
"test" // from the build/classes/java/test directory
)
);
});
}

@Test
public void testAccessControllerUsingCheckedRunnableThrowsException() {
assertThrows(IllegalArgumentException.class, () -> {
org.opensearch.javaagent.bootstrap.AccessController.doPrivilegedChecked((CheckedRunnable<IllegalArgumentException>) () -> {
throw new IllegalArgumentException("Test exception");
});
});
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*/

package org.opensearch.javaagent.bootstrap;
Copy link
Member Author

@cwperks cwperks May 20, 2025

Choose a reason for hiding this comment

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

I don't know the right module for this code. The server has a dependency on this lib, but its marked as compileOnly. How are the other classes in this module (like AgentPolicy) available at runtime?

Copy link
Member

Choose a reason for hiding this comment

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

I believe jars passed to the JVM via -javaagent are available on the classpath, so the compileOnly dependency is making the assumption this will be provided at runtime via a -javaagent.


import java.util.concurrent.Callable;
import java.util.function.Supplier;

/**
* Utility class to run code in a privileged block.
*/
public final class AccessController {
/**
* Don't allow instantiation an {@code AccessController}
*/
private AccessController() {}

/**
* Performs the specified action in a privileged block.
*
* <p> If the action's {@code run} method throws an (unchecked)
* exception, it will propagate through this method.
*
* @param action the action to be performed
*/
public static void doPrivileged(Runnable action) {
action.run();
}

Check warning on line 33 in libs/agent-sm/bootstrap/src/main/java/org/opensearch/javaagent/bootstrap/AccessController.java

View check run for this annotation

Codecov / codecov/patch

libs/agent-sm/bootstrap/src/main/java/org/opensearch/javaagent/bootstrap/AccessController.java#L32-L33

Added lines #L32 - L33 were not covered by tests

/**
* Performs the specified action.
*
* <p> If the action's {@code run} method throws an <i>unchecked</i>
* exception, it will propagate through this method.
*
* @param <T> the type of the value returned by the
* PrivilegedExceptionAction's {@code run} method
*
* @param action the action to be performed
*
* @return the value returned by the action's {@code run} method
*/
public static <T> T doPrivileged(Supplier<T> action) {
return action.get();

Check warning on line 49 in libs/agent-sm/bootstrap/src/main/java/org/opensearch/javaagent/bootstrap/AccessController.java

View check run for this annotation

Codecov / codecov/patch

libs/agent-sm/bootstrap/src/main/java/org/opensearch/javaagent/bootstrap/AccessController.java#L49

Added line #L49 was not covered by tests
}

/**
* Performs the specified action.
*
* <p> If the action's {@code run} method throws an <i>unchecked</i>
* exception, it will propagate through this method.
*
* @param <T> the type of the value returned by the
* PrivilegedExceptionAction's {@code run} method
*
* @param action the action to be performed
*
* @return the value returned by the action's {@code run} method
*
* @throws Exception if the specified action's
* {@code call} method threw a <i>checked</i> exception
*/
public static <T> T doPrivilegedChecked(Callable<T> action) throws Exception {
return action.call();

Check warning on line 69 in libs/agent-sm/bootstrap/src/main/java/org/opensearch/javaagent/bootstrap/AccessController.java

View check run for this annotation

Codecov / codecov/patch

libs/agent-sm/bootstrap/src/main/java/org/opensearch/javaagent/bootstrap/AccessController.java#L69

Added line #L69 was not covered by tests
}

/**
* Performs the specified action in a privileged block.
*
* <p> If the action's {@code run} method throws an (unchecked)
* exception, it will propagate through this method.
*
* @param action the action to be performed
*
* @throws T if the specified action's
* {@code call} method threw a <i>checked</i> exception
*/
public static <T extends Exception> void doPrivilegedChecked(CheckedRunnable<T> action) throws T {
action.run();
}

Check warning on line 85 in libs/agent-sm/bootstrap/src/main/java/org/opensearch/javaagent/bootstrap/AccessController.java

View check run for this annotation

Codecov / codecov/patch

libs/agent-sm/bootstrap/src/main/java/org/opensearch/javaagent/bootstrap/AccessController.java#L84-L85

Added lines #L84 - L85 were not covered by tests

/**
* A functional interface that represents a runnable action that can throw a checked exception.
*
* @param <E> the type of the exception that can be thrown
*/
public interface CheckedRunnable<E extends Exception> {

/**
* Executes the action.
*
* @throws E
*/
void run() throws E;
}
}
Loading
Loading