From 07e79e3bec7732cc347bf36d934631e6f87a8216 Mon Sep 17 00:00:00 2001 From: Andriy Redko Date: Mon, 11 Mar 2024 21:26:35 -0400 Subject: [PATCH] The org.opensearch.bootstrap.Security should support codebase for JAR files with classifiers (#12586) Signed-off-by: Andriy Redko --- CHANGELOG.md | 1 + .../org/opensearch/bootstrap/Security.java | 71 ++++++++++++++----- .../opensearch/bootstrap/SecurityTests.java | 23 ++++++ .../bootstrap/test-codebases.policy | 25 +++++++ .../org/opensearch/bootstrap/test.policy | 13 ++++ 5 files changed, 116 insertions(+), 17 deletions(-) create mode 100644 server/src/test/resources/org/opensearch/bootstrap/test-codebases.policy create mode 100644 server/src/test/resources/org/opensearch/bootstrap/test.policy diff --git a/CHANGELOG.md b/CHANGELOG.md index 3e7229a2a0336..1ea80e90b1763 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -106,6 +106,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Add shard id property to SearchLookup for use in field types provided by plugins ([#1063](https://github.com/opensearch-project/OpenSearch/pull/1063)) - [Tiered caching] Make IndicesRequestCache implementation configurable [EXPERIMENTAL] ([#12533](https://github.com/opensearch-project/OpenSearch/pull/12533)) - 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)) diff --git a/server/src/main/java/org/opensearch/bootstrap/Security.java b/server/src/main/java/org/opensearch/bootstrap/Security.java index a48bbd61016e3..53b1d990f9a0c 100644 --- a/server/src/main/java/org/opensearch/bootstrap/Security.java +++ b/server/src/main/java/org/opensearch/bootstrap/Security.java @@ -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; @@ -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() {} @@ -231,33 +235,45 @@ static Policy readPolicy(URL policyFile, Map codebases) { try { List propertiesSet = new ArrayList<>(); try { + final Map, String> jarsWithPossibleClassifiers = new HashMap<>(); // set codebase properties for (Map.Entry 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, String> jarWithPossibleClassifier : jarsWithPossibleClassifiers.entrySet()) { + final Map.Entry 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(); } + + addCodebaseToSystemProperties(propertiesSet, url, property, aliasProperty); } + return Policy.getInstance("JavaPolicy", new URIParameter(policyFile.toURI())); } finally { // clear codebase properties @@ -270,6 +286,27 @@ static Policy readPolicy(URL policyFile, Map codebases) { } } + /** adds the codebase to properties and System properties */ + @SuppressForbidden(reason = "accesses System properties to configure codebases") + private static void addCodebaseToSystemProperties(List 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() + ); + } + } + 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() + ); + } + } + /** returns dynamic Permissions to configured paths and bind ports */ static Permissions createPermissions(Environment environment) throws IOException { Permissions policy = new Permissions(); diff --git a/server/src/test/java/org/opensearch/bootstrap/SecurityTests.java b/server/src/test/java/org/opensearch/bootstrap/SecurityTests.java index 69e561bb8fd89..76353aea03257 100644 --- a/server/src/test/java/org/opensearch/bootstrap/SecurityTests.java +++ b/server/src/test/java/org/opensearch/bootstrap/SecurityTests.java @@ -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 { @@ -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 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) + ); + } } diff --git a/server/src/test/resources/org/opensearch/bootstrap/test-codebases.policy b/server/src/test/resources/org/opensearch/bootstrap/test-codebases.policy new file mode 100644 index 0000000000000..0dc754ccb0c57 --- /dev/null +++ b/server/src/test/resources/org/opensearch/bootstrap/test-codebases.policy @@ -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}" { +}; diff --git a/server/src/test/resources/org/opensearch/bootstrap/test.policy b/server/src/test/resources/org/opensearch/bootstrap/test.policy new file mode 100644 index 0000000000000..7b0a9b3d5d709 --- /dev/null +++ b/server/src/test/resources/org/opensearch/bootstrap/test.policy @@ -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"; +};