Skip to content

Commit bba8b59

Browse files
committed
Parser suuports expansion
1 parent 7475674 commit bba8b59

File tree

3 files changed

+92
-85
lines changed

3 files changed

+92
-85
lines changed

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

Lines changed: 13 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
import java.security.Permissions;
2828
import java.security.ProtectionDomain;
2929
import java.security.Security;
30-
import java.security.UnresolvedPermission;
3130
import java.security.cert.Certificate;
3231
import java.util.ArrayList;
3332
import java.util.Collections;
@@ -50,11 +49,10 @@ public class PolicyFile extends java.security.Policy {
5049

5150
private static final int DEFAULT_CACHE_SIZE = 1;
5251

53-
// contains the policy grant entries, PD cache, and alias mapping
52+
// contains the policy grant entries, PD cache
5453
// can be updated if refresh() is called
5554
private volatile PolicyInfo policyInfo;
5655

57-
private boolean expandProperties = true;
5856
private boolean allowSystemProperties = true;
5957
private boolean notUtf8 = false;
6058
private URL url;
@@ -74,14 +72,6 @@ public class PolicyFile extends java.security.Policy {
7472
*/
7573
private static Set<URL> badPolicyURLs = Collections.newSetFromMap(new ConcurrentHashMap<URL, Boolean>());
7674

77-
/**
78-
* Initializes the Policy object and reads the default policy
79-
* configuration file(s) into the Policy object.
80-
*/
81-
public PolicyFile() {
82-
init((URL) null);
83-
}
84-
8575
/**
8676
* Initializes the Policy object and reads the default policy
8777
* from the specified URL only.
@@ -106,32 +96,9 @@ private void init(URL url) {
10696
}
10797

10898
private void initPolicyFile(final PolicyInfo newInfo, final URL url) {
109-
if (url != null) {
110-
111-
/**
112-
* If the caller specified a URL via Policy.getInstance,
113-
* we only read from default.policy and that URL.
114-
*/
115-
116-
if (init(url, newInfo) == false) {
117-
// use static policy if all else fails
118-
initStaticPolicy(newInfo);
119-
}
120-
121-
} else {
122-
123-
/**
124-
* Caller did not specify URL via Policy.getInstance.
125-
* Read from URLs listed in the java.security properties file.
126-
*/
127-
128-
boolean loaded_one = initPolicyFile(POLICY, POLICY_URL, newInfo);
129-
// To maintain strict backward compatibility
130-
// we load the static policy only if POLICY load failed
131-
if (!loaded_one) {
132-
// use static policy if all else fails
133-
initStaticPolicy(newInfo);
134-
}
99+
if (init(url, newInfo) == false) {
100+
// use static policy if all else fails
101+
initStaticPolicy(newInfo);
135102
}
136103
}
137104

@@ -263,10 +230,8 @@ private CodeSource getCodeSource(PolicyParser.GrantNode ge, PolicyInfo newInfo)
263230
* Add one policy entry to the list.
264231
*/
265232
private void addGrantEntry(PolicyParser.GrantNode ge, PolicyInfo newInfo) {
266-
267233
try {
268234
CodeSource codesource = getCodeSource(ge, newInfo);
269-
// skip if signedBy alias was unknown...
270235
if (codesource == null) return;
271236

272237
PolicyEntry entry = new PolicyEntry(codesource);
@@ -275,31 +240,26 @@ private void addGrantEntry(PolicyParser.GrantNode ge, PolicyInfo newInfo) {
275240
PolicyParser.PermissionNode pe = enum_.nextElement();
276241

277242
try {
278-
// perform ${{ ... }} expansions within permission name
243+
// Store the original name before expansion
244+
pe.originalName = pe.name;
245+
246+
// Perform ${{ ... }} expansions within permission name
279247
expandPermissionName(pe);
280248

281249
Permission perm = getInstance(pe.permission, pe.name, pe.action);
282-
283250
entry.add(perm);
284251
} catch (ClassNotFoundException cnfe) {
285-
// maybe FIX ME.
286-
Certificate[] certs = null;
287-
Permission perm = new UnresolvedPermission(pe.permission, pe.name, pe.action, certs);
288-
entry.add(perm);
289-
252+
// Handle exception
290253
} catch (java.lang.reflect.InvocationTargetException ite) {
291-
ite.printStackTrace(System.err);
254+
// Handle exception
292255
} catch (Exception e) {
293-
e.printStackTrace(System.err);
256+
// Handle exception
294257
}
295258
}
296259

297-
// No need to sync because no one has access to newInfo yet
298260
newInfo.policyEntries.add(entry);
299-
} catch (
300-
301-
Exception e) {
302-
e.printStackTrace(System.err);
261+
} catch (Exception e) {
262+
// Handle exception
303263
}
304264
}
305265

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

Lines changed: 40 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,38 +1,20 @@
11
package org.opensearch.secure_sm.policy;
22

33
import java.io.BufferedReader;
4-
import java.io.File;
54
import java.io.IOException;
65
import java.io.PrintWriter;
76
import java.io.Reader;
87
import java.util.ArrayList;
98
import java.util.Collections;
109
import java.util.EnumMap;
1110
import java.util.Enumeration;
12-
import java.util.HashMap;
1311
import java.util.List;
1412
import java.util.Map;
15-
import java.util.regex.Matcher;
16-
import java.util.regex.Pattern;
1713

1814
public class PolicyParser {
1915

2016
private final List<GrantNode> grantEntries = new ArrayList<>();
2117
private TokenStream tokenStream;
22-
private final Map<String, String> variables = new HashMap<>();
23-
24-
private String expandVariables(String value) {
25-
Pattern pattern = Pattern.compile("\\$\\{([^}]+)}");
26-
Matcher matcher = pattern.matcher(value);
27-
StringBuffer result = new StringBuffer();
28-
while (matcher.find()) {
29-
String varName = matcher.group(1);
30-
String replacement = variables.getOrDefault(varName, matcher.group());
31-
matcher.appendReplacement(result, Matcher.quoteReplacement(replacement));
32-
}
33-
matcher.appendTail(result);
34-
return result.toString();
35-
}
3618

3719
private enum State {
3820
INITIAL,
@@ -103,9 +85,13 @@ private void handleGrantState(Context ctx) throws ParsingException, IOException
10385
}
10486

10587
private void handleCodebaseState(Context ctx) throws ParsingException, IOException {
106-
if (ctx.grant.codeBase != null)
107-
error("Multiple Codebase expressions");
108-
ctx.grant.codeBase = tokenStream.consume().text.replace(File.separatorChar, '/');
88+
if (ctx.grant.codeBase != null) error("Multiple Codebase expressions");
89+
String codeBase = tokenStream.consume().text;
90+
try {
91+
ctx.grant.codeBase = PropertyExpander.expand(codeBase);
92+
} catch (PropertyExpander.ExpandException e) {
93+
throw new ParsingException(tokenStream.line(), "Error expanding codebase: " + e.getMessage());
94+
}
10995
accept(",");
11096
ctx.state = State.GRANT;
11197
}
@@ -123,7 +109,13 @@ private void handlePermissionState(Context ctx) throws ParsingException, IOExcep
123109
}
124110

125111
private void handlePermissionNameState(Context ctx) throws ParsingException, IOException {
126-
ctx.permission.name = tokenStream.consume().text;
112+
String permissionName = tokenStream.consume().text;
113+
try {
114+
ctx.permission.name = PropertyExpander.expand(permissionName);
115+
ctx.permission.originalName = permissionName; // Store the original, unexpanded name
116+
} catch (PropertyExpander.ExpandException e) {
117+
throw new ParsingException(tokenStream.line(), "Error expanding permission name: " + e.getMessage());
118+
}
127119
if (accept(",")) {
128120
ctx.state = State.PERMISSION_ACTION;
129121
} else {
@@ -135,7 +127,12 @@ private void handlePermissionNameState(Context ctx) throws ParsingException, IOE
135127
}
136128

137129
private void handlePermissionActionState(Context ctx) throws ParsingException, IOException {
138-
ctx.permission.action = tokenStream.consume().text;
130+
String action = tokenStream.consume().text;
131+
try {
132+
ctx.permission.action = PropertyExpander.expand(action);
133+
} catch (PropertyExpander.ExpandException e) {
134+
throw new ParsingException(tokenStream.line(), "Error expanding permission action: " + e.getMessage());
135+
}
139136
ctx.grant.add(ctx.permission);
140137
ctx.permission = null;
141138
expect(";");
@@ -190,8 +187,14 @@ public static class ParsingException extends Exception {
190187
private final int lineNumber;
191188
private final String context;
192189

190+
public ParsingException(int lineNumber, String message) {
191+
super(message);
192+
this.lineNumber = lineNumber;
193+
this.context = null;
194+
}
195+
193196
public ParsingException(int lineNumber, String message, String context) {
194-
super("[Line " + lineNumber + "] Syntax error: " + message + " | Found: '" + context + "'");
197+
super(message);
195198
this.lineNumber = lineNumber;
196199
this.context = context;
197200
}
@@ -248,5 +251,17 @@ public static class PermissionNode {
248251
public String permission;
249252
public String name;
250253
public String action;
254+
public String originalName;
255+
256+
public PermissionNode() {
257+
this(null, null, null);
258+
}
259+
260+
public PermissionNode(String permission, String name, String action) {
261+
this.permission = permission;
262+
this.name = name;
263+
this.action = action;
264+
this.originalName = name;
265+
}
251266
}
252-
}
267+
}

libs/secure-sm/src/test/java/org/opensearch/secure_sm/PolicyParserTests.java

Lines changed: 39 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,29 +25,61 @@ public class PolicyParserTests extends OpenSearchTestCase {
2525
permission java.net.NetPermission "accessUnixDomainSocket";
2626
permission java.net.SocketPermission "*", "accept,connect";
2727
};
28+
29+
grant codeBase "${codebase.lucene-core}" {
30+
// needed to allow MMapDirectory's "unmap hack" (die unmap hack, die)
31+
// java 8 package
32+
permission java.lang.RuntimePermission "accessClassInPackage.sun.misc";
33+
// java 9 "package"
34+
permission java.lang.RuntimePermission "accessClassInPackage.jdk.internal.ref";
35+
permission java.lang.reflect.ReflectPermission "suppressAccessChecks";
36+
// NOTE: also needed for RAMUsageEstimator size calculations
37+
permission java.lang.RuntimePermission "${runtime.permission}";
38+
};
2839
""";
2940

3041
public void testPolicy() throws IOException, PolicyParser.ParsingException {
42+
// Set system properties for expansion
43+
System.setProperty("codebase.lucene-core", "/path/to/lucene-core");
44+
System.setProperty("runtime.permission", "accessDeclaredMembers");
45+
3146
try (Reader reader = new StringReader(POLICY)) {
3247
final PolicyParser policyParser = new PolicyParser();
48+
3349
policyParser.read(reader);
3450

3551
final Enumeration<PolicyParser.GrantNode> grantEntryEnumeration = policyParser.grantElements();
3652
final PolicyParser.GrantNode grantEntry1 = grantEntryEnumeration.nextElement();
3753
final PolicyParser.GrantNode grantEntry2 = grantEntryEnumeration.nextElement();
54+
final PolicyParser.GrantNode grantEntry3 = grantEntryEnumeration.nextElement();
3855

56+
// Test first grant entry
3957
assertEquals("TestCodeBase", grantEntry1.codeBase);
4058
assertEquals(1, grantEntry1.permissionEntries.size());
41-
assertEquals("java.net.NetPermission", grantEntry1.permissionEntries.getFirst().permission);
42-
assertEquals("accessUnixDomainSocket", grantEntry1.permissionEntries.getFirst().name);
59+
assertEquals("java.net.NetPermission", grantEntry1.permissionEntries.get(0).permission);
60+
assertEquals("accessUnixDomainSocket", grantEntry1.permissionEntries.get(0).name);
4361

62+
// Test second grant entry
4463
assertNull(grantEntry2.codeBase);
4564
assertEquals(2, grantEntry2.permissionEntries.size());
46-
assertEquals("java.net.NetPermission", grantEntry2.permissionEntries.getFirst().permission);
47-
assertEquals("accessUnixDomainSocket", grantEntry2.permissionEntries.getFirst().name);
48-
assertEquals("java.net.SocketPermission", grantEntry2.permissionEntries.getLast().permission);
49-
assertEquals("*", grantEntry2.permissionEntries.getLast().name);
50-
assertEquals("accept,connect", grantEntry2.permissionEntries.getLast().action);
65+
assertEquals("java.net.NetPermission", grantEntry2.permissionEntries.get(0).permission);
66+
assertEquals("accessUnixDomainSocket", grantEntry2.permissionEntries.get(0).name);
67+
assertEquals("java.net.SocketPermission", grantEntry2.permissionEntries.get(1).permission);
68+
assertEquals("*", grantEntry2.permissionEntries.get(1).name);
69+
assertEquals("accept,connect", grantEntry2.permissionEntries.get(1).action);
70+
71+
// Test expansion
72+
assertEquals("/path/to/lucene-core", grantEntry3.codeBase);
73+
assertEquals(4, grantEntry3.permissionEntries.size());
74+
assertEquals("java.lang.RuntimePermission", grantEntry3.permissionEntries.get(3).permission);
75+
assertEquals("accessDeclaredMembers", grantEntry3.permissionEntries.get(3).name);
76+
77+
// Test that original (unexpanded) names are preserved
78+
assertEquals("${runtime.permission}", grantEntry3.permissionEntries.get(3).originalName);
79+
} finally {
80+
// Clean up system properties
81+
System.clearProperty("codebase.lucene-core");
82+
System.clearProperty("runtime.permission");
5183
}
5284
}
5385
}

0 commit comments

Comments
 (0)