From ca72b98aefafb8a47652a9d6cf8e03bb853966bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Tarti=C3=A8re?= Date: Wed, 16 Aug 2023 10:58:39 -1000 Subject: [PATCH 1/8] Do not change shared data permissions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The files installed in /usr/share/opensearch do not need to be owned by the opensearch user. The default ownership from Debian packages, root:root is fine and should be preferred. Remove the post-installation line that customized these permissions. Signed-off-by: Romain Tartière --- scripts/pkg/build_templates/opensearch/deb/debian/postinst | 1 - 1 file changed, 1 deletion(-) diff --git a/scripts/pkg/build_templates/opensearch/deb/debian/postinst b/scripts/pkg/build_templates/opensearch/deb/debian/postinst index cc84020eb7..34ccf06b90 100755 --- a/scripts/pkg/build_templates/opensearch/deb/debian/postinst +++ b/scripts/pkg/build_templates/opensearch/deb/debian/postinst @@ -38,7 +38,6 @@ if ! grep -q '## OpenSearch Performance Analyzer' ${config_dir}/jvm.options; the fi # Set owner -chown -R opensearch.opensearch ${product_dir} chown -R opensearch.opensearch ${config_dir} chown -R opensearch.opensearch ${log_dir} chown -R opensearch.opensearch ${data_dir} From 66d44ad976af462a6531a4720de9c7379dcc50c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Tarti=C3=A8re?= Date: Wed, 16 Aug 2023 11:01:47 -1000 Subject: [PATCH 2/8] Restrict access to OpenSearch configuration files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The configuartion files installed by the opensearch package can contain sensitive information such as clear-text passwords (e.g. to reach an LDAP directory to authenticate users). When installing from a tarball, the configuration file are only accessible to the owner and members of the group of the file. Adjust the post-installation script to setup permissions for these files similar to the ones of the tarball. Currently, OpenSearch create files in the configuartion directory on first start (opensearch.keystore), so we cannot change the configuration directory ownership to prevent the service from tinkering with these files. Signed-off-by: Romain Tartière --- scripts/pkg/build_templates/opensearch/deb/debian/postinst | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/scripts/pkg/build_templates/opensearch/deb/debian/postinst b/scripts/pkg/build_templates/opensearch/deb/debian/postinst index 34ccf06b90..5f9be46c12 100755 --- a/scripts/pkg/build_templates/opensearch/deb/debian/postinst +++ b/scripts/pkg/build_templates/opensearch/deb/debian/postinst @@ -37,8 +37,11 @@ if ! grep -q '## OpenSearch Performance Analyzer' ${config_dir}/jvm.options; the echo "--add-opens=jdk.attach/sun.tools.attach=ALL-UNNAMED" >> ${config_dir}/jvm.options fi -# Set owner +# Set ownership and permissions +# FIXME: the opensearch service should not have w permission in the config directory chown -R opensearch.opensearch ${config_dir} +chmod -R o-rx ${config_dir} + chown -R opensearch.opensearch ${log_dir} chown -R opensearch.opensearch ${data_dir} chown -R opensearch.opensearch ${pid_dir} From f5c8cb3bf6dceeed81a3f85a3520b15e8811ea01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Tarti=C3=A8re?= Date: Wed, 16 Aug 2023 11:08:39 -1000 Subject: [PATCH 3/8] Adjust ownership and permissions of the log directory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The OpenSearch logs may contain sensitive data. In order to prevent random users from accessing log files, Debian packages often setup the log directory so that only the service can write to it, members of the adm group can read these files, and other users are not granted access. Adjust the log directory owner and permissions to match that behavior. Signed-off-by: Romain Tartière --- scripts/pkg/build_templates/opensearch/deb/debian/postinst | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scripts/pkg/build_templates/opensearch/deb/debian/postinst b/scripts/pkg/build_templates/opensearch/deb/debian/postinst index 5f9be46c12..d285c25a85 100755 --- a/scripts/pkg/build_templates/opensearch/deb/debian/postinst +++ b/scripts/pkg/build_templates/opensearch/deb/debian/postinst @@ -42,7 +42,9 @@ fi chown -R opensearch.opensearch ${config_dir} chmod -R o-rx ${config_dir} -chown -R opensearch.opensearch ${log_dir} +chown -R opensearch.adm ${log_dir} +chmod 750 ${log_dir} + chown -R opensearch.opensearch ${data_dir} chown -R opensearch.opensearch ${pid_dir} From a9053571dc32a34ccf0e701397fdbd618228f1a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Tarti=C3=A8re?= Date: Wed, 16 Aug 2023 15:32:28 -1000 Subject: [PATCH 4/8] Adjust permissions of the data directory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Other users on the system have no reason to access the OpenSearch raw data files. Make sure these files are only available to the user running the service. Signed-off-by: Romain Tartière --- scripts/pkg/build_templates/opensearch/deb/debian/postinst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scripts/pkg/build_templates/opensearch/deb/debian/postinst b/scripts/pkg/build_templates/opensearch/deb/debian/postinst index d285c25a85..71e25700d6 100755 --- a/scripts/pkg/build_templates/opensearch/deb/debian/postinst +++ b/scripts/pkg/build_templates/opensearch/deb/debian/postinst @@ -46,6 +46,8 @@ chown -R opensearch.adm ${log_dir} chmod 750 ${log_dir} chown -R opensearch.opensearch ${data_dir} +chmod 750 ${data_dir} + chown -R opensearch.opensearch ${pid_dir} # Reload systemctl daemon From eb0e032a7d7f35b8e99217e1b761955f361c78a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Tarti=C3=A8re?= Date: Wed, 16 Aug 2023 15:34:29 -1000 Subject: [PATCH 5/8] Sync rpm owner/permissions with the deb ones MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that the deb permissions and files ownerships are better, adjust the rpm receipe is a similar fashion for a consistent experience accross different operating systems. Signed-off-by: Romain Tartière --- .../opensearch/rpm/opensearch.rpm.spec | 27 +++++++++++-------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/scripts/pkg/build_templates/opensearch/rpm/opensearch.rpm.spec b/scripts/pkg/build_templates/opensearch/rpm/opensearch.rpm.spec index f2df1879e0..af26c2f0f9 100644 --- a/scripts/pkg/build_templates/opensearch/rpm/opensearch.rpm.spec +++ b/scripts/pkg/build_templates/opensearch/rpm/opensearch.rpm.spec @@ -68,7 +68,8 @@ if [ ! -f %{buildroot}%{data_dir}/performance_analyzer_enabled.conf ]; then echo 'true' > %{buildroot}%{data_dir}/performance_analyzer_enabled.conf fi # Change Permissions -chmod -Rf a+rX,u+w,g-w,o-w %{buildroot}/* +chmod -Rf g-s %{buildroot}/* +chmod -Rf u=rwX,g=rX,o= %{buildroot}/etc exit 0 %pre @@ -150,13 +151,6 @@ exit 0 # Permissions %defattr(-, %{name}, %{name}) -# Root dirs/docs/licenses -%dir %{product_dir} -%doc %{product_dir}/NOTICE.txt -%doc %{product_dir}/README.md -%license %{product_dir}/LICENSE.txt -%{product_dir}/manifest.yml - # Config dirs/files %dir %{config_dir} %{config_dir}/jvm.options.d @@ -175,6 +169,20 @@ exit 0 %attr(0644, root, root) %config(noreplace) %{_prefix}/lib/sysctl.d/%{name}.conf %attr(0644, root, root) %config(noreplace) %{_prefix}/lib/tmpfiles.d/%{name}.conf +%dir %attr(750, %{name}, %{name}) %{data_dir} +%dir %attr(750, %{name}, %{name}) %{log_dir} +%dir %attr(750, %{name}, %{name}) %{pid_dir} + +# Permissions +%defattr(-, root, root) + +# Root dirs/docs/licenses +%dir %{product_dir} +%doc %{product_dir}/NOTICE.txt +%doc %{product_dir}/README.md +%license %{product_dir}/LICENSE.txt +%{product_dir}/manifest.yml + # Main dirs %{product_dir}/bin %{product_dir}/jdk @@ -182,9 +190,6 @@ exit 0 %{product_dir}/modules %{product_dir}/performance-analyzer-rca %{product_dir}/plugins -%{log_dir} -%{pid_dir} -%dir %{data_dir} # Symlinks %{product_dir}/data From ed1636d67d9a636a3a514433726eab3a8a04768c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Tarti=C3=A8re?= Date: Thu, 17 Aug 2023 14:58:53 -1000 Subject: [PATCH 6/8] Make permisson more explicit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of adding and removing permission bits, use explicit values. Signed-off-by: Romain Tartière --- scripts/pkg/build_templates/opensearch/deb/debian/postinst | 2 +- .../opensearch/deb/debmake_opensearch_install.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/pkg/build_templates/opensearch/deb/debian/postinst b/scripts/pkg/build_templates/opensearch/deb/debian/postinst index 71e25700d6..a192b660b1 100755 --- a/scripts/pkg/build_templates/opensearch/deb/debian/postinst +++ b/scripts/pkg/build_templates/opensearch/deb/debian/postinst @@ -40,7 +40,7 @@ fi # Set ownership and permissions # FIXME: the opensearch service should not have w permission in the config directory chown -R opensearch.opensearch ${config_dir} -chmod -R o-rx ${config_dir} +chmod -R u=rwX,g=rX,o= ${config_dir} chown -R opensearch.adm ${log_dir} chmod 750 ${log_dir} diff --git a/scripts/pkg/build_templates/opensearch/deb/debmake_opensearch_install.sh b/scripts/pkg/build_templates/opensearch/deb/debmake_opensearch_install.sh index 206ca6d6d6..5a4a341ec3 100755 --- a/scripts/pkg/build_templates/opensearch/deb/debmake_opensearch_install.sh +++ b/scripts/pkg/build_templates/opensearch/deb/debmake_opensearch_install.sh @@ -41,6 +41,6 @@ ln -s ${data_dir} ${buildroot}${product_dir}/data ln -s ${log_dir} ${buildroot}${product_dir}/logs # Change Permissions -chmod -Rf a+rX,u+w,g-w,o-w ${buildroot}/* +chmod -Rf u=rwX,g=rX,o=rX ${buildroot}/* exit 0 From 0b4c2774ef752bbae6ff726ecc6d6edfcc481cc7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Tarti=C3=A8re?= Date: Fri, 18 Aug 2023 13:04:26 -1000 Subject: [PATCH 7/8] Restore previous absence of %dir for logs/pid MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit They where changed unexpectedly in eb0e032a7d7f35b8e99217e1b761955f361c78a3 but while the change seems noop, better limit the number of changes to avoid unexpected regressions. Signed-off-by: Romain Tartière --- .../pkg/build_templates/opensearch/rpm/opensearch.rpm.spec | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/pkg/build_templates/opensearch/rpm/opensearch.rpm.spec b/scripts/pkg/build_templates/opensearch/rpm/opensearch.rpm.spec index af26c2f0f9..ea2ddb2108 100644 --- a/scripts/pkg/build_templates/opensearch/rpm/opensearch.rpm.spec +++ b/scripts/pkg/build_templates/opensearch/rpm/opensearch.rpm.spec @@ -170,8 +170,8 @@ exit 0 %attr(0644, root, root) %config(noreplace) %{_prefix}/lib/tmpfiles.d/%{name}.conf %dir %attr(750, %{name}, %{name}) %{data_dir} -%dir %attr(750, %{name}, %{name}) %{log_dir} -%dir %attr(750, %{name}, %{name}) %{pid_dir} +%attr(750, %{name}, %{name}) %{log_dir} +%attr(750, %{name}, %{name}) %{pid_dir} # Permissions %defattr(-, root, root) From 71c7c34cc5a30dde0933b53c7356fae86c3df3eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Tarti=C3=A8re?= Date: Mon, 21 Aug 2023 08:17:00 -1000 Subject: [PATCH 8/8] Also explicitly remove SGID bit when building .deb MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The presence of this bit on the files seems to have no consequence on the final package as the installed files do not have the SGID bit set. But for consistency with the rpm packaging, unset the bit before building the package. Signed-off-by: Romain Tartière --- .../build_templates/opensearch/deb/debmake_opensearch_install.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/pkg/build_templates/opensearch/deb/debmake_opensearch_install.sh b/scripts/pkg/build_templates/opensearch/deb/debmake_opensearch_install.sh index 5a4a341ec3..ff87e544d5 100755 --- a/scripts/pkg/build_templates/opensearch/deb/debmake_opensearch_install.sh +++ b/scripts/pkg/build_templates/opensearch/deb/debmake_opensearch_install.sh @@ -41,6 +41,7 @@ ln -s ${data_dir} ${buildroot}${product_dir}/data ln -s ${log_dir} ${buildroot}${product_dir}/logs # Change Permissions +chmod -Rf g-s ${buildroot}/* chmod -Rf u=rwX,g=rX,o=rX ${buildroot}/* exit 0