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

The org.opensearch.bootstrap.Security should support codebase for JAR files with classifiers #12586

Merged
merged 1 commit into from
Mar 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 @@ -105,6 +105,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- Support for returning scores in matched queries ([#11626](https://github.com/opensearch-project/OpenSearch/pull/11626))
- Add shard id property to SearchLookup for use in field types provided by plugins ([#1063](https://github.com/opensearch-project/OpenSearch/pull/1063))
- Add kuromoji_completion analyzer and filter ([#4835](https://github.com/opensearch-project/OpenSearch/issues/4835))
- The org.opensearch.bootstrap.Security should support codebase for JAR files with classifiers ([#12586](https://github.com/opensearch-project/OpenSearch/issues/12586))

### Dependencies
- Bump `peter-evans/find-comment` from 2 to 3 ([#12288](https://github.com/opensearch-project/OpenSearch/pull/12288))
Expand Down
71 changes: 54 additions & 17 deletions server/src/main/java/org/opensearch/bootstrap/Security.java
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import static org.opensearch.bootstrap.FilePermissionUtils.addDirectoryPath;
import static org.opensearch.bootstrap.FilePermissionUtils.addSingleFilePath;
Expand Down Expand Up @@ -121,6 +123,8 @@
*/
@SuppressWarnings("removal")
final class Security {
private static final Pattern CODEBASE_JAR_WITH_CLASSIFIER = Pattern.compile("^(.+)-\\d+\\.\\d+[^-]*.*?[-]?([^-]+)?\\.jar$");

/** no instantiation */
private Security() {}

Expand Down Expand Up @@ -231,33 +235,45 @@
try {
List<String> propertiesSet = new ArrayList<>();
try {
final Map<Map.Entry<String, URL>, String> jarsWithPossibleClassifiers = new HashMap<>();
andrross marked this conversation as resolved.
Show resolved Hide resolved
// set codebase properties
for (Map.Entry<String, URL> codebase : codebases.entrySet()) {
String name = codebase.getKey();
URL url = codebase.getValue();
final String name = codebase.getKey();
final URL url = codebase.getValue();

// We attempt to use a versionless identifier for each codebase. This assumes a specific version
// format in the jar filename. While we cannot ensure all jars in all plugins use this format, nonconformity
// only means policy grants would need to include the entire jar filename as they always have before.
final Matcher matcher = CODEBASE_JAR_WITH_CLASSIFIER.matcher(name);
if (matcher.matches() && matcher.group(2) != null) {
// There is a JAR that, possibly, has a classifier or SNAPSHOT at the end, examples are:
// - netty-tcnative-boringssl-static-2.0.61.Final-linux-x86_64.jar
// - kafka-server-common-3.6.1-test.jar
// - lucene-core-9.11.0-snapshot-8a555eb.jar
// - zstd-jni-1.5.5-5.jar
jarsWithPossibleClassifiers.put(codebase, matcher.group(2));
} else {
String property = "codebase." + name;
String aliasProperty = "codebase." + name.replaceFirst("-\\d+\\.\\d+.*\\.jar", "");
addCodebaseToSystemProperties(propertiesSet, url, property, aliasProperty);
}
}

// set codebase properties for JARs that might present with classifiers
for (Map.Entry<Map.Entry<String, URL>, String> jarWithPossibleClassifier : jarsWithPossibleClassifiers.entrySet()) {
final Map.Entry<String, URL> codebase = jarWithPossibleClassifier.getKey();
final String name = codebase.getKey();
final URL url = codebase.getValue();

String property = "codebase." + name;
String aliasProperty = "codebase." + name.replaceFirst("-\\d+\\.\\d+.*\\.jar", "");
if (aliasProperty.equals(property) == false) {
propertiesSet.add(aliasProperty);
String previous = System.setProperty(aliasProperty, url.toString());
if (previous != null) {
throw new IllegalStateException(
"codebase property already set: " + aliasProperty + " -> " + previous + ", cannot set to " + url.toString()
);
}
}
propertiesSet.add(property);
String previous = System.setProperty(property, url.toString());
if (previous != null) {
throw new IllegalStateException(
"codebase property already set: " + property + " -> " + previous + ", cannot set to " + url.toString()
);
if (System.getProperties().containsKey(aliasProperty)) {
aliasProperty = aliasProperty + "@" + jarWithPossibleClassifier.getValue();
andrross marked this conversation as resolved.
Show resolved Hide resolved
}

addCodebaseToSystemProperties(propertiesSet, url, property, aliasProperty);
}

return Policy.getInstance("JavaPolicy", new URIParameter(policyFile.toURI()));
} finally {
// clear codebase properties
Expand All @@ -270,6 +286,27 @@
}
}

/** adds the codebase to properties and System properties */
@SuppressForbidden(reason = "accesses System properties to configure codebases")
private static void addCodebaseToSystemProperties(List<String> propertiesSet, final URL url, String property, String aliasProperty) {
if (aliasProperty.equals(property) == false) {
propertiesSet.add(aliasProperty);
String previous = System.setProperty(aliasProperty, url.toString());
if (previous != null) {
throw new IllegalStateException(
"codebase property already set: " + aliasProperty + " -> " + previous + ", cannot set to " + url.toString()

Check warning on line 297 in server/src/main/java/org/opensearch/bootstrap/Security.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/bootstrap/Security.java#L296-L297

Added lines #L296 - L297 were not covered by tests
);
}
}
propertiesSet.add(property);
String previous = System.setProperty(property, url.toString());
if (previous != null) {
throw new IllegalStateException(
"codebase property already set: " + property + " -> " + previous + ", cannot set to " + url.toString()

Check warning on line 305 in server/src/main/java/org/opensearch/bootstrap/Security.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/bootstrap/Security.java#L304-L305

Added lines #L304 - L305 were not covered by tests
);
}
}

/** returns dynamic Permissions to configured paths and bind ports */
static Permissions createPermissions(Environment environment) throws IOException {
Permissions policy = new Permissions();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,12 @@
import org.opensearch.test.OpenSearchTestCase;

import java.io.IOException;
import java.net.URL;
import java.nio.file.Files;
import java.nio.file.Path;
import java.security.AccessController;
import java.security.PrivilegedAction;
import java.util.Map;

public class SecurityTests extends OpenSearchTestCase {

Expand Down Expand Up @@ -80,4 +84,23 @@ public void testProcessExecution() throws Exception {
fail("didn't get expected exception");
} catch (SecurityException expected) {}
}

public void testReadPolicyWithCodebases() throws IOException {
final Map<String, URL> codebases = Map.of(
"test-netty-tcnative-boringssl-static-2.0.61.Final-linux-x86_64.jar",
new URL("file://test-netty-tcnative-boringssl-static-2.0.61.Final-linux-x86_64.jar"),
"test-kafka-server-common-3.6.1.jar",
new URL("file://test-kafka-server-common-3.6.1.jar"),
"test-kafka-server-common-3.6.1-test.jar",
new URL("file://test-kafka-server-common-3.6.1-test.jar"),
"test-lucene-core-9.11.0-snapshot-8a555eb.jar",
new URL("file://test-lucene-core-9.11.0-snapshot-8a555eb.jar"),
"test-zstd-jni-1.5.5-5.jar",
new URL("file://test-zstd-jni-1.5.5-5.jar")
);

AccessController.doPrivileged(
(PrivilegedAction<?>) () -> Security.readPolicy(SecurityTests.class.getResource("test-codebases.policy"), codebases)
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/*
* 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.
*/

/*
* Modifications Copyright OpenSearch Contributors. See
* GitHub history for details.
*/

//// additional test framework permissions.
//// These are mock objects and test management that we allow test framework libs
//// to provide on our behalf. But tests themselves cannot do this stuff!

grant codeBase "${codebase.zstd-jni}" {
};

grant codeBase "${codebase.kafka-server-common}" {
};

grant codeBase "${codebase.kafka-server-common@test}" {
};
13 changes: 13 additions & 0 deletions server/src/test/resources/org/opensearch/bootstrap/test.policy
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/*
* 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.
*/

grant {
// allow to test Security policy and codebases
permission java.util.PropertyPermission "*", "read,write";
permission java.security.SecurityPermission "createPolicy.JavaPolicy";
};
Loading