diff --git a/pom.xml b/pom.xml index fd2f4c6ef4a..327ba8be7a9 100644 --- a/pom.xml +++ b/pom.xml @@ -774,7 +774,9 @@ org/elasticsearch/test/**/* - org/apache/lucene/util/AbstractRandomizedTest.java + org/elasticsearch/bootstrap/BootstrapForTesting.class + org/elasticsearch/common/cli/CliToolTestCase.class + org/elasticsearch/common/cli/CliToolTestCase$*.class @@ -1535,9 +1537,7 @@ org/elasticsearch/test/**/* - org/elasticsearch/cache/recycler/MockPageCacheRecycler.class - org/apache/lucene/util/AbstractRandomizedTest.class - org/apache/lucene/util/AbstractRandomizedTest$*.class + org/elasticsearch/bootstrap/BootstrapForTesting.class org/elasticsearch/common/cli/CliToolTestCase.class org/elasticsearch/common/cli/CliToolTestCase$*.class diff --git a/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java b/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java index e4a86baf9ab..cda029f84f7 100644 --- a/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java +++ b/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java @@ -58,7 +58,7 @@ public class Bootstrap { private static volatile Bootstrap INSTANCE; - private Node node; + private volatile Node node; private final CountDownLatch keepAliveLatch = new CountDownLatch(1); private final Thread keepAliveThread; @@ -144,19 +144,22 @@ public class Bootstrap { initializeNatives(settings.getAsBoolean("bootstrap.mlockall", false), settings.getAsBoolean("bootstrap.ctrlhandler", true)); - NodeBuilder nodeBuilder = NodeBuilder.nodeBuilder().settings(settings).loadConfigSettings(false); - node = nodeBuilder.build(); if (addShutdownHook) { Runtime.getRuntime().addShutdownHook(new Thread() { @Override public void run() { - node.close(); + if (node != null) { + node.close(); + } } }); } // install SM after natives, shutdown hooks, etc. setupSecurity(settings, environment); + + NodeBuilder nodeBuilder = NodeBuilder.nodeBuilder().settings(settings).loadConfigSettings(false); + node = nodeBuilder.build(); } /** diff --git a/src/main/java/org/elasticsearch/bootstrap/Security.java b/src/main/java/org/elasticsearch/bootstrap/Security.java index 7874f8002b0..4dccd1843e1 100644 --- a/src/main/java/org/elasticsearch/bootstrap/Security.java +++ b/src/main/java/org/elasticsearch/bootstrap/Security.java @@ -23,7 +23,10 @@ import org.elasticsearch.common.SuppressForbidden; import org.elasticsearch.env.Environment; import java.io.*; +import java.nio.file.AccessMode; +import java.nio.file.FileAlreadyExistsException; import java.nio.file.Files; +import java.nio.file.NotDirectoryException; import java.nio.file.Path; import java.security.Permissions; import java.security.Policy; @@ -34,7 +37,7 @@ import java.security.Policy; * We use a template file (the one we test with), and add additional * permissions based on the environment (data paths, etc) */ -public class Security { +class Security { /** * Initializes securitymanager for the environment @@ -74,22 +77,49 @@ public class Security { } /** Add access to path (and all files underneath it */ - public static void addPath(Permissions policy, Path path, String permissions) throws IOException { + static void addPath(Permissions policy, Path path, String permissions) throws IOException { // paths may not exist yet - Files.createDirectories(path); + ensureDirectoryExists(path); + // add each path twice: once for itself, again for files underneath it policy.add(new FilePermission(path.toString(), permissions)); policy.add(new FilePermission(path.toString() + path.getFileSystem().getSeparator() + "-", permissions)); } + + /** + * Ensures configured directory {@code path} exists. + * @throws IOException if {@code path} exists, but is not a directory, not accessible, or broken symbolic link. + */ + static void ensureDirectoryExists(Path path) throws IOException { + // this isn't atomic, but neither is createDirectories. + if (Files.isDirectory(path)) { + // verify access, following links (throws exception if something is wrong) + // we only check READ as a sanity test + path.getFileSystem().provider().checkAccess(path.toRealPath(), AccessMode.READ); + } else { + // doesn't exist, or not a directory + try { + Files.createDirectories(path); + } catch (FileAlreadyExistsException e) { + // convert optional specific exception so the context is clear + IOException e2 = new NotDirectoryException(path.toString()); + e2.addSuppressed(e); + throw e2; + } + } + } /** Simple checks that everything is ok */ @SuppressForbidden(reason = "accesses jvm default tempdir as a self-test") - public static void selfTest() { + static void selfTest() throws IOException { // check we can manipulate temporary files try { - Files.delete(Files.createTempFile(null, null)); - } catch (IOException ignored) { - // potentially virus scanner + Path p = Files.createTempFile(null, null); + try { + Files.delete(p); + } catch (IOException ignored) { + // potentially virus scanner + } } catch (SecurityException problem) { throw new SecurityException("Security misconfiguration: cannot access java.io.tmpdir", problem); } diff --git a/src/main/java/org/elasticsearch/common/jna/Natives.java b/src/main/java/org/elasticsearch/common/jna/Natives.java index 69f412bda6c..fa8e074713a 100644 --- a/src/main/java/org/elasticsearch/common/jna/Natives.java +++ b/src/main/java/org/elasticsearch/common/jna/Natives.java @@ -72,9 +72,9 @@ public class Natives { } try { return CLibrary.geteuid() == 0; - } catch (Throwable error) { - logger.warn("unable to determine euid", error); - return false; // don't know + } catch (UnsatisfiedLinkError e) { + // this will have already been logged by Kernel32Library, no need to repeat it + return false; } } diff --git a/src/test/java/org/elasticsearch/test/SecurityBootstrap.java b/src/test/java/org/elasticsearch/bootstrap/BootstrapForTesting.java similarity index 93% rename from src/test/java/org/elasticsearch/test/SecurityBootstrap.java rename to src/test/java/org/elasticsearch/bootstrap/BootstrapForTesting.java index 21ad1e7ac19..42eb82db4f2 100644 --- a/src/test/java/org/elasticsearch/test/SecurityBootstrap.java +++ b/src/test/java/org/elasticsearch/bootstrap/BootstrapForTesting.java @@ -17,7 +17,7 @@ * under the License. */ -package org.elasticsearch.test; +package org.elasticsearch.bootstrap; import org.apache.lucene.util.TestSecurityManager; import org.elasticsearch.bootstrap.Bootstrap; @@ -33,13 +33,14 @@ import java.util.Objects; import static com.carrotsearch.randomizedtesting.RandomizedTest.systemPropertyAsBoolean; /** - * Installs test security manager (ensures it happens regardless of which + * Initializes natives and installs test security manager + * (init'd early by base classes to ensure it happens regardless of which * test case happens to be first, test ordering, etc). *

* The idea is to mimic as much as possible what happens with ES in production * mode (e.g. assign permissions and install security manager the same way) */ -class SecurityBootstrap { +public class BootstrapForTesting { // TODO: can we share more code with the non-test side here // without making things complex??? @@ -77,5 +78,5 @@ class SecurityBootstrap { } // does nothing, just easy way to make sure the class is loaded. - static void ensureInitialized() {} + public static void ensureInitialized() {} } diff --git a/src/test/java/org/elasticsearch/bootstrap/SecurityTests.java b/src/test/java/org/elasticsearch/bootstrap/SecurityTests.java index f50342b4e9e..a0b2f420330 100644 --- a/src/test/java/org/elasticsearch/bootstrap/SecurityTests.java +++ b/src/test/java/org/elasticsearch/bootstrap/SecurityTests.java @@ -25,6 +25,8 @@ import org.elasticsearch.env.Environment; import org.elasticsearch.test.ElasticsearchTestCase; import java.io.FilePermission; +import java.io.IOException; +import java.nio.file.Files; import java.nio.file.Path; import java.security.Permissions; @@ -110,4 +112,68 @@ public class SecurityTests extends ElasticsearchTestCase { // PID file: r/w assertTrue(permissions.implies(new FilePermission(environment.pidFile().toString(), "read,readlink,write,delete"))); } + + public void testEnsureExists() throws IOException { + Path p = createTempDir(); + + // directory exists + Path exists = p.resolve("exists"); + Files.createDirectory(exists); + Security.ensureDirectoryExists(exists); + Files.createTempFile(exists, null, null); + } + + public void testEnsureNotExists() throws IOException { + Path p = createTempDir(); + + // directory does not exist: create it + Path notExists = p.resolve("notexists"); + Security.ensureDirectoryExists(notExists); + Files.createTempFile(notExists, null, null); + } + + public void testEnsureRegularFile() throws IOException { + Path p = createTempDir(); + + // regular file + Path regularFile = p.resolve("regular"); + Files.createFile(regularFile); + try { + Security.ensureDirectoryExists(regularFile); + fail("didn't get expected exception"); + } catch (IOException expected) {} + } + + public void testEnsureSymlink() throws IOException { + Path p = createTempDir(); + + Path exists = p.resolve("exists"); + Files.createDirectory(exists); + + // symlink + Path linkExists = p.resolve("linkExists"); + try { + Files.createSymbolicLink(linkExists, exists); + } catch (UnsupportedOperationException | IOException e) { + assumeNoException("test requires filesystem that supports symbolic links", e); + } + Security.ensureDirectoryExists(linkExists); + Files.createTempFile(linkExists, null, null); + } + + public void testEnsureBrokenSymlink() throws IOException { + Path p = createTempDir(); + + // broken symlink + Path brokenLink = p.resolve("brokenLink"); + try { + Files.createSymbolicLink(brokenLink, p.resolve("nonexistent")); + } catch (UnsupportedOperationException | IOException e) { + assumeNoException("test requires filesystem that supports symbolic links", e); + } + try { + Security.ensureDirectoryExists(brokenLink); + fail("didn't get expected exception"); + } catch (IOException expected) {} + } } diff --git a/src/test/java/org/elasticsearch/test/ElasticsearchTestCase.java b/src/test/java/org/elasticsearch/test/ElasticsearchTestCase.java index a56db25f075..299a60a9715 100644 --- a/src/test/java/org/elasticsearch/test/ElasticsearchTestCase.java +++ b/src/test/java/org/elasticsearch/test/ElasticsearchTestCase.java @@ -30,12 +30,14 @@ import com.carrotsearch.randomizedtesting.generators.RandomPicks; import com.carrotsearch.randomizedtesting.generators.RandomStrings; import com.carrotsearch.randomizedtesting.rules.TestRuleAdapter; import com.google.common.base.Predicate; + import org.apache.lucene.uninverting.UninvertingReader; import org.apache.lucene.util.LuceneTestCase; import org.apache.lucene.util.LuceneTestCase.SuppressCodecs; import org.apache.lucene.util.TestUtil; import org.apache.lucene.util.TimeUnits; import org.elasticsearch.Version; +import org.elasticsearch.bootstrap.BootstrapForTesting; import org.elasticsearch.client.Requests; import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.cluster.routing.DjbHashFunction; @@ -93,7 +95,7 @@ import static com.google.common.collect.Lists.newArrayList; public abstract class ElasticsearchTestCase extends LuceneTestCase { static { - SecurityBootstrap.ensureInitialized(); + BootstrapForTesting.ensureInitialized(); } protected final ESLogger logger = Loggers.getLogger(getClass()); diff --git a/src/test/java/org/elasticsearch/test/ElasticsearchTokenStreamTestCase.java b/src/test/java/org/elasticsearch/test/ElasticsearchTokenStreamTestCase.java index 8374472dba8..e5fd76b518a 100644 --- a/src/test/java/org/elasticsearch/test/ElasticsearchTokenStreamTestCase.java +++ b/src/test/java/org/elasticsearch/test/ElasticsearchTokenStreamTestCase.java @@ -26,6 +26,7 @@ import org.apache.lucene.analysis.BaseTokenStreamTestCase; import org.apache.lucene.util.LuceneTestCase; import org.apache.lucene.util.TimeUnits; import org.elasticsearch.Version; +import org.elasticsearch.bootstrap.BootstrapForTesting; import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.common.settings.ImmutableSettings; import org.elasticsearch.test.junit.listeners.ReproduceInfoPrinter; @@ -43,7 +44,7 @@ import org.elasticsearch.test.junit.listeners.ReproduceInfoPrinter; public abstract class ElasticsearchTokenStreamTestCase extends BaseTokenStreamTestCase { static { - SecurityBootstrap.ensureInitialized(); + BootstrapForTesting.ensureInitialized(); } public static Version randomVersion() {