Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Plugin script to set proper plugin bin dir attributes #14088

Merged
merged 1 commit into from
Oct 14, 2015
Merged

Plugin script to set proper plugin bin dir attributes #14088

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 24 additions & 12 deletions core/src/main/java/org/elasticsearch/plugins/PluginManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,25 +359,37 @@ private void copyBinDirectory(Path sourcePluginBinDirectory, Path destPluginBinD
throw new IOException("Could not move [" + sourcePluginBinDirectory + "] to [" + destPluginBinDirectory + "]", e);
}
if (Environment.getFileStore(destPluginBinDirectory).supportsFileAttributeView(PosixFileAttributeView.class)) {
// add read and execute permissions to existing perms, so execution will work.
// read should generally be set already, but set it anyway: don't rely on umask...
final Set<PosixFilePermission> executePerms = new HashSet<>();
executePerms.add(PosixFilePermission.OWNER_READ);
executePerms.add(PosixFilePermission.GROUP_READ);
executePerms.add(PosixFilePermission.OTHERS_READ);
executePerms.add(PosixFilePermission.OWNER_EXECUTE);
executePerms.add(PosixFilePermission.GROUP_EXECUTE);
executePerms.add(PosixFilePermission.OTHERS_EXECUTE);
PosixFileAttributes parentDirAttributes = Files.getFileAttributeView(destPluginBinDirectory.getParent(), PosixFileAttributeView.class).readAttributes();
//copy permissions from parent bin directory
Set<PosixFilePermission> filePermissions = new HashSet<>();
for (PosixFilePermission posixFilePermission : parentDirAttributes.permissions()) {
switch (posixFilePermission) {
case OWNER_EXECUTE:
case GROUP_EXECUTE:
case OTHERS_EXECUTE:
break;
default:
filePermissions.add(posixFilePermission);
}
}
// add file execute permissions to existing perms, so execution will work.
filePermissions.add(PosixFilePermission.OWNER_EXECUTE);
filePermissions.add(PosixFilePermission.GROUP_EXECUTE);
filePermissions.add(PosixFilePermission.OTHERS_EXECUTE);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we wanted the default to be 750 right? If so then we shouldn't set OTHERS_EXECUTE

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure what we want to do. I think the original description of the issue doesn't necessarily makes sense for the bin directory. I thought that leaving what we already have shouldn't hurt, especially given that all other scripts that we have under bin (e.g. elasticsearch, plugin) have execute permissions for others too. The change is that we set user and group and we inherit other permissions form the parent dir instead of relying on what we already have on the file and forcing read access (what we did before). I am open to discussion of course :)

Files.walkFileTree(destPluginBinDirectory, new SimpleFileVisitor<Path>() {
@Override
public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
if (attrs.isRegularFile()) {
Set<PosixFilePermission> perms = Files.getPosixFilePermissions(file);
perms.addAll(executePerms);
Files.setPosixFilePermissions(file, perms);
setPosixFileAttributes(file, parentDirAttributes.owner(), parentDirAttributes.group(), filePermissions);
}
return FileVisitResult.CONTINUE;
}

@Override
public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) throws IOException {
setPosixFileAttributes(dir, parentDirAttributes.owner(), parentDirAttributes.group(), parentDirAttributes.permissions());
return FileVisitResult.CONTINUE;
}
});
} else {
terminal.println(VERBOSE, "Skipping posix permissions - filestore doesn't support posix permission");
Expand Down
36 changes: 35 additions & 1 deletion core/src/test/java/org/elasticsearch/plugins/PluginManagerPermissionTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import java.util.zip.ZipEntry;
import java.util.zip.ZipOutputStream;

import static java.nio.file.attribute.PosixFilePermission.*;
import static org.elasticsearch.common.settings.Settings.settingsBuilder;
import static org.elasticsearch.plugins.PluginInfoTests.writeProperties;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.*;
Expand Down Expand Up @@ -283,6 +284,39 @@ public void testConfigDirectoryOwnerGroupAndPermissions() throws IOException {
assertThat(pluginConfigFileAttributes.permissions(), equalTo(expectedFilePermissions));
}

public void testBinDirectoryOwnerGroupAndPermissions() throws IOException {
assumeTrue("File system does not support permissions, skipping", supportsPermissions);
URL pluginUrl = createPlugin(true, false);
PluginManager pluginManager = new PluginManager(environment, pluginUrl, PluginManager.OutputMode.VERBOSE, TimeValue.timeValueSeconds(10));
pluginManager.downloadAndExtract(pluginName, terminal);
PosixFileAttributes parentFileAttributes = Files.getFileAttributeView(environment.binFile(), PosixFileAttributeView.class).readAttributes();
Path binPath = environment.binFile().resolve(pluginName);
PosixFileAttributes pluginBinDirAttributes = Files.getFileAttributeView(binPath, PosixFileAttributeView.class).readAttributes();
assertThat(pluginBinDirAttributes.owner(), equalTo(parentFileAttributes.owner()));
assertThat(pluginBinDirAttributes.group(), equalTo(parentFileAttributes.group()));
assertThat(pluginBinDirAttributes.permissions(), equalTo(parentFileAttributes.permissions()));
Path executableFile = binPath.resolve("my-binary");
PosixFileAttributes pluginExecutableFileAttributes = Files.getFileAttributeView(executableFile, PosixFileAttributeView.class).readAttributes();
assertThat(pluginExecutableFileAttributes.owner(), equalTo(parentFileAttributes.owner()));
assertThat(pluginExecutableFileAttributes.group(), equalTo(parentFileAttributes.group()));
Set<PosixFilePermission> expectedFilePermissions = new HashSet<>();
expectedFilePermissions.add(OWNER_EXECUTE);
expectedFilePermissions.add(GROUP_EXECUTE);
expectedFilePermissions.add(OTHERS_EXECUTE);
for (PosixFilePermission parentPermission : parentFileAttributes.permissions()) {
switch(parentPermission) {
case OWNER_EXECUTE:
case GROUP_EXECUTE:
case OTHERS_EXECUTE:
break;
default:
expectedFilePermissions.add(parentPermission);
}
}

assertThat(pluginExecutableFileAttributes.permissions(), equalTo(expectedFilePermissions));
}

private URL createPlugin(boolean withBinDir, boolean withConfigDir) throws IOException {
final Path structure = createTempDir().resolve("fake-plugin");
writeProperties(structure, "description", "fake desc",
Expand All @@ -301,7 +335,7 @@ private URL createPlugin(boolean withBinDir, boolean withConfigDir) throws IOExc
// create executable
Path executable = binDir.resolve("my-binary");
Files.createFile(executable);
Files.setPosixFilePermissions(executable, PosixFilePermissions.fromString("rwxr-xr-x"));
Files.setPosixFilePermissions(executable, PosixFilePermissions.fromString("rw-r--r--"));
}
if (withConfigDir) {
// create bin dir
Expand Down
9 changes: 7 additions & 2 deletions qa/vagrant/src/test/resources/packaging/scripts/plugins.bash
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,13 @@ install_jvm_example() {
local relativePath=${1:-$(readlink -m jvm-example-*.zip)}
install_jvm_plugin jvm-example "$relativePath"

assert_file_exist "$ESHOME/bin/jvm-example"
assert_file_exist "$ESHOME/bin/jvm-example/test"
#owner group and permissions vary depending on how es was installed
#just make sure that everything is the same as the parent bin dir, which was properly set up during install
bin_user=$(find "$ESHOME/bin" -maxdepth 0 -printf "%u")
bin_owner=$(find "$ESHOME/bin" -maxdepth 0 -printf "%g")
bin_privileges=$(find "$ESHOME/bin" -maxdepth 0 -printf "%m")
assert_file "$ESHOME/bin/jvm-example" d $bin_user $bin_owner $bin_privileges
assert_file "$ESHOME/bin/jvm-example/test" f $bin_user $bin_owner $bin_privileges

#owner group and permissions vary depending on how es was installed
#just make sure that everything is the same as $CONFIG_DIR, which was properly set up during install
Expand Down