Entitle com.unboundid.ldap.listener as test package (#130706)

Author: mosche

test/framework/src/main/java/org/elasticsearch/entitlement/bootstrap/TestEntitlementBootstrap.java

@@ -87,6 +87,10 @@ public static void setTriviallyAllowingTestCode(boolean newValue) {
         policyManager.setTriviallyAllowingTestCode(newValue);
     }
 
+    public static void setEntitledTestPackages(String[] entitledTestPackages) {
+        policyManager.setEntitledTestPackages(entitledTestPackages);
+    }
+
     public static void reset() {
         if (policyManager != null) {
             policyManager.reset();

test/framework/src/main/java/org/elasticsearch/entitlement/runtime/policy/TestPolicyManager.java

@@ -9,6 +9,7 @@
 
 package org.elasticsearch.entitlement.runtime.policy;
 
+import org.elasticsearch.common.util.ArrayUtils;
 import org.elasticsearch.entitlement.runtime.policy.entitlements.Entitlement;
 import org.elasticsearch.test.ESTestCase;
 
@@ -17,6 +18,7 @@
 import java.nio.file.Path;
 import java.security.CodeSource;
 import java.security.ProtectionDomain;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
@@ -29,6 +31,7 @@ public class TestPolicyManager extends PolicyManager {
 
     boolean isActive;
     boolean isTriviallyAllowingTestCode;
+    String[] entitledTestPackages = TEST_FRAMEWORK_PACKAGE_PREFIXES;
 
     /**
      * We don't have modules in tests, so we can't use the inherited map of entitlements per module.
@@ -60,6 +63,16 @@ public void setTriviallyAllowingTestCode(boolean newValue) {
         this.isTriviallyAllowingTestCode = newValue;
     }
 
+    public void setEntitledTestPackages(String... entitledTestPackages) {
+        assertNoRedundantPrefixes(TEST_FRAMEWORK_PACKAGE_PREFIXES, entitledTestPackages, false);
+        if (entitledTestPackages.length > 1) {
+            assertNoRedundantPrefixes(entitledTestPackages, entitledTestPackages, true);
+        }
+        String[] packages = ArrayUtils.concat(this.entitledTestPackages, entitledTestPackages);
+        Arrays.sort(packages);
+        this.entitledTestPackages = packages;
+    }
+
     /**
      * Called between tests so each test is not affected by prior tests
      */
@@ -110,19 +123,47 @@ private boolean isEntitlementClass(Class<?> requestingClass) {
             && (requestingClass.getName().contains("Test") == false);
     }
 
-    @Deprecated // TODO: reevaluate whether we want this.
-    // If we can simply check for dependencies the gradle worker has that aren't
-    // declared in the gradle config (namely org.gradle) that would be simpler.
     private boolean isTestFrameworkClass(Class<?> requestingClass) {
-        String packageName = requestingClass.getPackageName();
-        for (String prefix : TEST_FRAMEWORK_PACKAGE_PREFIXES) {
-            if (packageName.startsWith(prefix)) {
+        return isTestFrameworkClass(entitledTestPackages, requestingClass.getPackageName());
+    }
+
+    // no redundant entries allowed, see assertNoRedundantPrefixes
+    static boolean isTestFrameworkClass(String[] sortedPrefixes, String packageName) {
+        int idx = Arrays.binarySearch(sortedPrefixes, packageName);
+        if (idx >= 0) {
+            return true;
+        }
+        idx = -idx - 2; // candidate package index (insertion point - 1)
+        if (idx >= 0 && idx < sortedPrefixes.length) {
+            String candidate = sortedPrefixes[idx];
+            if (packageName.startsWith(candidate)
+                && (packageName.length() == candidate.length() || packageName.charAt(candidate.length()) == '.')) {
                 return true;
             }
         }
         return false;
     }
 
+    private static boolean isNotPrefixMatch(String name, String prefix, boolean discardExactMatch) {
+        assert prefix.endsWith(".") == false : "Invalid package prefix ending with '.' [" + prefix + "]";
+        if (name == prefix || name.startsWith(prefix)) {
+            if (name.length() == prefix.length()) {
+                return discardExactMatch;
+            }
+            return false == (name.length() > prefix.length() && name.charAt(prefix.length()) == '.');
+        }
+        return true;
+    }
+
+    static void assertNoRedundantPrefixes(String[] setA, String[] setB, boolean discardExactMatch) {
+        for (String a : setA) {
+            for (String b : setB) {
+                assert isNotPrefixMatch(a, b, discardExactMatch) && isNotPrefixMatch(b, a, discardExactMatch)
+                    : "Redundant prefix entries: [" + a + ", " + b + "]";
+            }
+        }
+    }
+
     private boolean isTestCode(Class<?> requestingClass) {
         // TODO: Cache this? It's expensive
         for (Class<?> candidate = requireNonNull(requestingClass); candidate != null; candidate = candidate.getDeclaringClass()) {
@@ -163,6 +204,10 @@ private boolean isTestCode(Class<?> requestingClass) {
         "org.bouncycastle.jsse.provider" // Used in test code if FIPS is enabled, support more fine-grained config in ES-12128
     };
 
+    static {
+        Arrays.sort(TEST_FRAMEWORK_PACKAGE_PREFIXES);
+    }
+
     @Override
     protected ModuleEntitlements getEntitlements(Class<?> requestingClass) {
         return classEntitlementsMap.computeIfAbsent(requestingClass, this::computeEntitlements);

test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java

@@ -517,16 +517,30 @@ protected void afterIfSuccessful() throws Exception {}
     public @interface WithEntitlementsOnTestCode {
     }
 
+    @Retention(RetentionPolicy.RUNTIME)
+    @Target(ElementType.TYPE)
+    @Inherited
+    public @interface EntitledTestPackages {
+        String[] value();
+    }
+
     @BeforeClass
     public static void setupEntitlementsForClass() {
         boolean withoutEntitlements = getTestClass().isAnnotationPresent(WithoutEntitlements.class);
         boolean withEntitlementsOnTestCode = getTestClass().isAnnotationPresent(WithEntitlementsOnTestCode.class);
+        EntitledTestPackages entitledPackages = getTestClass().getAnnotation(EntitledTestPackages.class);
+
         if (TestEntitlementBootstrap.isEnabledForTest()) {
             TestEntitlementBootstrap.setActive(false == withoutEntitlements);
             TestEntitlementBootstrap.setTriviallyAllowingTestCode(false == withEntitlementsOnTestCode);
+            if (entitledPackages != null) {
+                assert withEntitlementsOnTestCode == false : "Cannot use @WithEntitlementsOnTestCode together with @EntitledTestPackages";
+                assert entitledPackages.value().length > 0 : "No test packages specified in @EntitledTestPackages";
+                TestEntitlementBootstrap.setEntitledTestPackages(entitledPackages.value());
+            }
         } else if (withEntitlementsOnTestCode) {
             throw new AssertionError(
-                "Cannot use WithEntitlementsOnTestCode on tests that are not configured to use entitlements for testing"
+                "Cannot use @WithEntitlementsOnTestCode on tests that are not configured to use entitlements for testing"
             );
         }
     }

test/framework/src/test/java/org/elasticsearch/entitlement/runtime/policy/TestPolicyManagerTests.java

@@ -13,11 +13,15 @@
 import org.elasticsearch.test.ESTestCase;
 import org.junit.Before;
 
+import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import static org.elasticsearch.entitlement.runtime.policy.PolicyManager.ComponentKind.PLUGIN;
+import static org.hamcrest.Matchers.both;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.equalTo;
 
 public class TestPolicyManagerTests extends ESTestCase {
     TestPolicyManager policyManager;
@@ -60,4 +64,43 @@ public void testIsTriviallyAllowed() {
         policyManager.setTriviallyAllowingTestCode(false);
         assertFalse(policyManager.isTriviallyAllowed(getClass()));
     }
+
+    public void testDefaultEntitledTestPackages() {
+        String[] testPackages = policyManager.entitledTestPackages.clone();
+        TestPolicyManager.assertNoRedundantPrefixes(testPackages, testPackages, true);
+
+        Arrays.sort(testPackages);
+        assertThat("Entitled test framework packages are not sorted", policyManager.entitledTestPackages, equalTo(testPackages));
+    }
+
+    public void testRejectSetRedundantEntitledTestPackages() {
+        var throwable = expectThrows(AssertionError.class, () -> policyManager.setEntitledTestPackages("org.apache.lucene.tests"));
+        var baseMatcher = both(containsString("Redundant prefix entries"));
+        assertThat(throwable.getMessage(), baseMatcher.and(containsString("org.apache.lucene.tests, org.apache.lucene.tests")));
+
+        throwable = expectThrows(AssertionError.class, () -> policyManager.setEntitledTestPackages("org.apache.lucene"));
+        assertThat(throwable.getMessage(), baseMatcher.and(containsString("org.apache.lucene.tests, org.apache.lucene")));
+
+        throwable = expectThrows(AssertionError.class, () -> policyManager.setEntitledTestPackages("org.apache.lucene.tests.whatever"));
+        assertThat(throwable.getMessage(), baseMatcher.and(containsString("org.apache.lucene.tests, org.apache.lucene.tests.whatever")));
+
+        throwable = expectThrows(AssertionError.class, () -> policyManager.setEntitledTestPackages("my.package", "my.package.sub"));
+        assertThat(throwable.getMessage(), baseMatcher.and(containsString("my.package, my.package.sub")));
+
+        throwable = expectThrows(AssertionError.class, () -> policyManager.setEntitledTestPackages("trailing.dot."));
+        assertThat(throwable.getMessage(), containsString("Invalid package prefix ending with '.' [trailing.dot.]"));
+    }
+
+    public void testIsTestFrameworkClass() {
+        String[] sortedPrefixes = { "a.b", "a.bc", "a.c" };
+
+        assertTrue(TestPolicyManager.isTestFrameworkClass(sortedPrefixes, "a.b"));
+        assertTrue(TestPolicyManager.isTestFrameworkClass(sortedPrefixes, "a.b.c"));
+        assertTrue(TestPolicyManager.isTestFrameworkClass(sortedPrefixes, "a.bc"));
+        assertTrue(TestPolicyManager.isTestFrameworkClass(sortedPrefixes, "a.bc.a"));
+
+        assertFalse(TestPolicyManager.isTestFrameworkClass(sortedPrefixes, "a"));
+        assertFalse(TestPolicyManager.isTestFrameworkClass(sortedPrefixes, "a.ba"));
+        assertFalse(TestPolicyManager.isTestFrameworkClass(sortedPrefixes, "a.bcc"));
+    }
 }

x-pack/plugin/core/src/main/plugin-metadata/entitlement-policy.yaml

@@ -18,7 +18,6 @@ org.apache.httpcomponents.httpasyncclient:
   - manage_threads
 unboundid.ldapsdk:
   - set_https_connection_properties # TODO: review if we need this once we have proper test coverage
-  - inbound_network # For com.unboundid.ldap.listener.LDAPListener
   - outbound_network
   - manage_threads
   - write_system_properties:

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ldap/ActiveDirectoryRealmTests.java

@@ -33,6 +33,7 @@
 import org.elasticsearch.script.ScriptService;
 import org.elasticsearch.script.mustache.MustacheScriptEngine;
 import org.elasticsearch.test.ESTestCase;
+import org.elasticsearch.test.ESTestCase.EntitledTestPackages;
 import org.elasticsearch.threadpool.TestThreadPool;
 import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.watcher.ResourceWatcherService;
@@ -106,6 +107,7 @@
  * The username used to authenticate then has to be in the form of CN=user. Finally the username needs to be added as an
  * additional bind DN with a password in the test setup since it really is not a DN in the ldif file
  */
+@EntitledTestPackages(value = { "com.unboundid.ldap.listener" }) // tests start LDAP server that listens for incoming connections
 public class ActiveDirectoryRealmTests extends ESTestCase {
 
     private static final String PASSWORD = "password";

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ldap/support/LdapTestCase.java

@@ -29,6 +29,7 @@
 import org.elasticsearch.core.TimeValue;
 import org.elasticsearch.env.TestEnvironment;
 import org.elasticsearch.test.ESTestCase;
+import org.elasticsearch.test.ESTestCase.EntitledTestPackages;
 import org.elasticsearch.watcher.ResourceWatcherService;
 import org.elasticsearch.xpack.core.XPackSettings;
 import org.elasticsearch.xpack.core.security.authc.RealmConfig;
@@ -73,6 +74,7 @@
 import static org.elasticsearch.xpack.core.security.authc.ldap.support.SessionFactorySettings.URLS_SETTING;
 import static org.hamcrest.Matchers.is;
 
+@EntitledTestPackages(value = { "com.unboundid.ldap.listener" }) // tests start LDAP server that listens for incoming connections
 public abstract class LdapTestCase extends ESTestCase {
 
     protected static final RealmConfig.RealmIdentifier REALM_IDENTIFIER = new RealmConfig.RealmIdentifier("ldap", "ldap1");