Explicitly set packaging permissions

This changes our packaging to be explicit about the permissions of files
and directories in the tar.gz, rpm, and deb packages. This is to protect
against a user having an incorrectly set umask when installing.

Additionally, plugins that are installed now have their permissions set
by the plugin installation so that plugins that may have been packaged
with incorrect permissions are secured.

Resolves #17634

core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java

@@ -51,6 +51,7 @@ import java.nio.file.attribute.PosixFilePermission;
 import java.nio.file.attribute.PosixFilePermissions;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.LinkedHashSet;
 import java.util.List;
@@ -134,6 +135,30 @@ class InstallPluginCommand extends Command {
     private final OptionSpec<Void> batchOption;
     private final OptionSpec<String> arguments;
 
+    public static final Set<PosixFilePermission> DIR_AND_EXECUTABLE_PERMS;
+    public static final Set<PosixFilePermission> FILE_PERMS;
+
+    static {
+        Set<PosixFilePermission> dirAndExecutablePerms = new HashSet<>(7);
+        // Directories and executables get chmod 755
+        dirAndExecutablePerms.add(PosixFilePermission.OWNER_EXECUTE);
+        dirAndExecutablePerms.add(PosixFilePermission.OWNER_READ);
+        dirAndExecutablePerms.add(PosixFilePermission.OWNER_WRITE);
+        dirAndExecutablePerms.add(PosixFilePermission.GROUP_EXECUTE);
+        dirAndExecutablePerms.add(PosixFilePermission.GROUP_READ);
+        dirAndExecutablePerms.add(PosixFilePermission.OTHERS_READ);
+        dirAndExecutablePerms.add(PosixFilePermission.OTHERS_EXECUTE);
+        DIR_AND_EXECUTABLE_PERMS = Collections.unmodifiableSet(dirAndExecutablePerms);
+
+        Set<PosixFilePermission> filePerms = new HashSet<>(4);
+        // Files get chmod 644
+        filePerms.add(PosixFilePermission.OWNER_READ);
+        filePerms.add(PosixFilePermission.OWNER_WRITE);
+        filePerms.add(PosixFilePermission.GROUP_READ);
+        filePerms.add(PosixFilePermission.OTHERS_READ);
+        FILE_PERMS = Collections.unmodifiableSet(filePerms);
+    }
+
     InstallPluginCommand(Environment env) {
         super("Install a plugin");
         this.env = env;
@@ -298,15 +323,7 @@ class InstallPluginCommand extends Command {
 
     private Path stagingDirectory(Path pluginsDir) throws IOException {
         try {
-            Set<PosixFilePermission> perms = new HashSet<>();
+            return Files.createTempDirectory(pluginsDir, ".installing-", PosixFilePermissions.asFileAttribute(DIR_AND_EXECUTABLE_PERMS));
-            perms.add(PosixFilePermission.OWNER_EXECUTE);
-            perms.add(PosixFilePermission.OWNER_READ);
-            perms.add(PosixFilePermission.OWNER_WRITE);
-            perms.add(PosixFilePermission.GROUP_READ);
-            perms.add(PosixFilePermission.GROUP_EXECUTE);
-            perms.add(PosixFilePermission.OTHERS_READ);
-            perms.add(PosixFilePermission.OTHERS_EXECUTE);
-            return Files.createTempDirectory(pluginsDir, ".installing-", PosixFilePermissions.asFileAttribute(perms));
         } catch (IllegalArgumentException e) {
             // Jimfs throws an IAE where it should throw an UOE
             // remove when google/jimfs#30 is integrated into Jimfs
@@ -409,6 +426,15 @@ class InstallPluginCommand extends Command {
             }
 
             Files.move(tmpRoot, destination, StandardCopyOption.ATOMIC_MOVE);
+            try (DirectoryStream<Path> stream = Files.newDirectoryStream(destination)) {
+                for (Path pluginFile : stream) {
+                    if (Files.isDirectory(pluginFile)) {
+                        setFileAttributes(pluginFile, DIR_AND_EXECUTABLE_PERMS);
+                    } else {
+                        setFileAttributes(pluginFile, FILE_PERMS);
+                    }
+                }
+            }
             terminal.println("-> Installed " + info.getName());
 
         } catch (Exception installProblem) {
@@ -427,17 +453,7 @@ class InstallPluginCommand extends Command {
             throw new UserError(ExitCodes.IO_ERROR, "bin in plugin " + info.getName() + " is not a directory");
         }
         Files.createDirectory(destBinDir);
-
+        setFileAttributes(destBinDir, DIR_AND_EXECUTABLE_PERMS);
-        // setup file attributes for the installed files to those of the parent dir
-        final Set<PosixFilePermission> perms = new HashSet<>();
-        final PosixFileAttributeView binAttributeView = Files.getFileAttributeView(destBinDir.getParent(), PosixFileAttributeView.class);
-        if (binAttributeView != null) {
-            perms.addAll(binAttributeView.readAttributes().permissions());
-            // setting execute bits, since this just means "the file is executable", and actual execution requires read
-            perms.add(PosixFilePermission.OWNER_EXECUTE);
-            perms.add(PosixFilePermission.GROUP_EXECUTE);
-            perms.add(PosixFilePermission.OTHERS_EXECUTE);
-        }
 
         try (DirectoryStream<Path> stream = Files.newDirectoryStream(tmpBinDir)) {
             for (Path srcFile : stream) {
@@ -449,11 +465,7 @@ class InstallPluginCommand extends Command {
 
                 Path destFile = destBinDir.resolve(tmpBinDir.relativize(srcFile));
                 Files.copy(srcFile, destFile);
-
+                setFileAttributes(destFile, DIR_AND_EXECUTABLE_PERMS);
-                final PosixFileAttributeView view = Files.getFileAttributeView(destFile, PosixFileAttributeView.class);
-                if (view != null) {
-                    view.setPermissions(perms);
-                }
             }
         }
         IOUtils.rm(tmpBinDir); // clean up what we just copied
@@ -468,8 +480,8 @@ class InstallPluginCommand extends Command {
             throw new UserError(ExitCodes.IO_ERROR, "config in plugin " + info.getName() + " is not a directory");
         }
 
-        // create the plugin's config dir "if necessary"
         Files.createDirectories(destConfigDir);
+        setFileAttributes(destConfigDir, DIR_AND_EXECUTABLE_PERMS);
         final PosixFileAttributeView destConfigDirAttributesView =
             Files.getFileAttributeView(destConfigDir.getParent(), PosixFileAttributeView.class);
         final PosixFileAttributes destConfigDirAttributes =
@@ -487,6 +499,7 @@ class InstallPluginCommand extends Command {
                 Path destFile = destConfigDir.resolve(tmpConfigDir.relativize(srcFile));
                 if (Files.exists(destFile) == false) {
                     Files.copy(srcFile, destFile);
+                    setFileAttributes(destFile, FILE_PERMS);
                     if (destConfigDirAttributes != null) {
                         setOwnerGroup(destFile, destConfigDirAttributes);
                     }
@@ -504,4 +517,13 @@ class InstallPluginCommand extends Command {
         fileAttributeView.setGroup(attributes.group());
     }
 
+    /**
+     * Sets the attributes for a path iff posix attributes are supported
+     */
+    private static void setFileAttributes(final Path path, final Set<PosixFilePermission> permissions) throws IOException {
+        PosixFileAttributeView fileAttributeView = Files.getFileAttributeView(path, PosixFileAttributeView.class);
+        if (fileAttributeView != null) {
+            Files.setPosixFilePermissions(path, permissions);
+        }
+    }
 }

distribution/build.gradle

@@ -239,6 +239,8 @@ configure(subprojects.findAll { ['deb', 'rpm'].contains(it.name) }) {
     dependsOn createEtc, createEtcScripts
     with configFiles
     into "${packagingFiles}/etc/elasticsearch"
+    fileMode 0640
+    dirMode 0750
     /* Explicitly declare the output files so this task doesn't consider itself
       up to date when the directory is created, which it would by default. And
       that'll happen when createEtc runs. */
@@ -303,6 +305,8 @@ configure(subprojects.findAll { ['deb', 'rpm'].contains(it.name) }) {
     postUninstall file("${scripts}/postrm")
 
     into '/usr/share/elasticsearch'
+    fileMode 0644
+    dirMode 0755
     user 'root'
     permissionGroup 'root'
     with libFiles
@@ -344,6 +348,8 @@ configure(subprojects.findAll { ['deb', 'rpm'].contains(it.name) }) {
     configurationFile project.expansions['path.env']
     into(new File(project.expansions['path.env']).getParent()) {
       from "${project.packagingFiles}/env/elasticsearch"
+      fileMode 0644
+      dirMode 0755
     }
 
     /**
@@ -351,6 +357,7 @@ configure(subprojects.findAll { ['deb', 'rpm'].contains(it.name) }) {
      */
     Closure suckUpEmptyDirectories = { path, u, g ->
       into(path) {
+        fileMode 0755
         from "${packagingFiles}/${path}"
         includeEmptyDirs true
         createDirectoryEntry true

distribution/deb/build.gradle

@@ -31,6 +31,7 @@ task buildDeb(type: Deb) {
   }
   into('/usr/share/doc/elasticsearch') {
     from "${project.packagingFiles}/copyright"
+    fileMode 0644
   }
 }
 

distribution/rpm/build.gradle

@@ -38,6 +38,8 @@ task buildRpm(type: Rpm) {
   license '2009'
   distribution 'Elasticsearch'
   vendor 'Elasticsearch'
+  dirMode 0755
+  fileMode 0644
   // TODO ospackage doesn't support icon but we used to have one
 }
 

distribution/src/main/packaging/scripts/postinst

@@ -99,5 +99,11 @@ fi
 chown -R $ES_USER:$ES_GROUP /var/lib/elasticsearch
 chown -R $ES_USER:$ES_GROUP /var/log/elasticsearch
 chown -R root:$ES_GROUP /etc/elasticsearch
+chmod 0750 /etc/elasticsearch
+chmod 0750 /etc/elasticsearch/scripts
+
+if [ -f /etc/sysconfig/elasticsearch ]; then
+   chmod 0644 /etc/sysconfig/elasticsearch
+fi
 
 ${scripts.footer}

distribution/tar/build.gradle

@@ -22,6 +22,8 @@ task buildTar(type: Tar) {
   extension = 'tar.gz'
   with archivesFiles
   compression = Compression.GZIP
+  dirMode 0755
+  fileMode 0644
 }
 
 artifacts {

qa/evil-tests/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java

@@ -236,11 +236,7 @@ public class InstallPluginCommandTests extends ESTestCase {
                     assertFalse("not a dir", Files.isDirectory(file));
                     if (isPosix) {
                         PosixFileAttributes attributes = Files.readAttributes(file, PosixFileAttributes.class);
-                        Set<PosixFilePermission> expectedPermissions = new HashSet<>(binAttributes.permissions());
+                        assertEquals(InstallPluginCommand.DIR_AND_EXECUTABLE_PERMS, attributes.permissions());
-                        expectedPermissions.add(PosixFilePermission.OWNER_EXECUTE);
-                        expectedPermissions.add(PosixFilePermission.GROUP_EXECUTE);
-                        expectedPermissions.add(PosixFilePermission.OTHERS_EXECUTE);
-                        assertEquals(expectedPermissions, attributes.permissions());
                     }
                 }
             }

qa/vagrant/src/test/resources/packaging/scripts/packaging_test_utils.bash

@@ -134,14 +134,18 @@ skip_not_zip() {
 
 assert_file_exist() {
     local file="$1"
-    echo "Should exist: ${file}"
+    if [ ! -e "$file" ]; then
+        echo "Should exist: ${file} but does not"
+    fi
     local file=$(readlink -m "${file}")
     [ -e "$file" ]
 }
 
 assert_file_not_exist() {
     local file="$1"
-    echo "Should not exist: ${file}"
+    if [ -e "$file" ]; then
+        echo "Should not exist: ${file} but does"
+    fi
     local file=$(readlink -m "${file}")
     [ ! -e "$file" ]
 }
@@ -156,28 +160,38 @@ assert_file() {
     assert_file_exist "$file"
 
     if [ "$type" = "d" ]; then
-        echo "And be a directory...."
+        if [ ! -d "$file" ]; then
+            echo "[$file] should be a directory but is not"
+        fi
         [ -d "$file" ]
     else
-        echo "And be a regular file...."
+        if [ ! -f "$file" ]; then
+            echo "[$file] should be a regular file but is not"
+        fi
         [ -f "$file" ]
     fi
 
     if [ "x$user" != "x" ]; then
         realuser=$(find "$file" -maxdepth 0 -printf "%u")
-        echo "Expected user: $user, found $realuser"
+        if [ "$realuser" != "$user" ]; then
+            echo "Expected user: $user, found $realuser [$file]"
+        fi
         [ "$realuser" = "$user" ]
     fi
 
     if [ "x$group" != "x" ]; then
         realgroup=$(find "$file" -maxdepth 0 -printf "%g")
-        echo "Expected group: $group, found $realgroup"
+        if [ "$realgroup" != "$group" ]; then
+            echo "Expected group: $group, found $realgroup [$file]"
+        fi
         [ "$realgroup" = "$group" ]
     fi
 
     if [ "x$privileges" != "x" ]; then
         realprivileges=$(find "$file" -maxdepth 0 -printf "%m")
-        echo "Expected privileges: $privileges, found $realprivileges"
+        if [ "$realprivileges" != "$privileges" ]; then
+            echo "Expected privileges: $privileges, found $realprivileges [$file]"
+        fi
         [ "$realprivileges" = "$privileges" ]
     fi
 }
@@ -190,10 +204,8 @@ assert_module_or_plugin_directory() {
     #just make sure that everything is the same as $CONFIG_DIR, which was properly set up during install
     config_user=$(find "$ESHOME" -maxdepth 0 -printf "%u")
     config_owner=$(find "$ESHOME" -maxdepth 0 -printf "%g")
-    # directories should use the user file-creation mask
-    config_privileges=$(executable_privileges_for_user_from_umask $ESPLUGIN_COMMAND_USER)
 
-    assert_file $directory d $config_user $config_owner $(printf "%o" $config_privileges)
+    assert_file $directory d $config_user $config_owner 755
 }
 
 assert_module_or_plugin_file() {
@@ -201,11 +213,7 @@ assert_module_or_plugin_file() {
     shift
 
     assert_file_exist "$(readlink -m $file)"
-
+    assert_file $file f $config_user $config_owner 644
-    # config files should not be executable and otherwise use the user
-    # file-creation mask
-    expected_file_privileges=$(file_privileges_for_user_from_umask $ESPLUGIN_COMMAND_USER)
-    assert_file $file f $config_user $config_owner $(printf "%o" $expected_file_privileges)
 }
 
 assert_output() {

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

@@ -84,20 +84,16 @@ install_jvm_example() {
     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" d $bin_user $bin_owner 755
-    assert_file "$ESHOME/bin/jvm-example/test" f $bin_user $bin_owner $bin_privileges
+    assert_file "$ESHOME/bin/jvm-example/test" f $bin_user $bin_owner 755
 
     #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
     config_user=$(find "$ESCONFIG" -maxdepth 0 -printf "%u")
     config_owner=$(find "$ESCONFIG" -maxdepth 0 -printf "%g")
     # directories should user the user file-creation mask
-    config_privileges=$(executable_privileges_for_user_from_umask $ESPLUGIN_COMMAND_USER)
+    assert_file "$ESCONFIG/jvm-example" d $config_user $config_owner 755
-    assert_file "$ESCONFIG/jvm-example" d $config_user $config_owner $(printf "%o" $config_privileges)
+    assert_file "$ESCONFIG/jvm-example/example.yaml" f $config_user $config_owner 644
-    # config files should not be executable and otherwise use the user
-    # file-creation mask
-    expected_file_privileges=$(file_privileges_for_user_from_umask $ESPLUGIN_COMMAND_USER)
-    assert_file "$ESCONFIG/jvm-example/example.yaml" f $config_user $config_owner $(printf "%o" $expected_file_privileges)
 
     echo "Running jvm-example's bin script...."
     "$ESHOME/bin/jvm-example/test" | grep test

qa/vagrant/src/test/resources/packaging/scripts/tar.bash

@@ -33,7 +33,7 @@ install_archive() {
     echo "Unpacking tarball to $ESHOME"
     rm -rf /tmp/untar
     mkdir -p /tmp/untar
-    tar -xzf elasticsearch*.tar.gz -C /tmp/untar
+    tar -xzpf elasticsearch*.tar.gz -C /tmp/untar
 
     find /tmp/untar -depth -type d -name 'elasticsearch*' -exec mv {} "$ESHOME" \; > /dev/null