[RFC] Simplify async path based VFS functions: remove tevent and pthreadpool_tevent wrappers

[Stefan Metzmacher]

Am 19.12.18 um 14:02 schrieb Ralph Böhme:
> Hi folks,
> 
> as you may or may not know, since a few months Samba has a fancy new
> machinery
> that was added to properly support path based functions like getxattr()
> in the
> Samba VFS.
> 
> The machinery consists of:
> 
> 1. tevent wrapper
> 
> A tevent wrapper associates an existing tevent context with a bunch of
> function
> pointers that get called before and after the tevent event handlers.
> 
> This is useful to hide the complexity of becoming the correct user at
> the right
> time, especially in callbacks from tevent.

Here's a little bit of background why I have chosen that strategy
(actually I started with it in 2012 already).

Until now it was the job of the core code in smbd to take care
of any impersonation for the currently processed request.

We have change_to_user() and set_current_service() in the central
protocol processing in order to support multiple sessions on
an SMB connection.

In order to support oplock related async delays, we use an
infrastructure to put requests from the SMB_VFS layer
onto a deferred queue, while we're waiting for an internal notification
message to arrive.

Once we're notified that there's a change to get any further,
the deferred request is replayed internally, which means we completely
redo the protocol processing and the resulting impersonation
before calling into the SMB_VFS layer again.

This makes it relatively hard for SMB_VFS modules to keep state
of the already started request, while the module don't have to
take care about impersonation.

For file handle based calls like pread, pwrite, fsync, offload_read and
offload_write we already added SMB_VFS_*_SEND/RECV hooks to make use of
our typical tevent_req based async request handling, which makes it easy
to have a per request state. But there's no impersonation enforcement at
all, because the tevent event handlers don't restore the impersonation
of the current request. And we may change back to root or any random
other user attached to a different SMB session on the same connection.

All implementations of SMB_VFS_*_SEND/RECV within upstream Samba
releases (<= 4.9.x) should not need impersonation for these calls as
they all operate on an already existing file descriptor. But I already
saw SMB VFS modules of OEMs, which may rely on impersonation being
correct during the whole lifetime of an async request.

In order to make it possible to add path based SMB_VFS_*_SEND/RECV
we need a solution that makes it very hard for module writers to get the
impersonation of async requests wrong and introduce security problems.

With the existing strategy (that the core smbd handles all
impersonation) in mind I implemented impersonation wrappers
using tevent context wrappers. And the SMB_VFS_*_SEND() function
would then only get the wrapper context and all impersonation is
done for free.

> 2. pthreadpool_tevent wrapper
> 
> pthreadpool_tevent wrapper hides the complexity of impersonating the
> correct
> user and changing the current working directory to the base directory of a
> service from the worker thread.
> 
> 3. VFS glue
> 
> This is a structure and some functions that bring together user and root
> pthreadpool_tevent and tevent wrappers, taking into account system
> support for
> per-thread credentials and per-thread current working directory support.
> 
> VFS implementations can use the VFS glue and the supporting functions
> that act
> on the glue in a boilerplate fashion to implement path based VFS functions,
> including support for funning worker thread job functions as root and in
> the
> correct working directory.

In order to hide more complexity I also added pthreadpool_tevent wrapper
and the SMB_VFS glue. These are very complex inside, but make it
relatively easy for SMB_VFS modules.

> The longterm plan is to pass an additional argument to all syscall
> wrapping VFS
> functions with the required internal information to perform
> impersonation and
> chdir(), eg an "impersonation_context".
> 
> This, together with some helper functions that work on the object and
> can be
> used the implementation of VFS functions, should result in reasonable
> complexity
> both in the implementation and for the user of the new API.

We're moving more and more to file handle based functions,
which is a result of the clean SMB2/3 protocol (and we try to
get rid of SMB1). At the same time we also try to move syscalls
to helper threads.

This means doing the impersonation in the main smbd process/thread
will not be needed most of the time.

So we thought that removing the complexity of the tevent/pthread_tevent
wrappers would make it easier to understand the core smbd code and
make it more explicit where impersonation might be required.

While we'll pass some kind of explicit impersonation context through
the SMB_VFS layers, we'll also provide helper functions, which make
it easier for module writers to trigger the impersonation, by just
writing little syscall wrappers like this:

static void vfswrap_getxattrat_do(void *private_data)
{
        struct vfswrap_getxattrat_state *state = talloc_get_type_abort(
                private_data, struct vfswrap_getxattrat_state);
        int ret;

        /*
         * Here we simulate a getxattrat()
         * call using fchdir();getxattr()
         */

        ret = fchdir(state->dirfd);
        if (ret == -1) {
                state->xattr_size = -1;
                state->vfs_aio_state.error = errno;
                goto end_profile;
        }

        state->xattr_size = getxattr(state->name,
                                     state->xattr_name,
                                     state->xattr_value,
                                     state->xattr_bufsize);
}

Helper functions will execute such functions in the desired
impersonation state. Depending on the available OS features
and configuration this can run in the main thread or within
a helper thread.

Pure user space file system may skip impersonation completely.

I think this is a good long term goal, but it will take some
time and might not be completed until we remove support for
SMB1.

metze


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20181219/9d086e0d/signature.sig>