From mboxrd@z Thu Jan  1 00:00:00 1970
From: Michael Tremer <michael.tremer@ipfire.org>
To: development@lists.ipfire.org
Subject: Re: [PATCH 2/2] run Tor under dedicated user
Date: Thu, 14 Mar 2019 15:04:16 +0000
Message-ID: <2273FCAB-AAC1-4ABC-B851-FD34460F2269@ipfire.org>
In-Reply-To: <58a2b0d7-7d03-a3d2-fdcb-45809c098cbd@ipfire.org>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============3642424571867693895=="
List-Id: <development.lists.ipfire.org>

--===============3642424571867693895==
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

Hi,

> On 14 Mar 2019, at 14:58, Peter M=C3=BCller <peter.mueller(a)ipfire.org> wr=
ote:
>=20
> Hello Michael,
>=20
>> Hi,
>>=20
>> There is a problem in the script:
> :-(
>>=20
>>> On 11 Mar 2019, at 20:07, Peter M=C3=BCller <peter.mueller(a)ipfire.org> =
wrote:
>>>=20
>>> This allows more-fine granular firewall rules (see first patch for
>>> further information). Further, it prevents other services running as
>>> "nobody" (Apache, ...) from reading Tor relay keys.
>>>=20
>>> Fixes #11779.
>>>=20
>>> Signed-off-by: Peter M=C3=BCller <peter.mueller(a)ipfire.org>
>>> ---
>>> lfs/tor                 |  6 +++---
>>> src/paks/tor/install.sh | 15 ++++++++++++++-
>>> 2 files changed, 17 insertions(+), 4 deletions(-)
>>>=20
>>> diff --git a/lfs/tor b/lfs/tor
>>> index 384b1b213..2b0e0903a 100644
>>> --- a/lfs/tor
>>> +++ b/lfs/tor
>>> @@ -32,7 +32,7 @@ DL_FROM    =3D $(URL_IPFIRE)
>>> DIR_APP    =3D $(DIR_SRC)/$(THISAPP)
>>> TARGET     =3D $(DIR_INFO)/$(THISAPP)
>>> PROG       =3D tor
>>> -PAK_VER    =3D 34
>>> +PAK_VER    =3D 35
>>>=20
>>> DEPS       =3D ""
>>>=20
>>> @@ -82,8 +82,8 @@ $(TARGET) : $(patsubst %,$(DIR_DL)/%,$(objects))
>>> 			--prefix=3D/usr \
>>> 			--sysconfdir=3D/etc \
>>> 			--localstatedir=3D/var \
>>> -			--with-tor-user=3Dnobody \
>>> -			--with-tor-group=3Dnobody
>>> +			--with-tor-user=3Dtor \
>>> +			--with-tor-group=3Dtor
>>>=20
>>> 	cd $(DIR_APP) && make $(MAKETUNING)
>>> 	cd $(DIR_APP) && make install
>>> diff --git a/src/paks/tor/install.sh b/src/paks/tor/install.sh
>>> index 31c5fecae..e1ed33331 100644
>>> --- a/src/paks/tor/install.sh
>>> +++ b/src/paks/tor/install.sh
>>> @@ -17,11 +17,24 @@
>>> # along with IPFire; if not, write to the Free Software                  =
  #
>>> # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307 US=
A #
>>> #                                                                        =
  #
>>> -# Copyright (C) 2007 IPFire-Team <info(a)ipfire.org>.                   =
     #
>>> +# Copyright (C) 2007-2019 IPFire-Team <info(a)ipfire.org>.              =
     #
>>> #                                                                        =
  #
>>> #########################################################################=
###
>>> #
>>> . /opt/pakfire/lib/functions.sh
>>> +
>>> +# Run Tor as dedicated user and make sure user and group exist
>>> +if ! getent group tor &>/dev/null; then
>>> +       groupadd -g 119 tor
>>> +fi
>>> +
>>> +if ! getent passwd tor; then
>>> +       useradd -u 119 -g tor -d /var/empty -s /bin/false tor
>>> +
>>> +       # Adjust some folder permission for new UID/GID
>>> +       chown -R tor:tor /var/lib/tor /var/ipfire/tor
>>=20
>> You are only changing these directories when the user is being created.
> Yes, this is intentional.
>>=20
>> If the add-on is uninstalled and later installed again the files will have=
 the wrong owner because they are created as somebody else in the build proce=
ss.
>>=20
>> So the chown line should be in the build process. The user should also be =
put into /etc/passwd and /etc/group so that it is always present on all syste=
ms as well as during the build process to assign correct ownership of the tho=
se directories.
> I tried to run the chown command during the build process, but it failed,
> as the user Tor was unavailable at build time.
>=20
> As I saw the patches were merged for Core Update 130, I will add some addit=
ional
> patches for adding the Tor user during build time. Do you think manually ad=
ding
> the user via src/paks/tor/install.sh will be still necessary then?

Silly me has merged the patch and forgotten about the ownership issue :)

I suppose moving the chown command after the if clause would suffice.

>=20
> Thanks for any hints.
>=20
> Best regards,
> Peter M=C3=BCller
> --=20
> The road to Hades is easy to travel.
> 	-- Bion of Borysthenes


--===============3642424571867693895==--