Re: [devel] [PATCH hasher-priv v1 1/3] Make a daemon from the hasher-priv

[Arseny Maslennikov <ar@cs.msu.ru>]

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

On Thu, Oct 01, 2020 at 09:43:04PM +0200, Alexey Gladkov wrote:
> On Thu, Sep 17, 2020 at 04:10:13PM +0300, Arseny Maslennikov wrote:
> > On Fri, Dec 13, 2019 at 12:42:03PM +0100, Alex Gladkov wrote:
> > > From: Alexey Gladkov <legion@altlinux.org>
> > > 
> > > All privileged operations moved to the daemon. Commands to the server
> > > are sent through the unix domain socket. The credentials which the sender
> > > specifies are checked by the kernel. The hasher-priv no longer SUID.
> > 
> > I'm going to suggest some English literacy/typo improvements in a separate
> > email.
> > 
> > > 
> > > For each user server creates a separate session process that executes
> > > commands only from the user who created it. The session process ends
> > 
> > Since that new process will still be privileged, why the additional fork
> > and the new listening socket inode? Is the strive for less complex
> > daemon source code the only driver for that, albeit a perfectly viable
> > one?
> 
> We have privileged server. When a user request comes in a session daemon
> is forked for him that opens a socket for such user. A separate process
> is created for each job. If the session daemon is inactive, then after a
> session_timeout it will terminate.

Yes, this is well explained already.

> This is done to isolate one user from
> another. You cannot DoS the main server.

So you mean resource exhaustion. What kind of resource?
CPU time? A malicious client can uselessly connect to the main daemon,
send IPC commands, get kicked, repeat.

Open FDs come to mind. It makes sense for the main daemon to close the
client connection after the serving "session" daemon spawn and the
response; the main daemon probably already does, it should be obvious
from the patch.

> 
> > > after a certain period of inactivity.
> > 
> > No problem with that, but IMO we still should be careful
> > about resource leaks.
> 
> The admin controls the number of users and hence the number of possible
> session daemons.
> 
> > > 
> > 
> > 
> > 
> > > Signed-off-by: Alexey Gladkov <legion@altlinux.org>
> > > ---
> > >  hasher-priv/.gitignore      |   1 +
> > >  hasher-priv/DESIGN          | 281 ++++++++++++++++----------
> > 
> > I'd like to see a formal description of the client-server protocol in e.
> > g. hasher-priv/IPC, that shows the intended message exchanges and
> > meanings. The message contents can be rather easily inferred from the C
> > headers, but the semantics cannot. This ends up as a source of
> > programming errors, see the other emails.
> > 
> > The relevant information is probably already present in the DESIGN file,
> > but, as this is documentation for humans, we should wrap it into a form
> > that's easier to comprehend without wasting much time.
> > 
> > I'm also worried about the message format using plain C ABI structs.
> > We're not going to use it over the network on machines with different
> > architectures, and we're never going to support a client and server from
> > different package builds, sure. But is this enough of a justification?
> 
> Don't be fooled that a socket is being used. The hasher-provd will not be
> able to work over the network. The file descriptors are passed over
> the socket and privileges are checked.

Yes, this was already well explained.
Quote:
>> We're not going to use it over the network on machines with different
>> architectures, and we're never going to support a client and server from
>> different package builds, sure.

> <...>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]