Skip to content

Commit 972e303

Browse files
cwperkscwperks
authored
Create equivalents of JSM's AccessController in the java agent (opensearch-project#18346)
* Create OpenSearch replacements for widely used methods in AccessController Signed-off-by: Craig Perkins <[email protected]> * Fix javadoc Signed-off-by: Craig Perkins <[email protected]> * Remove getException Signed-off-by: Craig Perkins <[email protected]> * Remove other instance of apiNote Signed-off-by: Craig Perkins <[email protected]> * Modify javadoc and restart stuck CI checks Signed-off-by: Craig Perkins <[email protected]> * Remove mistakenly added line Signed-off-by: Craig Perkins <[email protected]> * Add to CHANGELOG Signed-off-by: Craig Perkins <[email protected]> * Address code review feedback Signed-off-by: Craig Perkins <[email protected]> * Use callable and runnable Signed-off-by: Craig Perkins <[email protected]> * Use Callable Signed-off-by: Craig Perkins <[email protected]> * Add checked equivalents to interface Signed-off-by: Craig Perkins <[email protected]> * Add throws IllegalArgumentException Signed-off-by: Craig Perkins <[email protected]> * Fix precommit Signed-off-by: Craig Perkins <[email protected]> * Show example of replacement in a module Signed-off-by: Craig Perkins <[email protected]> * Address code review comments Signed-off-by: Craig Perkins <[email protected]> * Fix precommit Signed-off-by: Craig Perkins <[email protected]> * Address code review comments Signed-off-by: Craig Perkins <[email protected]> * Create separate agent-api lib and remove compileOnlyApi Signed-off-by: Craig Perkins <[email protected]> * Re-use agent-policy lib Signed-off-by: Craig Perkins <[email protected]> * Address review comments Signed-off-by: Craig Perkins <[email protected]> * Move to secure_sm package Signed-off-by: Craig Perkins <[email protected]> * Fix conflicts in CHANGELOG Signed-off-by: Craig Perkins <[email protected]> --------- Signed-off-by: Craig Perkins <[email protected]>Signed-off-by: TJ Neuenfeldt <[email protected]>
1 parent db1b46c commit 972e303

File tree

7 files changed

+327
-36
lines changed

7 files changed

+327
-36
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
99
- Ability to run Code Coverage with Gradle and produce the jacoco reports locally ([#18509](https://github.com/opensearch-project/OpenSearch/issues/18509))
1010
- Add NodeResourceUsageStats to ClusterInfo ([#18480](https://github.com/opensearch-project/OpenSearch/issues/18472))
1111
- Introduce SecureHttpTransportParameters experimental API (to complement SecureTransportParameters counterpart) ([#18572](https://github.com/opensearch-project/OpenSearch/issues/18572))
12+
- Create equivalents of JSM's AccessController in the java agent ([#18346](https://github.com/opensearch-project/OpenSearch/issues/18346))
1213

1314
### Changed
1415
- Update Subject interface to use CheckedRunnable ([#18570](https://github.com/opensearch-project/OpenSearch/issues/18570))

‎libs/agent-sm/agent-policy/src/main/java/org/opensearch/secure_sm/AccessController.java

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
*
4+
* The OpenSearch Contributors require contributions made to
5+
* this file be licensed under the Apache-2.0 license or a
6+
* compatible open source license.
7+
*/
8+
9+
package org.opensearch.secure_sm;
10+
11+
import java.util.concurrent.Callable;
12+
import java.util.function.Supplier;
13+
14+
/**
15+
* A utility class that provides methods to perform actions in a privileged context.
16+
*
17+
* This class is a replacement for Java's {@code java.security.AccessController} functionality which is marked for
18+
* removal. All new code should use this class instead of the JDK's {@code AccessController}.
19+
*
20+
* Running code in a privileged context will ensure that the code has the necessary permissions
21+
* without traversing through the entire call stack. See {@code org.opensearch.javaagent.StackCallerProtectionDomainChainExtractor}
22+
*
23+
* Example usages:
24+
* <pre>
25+
* {@code
26+
* AccessController.doPrivileged(() -> {
27+
* // code that requires privileges
28+
* });
29+
* }
30+
* </pre>
31+
*
32+
* Example usage with a return value and checked exception:
33+
*
34+
* <pre>
35+
* {@code
36+
* T something = AccessController.doPrivilegedChecked(() -> {
37+
* // code that requires privileges and may throw a checked exception
38+
* return something;
39+
* // or
40+
* throw new Exception();
41+
* });
42+
* }
43+
* </pre>
44+
*/
45+
public final class AccessController {
46+
/**
47+
* Don't allow instantiation an {@code AccessController}
48+
*/
49+
private AccessController() {}
50+
51+
/**
52+
* Performs the specified action in a privileged block.
53+
*
54+
* <p> If the action's {@code run} method throws an (unchecked)
55+
* exception, it will propagate through this method.
56+
*
57+
* @param action the action to be performed
58+
*/
59+
public static void doPrivileged(Runnable action) {
60+
action.run();
61+
}
62+
63+
/**
64+
* Performs the specified action.
65+
*
66+
* <p> If the action's {@code run} method throws an <i>unchecked</i>
67+
* exception, it will propagate through this method.
68+
*
69+
* @param <T> the type of the value returned by the
70+
* PrivilegedExceptionAction's {@code run} method
71+
*
72+
* @param action the action to be performed
73+
*
74+
* @return the value returned by the action's {@code run} method
75+
*/
76+
public static <T> T doPrivileged(Supplier<T> action) {
77+
return action.get();
78+
}
79+
80+
/**
81+
* Performs the specified action.
82+
*
83+
* <p> If the action's {@code run} method throws an <i>unchecked</i>
84+
* exception, it will propagate through this method.
85+
*
86+
* @param <T> the type of the value returned by the
87+
* PrivilegedExceptionAction's {@code run} method
88+
*
89+
* @param action the action to be performed
90+
*
91+
* @return the value returned by the action's {@code run} method
92+
*
93+
* @throws Exception if the specified action's
94+
* {@code call} method threw a <i>checked</i> exception
95+
*/
96+
public static <T> T doPrivilegedChecked(Callable<T> action) throws Exception {
97+
return action.call();
98+
}
99+
100+
/**
101+
* Performs the specified action in a privileged block.
102+
*
103+
* <p> If the action's {@code run} method throws an (unchecked)
104+
* exception, it will propagate through this method.
105+
*
106+
* @param action the action to be performed
107+
*
108+
* @throws T if the specified action's
109+
* {@code call} method threw a <i>checked</i> exception
110+
*/
111+
public static <T extends Exception> void doPrivilegedChecked(CheckedRunnable<T> action) throws T {
112+
action.run();
113+
}
114+
115+
/**
116+
* A functional interface that represents a runnable action that can throw a checked exception.
117+
*
118+
* @param <E> the type of the exception that can be thrown
119+
*/
120+
public interface CheckedRunnable<E extends Exception> {
121+
122+
/**
123+
* Executes the action.
124+
*
125+
* @throws E
126+
*/
127+
void run() throws E;
128+
}
129+
}

‎libs/agent-sm/agent-policy/src/main/java/org/opensearch/secure_sm/package-info.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
*
4+
* The OpenSearch Contributors require contributions made to
5+
* this file be licensed under the Apache-2.0 license or a
6+
* compatible open source license.
7+
*/
8+
9+
/**
10+
* Classes for running code in a privileged context
11+
*/
12+
package org.opensearch.secure_sm;

‎libs/agent-sm/agent/build.gradle

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ dependencies {
1414
implementation "net.bytebuddy:byte-buddy:${versions.bytebuddy}"
1515
compileOnly "com.google.code.findbugs:jsr305:3.0.2"
1616

17+
testImplementation project(":libs:agent-sm:agent-policy")
1718
testImplementation "junit:junit:${versions.junit}"
1819
testImplementation "org.hamcrest:hamcrest:${versions.hamcrest}"
1920
}

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

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import java.lang.StackWalker.StackFrame;
1212
import java.security.ProtectionDomain;
1313
import java.util.Collection;
14+
import java.util.Set;
1415
import java.util.function.Function;
1516
import java.util.stream.Collectors;
1617
import java.util.stream.Stream;
@@ -24,6 +25,18 @@ public final class StackCallerProtectionDomainChainExtractor implements Function
2425
*/
2526
public static final StackCallerProtectionDomainChainExtractor INSTANCE = new StackCallerProtectionDomainChainExtractor();
2627

28+
private static final StackWalker STACK_WALKER = StackWalker.getInstance(StackWalker.Option.RETAIN_CLASS_REFERENCE);
29+
/**
30+
* Classes that are used to check if the stack frame is from AccessController. Temporarily supports both the
31+
* AccessController from the JDK (marked for removal) and its replacement in the Java Agent.
32+
*/
33+
private static final Set<String> ACCESS_CONTROLLER_CLASSES = Set.of(
34+
"java.security.AccessController",
35+
"org.opensearch.secure_sm.AccessController"
36+
);
37+
38+
private static final Set<String> DO_PRIVILEGED_METHODS = Set.of("doPrivileged", "doPrivilegedChecked");
39+
2740
/**
2841
* Constructor
2942
*/
@@ -36,7 +49,7 @@ private StackCallerProtectionDomainChainExtractor() {}
3649
@Override
3750
public Collection<ProtectionDomain> apply(Stream<StackFrame> frames) {
3851
return frames.takeWhile(
39-
frame -> !(frame.getClassName().equals("java.security.AccessController") && frame.getMethodName().equals("doPrivileged"))
52+
frame -> !(ACCESS_CONTROLLER_CLASSES.contains(frame.getClassName()) && DO_PRIVILEGED_METHODS.contains(frame.getMethodName()))
4053
)
4154
.map(StackFrame::getDeclaringClass)
4255
.map(Class::getProtectionDomain)

‎libs/agent-sm/agent/src/test/java/org/opensearch/javaagent/StackCallerProtectionDomainExtractorTests.java

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,14 @@
1919
import java.security.ProtectionDomain;
2020
import java.util.List;
2121
import java.util.Set;
22+
import java.util.function.Supplier;
2223
import java.util.stream.Collectors;
2324

2425
import static org.hamcrest.MatcherAssert.assertThat;
2526
import static org.hamcrest.Matchers.containsInAnyOrder;
2627
import static org.hamcrest.Matchers.hasItem;
2728
import static org.junit.Assert.assertEquals;
29+
import static org.junit.Assert.assertThrows;
2830

2931
public class StackCallerProtectionDomainExtractorTests {
3032

@@ -115,4 +117,144 @@ public Void run() {
115117
}
116118
});
117119
}
120+
121+
@Test
122+
public void testStackTruncationWithOpenSearchAccessController() {
123+
org.opensearch.secure_sm.AccessController.doPrivileged(() -> {
124+
StackCallerProtectionDomainChainExtractor extractor = StackCallerProtectionDomainChainExtractor.INSTANCE;
125+
Set<ProtectionDomain> protectionDomains = (Set<ProtectionDomain>) extractor.apply(captureStackFrames().stream());
126+
assertEquals(1, protectionDomains.size());
127+
List<String> simpleNames = protectionDomains.stream().map(pd -> {
128+
try {
129+
return pd.getCodeSource().getLocation().toURI();
130+
} catch (URISyntaxException e) {
131+
throw new RuntimeException(e);
132+
}
133+
})
134+
.map(URI::getPath)
135+
.map(Paths::get)
136+
.map(Path::getFileName)
137+
.map(Path::toString)
138+
// strip trailing “-VERSION.jar” if present
139+
.map(name -> name.replaceFirst("-\\d[\\d\\.]*\\.jar$", ""))
140+
// otherwise strip “.jar”
141+
.map(name -> name.replaceFirst("\\.jar$", ""))
142+
.toList();
143+
assertThat(
144+
simpleNames,
145+
containsInAnyOrder(
146+
"test" // from the build/classes/java/test directory
147+
)
148+
);
149+
});
150+
}
151+
152+
@Test
153+
public void testStackTruncationWithOpenSearchAccessControllerUsingSupplier() {
154+
org.opensearch.secure_sm.AccessController.doPrivileged((Supplier<Void>) () -> {
155+
StackCallerProtectionDomainChainExtractor extractor = StackCallerProtectionDomainChainExtractor.INSTANCE;
156+
Set<ProtectionDomain> protectionDomains = (Set<ProtectionDomain>) extractor.apply(captureStackFrames().stream());
157+
assertEquals(1, protectionDomains.size());
158+
List<String> simpleNames = protectionDomains.stream().map(pd -> {
159+
try {
160+
return pd.getCodeSource().getLocation().toURI();
161+
} catch (URISyntaxException e) {
162+
throw new RuntimeException(e);
163+
}
164+
})
165+
.map(URI::getPath)
166+
.map(Paths::get)
167+
.map(Path::getFileName)
168+
.map(Path::toString)
169+
// strip trailing “-VERSION.jar” if present
170+
.map(name -> name.replaceFirst("-\\d[\\d\\.]*\\.jar$", ""))
171+
// otherwise strip “.jar”
172+
.map(name -> name.replaceFirst("\\.jar$", ""))
173+
.toList();
174+
assertThat(
175+
simpleNames,
176+
containsInAnyOrder(
177+
"test" // from the build/classes/java/test directory
178+
)
179+
);
180+
return null;
181+
});
182+
}
183+
184+
@Test
185+
public void testStackTruncationWithOpenSearchAccessControllerUsingCallable() throws Exception {
186+
org.opensearch.secure_sm.AccessController.doPrivilegedChecked(() -> {
187+
StackCallerProtectionDomainChainExtractor extractor = StackCallerProtectionDomainChainExtractor.INSTANCE;
188+
Set<ProtectionDomain> protectionDomains = (Set<ProtectionDomain>) extractor.apply(captureStackFrames().stream());
189+
assertEquals(1, protectionDomains.size());
190+
List<String> simpleNames = protectionDomains.stream().map(pd -> {
191+
try {
192+
return pd.getCodeSource().getLocation().toURI();
193+
} catch (URISyntaxException e) {
194+
throw new RuntimeException(e);
195+
}
196+
})
197+
.map(URI::getPath)
198+
.map(Paths::get)
199+
.map(Path::getFileName)
200+
.map(Path::toString)
201+
// strip trailing “-VERSION.jar” if present
202+
.map(name -> name.replaceFirst("-\\d[\\d\\.]*\\.jar$", ""))
203+
// otherwise strip “.jar”
204+
.map(name -> name.replaceFirst("\\.jar$", ""))
205+
.toList();
206+
assertThat(
207+
simpleNames,
208+
containsInAnyOrder(
209+
"test" // from the build/classes/java/test directory
210+
)
211+
);
212+
return null;
213+
});
214+
}
215+
216+
@Test
217+
public void testAccessControllerUsingCallableThrowsException() {
218+
assertThrows(IllegalArgumentException.class, () -> {
219+
org.opensearch.secure_sm.AccessController.doPrivilegedChecked(() -> { throw new IllegalArgumentException("Test exception"); });
220+
});
221+
}
222+
223+
@Test
224+
public void testStackTruncationWithOpenSearchAccessControllerUsingCheckedRunnable() throws IllegalArgumentException {
225+
org.opensearch.secure_sm.AccessController.doPrivilegedChecked(() -> {
226+
StackCallerProtectionDomainChainExtractor extractor = StackCallerProtectionDomainChainExtractor.INSTANCE;
227+
Set<ProtectionDomain> protectionDomains = (Set<ProtectionDomain>) extractor.apply(captureStackFrames().stream());
228+
assertEquals(1, protectionDomains.size());
229+
List<String> simpleNames = protectionDomains.stream().map(pd -> {
230+
try {
231+
return pd.getCodeSource().getLocation().toURI();
232+
} catch (URISyntaxException e) {
233+
throw new RuntimeException(e);
234+
}
235+
})
236+
.map(URI::getPath)
237+
.map(Paths::get)
238+
.map(Path::getFileName)
239+
.map(Path::toString)
240+
// strip trailing “-VERSION.jar” if present
241+
.map(name -> name.replaceFirst("-\\d[\\d\\.]*\\.jar$", ""))
242+
// otherwise strip “.jar”
243+
.map(name -> name.replaceFirst("\\.jar$", ""))
244+
.toList();
245+
assertThat(
246+
simpleNames,
247+
containsInAnyOrder(
248+
"test" // from the build/classes/java/test directory
249+
)
250+
);
251+
});
252+
}
253+
254+
@Test
255+
public void testAccessControllerUsingCheckedRunnableThrowsException() {
256+
assertThrows(IllegalArgumentException.class, () -> {
257+
org.opensearch.secure_sm.AccessController.doPrivilegedChecked(() -> { throw new IllegalArgumentException("Test exception"); });
258+
});
259+
}
118260
}

0 commit comments

Comments
 (0)