Re: AF_UNIX status report

[Ken Brown <kbrown@cornell.edu>]

On 11/7/2020 5:25 PM, Ken Brown via Cygwin-developers wrote:
> On 11/6/2020 4:12 AM, Corinna Vinschen wrote:
>> On Nov  5 18:41, Ken Brown via Cygwin-developers wrote:
>>> On 11/5/2020 12:21 PM, Corinna Vinschen wrote:
>>>> On Nov  5 09:23, Ken Brown via Cygwin-developers wrote:
>>>>> OK, here's how I imagine this working:
>>>>>
>>>>> A process wants to send a file descriptor fd, so it creates a msghdr with an
>>>>> SCM_RIGHTS cmsghdr and calls sendmsg.  The latter creates and sends an admin
>>>>> packet A containing the fhandler for fd, and then it sends the original
>>>>> packet P.
>>>>>
>>>>> At the receiving end, recvmsg sees packet A first (recvmsg is always
>>>>> checking for admin packets anyway whenever it's called).  It stores the
>>>>> fhandler somewhere.  When it then reads packet P, it retrieves the stored
>>>>> fhandler, fiddles with it (duplicating handles, etc.), and creates the new
>>>>> file descriptor.
>>>>
>>>> Actually, this needs to be implemented in a source/dest-independent
>>>> manner.  Only the server of the named pipe can impersonate the client.
>>>> So the server side should do the job of duplicating the handles.  If the
>>>> sever is also the source of SCM_RIGHTS, it should send the fhandler with
>>>> already duplicated handles.
>>>
>>> The only example of pipe client impersonation I can find in the Cygwin code
>>> is in fhandler_pty_master::pty_master_thread.  Is this a good model to
>>> follow?  If not, can you point me to other examples somewhere?
>>>
>>> AFAICT, the only reason for the impersonation is to check that the client
>>> has appropriate permissions before trying to duplicate handles from the
>>> server process to the client process.  Is that right?  What would go wrong
>>> if we didn't check this?  Is the issue that the client process would have
>>> handles that it can't access?
>>
>> Maybe I'm overthinking this.  A typical scenario for SCM_RIGHTS
>> involves a privileged and an unprivileged process.  The privileged
>> process sends an fd to the unprivileged process.  In this case the
>> sending process has admin rights anyway and can duplicate the handles
>> into the receiving process without having to impersonate.
>>
>> Either way, if both processes are running under the same user, or at
>> least one of the processes has admin rights, no impersonation is
>> required.  But since we don't know if the admin process is the sender or
>> the receiver, both sides must be capable of duplicating the handles.
>>
>> So, only if both processes are unprivileged, we would need to
>> impersonate.  This will almost always fail, unless both processes have
>> been started from (for instance) the same ssh session or one of the user
>> accounts has the SeImpersonatePrivilege privilege.
>>
>> Maybe we should just skip the latter scenario for a start.
> 
> Good!  That way I don't have to get sidetracked learning about impersonation.
> 
> Here's another issue involving serialization.  I'm not sure it's enough to just 
> fiddle with the handles and then send the fhandler.  We also need to send the 
> strings that are in the path_conv member of the fhandler.  [I just noticed that 
> you added path_conv serialization/deserialization recently, which should help 
> with this.]  This increases the size of the data to the point where I think we 
> need to send more than one packet when we're sending SCM_RIGHTS.
> 
> Alternatively, instead of trying to send the fhandler and string(s) over the 
> socket, we could store a copy of the fhandler, along with the serialized pc, in 
> a named shared memory block.  The name could be something like 
> "scm_rights.<installation_key>.<device>.<inode>".  Then the sender would only 
> have to send the device and inode, and the receiver could open the shared memory 
> and reconstruct everything.

Apart from this annoying issue of packets being too long, here's how I imagine 
serialization/deserialization working.  All the code below is untested and 
probably full of errors, but I hope it gives the idea.  Note: get_size() below 
is the virtual method that I called size() in an earlier message.

struct fh_flat
{
   fhandler_union fhu;
   size_t name_len;
   char data[0];
};

/* Prerequisites: Modify fhandler_*::dup to take an optional winpid
    argument specifying the process into which we want to duplicate
    handles.  Similarly modify dtable::dup_worker.  */
void *
serialize (int fd, DWORD winpid, size_t &len)
{
   fh_flat *fhf;
   size_t nlen = 0;
   cygheap_fdget cfd (fd);

   if (cfd < 0)
     {
       set_errno (EBADF);
       len = 0;
       return NULL;
     }
   /* Make a copy of the fhandler with handles duplicated for winpid. */
   fhandler_base *copy = cygheap->fdtab->dup_worker (cfd, 0, winpid);
   if (copy->get_name ())
     nlen = strlen (copy->get_name ()) + 1;
   len = sizeof (fh_flat) + nlen;
   fhf = (fh_flat *) cmalloc (HEAP_FHANDLER, len);
   if (!fhf)
     {
       set_errno (ENOMEM);
       len = 0;
       return NULL;
     }
   memcpy (&fhf->fhu, copy, copy->get_size ());
   fhf->name_len = nlen;
   if (nlen)
     strcpy (fhf->data, copy->get_name ());
   return fhf;
}

/* Return new fd, or -1 on error. */
int
deserialize (void *bufp)
{
   fh_flat *fhf = (fh_flat *) bufp;
   cygheap_fdnew cfd;

   if (cfd < 0)
     return -1;
   /* Create an fhandler of the right derived class. */
   device dev = ((fhandler_base) fhf->fhu).dev ();
   fhandler_base *fh = build_fh_dev (dev);
   if (!fh)
     {
       debug_printf ("build_fh_dev failed");
       return -1;
     }
   memcpy (fh, &fhf->fhu, fh->get_size ());
   fh->set_name (fhf->data);
   cfd = fh;
   return cfd;
}

Does this approach look OK?

Ken