Subject: [bug#47849] [PATCH 1/1] services: Add a service for the Jami daemon.

From: Maxim Cournoyer <[email-masked]>
Date: Mon, 19 Apr 2021 11:42:13 -0400

Hi Maxime,

Maxime Devos <[email-masked]> writes:

> Maxim Cournoyer schreef op ma 19-04-2021 om 08:07 [-0400]:
>> Hi Maxime!
>> 
>> Maxime Devos <[email-masked]> writes:
>> 
>> > Maxim Cournoyer schreef op za 17-04-2021 om 16:06 [-0400]:
>> > +                      [...]
>> > +                       (delete-file-recursively "/var/lib/jami/accounts"))
>> > 
>> > You might want to verify whether /var/lib/jami/{.cache,.config,.local/share,.local}
>> > aren't symbolic links.  That way, if the Jami daemon is compromised (due to buffer
>> > overflow --> arbitrary code execution or something), the attacker can't trick the
>> > shepherd service into deleting arbitrary directories.
>> 
>> It would only be able to delete directories that are world writable
>> though, right?  Seems the opportunity to cause damage is limited, but
>> it's a simple check to add, so I'll do it.
>
> Let's step through the relevant code of the shepherd service.
>
> (shepherd-service
>   (documentation "Run the Jami daemon.")
>   [blabla]
>   (start
>     #~(lambda args
>         [other stuff]
>         (when [blabla, and a 'catch' form]
>           (delete-file-recursively "/var/lib/jami/.cache/jami")
>           [etcetera])
>          (let* (([blabla])
>                 (user (passwd:uid [blabla jami user]))
>                 (group [likewise]))
>            [blabla] (chown accounts-dir user group)))
>         ;; Start the daemon
>         (define daemon-pid
>           (fork+exec-command [blabla Jami cmdline arguments]
>             #:user "jami" #:group "jami" [blabla]))
>         [blabla]))
>    (stop [blabla]))         
>
> Remember that the shepherd daemon is run as root (and therefore
> has read-write-execute access to everything).  The 'start' procedure
> (and 'stop' procedure) are run _within_ the shepherd daemon.  Thus, the
> 'start' gexp is run as root.

Ah yes, looking the service definition it's obvious.  Sorry for missing
that earlier :-).

> As the start procedure didn't change the uid/gid from root to something
> else, (delete-file-recursively "/var/lib/jami/.cache/jami") is run as
> root.  IIUC, root user can read/write anything, ignoring things like "user"
> and "group".  World-writability is not required.
>
>> What about if the daemon was
>> run in a container (your suggestion in a following email, to which I
>> agree would be a good thing)?  It would prevent this kind of attack,
>> right?
>
> I don't see how that would help.  It is the _shepherd daemon_ (that runs
> as root) that runs (delete-file-recursively ...), not the attacker (from
> within the compromised jami-daemon process).  Perhaps this is cleared up
> by my previous response?

Indeed!  Thanks for taking the time to make it clear for me!

I'll address this in a v2 patch series, hopefully resolving the issues
with the daemon starting in double the first time the system is
reconfigured (something to do with d-bus autospawning I think -- perhaps
the D-Bus ping method is enough to spawn the process if it was not yet
up and running).

It'll take me some time to get to it; probably after the v1.3.0 is
released.

Thank you!

Maxim