Remove spurious SGID (opensearch-project#9447)

Setting the SGID bit on directories is maybe something some users will want to use, but setting it by default for all users does not really make sense and when packaging OpenSearch, we need to remove this customization when building packges. This was added to ElasticSearch to make it possible to manage the keystore as root while the service runs as an unprivileged user. Without the SGID trick, the generated keystore was owned by root and ElasticSearch could not access it. It is preferable to manage the keystore with non-root privileges, and this hack is not required in this case. Stick to the default permissions and remove this personalization. Signed-off-by: Romain Tartière <[email protected]> Signed-off-by: Shivansh Arora <[email protected]>
Gaurav614 · Apr 25, 2024 · d81429b · d81429b
1 parent 7b4db49
commit d81429b
Show file tree

Hide file tree

Showing 4 changed files with 11 additions and 10 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -123,6 +123,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 ### Deprecated
 
 ### Removed
+- Remove spurious SGID bit on directories ([#9447](https://github.com/opensearch-project/OpenSearch/pull/9447))
 
 ### Fixed
 - Fix ignore_missing parameter has no effect when using template snippet in rename ingest processor ([#9725](https://github.com/opensearch-project/OpenSearch/pull/9725))

diff --git a/distribution/packages/build.gradle b/distribution/packages/build.gradle
@@ -213,7 +213,7 @@ Closure commonPackageConfig(String type, boolean jdk, String architecture) {
     configurationFile '/etc/opensearch/jvm.options'
     configurationFile '/etc/opensearch/log4j2.properties'
     from("${packagingFiles}") {
-      dirMode 02750
+      dirMode 0750
       into('/etc')
       permissionGroup 'opensearch'
       includeEmptyDirs true
@@ -223,7 +223,7 @@ Closure commonPackageConfig(String type, boolean jdk, String architecture) {
     }
     from("${packagingFiles}/etc/opensearch") {
       into('/etc/opensearch')
-      dirMode 02750
+      dirMode 0750
       fileMode 0660
       permissionGroup 'opensearch'
       includeEmptyDirs true
@@ -281,8 +281,8 @@ Closure commonPackageConfig(String type, boolean jdk, String architecture) {
         dirMode mode
       }
     }
-    copyEmptyDir('/var/log/opensearch', 'opensearch', 'opensearch', 02750)
-    copyEmptyDir('/var/lib/opensearch', 'opensearch', 'opensearch', 02750)
+    copyEmptyDir('/var/log/opensearch', 'opensearch', 'opensearch', 0750)
+    copyEmptyDir('/var/lib/opensearch', 'opensearch', 'opensearch', 0750)
     copyEmptyDir('/usr/share/opensearch/plugins', 'root', 'root', 0755)
 
     into '/usr/share/opensearch'

diff --git a/distribution/packages/src/deb/lintian/opensearch b/distribution/packages/src/deb/lintian/opensearch
@@ -15,11 +15,11 @@ missing-dep-on-jarwrapper
 
 # we prefer to not make our config and log files world readable
 non-standard-file-perm etc/default/opensearch 0660 != 0644
-non-standard-dir-perm etc/opensearch/ 2750 != 0755
-non-standard-dir-perm etc/opensearch/jvm.options.d/ 2750 != 0755
+non-standard-dir-perm etc/opensearch/ 0750 != 0755
+non-standard-dir-perm etc/opensearch/jvm.options.d/ 0750 != 0755
 non-standard-file-perm etc/opensearch/*
-non-standard-dir-perm var/lib/opensearch/ 2750 != 0755
-non-standard-dir-perm var/log/opensearch/ 2750 != 0755
+non-standard-dir-perm var/lib/opensearch/ 0750 != 0755
+non-standard-dir-perm var/log/opensearch/ 0750 != 0755
 executable-is-not-world-readable etc/init.d/opensearch 0750
 non-standard-file-permissions-for-etc-init.d-script etc/init.d/opensearch 0750 != 0755
 

diff --git a/qa/os/src/test/java/org/opensearch/packaging/util/Packages.java b/qa/os/src/test/java/org/opensearch/packaging/util/Packages.java
@@ -194,11 +194,11 @@ private static void verifyInstallation(Installation opensearch, Distribution dis
 
         // we shell out here because java's posix file permission view doesn't support special modes
         assertThat(opensearch.config, file(Directory, "root", "opensearch", p750));
-        assertThat(sh.run("find \"" + opensearch.config + "\" -maxdepth 0 -printf \"%m\"").stdout, containsString("2750"));
+        assertThat(sh.run("find \"" + opensearch.config + "\" -maxdepth 0 -printf \"%m\"").stdout, containsString("750"));
 
         final Path jvmOptionsDirectory = opensearch.config.resolve("jvm.options.d");
         assertThat(jvmOptionsDirectory, file(Directory, "root", "opensearch", p750));
-        assertThat(sh.run("find \"" + jvmOptionsDirectory + "\" -maxdepth 0 -printf \"%m\"").stdout, containsString("2750"));
+        assertThat(sh.run("find \"" + jvmOptionsDirectory + "\" -maxdepth 0 -printf \"%m\"").stdout, containsString("750"));
 
         Stream.of("opensearch.keystore", "opensearch.yml", "jvm.options", "log4j2.properties")
             .forEach(configFile -> assertThat(opensearch.config(configFile), file(File, "root", "opensearch", p660)));