Defaut config permission too relaxed in deb and rpm packages

[smortex]

After installing the Debian package, all users can read all configuration files:

$ ls -dl /etc/opensearch                    
drwxr-xr-x 9 opensearch opensearch 4096 25 juil. 15:57 /etc/opensearch/
$ ls -l /etc/opensearch/opensearch.yml 
-rw-r--r-- 1 opensearch opensearch 6503 25 juil. 15:57 /etc/opensearch/opensearch.yml
$ ls -dl /etc/opensearch/opensearch-security
drwxr-xr-x 2 opensearch opensearch 4096 25 juil. 15:57 /etc/opensearch/opensearch-security/
$ ls -l /etc/opensearch/opensearch-security 
total 76
-rw-r--r-- 1 opensearch opensearch    50 14 oct.   2022 action_groups.yml
-rw-r--r-- 1 opensearch opensearch  1973 14 oct.   2022 allowlist.yml
-rw-r--r-- 1 opensearch opensearch  2541 14 oct.   2022 audit.yml
-rw-r--r-- 1 opensearch opensearch 10063 14 oct.   2022 config.yml
-rw-r--r-- 1 opensearch opensearch  1689 14 oct.   2022 internal_users.yml
-rw-r--r-- 1 opensearch opensearch   154 14 oct.   2022 nodes_dn.yml
-rw-r--r-- 1 opensearch opensearch 12381 14 oct.   2022 opensearch.yml.example
-rw-r--r-- 1 opensearch opensearch   844 14 oct.   2022 roles_mapping.yml
-rw-r--r-- 1 opensearch opensearch 12649 14 oct.   2022 roles.yml
-rw-r--r-- 1 opensearch opensearch   170 14 oct.   2022 tenants.yml
-rw-r--r-- 1 opensearch opensearch  1973 14 oct.   2022 whitelist.yml

Some of these files contain or can contain sensitive data, for example:

• internal_users.yml contain hashed password;
• config.yml can contain clear-text credential for authentication against an LDAP directory.

Upon initial investigation, this is the result of this chmod on the whole tree of files installed by the package:

opensearch-build/scripts/pkg/build_templates/opensearch/deb/debmake_opensearch_install.sh

Line 44 in b422ae0

chmod -Rf a+rX,u+w,g-w,o-w ${buildroot}/*

It looks like the tarball of OpenSearch has different and more restrictive permissions by default

Filename	Debian package permission	Tarball permission
config (read /etc/opensearch with the Debian package)	755	755
config/opensearch.yml	644	640
config/opensearch-security	755	750
config/opensearch-security/*.yml	644	640

I am opening this issue in order to discuss this packaging issue and fix it. IMHO, the tarball default permissions are better than the Debian ones, but there is still room for improvement such as changing files ownership to root:opensearch in order to prevent a compromised service from rewriting it's configuration (and yes, currently all the files installed by the package are owned by the opensearch user so he can overwrite all the application code, but let's tackle this in another PR).

What do you think?

[smortex]

Cc @peterzhuamazon and @mnin who contributed to the awesome Debian packaging and might have an opinion on this!

[rishabh6788]

Tagging @peterzhuamazon to take a look.

[peterzhuamazon]

Hi @smortex I am currently a bit busy on another project.
But will try to do some digging soon.

I think in the rpm pkg I did something on the spec file for such permissions, but in deb pkg I assume there might have missed something in the user scripts. Thanks.

[etgraylog]

It seems ownership & permission-bits set by the DEB & RPM packages are similar, if not the same. For reference, here they are from OpenSearch 2.9.0-1 RPM package:

[root@localhost /]# cat /etc/system-release; yum list --installed | grep opensearch
CentOS Stream release 8
opensearch.x86_64                     2.9.0-1                             @opensearch-2.x
[root@localhost /]# ls -ld /etc/opensearch
drwxr-sr-x. 9 opensearch opensearch 4096 Aug  9 10:01 /etc/opensearch
[root@localhost /]# ls -l /etc/opensearch/opensearch.yml
-rw-r--r--. 1 opensearch opensearch 6503 Aug  9 10:01 /etc/opensearch/opensearch.yml
[root@localhost /]# ls -dl /etc/opensearch/opensearch-security
drwxr-xr-x. 2 opensearch opensearch 4096 Aug  9 10:00 /etc/opensearch/opensearch-security
[root@localhost /]# ls -l /etc/opensearch/opensearch-security
total 76
-rw-r--r--. 1 opensearch opensearch    50 Jul 18 17:54 action_groups.yml
-rw-r--r--. 1 opensearch opensearch  1973 Jul 18 17:54 allowlist.yml
-rw-r--r--. 1 opensearch opensearch  2541 Jul 18 17:54 audit.yml
-rw-r--r--. 1 opensearch opensearch 10063 Jul 18 17:54 config.yml
-rw-r--r--. 1 opensearch opensearch  1689 Jul 18 17:54 internal_users.yml
-rw-r--r--. 1 opensearch opensearch   154 Jul 18 17:54 nodes_dn.yml
-rw-r--r--. 1 opensearch opensearch 12381 Jul 18 17:54 opensearch.yml.example
-rw-r--r--. 1 opensearch opensearch   844 Jul 18 17:54 roles_mapping.yml
-rw-r--r--. 1 opensearch opensearch 12649 Jul 18 17:54 roles.yml
-rw-r--r--. 1 opensearch opensearch   170 Jul 18 17:54 tenants.yml
-rw-r--r--. 1 opensearch opensearch  1973 Jul 18 17:54 whitelist.yml
[root@localhost /]#

I suppose 3 new lines could be introduced to the build_templates/ opensearch.rpm.spec & postinit to set the same file permission-bits as the Tar-file does? I created PR #3858 to propose these changes.

chown 640 opensearch.opensearch ${config_dir}/opensearch.yml
chown 640 opensearch.opensearch ${config_dir}/opensearch-security
chown 640 opensearch.opensearch ${config_dir}/opensearch-security/*.yml

[mnin]

Hey @smortex,

thank you for the inquiry!

I was looking into this together with @etgraylog.

We checked the DEB and RPM results in the /etc/opensearch directories after successfully installed the packages. We don't saw any differences for the user/group ids and the permissions.

OpenSearch-build is calling the /usr/share/opensearch/plugins/opensearch-security/tools/install_demo_configuration.sh script after the installation via https://github.com/opensearch-project/opensearch-build/blob/main/scripts/pkg/build_templates/opensearch/deb/debian/postinst#L24 (or in the RPM
package https://github.com/opensearch-project/opensearch-build/blob/main/scripts/pkg/build_templates/opensearch/rpm/opensearch.rpm.spec#L96).

This https://github.com/opensearch-project/security/blob/main/tools/install_demo_configuration.sh might be the right place to adjust the file permissions for security related directories and files.

@peterzhuamazon what do you think?

[peterzhuamazon]

Hi,

In tar, the install demo configuration is also run, so if tar is preserving the permissions after running the script, rpm/deb should have the same.

It seems like chmod -Rf a+rX,u+w,g-w,o-w ${buildroot}/* is indeed the cause to me tho, unless I miss anything right here.

I think that install demo configuration script is going away at some point, so I am hesitate to introduce more changes in there or just fix with the closing #3858.

Tho even then, #3858 is putting lines in the wrong location where if security plugin is not present, the building of the pkgs will fail.

I am ok to re-open #3858 and fix it through the spec file/post script tho.

Let me know,

Thanks.

[etgraylog]

Tho even then, #3858 is putting lines in the wrong location where if security plugin is not present, the building of the pkgs will fail.

I am ok to re-open #3858 and fix it through the spec file/post script tho.

The repository that the PR was created from has already been deleted, so I don't think the PR can be re-opened.

Moving the lines proposed in #3858 for opensearch-security to the final selection constructs containing 2 echo commands like this for example would avoid build failure if security plugin is not present. If that is acceptable, I will prepare a PR from it.

Of course we can also consider an entirely different approach that would change chmod -Rf a+rX,u+w,g-w,o-w ${buildroot}/*.

What do you think @mnin @peterzhuamazon ?

[smortex]

The tar archive permissions do not allow any permissions for "others", so we can stick to that, this seems okay.

[peterzhuamazon]

I think I remembered why I add the line:

I skipped %prep and %build due to OpenSearch does not require these stages, and run all the things in %install.

As a result, in order to make sure the permission is consistent, I manually copied this from the output into install + /bin/chmod -Rf a+rX,g-w,o-w, which is part of the steps to go through before running %build by rpmbuild I believe.

[artificial-intelligence]

as already said in the linked issue opened by me, I was not aware that there already was an open issue about this.

I still think this setting in gradle also might play a role in the insecure permissions?

But I'm not sure, as I'm not really familiar with the opensearch build process.

Knowing a thing or two about deb/rpm packaging I think it's safe to say that permission setting should be part of the package spec, e.g. post sections in the package.

Thanks for working on this and thanks to @peterzhuamazon for making me aware of this issue.

[peterzhuamazon]

as already said in the linked issue opened by me, I was not aware that there already was an open issue about this.

I still think this setting in gradle also might play a role in the insecure permissions?

But I'm not sure, as I'm not really familiar with the opensearch build process.

Knowing a thing or two about deb/rpm packaging I think it's safe to say that permission setting should be part of the package spec, e.g. post sections in the package.

Thanks for working on this and thanks to @peterzhuamazon for making me aware of this issue.

Hi @artificial-intelligence ,

When we release deb/rpm we extract all the files from the deb/rpm min pkg from opensearch core repo, install plugins, then repackage with either debmake or rpmbuild, and both of them is running chmod -Rf a+rX,u+w,g-w,o-w ${buildroot}/* so the initial perm has already been reset.

So the solution right now is one of these:

• Tweak the initial chmod -Rf a+rX,u+w,g-w,o-w ${buildroot}/* (either change or remove this line but I am hesitate on that)
• Add new perm settings like in the closed Issue 3815 #3858
• Tweak the security repository demo install script to include perm settings

Want to get thoughts from you guys on which to approach here, @mnin @smortex @etgraylog @artificial-intelligence .

Thanks.

[peterzhuamazon]

Have some offline talk with @smortex and he is currently looking into this.
Will sync up with him to try get a fix if possible.

Thanks.

[smortex]

Hey everyone!

With @peterzhuamazon help I could finally build .deb and .rpm packages on my work machine and could iterate to have packages that better fit my expectations. I opened #3898 with these fixes and invite you to give it a try.

For information, I built test packages (deb and rpm) using the following on Debian 11 (these are "minimal" packages with just OpenSearch core and the security plugin but nothing more):

Building a deb:

PATH="$PATH:/sbin" JAVA_HOME=/usr/lib/jvm/java-17-openjdk-amd64 ./build.sh manifests/2.9.0/opensearch-2.9.0.yml --component OpenSearch security -d deb -p linux -a x64
./assemble.sh deb/builds/opensearch/manifest.yml
ls opensearch-2.9.0-linux-x64.deb

Building a rpm:

PATH="$PATH:/sbin" JAVA_HOME=/usr/lib/jvm/java-17-openjdk-amd64 ./build.sh manifests/2.9.0/opensearch-2.9.0.yml --component OpenSearch security -d rpm -p linux -a x64
docker run -it --rm -v $PWD:$PWD -w $PWD opensearchstaging/ci-runner:ci-runner-rockylinux8-opensearch-build-v4 ./assemble.sh rpm/builds/opensearch/manifest.yml
ls opensearch-2.9.0-linux-x64.rpm

Thank you!

[smortex]

I still think this setting in gradle also might play a role in the insecure permissions?

Lol, I was wondering why the SGID bit was set on the /etc/opensearch directory on REHL but this seems to be the culprit. I saw no reason for it so removed it when building the rpm (the debian packaging seems to strip it automatically).

This code was added in opensearch-project/OpenSearch@2cf7a80 but the issue referenced in the commit message does not exist. I don't see the reason for this SGID bit and think we can remove it, but maybe someone has an idea about why this was introduced and we should keep it?

[peterzhuamazon]

I still think this setting in gradle also might play a role in the insecure permissions?

Lol, I was wondering why the SGID bit was set on the /etc/opensearch directory on REHL but this seems to be the culprit. I saw no reason for it so removed it when building the rpm (the debian packaging seems to strip it automatically).

This code was added in opensearch-project/OpenSearch@2cf7a80 but the issue referenced in the commit message does not exist. I don't see the reason for this SGID bit and think we can remove it, but maybe someone has an idea about why this was introduced and we should keep it?

Sync @nknize and @reta in this conversation about the bit, thanks.

[etgraylog]

The PR #3898 I think is the most comprehensive & sensible solution. Kudos @smortex !

[peterzhuamazon]

New changes:

There are some concerns on this change go into 2.10.0 as follows:

1. The change might not be backward compatible. Since permissions were pretty open before the customer and their automation might have the required permissions to deploy those with right user. We are unaware what might break for end user as well as testing side of things.
2. With current timeline it might be difficult to get documentation level changes in explaining why and whats.
3. opensearch-build repository lacks branching. We have 1.3.13 going on in parallel to 2.10.0. 1.x series is in maintenance mode so these enhancements should not go in as a part of 1.3.13

We have a offline discussion with @smortex and we agree on the next steps:

1. Reset the permission back to original for this release.
2. We will talk about the exact version to introduce this back in a community meeting.
3. After the version is decided we will add a notice on the next upcoming release notes for upcoming breaking change of this.
4. We open a new PR with these permission changes which target the version we agree on.

Thanks.

cc: @smortex @mnin @etgraylog @artificial-intelligence @bbarani @gaiksaya @peterzhuamazon

---

• Reset deb rpm permissions to original state #4041
• [Resume] Improve ownership and permissions of files in OpenSearch/Dashboards deb and rpm packages #4043
• Announce upcoming breaking changes in packaging #4051

[peterzhuamazon]

We have presented the above change in community meeting today (2023/10/17).
And we are currently looking to push this change by 2.13.0 release.
Please let us know if you have any comments on this. Thanks.

cc: @smortex @mnin @etgraylog @artificial-intelligence @bbarani @krisfreedain

[peterzhuamazon]

• [Resume] Improve ownership and permissions of files in OpenSearch/Dashboards deb and rpm packages #4043
• Support new deb/rpm permissions changes in #4043 on integTest #4534
• Allow 1.x deb/rpm to use the old build templates #4536
• Fix the incorrect location of pkg build_templates #4539
• Update 2.13.0 deb/rpm package permission change notice #4546
• Allow current user to join os/osd group during deb/rpm test  #4548
• Add current user to adm group to access logs #4553
• Cleanup deb/rpm integTest uninstallation properly #4557