Improve path management on init:

* Properly support symlinks (e.g. /tmp -> /mnt/tmp)
* Check all configured paths up front and deliver the best exception we can when things are wrong
* Initialize securitymanager earlier
* Fix too-loud error logging of Natives root check
This commit is contained in:
Robert Muir 2015-05-12 00:20:52 -04:00
parent 7c2a8dfb9e
commit 2cea5f2b38
4 changed files with 111 additions and 12 deletions

View File

@ -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();
}
/**

View File

@ -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;
@ -76,20 +79,47 @@ public class Security {
/** Add access to path (and all files underneath it */
public 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() {
public 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);
}

View File

@ -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;
}
}

View File

@ -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 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 e) {
assumeNoException("test requires filesystem that supports symbolic links", e);
}
try {
Security.ensureDirectoryExists(brokenLink);
fail("didn't get expected exception");
} catch (IOException expected) {}
}
}