Re: [PATCH 2/2] run Tor under dedicated user

[Michael Tremer <michael.tremer@ipfire.org>]

[-- Attachment #1: Type: text/plain, Size: 3845 bytes --]

Hi,

> On 14 Mar 2019, at 14:58, Peter Müller <peter.mueller(a)ipfire.org> wrote:
> 
> Hello Michael,
> 
>> Hi,
>> 
>> There is a problem in the script:
> :-(
>> 
>>> On 11 Mar 2019, at 20:07, Peter Müller <peter.mueller(a)ipfire.org> wrote:
>>> 
>>> 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.
>>> 
>>> Fixes #11779.
>>> 
>>> Signed-off-by: Peter Müller <peter.mueller(a)ipfire.org>
>>> ---
>>> lfs/tor                 |  6 +++---
>>> src/paks/tor/install.sh | 15 ++++++++++++++-
>>> 2 files changed, 17 insertions(+), 4 deletions(-)
>>> 
>>> diff --git a/lfs/tor b/lfs/tor
>>> index 384b1b213..2b0e0903a 100644
>>> --- a/lfs/tor
>>> +++ b/lfs/tor
>>> @@ -32,7 +32,7 @@ DL_FROM    = $(URL_IPFIRE)
>>> DIR_APP    = $(DIR_SRC)/$(THISAPP)
>>> TARGET     = $(DIR_INFO)/$(THISAPP)
>>> PROG       = tor
>>> -PAK_VER    = 34
>>> +PAK_VER    = 35
>>> 
>>> DEPS       = ""
>>> 
>>> @@ -82,8 +82,8 @@ $(TARGET) : $(patsubst %,$(DIR_DL)/%,$(objects))
>>> 			--prefix=/usr \
>>> 			--sysconfdir=/etc \
>>> 			--localstatedir=/var \
>>> -			--with-tor-user=nobody \
>>> -			--with-tor-group=nobody
>>> +			--with-tor-user=tor \
>>> +			--with-tor-group=tor
>>> 
>>> 	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 USA #
>>> #                                                                          #
>>> -# 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
>> 
>> You are only changing these directories when the user is being created.
> Yes, this is intentional.
>> 
>> 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 process.
>> 
>> 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 systems as well as during the build process to assign correct ownership of the those directories.
> I tried to run the chown command during the build process, but it failed,
> as the user Tor was unavailable at build time.
> 
> As I saw the patches were merged for Core Update 130, I will add some additional
> patches for adding the Tor user during build time. Do you think manually adding
> 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.

> 
> Thanks for any hints.
> 
> Best regards,
> Peter Müller
> -- 
> The road to Hades is easy to travel.
> 	-- Bion of Borysthenes