The org.opensearch.bootstrap.Security should support codebase for JAR…

… files with classifiers (opensearch-project#12586)

Signed-off-by: Andriy Redko <[email protected]>

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))

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<String, URL> codebases) {
         try {
             List<String> propertiesSet = new ArrayList<>();
             try {
+                final Map<Map.Entry<String, URL>, String> jarsWithPossibleClassifiers = new HashMap<>();
                 // 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();
                     }
+
+                    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<String, URL> codebases) {
         }
     }
 
+    /** 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()
+                );
+            }
+        }
+        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();

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<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)
+        );
+    }
 }

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}" {
+};

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";
+};