MIME-Version: 1.0
References: <22948c2fba2f4882ac4646501fd6ef3f@tower-net.de>
 <CAD+eXGR0gJH5UbrgE=5qvXaNgsRD+17tueooGdSY1NKPLcVrKw@mail.gmail.com> <49d7456930f237457bf7f3f5c50f96e4@tower-net.de>
In-Reply-To: <49d7456930f237457bf7f3f5c50f96e4@tower-net.de>
From: Konstantin Kulikov <k.kulikov2@gmail.com>
Date: Sun, 19 Jun 2022 16:42:35 +0300
Message-ID: <CAD+eXGQuATq8yW=LqSoyj04B=-egzfna_R6+9mOXXiiu+owq_A@mail.gmail.com>
Subject: Re: Security problem in how you manage users in package installations
To: Markus Kolb <alpinelinux+develml@tower-net.de>
Cc: Alpine Linux devel ML <~alpine/devel@lists.alpinelinux.org>
Content-Type: text/plain; charset="UTF-8"

> Some distros create system users only with predefined uid/gid.

There are ~500 *-openrc packages so I guess it can work.

> A first effective improvement would be, not to lower/remove restrictions
> of _any_ existing user in a kind of black box (installation script from
> admin perspective) during package installation.

adduser fails if user exists so I'm not sure what you mean here.

> Same proceeding would be good for file/directory permissions.
> For this there should be also taken care in e.g. checkpath of
> openrc-scripts. If the admin locks out users of a directory and during
> the next service start by a checkpath the directory becomes world
> accessible again, might be a problem. IMO openrc should not modify or
> create anything by checkpath but simply report about the undesired
> mismatch and abort. But there is no such option in checkpath to only
> check. The modification is implied, the name of the function is
> misleading. It is a "fixpath".

I don't agree that admin should be required to manually create
directories with correct permissions.
Maybe it's possible to checkpath with 700 and then give permissions to
others when required using ACLs(setfacl/getfacl)?

> I don't see a requirement for adding users like gitea/gogs to a
> website-content-group www-data.

I believe "someuser/www-data" is usually done to let apache/nginx
handle uploads and static file serving.
In gogs case this definitely shouldn't be done. Feel free to send a MR.

> What is happening below /var/log, /var/run in all the packages over time
> if there is no check what packages are doing?

Logging in alpine is in a shit state because openrc doesn't really
implement logging properly.
There was some talk about replacing it, no idea what state it is currently in.

> Btw. is it intended that you add user to the same group it has as
> primary group, so that it has ineffectively the same group 2 times?

Unlikely. Feel free to send MRs.

> Default installations need to be fixed here, but it is not foreseeable
> what configuration changes has been done on all the installations out
> there. Overwriting this more restrictive and safe can break
> installations using a different, custom configuration and directory
> structure.
> There could be provided an output to the user during the installation
> about the changes. In other distributions there would be a confirmation
> dialog.

apk doesn't do notifications. Postgres did fail installation in the
past when a major version was released,
but thankfully major postgres versions are now separate packages.

> How this should be done in Alpine?

I'd say get rid of www-data, but leave 755 for now.
Move directory creation from package() to initd script with checkpath.
This will let you edit initd on your installation and not fear for it
being overwritten on the next update.
You can get rid of setcap usage as well (see [1]).

Finally bring up your concerns and suggestions with TSC here [2] and
start a formal discussion.

[1] https://gitlab.alpinelinux.org/alpine/tsc/-/issues/45
[2] https://gitlab.alpinelinux.org/alpine/tsc