Fix plugin installation permissions

When installing plugin permissions, we try to set the permissions on all
installed files ourselves because a umask from the user could violate
everything needed to get the permissions right. Sadly, directories were
not handled correctly at all and so we were still left with broken
installations with umasks like 0077. This commit fixes this issue, adds
a thorough unit test for the situation, and most importantly, adds a
test that sets the umask before installing the plugin.

Relates #24527

distribution/build.gradle

@@ -249,6 +249,7 @@ configure(distributions.findAll { ['zip', 'tar', 'integ-test-zip'].contains(it.n
       }
       into('') {
         from {
+          dirMode 0755
           plugins.getParent()
         }
       }

distribution/tools/plugin-cli/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java

@@ -554,20 +554,20 @@ class InstallPluginCommand extends EnvironmentAwareCommand {
             Files.move(tmpRoot, destination, StandardCopyOption.ATOMIC_MOVE);
             Files.walkFileTree(destination, new SimpleFileVisitor<Path>() {
                 @Override
-                public FileVisitResult visitFile(Path pluginFile, BasicFileAttributes attrs) throws IOException {
-                    if (Files.isDirectory(pluginFile)) {
-                        setFileAttributes(pluginFile, PLUGIN_DIR_PERMS);
+                public FileVisitResult visitFile(final Path file, final BasicFileAttributes attrs) throws IOException {
+                    if ("bin".equals(file.getParent().getFileName().toString())) {
+                        setFileAttributes(file, BIN_FILES_PERMS);
                     } else {
-                        // There can also be "bin" directories under the plugin directory, storing native code executables
-                        Path parentDir = pluginFile.getParent().getFileName();
-                        if ("bin".equals(parentDir.toString())) {
-                            setFileAttributes(pluginFile, BIN_FILES_PERMS);
-                        } else {
-                            setFileAttributes(pluginFile, PLUGIN_FILES_PERMS);
-                        }
+                        setFileAttributes(file, PLUGIN_FILES_PERMS);
                     }
                     return FileVisitResult.CONTINUE;
                 }
+
+                @Override
+                public FileVisitResult postVisitDirectory(final Path dir, final IOException exc) throws IOException {
+                    setFileAttributes(dir, PLUGIN_DIR_PERMS);
+                    return FileVisitResult.CONTINUE;
+                }
             });
 
             terminal.println("-> Installed " + info.getName());

distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java

@@ -469,32 +469,65 @@ public class InstallPluginCommandTests extends ESTestCase {
         }
     }
 
-    public void testPlatformBinPermissions() throws Exception {
+    public void testPluginPermissions() throws Exception {
         assumeTrue("posix filesystem", isPosix);
-        Tuple<Path, Environment> env = createEnv(fs, temp);
-        Path pluginDir = createPluginDir(temp);
-        Path platformDir = pluginDir.resolve("platform");
-        Path platformNameDir = platformDir.resolve("linux-x86_64");
-        Path platformBinDir = platformNameDir.resolve("bin");
+
+        final Tuple<Path, Environment> env = createEnv(fs, temp);
+        final Path pluginDir = createPluginDir(temp);
+        final Path resourcesDir = pluginDir.resolve("resources");
+        final Path platformDir = pluginDir.resolve("platform");
+        final Path platformNameDir = platformDir.resolve("linux-x86_64");
+        final Path platformBinDir = platformNameDir.resolve("bin");
         Files.createDirectories(platformBinDir);
-        Path programFile = Files.createFile(platformBinDir.resolve("someprogram"));
-        // a file created with Files.createFile() should not have execute permissions
-        Set<PosixFilePermission> sourcePerms = Files.getPosixFilePermissions(programFile);
-        assertFalse(sourcePerms.contains(PosixFilePermission.OWNER_EXECUTE));
-        assertFalse(sourcePerms.contains(PosixFilePermission.GROUP_EXECUTE));
-        assertFalse(sourcePerms.contains(PosixFilePermission.OTHERS_EXECUTE));
-        String pluginZip = createPluginUrl("fake", pluginDir);
+
+        Files.createFile(pluginDir.resolve("fake-" + Version.CURRENT.toString() + ".jar"));
+        Files.createFile(platformBinDir.resolve("fake_executable"));
+        Files.createDirectory(resourcesDir);
+        Files.createFile(resourcesDir.resolve("resource"));
+
+        final String pluginZip = createPluginUrl("fake", pluginDir);
+
         installPlugin(pluginZip, env.v1());
         assertPlugin("fake", pluginDir, env.v2());
-        // check that the installed program has execute permissions, even though the one added to the plugin didn't
-        Path installedPlatformBinDir = env.v2().pluginsFile().resolve("fake").resolve("platform").resolve("linux-x86_64").resolve("bin");
-        assertTrue(Files.isDirectory(installedPlatformBinDir));
-        Path installedProgramFile = installedPlatformBinDir.resolve("someprogram");
-        assertTrue(Files.isRegularFile(installedProgramFile));
-        Set<PosixFilePermission> installedPerms = Files.getPosixFilePermissions(installedProgramFile);
-        assertTrue(installedPerms.contains(PosixFilePermission.OWNER_EXECUTE));
-        assertTrue(installedPerms.contains(PosixFilePermission.GROUP_EXECUTE));
-        assertTrue(installedPerms.contains(PosixFilePermission.OTHERS_EXECUTE));
+
+        final Path fake = env.v2().pluginsFile().resolve("fake");
+        final Path resources = fake.resolve("resources");
+        final Path platform = fake.resolve("platform");
+        final Path platformName = platform.resolve("linux-x86_64");
+        final Path bin = platformName.resolve("bin");
+        assert755(fake);
+        assert644(fake.resolve("fake-" + Version.CURRENT + ".jar"));
+        assert755(resources);
+        assert644(resources.resolve("resource"));
+        assert755(platform);
+        assert755(platformName);
+        assert755(bin.resolve("fake_executable"));
+    }
+
+    private void assert644(final Path path) throws IOException {
+        final Set<PosixFilePermission> permissions = Files.getPosixFilePermissions(path);
+        assertTrue(permissions.contains(PosixFilePermission.OWNER_READ));
+        assertTrue(permissions.contains(PosixFilePermission.OWNER_WRITE));
+        assertFalse(permissions.contains(PosixFilePermission.OWNER_EXECUTE));
+        assertTrue(permissions.contains(PosixFilePermission.GROUP_READ));
+        assertFalse(permissions.contains(PosixFilePermission.GROUP_WRITE));
+        assertFalse(permissions.contains(PosixFilePermission.GROUP_EXECUTE));
+        assertTrue(permissions.contains(PosixFilePermission.OTHERS_READ));
+        assertFalse(permissions.contains(PosixFilePermission.OTHERS_WRITE));
+        assertFalse(permissions.contains(PosixFilePermission.OTHERS_EXECUTE));
+    }
+
+    private void assert755(final Path path) throws IOException {
+        final Set<PosixFilePermission> permissions = Files.getPosixFilePermissions(path);
+        assertTrue(permissions.contains(PosixFilePermission.OWNER_READ));
+        assertTrue(permissions.contains(PosixFilePermission.OWNER_WRITE));
+        assertTrue(permissions.contains(PosixFilePermission.OWNER_EXECUTE));
+        assertTrue(permissions.contains(PosixFilePermission.GROUP_READ));
+        assertFalse(permissions.contains(PosixFilePermission.GROUP_WRITE));
+        assertTrue(permissions.contains(PosixFilePermission.GROUP_EXECUTE));
+        assertTrue(permissions.contains(PosixFilePermission.OTHERS_READ));
+        assertFalse(permissions.contains(PosixFilePermission.OTHERS_WRITE));
+        assertTrue(permissions.contains(PosixFilePermission.OTHERS_EXECUTE));
     }
 
     public void testConfig() throws Exception {

qa/vagrant/src/test/resources/packaging/tests/module_and_plugin_test_cases.bash

@@ -475,3 +475,7 @@ fi
     # restore ES_JAVA_OPTS
     export ES_JAVA_OPTS=$es_java_opts
 }
+
+@test "[$GROUP] test umask" {
+    install_jvm_example $(readlink -m jvm-example-*.zip) 0077
+}

qa/vagrant/src/test/resources/packaging/utils/plugins.bash

@@ -34,10 +34,15 @@
 install_plugin() {
     local name=$1
     local path="$2"
+    local umask="$3"
 
     assert_file_exist "$path"
 
-    sudo -E -u $ESPLUGIN_COMMAND_USER "$ESHOME/bin/elasticsearch-plugin" install -batch "file://$path"
+    if [ -z "$umask" ]; then
+      sudo -E -u $ESPLUGIN_COMMAND_USER "$ESHOME/bin/elasticsearch-plugin" install -batch "file://$path"
+    else
+      sudo -E -u $ESPLUGIN_COMMAND_USER bash -c "umask $umask && \"$ESHOME/bin/elasticsearch-plugin\" install -batch \"file://$path\""
+    fi
 
     assert_file_exist "$ESPLUGINS/$name"
     assert_file_exist "$ESPLUGINS/$name/plugin-descriptor.properties"
@@ -56,7 +61,7 @@ install_plugin() {
 install_jvm_plugin() {
     local name=$1
     local path="$2"
-    install_plugin $name "$path"
+    install_plugin $name "$path" $3
     assert_file_exist "$ESPLUGINS/$name/$name"*".jar"
 }
 
@@ -82,12 +87,16 @@ remove_plugin() {
 # placements for non-site plugins.
 install_jvm_example() {
     local relativePath=${1:-$(readlink -m jvm-example-*.zip)}
-    install_jvm_plugin jvm-example "$relativePath"
+    install_jvm_plugin jvm-example "$relativePath" $2
 
-    #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")
+
+    assert_file "$ESHOME/plugins/jvm-example" d $bin_user $bin_owner 755
+    assert_file "$ESHOME/plugins/jvm-example/jvm-example-$(cat version).jar" f $bin_user $bin_owner 644
+
+    #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
     assert_file "$ESHOME/bin/jvm-example" d $bin_user $bin_owner 755
     assert_file "$ESHOME/bin/jvm-example/test" f $bin_user $bin_owner 755