AF_UNIX status report

AF_UNIX status report

[Ken Brown]

I've made at least rudimentary implementations of all the fhandler_socket_unix 
functions (including those in select.cc) for which there were previously only 
placeholders.

I've pushed everything to topic/af_unix, including a merge with master as of a 
couple days ago.

I've cobbled together a few test programs and put them in 
winsup/cygwin/socket_tests on the topic/af_unix branch.  I haven't taken the 
time to automate the tests, so they all have to be run interactively.  There is 
a Makefile to build the test programs and a README.txt that shows how to run them.

One thing I haven't yet done is to think about (or systematically test) datagram 
sockets.  I'm sure there's quite a bit of code that won't work for them.

Aside from datagram sockets, there are still a few things that I'm working on, 
but I'm close to the point where I could use some input:

1. I've littered the code in fhandler_socket_unix.cc and select.cc with FIXME 
comments on which I'd like advice.

2. I haven't given any thought at all as to how to implement SCM_RIGHTS 
ancillary data.  I could definitely use suggestions on that before I start 
thrashing around.

Ken

Re: AF_UNIX status report

[Corinna Vinschen]

On Oct 26 18:04, Ken Brown via Cygwin-developers wrote:
> I've made at least rudimentary implementations of all the
> fhandler_socket_unix functions (including those in select.cc) for which
> there were previously only placeholders.
> 
> I've pushed everything to topic/af_unix, including a merge with master as of
> a couple days ago.
> 
> I've cobbled together a few test programs and put them in
> winsup/cygwin/socket_tests on the topic/af_unix branch.  I haven't taken the
> time to automate the tests, so they all have to be run interactively.  There
> is a Makefile to build the test programs and a README.txt that shows how to
> run them.
> 
> One thing I haven't yet done is to think about (or systematically test)
> datagram sockets.  I'm sure there's quite a bit of code that won't work for
> them.
> 
> Aside from datagram sockets, there are still a few things that I'm working
> on, but I'm close to the point where I could use some input:
> 
> 1. I've littered the code in fhandler_socket_unix.cc and select.cc with
> FIXME comments on which I'd like advice.

I'll look into it.

> 2. I haven't given any thought at all as to how to implement SCM_RIGHTS
> ancillary data.  I could definitely use suggestions on that before I start
> thrashing around.

I have only vague ideas at that point.  Assuming we can replace the
socket implemantation with the pipe implementation, what we have is a
pipe which can impersonate the peer at least from the server side, and
it knows the client process.  This in turn can be used to duplicate
handles.  So what we could do is to define fhandler methods which create
a matching serialization  and deserialization of the fhandler data, plus
duplicating the handles for the other process, sent over the pipe as
admin package.  This must work in either direction, regardless if the
server or the client sends the SCM_RIGHTS block.


Corinna

Re: AF_UNIX status report

[Ken Brown]

On 10/27/2020 5:43 AM, Corinna Vinschen wrote:
> On Oct 26 18:04, Ken Brown via Cygwin-developers wrote:
>> I've made at least rudimentary implementations of all the
>> fhandler_socket_unix functions (including those in select.cc) for which
>> there were previously only placeholders.
>>
>> I've pushed everything to topic/af_unix, including a merge with master as of
>> a couple days ago.
>>
>> I've cobbled together a few test programs and put them in
>> winsup/cygwin/socket_tests on the topic/af_unix branch.  I haven't taken the
>> time to automate the tests, so they all have to be run interactively.  There
>> is a Makefile to build the test programs and a README.txt that shows how to
>> run them.
>>
>> One thing I haven't yet done is to think about (or systematically test)
>> datagram sockets.  I'm sure there's quite a bit of code that won't work for
>> them.
>>
>> Aside from datagram sockets, there are still a few things that I'm working
>> on, but I'm close to the point where I could use some input:
>>
>> 1. I've littered the code in fhandler_socket_unix.cc and select.cc with
>> FIXME comments on which I'd like advice.
> 
> I'll look into it.
> 
>> 2. I haven't given any thought at all as to how to implement SCM_RIGHTS
>> ancillary data.  I could definitely use suggestions on that before I start
>> thrashing around.
> 
> I have only vague ideas at that point.  Assuming we can replace the
> socket implemantation with the pipe implementation, what we have is a
> pipe which can impersonate the peer at least from the server side, and
> it knows the client process.  This in turn can be used to duplicate
> handles.  So what we could do is to define fhandler methods which create
> a matching serialization  and deserialization of the fhandler data, plus
> duplicating the handles for the other process, sent over the pipe as
> admin package.  This must work in either direction, regardless if the
> server or the client sends the SCM_RIGHTS block.

This sounds reasonable.

I have no experience with serialization.  Do you happen to know of a good 
example that I could look at?

Thanks.

Ken

Re: AF_UNIX status report

[Joe Lowe]

On 2020-10-29 13:19, Ken Brown via Cygwin-developers wrote:
> On 10/27/2020 5:43 AM, Corinna Vinschen wrote:
>> On Oct 26 18:04, Ken Brown via Cygwin-developers wrote:
>>> I've made at least rudimentary implementations of all the
>>> fhandler_socket_unix functions (including those in select.cc) for which
>>> there were previously only placeholders.
>>>
>>> I've pushed everything to topic/af_unix, including a merge with 
>>> master as of
>>> a couple days ago.
>>>
>>> I've cobbled together a few test programs and put them in
>>> winsup/cygwin/socket_tests on the topic/af_unix branch.  I haven't 
>>> taken the
>>> time to automate the tests, so they all have to be run 
>>> interactively.  There
>>> is a Makefile to build the test programs and a README.txt that shows 
>>> how to
>>> run them.
>>>
>>> One thing I haven't yet done is to think about (or systematically test)
>>> datagram sockets.  I'm sure there's quite a bit of code that won't 
>>> work for
>>> them.
>>>
>>> Aside from datagram sockets, there are still a few things that I'm 
>>> working
>>> on, but I'm close to the point where I could use some input:
>>>
>>> 1. I've littered the code in fhandler_socket_unix.cc and select.cc with
>>> FIXME comments on which I'd like advice.
>>
>> I'll look into it.
>>
>>> 2. I haven't given any thought at all as to how to implement SCM_RIGHTS
>>> ancillary data.  I could definitely use suggestions on that before I 
>>> start
>>> thrashing around.
>>
>> I have only vague ideas at that point.  Assuming we can replace the
>> socket implemantation with the pipe implementation, what we have is a
>> pipe which can impersonate the peer at least from the server side, and
>> it knows the client process.  This in turn can be used to duplicate
>> handles.  So what we could do is to define fhandler methods which create
>> a matching serialization  and deserialization of the fhandler data, plus
>> duplicating the handles for the other process, sent over the pipe as
>> admin package.  This must work in either direction, regardless if the
>> server or the client sends the SCM_RIGHTS block.
> 
> This sounds reasonable.
> 
> I have no experience with serialization.  Do you happen to know of a 
> good example that I could look at?
> 
> Thanks.
> 
> Ken

I have experience building a secure implementation of SCM_RIGHTS type 
functionality over named pipe on Windows. This is not a small amount of 
code if you want to handle processes running as different users or 
privilege levels, and if you don't want to be a source of security 
vulnerabilities.

You might consider building an implementation of SCM_RIGHTS that is only 
expected to work for processes running as the same user and privilege 
level. At least this would be a good starting point. This would cover 
the requirements of some unix code bases that use SCM_RIGHTS , and 
avoids significant security issues and complexity.

Joe

Re: AF_UNIX status report

[Corinna Vinschen]

On Oct 29 14:53, Joe Lowe wrote:
> On 2020-10-29 13:19, Ken Brown via Cygwin-developers wrote:
> > On 10/27/2020 5:43 AM, Corinna Vinschen wrote:
> > > On Oct 26 18:04, Ken Brown via Cygwin-developers wrote:
> > > > I've made at least rudimentary implementations of all the
> > > > fhandler_socket_unix functions (including those in select.cc) for which
> > > > there were previously only placeholders.
> > > > 
> > > > I've pushed everything to topic/af_unix, including a merge with
> > > > master as of
> > > > a couple days ago.
> > > > 
> > > > I've cobbled together a few test programs and put them in
> > > > winsup/cygwin/socket_tests on the topic/af_unix branch.  I
> > > > haven't taken the
> > > > time to automate the tests, so they all have to be run
> > > > interactively.  There
> > > > is a Makefile to build the test programs and a README.txt that
> > > > shows how to
> > > > run them.
> > > > 
> > > > One thing I haven't yet done is to think about (or systematically test)
> > > > datagram sockets.  I'm sure there's quite a bit of code that
> > > > won't work for
> > > > them.
> > > > 
> > > > Aside from datagram sockets, there are still a few things that
> > > > I'm working
> > > > on, but I'm close to the point where I could use some input:
> > > > 
> > > > 1. I've littered the code in fhandler_socket_unix.cc and select.cc with
> > > > FIXME comments on which I'd like advice.
> > > 
> > > I'll look into it.
> > > 
> > > > 2. I haven't given any thought at all as to how to implement SCM_RIGHTS
> > > > ancillary data.  I could definitely use suggestions on that
> > > > before I start
> > > > thrashing around.
> > > 
> > > I have only vague ideas at that point.  Assuming we can replace the
> > > socket implemantation with the pipe implementation, what we have is a
> > > pipe which can impersonate the peer at least from the server side, and
> > > it knows the client process.  This in turn can be used to duplicate
> > > handles.  So what we could do is to define fhandler methods which create
> > > a matching serialization  and deserialization of the fhandler data, plus
> > > duplicating the handles for the other process, sent over the pipe as
> > > admin package.  This must work in either direction, regardless if the
> > > server or the client sends the SCM_RIGHTS block.
> > 
> > This sounds reasonable.
> > 
> > I have no experience with serialization.  Do you happen to know of a
> > good example that I could look at?

Unfortunately not.  Probably we can just send the entire fhandler and
the recipient fiddles the content in a per-class way, kind of like
fhandler::dup.

> I have experience building a secure implementation of SCM_RIGHTS type
> functionality over named pipe on Windows. This is not a small amount of code
> if you want to handle processes running as different users or privilege
> levels, and if you don't want to be a source of security vulnerabilities.

You're not interested to help coding this in Cygwin, by any chance?

> You might consider building an implementation of SCM_RIGHTS that is only
> expected to work for processes running as the same user and privilege level.
> At least this would be a good starting point. This would cover the
> requirements of some unix code bases that use SCM_RIGHTS , and avoids
> significant security issues and complexity.

This may be a good start, actually.  I'd love to get full privsep
working in OpenSSH, but that's not a key issue, given upstream
supports the preauth-only implementation as well, and this works
without SCM_RIGHTS.


Corinna

Re: AF_UNIX status report

[Ken Brown]

On 10/30/2020 5:20 AM, Corinna Vinschen wrote:
> On Oct 29 14:53, Joe Lowe wrote:
>> On 2020-10-29 13:19, Ken Brown via Cygwin-developers wrote:
>>> On 10/27/2020 5:43 AM, Corinna Vinschen wrote:
>>>> On Oct 26 18:04, Ken Brown via Cygwin-developers wrote:
>>>>> I've made at least rudimentary implementations of all the
>>>>> fhandler_socket_unix functions (including those in select.cc) for which
>>>>> there were previously only placeholders.
>>>>>
>>>>> I've pushed everything to topic/af_unix, including a merge with
>>>>> master as of
>>>>> a couple days ago.
>>>>>
>>>>> I've cobbled together a few test programs and put them in
>>>>> winsup/cygwin/socket_tests on the topic/af_unix branch.  I
>>>>> haven't taken the
>>>>> time to automate the tests, so they all have to be run
>>>>> interactively.  There
>>>>> is a Makefile to build the test programs and a README.txt that
>>>>> shows how to
>>>>> run them.
>>>>>
>>>>> One thing I haven't yet done is to think about (or systematically test)
>>>>> datagram sockets.  I'm sure there's quite a bit of code that
>>>>> won't work for
>>>>> them.
>>>>>
>>>>> Aside from datagram sockets, there are still a few things that
>>>>> I'm working
>>>>> on, but I'm close to the point where I could use some input:
>>>>>
>>>>> 1. I've littered the code in fhandler_socket_unix.cc and select.cc with
>>>>> FIXME comments on which I'd like advice.
>>>>
>>>> I'll look into it.
>>>>
>>>>> 2. I haven't given any thought at all as to how to implement SCM_RIGHTS
>>>>> ancillary data.  I could definitely use suggestions on that
>>>>> before I start
>>>>> thrashing around.
>>>>
>>>> I have only vague ideas at that point.  Assuming we can replace the
>>>> socket implemantation with the pipe implementation, what we have is a
>>>> pipe which can impersonate the peer at least from the server side, and
>>>> it knows the client process.  This in turn can be used to duplicate
>>>> handles.  So what we could do is to define fhandler methods which create
>>>> a matching serialization  and deserialization of the fhandler data, plus
>>>> duplicating the handles for the other process, sent over the pipe as
>>>> admin package.  This must work in either direction, regardless if the
>>>> server or the client sends the SCM_RIGHTS block.
>>>
>>> This sounds reasonable.
>>>
>>> I have no experience with serialization.  Do you happen to know of a
>>> good example that I could look at?
> 
> Unfortunately not.  Probably we can just send the entire fhandler and
> the recipient fiddles the content in a per-class way, kind of like
> fhandler::dup.

I'm working on implementing this, and I've bumped into an elementary C++ 
question.  In order to send the fhandler in an admin packet, I need to determine 
its size dynamically, given an (fhandler_base *) pointer to it.  AFAICS, this 
requires something like the following.

In the definition of class fhandler_base, put a virtual function

   virtual size_t size () const { return sizeof *this; }

and then repeat this essentially verbatim in every derived class:

   size_t size () const { return sizeof *this; }

Does this seem right?  I did an internet search and didn't find anything 
substantially different, although there were several suggestions to use 
templates in various ways.  I'm not convinced that using templates would 
actually improve the code, but I can do it if you think it's better.  See, for 
example,

 
https://stackoverflow.com/questions/7642614/find-size-of-derived-class-object-using-base-class-pointer

Ken

Re: AF_UNIX status report

[Corinna Vinschen]

On Nov  3 10:43, Ken Brown via Cygwin-developers wrote:
> On 10/30/2020 5:20 AM, Corinna Vinschen wrote:
> > On Oct 29 14:53, Joe Lowe wrote:
> > > On 2020-10-29 13:19, Ken Brown via Cygwin-developers wrote:
> > > > On 10/27/2020 5:43 AM, Corinna Vinschen wrote:
> > > > > On Oct 26 18:04, Ken Brown via Cygwin-developers wrote:
> > > > > > 2. I haven't given any thought at all as to how to implement SCM_RIGHTS
> > > > > > ancillary data.  I could definitely use suggestions on that
> > > > > > before I start
> > > > > > thrashing around.
> > > > > 
> > > > > I have only vague ideas at that point.  Assuming we can replace the
> > > > > socket implemantation with the pipe implementation, what we have is a
> > > > > pipe which can impersonate the peer at least from the server side, and
> > > > > it knows the client process.  This in turn can be used to duplicate
> > > > > handles.  So what we could do is to define fhandler methods which create
> > > > > a matching serialization  and deserialization of the fhandler data, plus
> > > > > duplicating the handles for the other process, sent over the pipe as
> > > > > admin package.  This must work in either direction, regardless if the
> > > > > server or the client sends the SCM_RIGHTS block.
> > > > 
> > > > This sounds reasonable.
> > > > 
> > > > I have no experience with serialization.  Do you happen to know of a
> > > > good example that I could look at?
> > 
> > Unfortunately not.  Probably we can just send the entire fhandler and
> > the recipient fiddles the content in a per-class way, kind of like
> > fhandler::dup.
> 
> I'm working on implementing this, and I've bumped into an elementary C++
> question.  In order to send the fhandler in an admin packet, I need to
> determine its size dynamically, given an (fhandler_base *) pointer to it.
> AFAICS, this requires something like the following.
> 
> In the definition of class fhandler_base, put a virtual function
> 
>   virtual size_t size () const { return sizeof *this; }
> 
> and then repeat this essentially verbatim in every derived class:
> 
>   size_t size () const { return sizeof *this; }
> 
> Does this seem right?  I did an internet search and didn't find anything
> substantially different, although there were several suggestions to use
> templates in various ways.  I'm not convinced that using templates would
> actually improve the code, but I can do it if you think it's better.

Actually, I don't like templates that much.  You could do the above, or
just always send a block of size fhandler_union.


Corinna

Re: AF_UNIX status report

[Ken Brown]

On 11/4/2020 7:03 AM, Corinna Vinschen wrote:
> On Nov  3 10:43, Ken Brown via Cygwin-developers wrote:
>> On 10/30/2020 5:20 AM, Corinna Vinschen wrote:
>>> On Oct 29 14:53, Joe Lowe wrote:
>>>> On 2020-10-29 13:19, Ken Brown via Cygwin-developers wrote:
>>>>> On 10/27/2020 5:43 AM, Corinna Vinschen wrote:
>>>>>> On Oct 26 18:04, Ken Brown via Cygwin-developers wrote:
>>>>>>> 2. I haven't given any thought at all as to how to implement SCM_RIGHTS
>>>>>>> ancillary data.  I could definitely use suggestions on that
>>>>>>> before I start
>>>>>>> thrashing around.
>>>>>>
>>>>>> I have only vague ideas at that point.  Assuming we can replace the
>>>>>> socket implemantation with the pipe implementation, what we have is a
>>>>>> pipe which can impersonate the peer at least from the server side, and
>>>>>> it knows the client process.  This in turn can be used to duplicate
>>>>>> handles.  So what we could do is to define fhandler methods which create
>>>>>> a matching serialization  and deserialization of the fhandler data, plus
>>>>>> duplicating the handles for the other process, sent over the pipe as
>>>>>> admin package.  This must work in either direction, regardless if the
>>>>>> server or the client sends the SCM_RIGHTS block.
>>>>>
>>>>> This sounds reasonable.
>>>>>
>>>>> I have no experience with serialization.  Do you happen to know of a
>>>>> good example that I could look at?
>>>
>>> Unfortunately not.  Probably we can just send the entire fhandler and
>>> the recipient fiddles the content in a per-class way, kind of like
>>> fhandler::dup.
>>
>> I'm working on implementing this, and I've bumped into an elementary C++
>> question.  In order to send the fhandler in an admin packet, I need to
>> determine its size dynamically, given an (fhandler_base *) pointer to it.
>> AFAICS, this requires something like the following.
>>
>> In the definition of class fhandler_base, put a virtual function
>>
>>    virtual size_t size () const { return sizeof *this; }
>>
>> and then repeat this essentially verbatim in every derived class:
>>
>>    size_t size () const { return sizeof *this; }
>>
>> Does this seem right?  I did an internet search and didn't find anything
>> substantially different, although there were several suggestions to use
>> templates in various ways.  I'm not convinced that using templates would
>> actually improve the code, but I can do it if you think it's better.
> 
> Actually, I don't like templates that much.  You could do the above, or
> just always send a block of size fhandler_union.

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.

Does this seem reasonable?  The main thing bothering me is the lack of 
atomicity.  I don't like the gap between the sending of the two packets A and P, 
and similarly for the receiving.  I thought about using the io_lock to at least 
make sure that the two packets are adjacent in the pipe, but I don't know if we 
want to tie up the io_lock for that long.

Also, the sending process might be sending several file descriptors at once, so 
that there would be several admin packets to be sent (unless we want to cram it 
all into one).

Ken

Re: AF_UNIX status report

[Corinna Vinschen]

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.

> Does this seem reasonable?  The main thing bothering me is the lack of
> atomicity.  I don't like the gap between the sending of the two packets A
> and P, and similarly for the receiving.  I thought about using the io_lock
> to at least make sure that the two packets are adjacent in the pipe, but I
> don't know if we want to tie up the io_lock for that long.
> 
> Also, the sending process might be sending several file descriptors at once,
> so that there would be several admin packets to be sent (unless we want to
> cram it all into one).

We can safely assume that pipe packets up to 64K are sent and received
atomically.

In most cases this shouldn't be much of a problem.  Most scenarios using
SCM_RIGHTS send no or only a minor payload.  Most scenarios share a
single or only a handful of descriptors.

Apart from that, Linux also defines SCM_MAX_FD, the max. number of
descriptors in a single sendmsg call.  If the number of descriptors
is larger, sendmsg returns EINVAL.  SCM_MAX_FD is 253 on Linux, but

What that means to us is, we can choose our own SCM_MAX_FD and just
return EINVAL if the number of descriptors is uncomfortably high.
The max. number of descriptors should be limited so that all descriptors
fit into 64K, or even 32K, just to leave space for payload.
Assuming a size of about 600 bytes per fhandler, 50 might be a good
candidate for SCM_MAX_FD.  I'd say even 32 would be sufficent for most
scenarios.

The idea would be to create the packet on the source side with all
fhandlers in the ancilliary data block, followed by the payload.
This should typically fit in a 64K package.  If not, only the
payload needs to be split into multiple packages.  Do we really
need atomicity there?  Not sure, but only then we'd need an io_lock.

Does that make sense?


Thanks,
Corinna

Re: AF_UNIX status report

[Ken Brown]

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.

Ah, OK.  I was thinking of it differently.  Rather than having the server 
impersonate the client, I was thinking that the sender would send its winpid as 
part of its admin packet, which the receiver could then use to get a handle to 
the sender's process.  The receiver could then duplicate the handles.  But maybe 
your approach is better.  I'll have to rethink it.

>> Does this seem reasonable?  The main thing bothering me is the lack of
>> atomicity.  I don't like the gap between the sending of the two packets A
>> and P, and similarly for the receiving.  I thought about using the io_lock
>> to at least make sure that the two packets are adjacent in the pipe, but I
>> don't know if we want to tie up the io_lock for that long.
>>
>> Also, the sending process might be sending several file descriptors at once,
>> so that there would be several admin packets to be sent (unless we want to
>> cram it all into one).
> 
> We can safely assume that pipe packets up to 64K are sent and received
> atomically.
> 
> In most cases this shouldn't be much of a problem.  Most scenarios using
> SCM_RIGHTS send no or only a minor payload.  Most scenarios share a
> single or only a handful of descriptors.
> 
> Apart from that, Linux also defines SCM_MAX_FD, the max. number of
> descriptors in a single sendmsg call.  If the number of descriptors
> is larger, sendmsg returns EINVAL.  SCM_MAX_FD is 253 on Linux, but
> 
> What that means to us is, we can choose our own SCM_MAX_FD and just
> return EINVAL if the number of descriptors is uncomfortably high.
> The max. number of descriptors should be limited so that all descriptors
> fit into 64K, or even 32K, just to leave space for payload.
> Assuming a size of about 600 bytes per fhandler, 50 might be a good
> candidate for SCM_MAX_FD.  I'd say even 32 would be sufficent for most
> scenarios.
> 
> The idea would be to create the packet on the source side with all
> fhandlers in the ancilliary data block, followed by the payload.
> This should typically fit in a 64K package.  If not, only the
> payload needs to be split into multiple packages.  Do we really
> need atomicity there?  Not sure, but only then we'd need an io_lock.
> 
> Does that make sense?

Yes.  Thanks.

Ken

Re: AF_UNIX status report

[Joe Lowe]

On 2020-11-05 11:01, 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.
> 
> Ah, OK.  I was thinking of it differently.  Rather than having the 
> server impersonate the client, I was thinking that the sender would send 
> its winpid as part of its admin packet, which the receiver could then 
> use to get a handle to the sender's process.  The receiver could then 
> duplicate the handles.  But maybe your approach is better.  I'll have to 
> rethink it.

SCM_RIGHTS on *nix; fd are retained by message buffering in the kernel. 
A sending process can close an fd after sendmsg is called, before 
recvmsg is called in the receiving process.

SCM_RIGHTS on *nix; fd are not added to a receiving process fd table 
until the SCM_RIGHTS message is read. This is a consideration for DOS 
attacks.

So I expect it is necessary to create a temp copy of each fd being sent, 
so the sender can close the original. And I expect it is necessary to 
use handshake/acks between the two processes; so the DuplicateHandle() 
call can happen in the correct process and not until the SCM_RIGHTS 
message is read.

>>> Does this seem reasonable?  The main thing bothering me is the lack of
>>> atomicity.  I don't like the gap between the sending of the two 
>>> packets A
>>> and P, and similarly for the receiving.  I thought about using the 
>>> io_lock
>>> to at least make sure that the two packets are adjacent in the pipe, 
>>> but I
>>> don't know if we want to tie up the io_lock for that long.
>>>
>>> Also, the sending process might be sending several file descriptors 
>>> at once,
>>> so that there would be several admin packets to be sent (unless we 
>>> want to
>>> cram it all into one).
>>
>> We can safely assume that pipe packets up to 64K are sent and received
>> atomically.
>>
>> In most cases this shouldn't be much of a problem.  Most scenarios using
>> SCM_RIGHTS send no or only a minor payload.  Most scenarios share a
>> single or only a handful of descriptors.
>>
>> Apart from that, Linux also defines SCM_MAX_FD, the max. number of
>> descriptors in a single sendmsg call.  If the number of descriptors
>> is larger, sendmsg returns EINVAL.  SCM_MAX_FD is 253 on Linux, but
>>
>> What that means to us is, we can choose our own SCM_MAX_FD and just
>> return EINVAL if the number of descriptors is uncomfortably high.
>> The max. number of descriptors should be limited so that all descriptors
>> fit into 64K, or even 32K, just to leave space for payload.
>> Assuming a size of about 600 bytes per fhandler, 50 might be a good
>> candidate for SCM_MAX_FD.  I'd say even 32 would be sufficent for most
>> scenarios.
>>
>> The idea would be to create the packet on the source side with all
>> fhandlers in the ancilliary data block, followed by the payload.
>> This should typically fit in a 64K package.  If not, only the
>> payload needs to be split into multiple packages.  Do we really
>> need atomicity there?  Not sure, but only then we'd need an io_lock.

Joe L.

Re: AF_UNIX status report

[Ken Brown]

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?

Ken

Re: AF_UNIX status report

[Ken Brown]

On 11/5/2020 2:54 PM, Joe Lowe wrote:
> 
> 
> On 2020-11-05 11:01, 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.
>>
>> Ah, OK.  I was thinking of it differently.  Rather than having the server 
>> impersonate the client, I was thinking that the sender would send its winpid 
>> as part of its admin packet, which the receiver could then use to get a handle 
>> to the sender's process.  The receiver could then duplicate the handles.  But 
>> maybe your approach is better.  I'll have to rethink it.
> 
> SCM_RIGHTS on *nix; fd are retained by message buffering in the kernel. A 
> sending process can close an fd after sendmsg is called, before recvmsg is 
> called in the receiving process.

I hadn't thought about that.

> SCM_RIGHTS on *nix; fd are not added to a receiving process fd table until the 
> SCM_RIGHTS message is read. This is a consideration for DOS attacks.
> 
> So I expect it is necessary to create a temp copy of each fd being sent, so the 
> sender can close the original.

Sounds right.

Ken

Re: AF_UNIX status report

[Corinna Vinschen]

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.


Corinna

Re: AF_UNIX status report

[Ken Brown]

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.

WDYT?

Ken

Re: AF_UNIX status report

[Ken Brown]

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

Re: AF_UNIX status report

[Corinna Vinschen]

On Nov  8 17:40, Ken Brown via Cygwin-developers wrote:
> On 11/7/2020 5:25 PM, Ken Brown via Cygwin-developers wrote:
> > On 11/6/2020 4:12 AM, Corinna Vinschen wrote:
> > > 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.

Hmm.  Not in all cases, I think.  For devices, major and minor numbers
should be sufficient.  Windows allows to regenerate the filename from
the handle, too.  For inet sockets there's no problem either.  Paths
under /proc and /registry are a problem, yeah.

> > [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.

device and inode are not sufficient.  This may collide with other
processes sharing an fd to the same file.  The code should generate a
unique identifier to name the shared mem and send this over the line.

> 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?

The duplicated handle has to be closed at one point but otherwise
the approach makes sense.


Corinna

Re: AF_UNIX status report

[Ken Brown]

On 11/9/2020 4:08 AM, Corinna Vinschen wrote:
> On Nov  8 17:40, Ken Brown via Cygwin-developers wrote:
>> On 11/7/2020 5:25 PM, Ken Brown via Cygwin-developers wrote:
>>> On 11/6/2020 4:12 AM, Corinna Vinschen wrote:
>>>> 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.
> 
> Hmm.  Not in all cases, I think.  For devices, major and minor numbers
> should be sufficient.  Windows allows to regenerate the filename from
> the handle, too.  For inet sockets there's no problem either.  Paths
> under /proc and /registry are a problem, yeah.
> 
>>> [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.
> 
> device and inode are not sufficient.  This may collide with other
> processes sharing an fd to the same file.  The code should generate a
> unique identifier to name the shared mem and send this over the line.
> 
>> 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?
> 
> The duplicated handle has to be closed at one point but otherwise
> the approach makes sense.

After wasting a ridiculous amount of time because of careless mistakes with 
handle duplication, I've finally gotten something working (currently for disk 
files only and with some limitations that have to removed).  I've pushed it to 
the topic/af_unix branch in case you want to review it and/or test it.

If you do test it, the main limitation currently is that the sending process 
can't exit until the receiving process has received and processed the SCM_RIGHTS 
data.

I'll keep working on improving this and removing limitations, but at least I'm 
confident now that the basic idea of sending serialized fhandlers works.  Thanks 
for suggesting that.  I wouldn't have known where to start otherwise.

Ken

Re: AF_UNIX status report

[Corinna Vinschen]

On Nov 17 14:57, Ken Brown via Cygwin-developers wrote:
> On 11/9/2020 4:08 AM, Corinna Vinschen wrote:
> > The duplicated handle has to be closed at one point but otherwise
> > the approach makes sense.
> 
> After wasting a ridiculous amount of time because of careless mistakes with
> handle duplication, I've finally gotten something working (currently for
> disk files only and with some limitations that have to removed).  I've
> pushed it to the topic/af_unix branch in case you want to review it and/or
> test it.

This is soooo fantastic!  Apart from files, the nexst most interesting
case is sharing a socket, probably.  We could activcate the 2nd half of
privilege separation in sshd then.

> If you do test it, the main limitation currently is that the sending process
> can't exit until the receiving process has received and processed the
> SCM_RIGHTS data.

While this is strictly a flaw, that may not be much of a limitation.
It would be interesting to know how many applications send descriptors
just to exit immediately.  Not so many, probably.

> I'll keep working on improving this and removing limitations, but at least
> I'm confident now that the basic idea of sending serialized fhandlers works.
> Thanks for suggesting that.  I wouldn't have known where to start otherwise.

Thanks for working on this stuff!


Corinna

Re: AF_UNIX status report

[Ken Brown]

On 11/18/2020 3:34 AM, Corinna Vinschen wrote:
> On Nov 17 14:57, Ken Brown via Cygwin-developers wrote:
>> On 11/9/2020 4:08 AM, Corinna Vinschen wrote:
>>> The duplicated handle has to be closed at one point but otherwise
>>> the approach makes sense.
>>
>> After wasting a ridiculous amount of time because of careless mistakes with
>> handle duplication, I've finally gotten something working (currently for
>> disk files only and with some limitations that have to removed).  I've
>> pushed it to the topic/af_unix branch in case you want to review it and/or
>> test it.
> 
> This is soooo fantastic!  Apart from files, the nexst most interesting
> case is sharing a socket, probably.  We could activcate the 2nd half of
> privilege separation in sshd then.

I've pushed a first attempt to implement sending socket descriptors, but I 
haven't yet tested it.  I'll try to find a small test program and then, if all 
goes well, take a look at sshd.

>> If you do test it, the main limitation currently is that the sending process
>> can't exit until the receiving process has received and processed the
>> SCM_RIGHTS data.
> 
> While this is strictly a flaw, that may not be much of a limitation.
> It would be interesting to know how many applications send descriptors
> just to exit immediately.  Not so many, probably.

I don't know, but I've reduced the effect of the limitation.  Now the sending 
process waits up to about 100ms for an ack from the receiving process before 
closing the socket.  I have no idea if 100ms is reasonable in the real world, 
but it's big enough for my test programs and small enough that I don't notice 
the delay when I run the programs interactively.

Ken

Re: AF_UNIX status report

[Corinna Vinschen]

On Nov 22 15:44, Ken Brown via Cygwin-developers wrote:
> On 11/18/2020 3:34 AM, Corinna Vinschen wrote:
> > On Nov 17 14:57, Ken Brown via Cygwin-developers wrote:
> > > On 11/9/2020 4:08 AM, Corinna Vinschen wrote:
> > > > The duplicated handle has to be closed at one point but otherwise
> > > > the approach makes sense.
> > > 
> > > After wasting a ridiculous amount of time because of careless mistakes with
> > > handle duplication, I've finally gotten something working (currently for
> > > disk files only and with some limitations that have to removed).  I've
> > > pushed it to the topic/af_unix branch in case you want to review it and/or
> > > test it.
> > 
> > This is soooo fantastic!  Apart from files, the nexst most interesting
> > case is sharing a socket, probably.  We could activcate the 2nd half of
> > privilege separation in sshd then.
> 
> I've pushed a first attempt to implement sending socket descriptors, but I
> haven't yet tested it.  I'll try to find a small test program and then, if
> all goes well, take a look at sshd.
> 
> > > If you do test it, the main limitation currently is that the sending process
> > > can't exit until the receiving process has received and processed the
> > > SCM_RIGHTS data.
> > 
> > While this is strictly a flaw, that may not be much of a limitation.
> > It would be interesting to know how many applications send descriptors
> > just to exit immediately.  Not so many, probably.
> 
> I don't know, but I've reduced the effect of the limitation.  Now the
> sending process waits up to about 100ms for an ack from the receiving
> process before closing the socket.  I have no idea if 100ms is reasonable in
> the real world, but it's big enough for my test programs and small enough
> that I don't notice the delay when I run the programs interactively.

100 ms may be a bit low under load.  Sounds like a good starting point, though.


Corinna

Re: AF_UNIX status report

[Ken Brown]

On 11/23/2020 3:43 AM, Corinna Vinschen wrote:
> On Nov 22 15:44, Ken Brown via Cygwin-developers wrote:
>> On 11/18/2020 3:34 AM, Corinna Vinschen wrote:
>>> On Nov 17 14:57, Ken Brown via Cygwin-developers wrote:
>>>> On 11/9/2020 4:08 AM, Corinna Vinschen wrote:
>>>>> The duplicated handle has to be closed at one point but otherwise
>>>>> the approach makes sense.
>>>>
>>>> After wasting a ridiculous amount of time because of careless mistakes with
>>>> handle duplication, I've finally gotten something working (currently for
>>>> disk files only and with some limitations that have to removed).  I've
>>>> pushed it to the topic/af_unix branch in case you want to review it and/or
>>>> test it.
>>>
>>> This is soooo fantastic!  Apart from files, the nexst most interesting
>>> case is sharing a socket, probably.  We could activcate the 2nd half of
>>> privilege separation in sshd then.
>>
>> I've pushed a first attempt to implement sending socket descriptors, but I
>> haven't yet tested it.  I'll try to find a small test program and then, if
>> all goes well, take a look at sshd.

I've now tested it with a small program that forks a subprocess, accepts a 
connection on an AF_INET socket, and sends the resulting socket descriptor to 
the child, using an AF_UNIX socketpair for parent-child communication.  It seems 
to work as expected.  The test is in winsup/cygwin/socket_tests on the 
topic/af_unix branch, with a description of how to run it in README.txt.

I took a quick glance at the openssh code, and I think I see places where 
pty/tty descriptors are sent.  For example, I see calls like mm_send_fd(sock, 
s->ttyfd).  So maybe I need to try to add support for that next.  This could 
take some time since I'm not familiar with the code for fhandler_termios or any 
of its derived classes, nor do I have any idea how to test sending that kind of fd.

Ken

Re: AF_UNIX status report

[Ken Brown]

On 11/26/2020 12:06 PM, Ken Brown via Cygwin-developers wrote:
> On 11/23/2020 3:43 AM, Corinna Vinschen wrote:
>> On Nov 22 15:44, Ken Brown via Cygwin-developers wrote:
>>> On 11/18/2020 3:34 AM, Corinna Vinschen wrote:
>>>> On Nov 17 14:57, Ken Brown via Cygwin-developers wrote:
>>>>> On 11/9/2020 4:08 AM, Corinna Vinschen wrote:
>>>>>> The duplicated handle has to be closed at one point but otherwise
>>>>>> the approach makes sense.
>>>>>
>>>>> After wasting a ridiculous amount of time because of careless mistakes with
>>>>> handle duplication, I've finally gotten something working (currently for
>>>>> disk files only and with some limitations that have to removed).  I've
>>>>> pushed it to the topic/af_unix branch in case you want to review it and/or
>>>>> test it.
>>>>
>>>> This is soooo fantastic!  Apart from files, the nexst most interesting
>>>> case is sharing a socket, probably.  We could activcate the 2nd half of
>>>> privilege separation in sshd then.
>>>
>>> I've pushed a first attempt to implement sending socket descriptors, but I
>>> haven't yet tested it.  I'll try to find a small test program and then, if
>>> all goes well, take a look at sshd.
> 
> I've now tested it with a small program that forks a subprocess, accepts a 
> connection on an AF_INET socket, and sends the resulting socket descriptor to 
> the child, using an AF_UNIX socketpair for parent-child communication.  It seems 
> to work as expected.  The test is in winsup/cygwin/socket_tests on the 
> topic/af_unix branch, with a description of how to run it in README.txt.
> 
> I took a quick glance at the openssh code, and I think I see places where 
> pty/tty descriptors are sent.  For example, I see calls like mm_send_fd(sock, 
> s->ttyfd).  So maybe I need to try to add support for that next.  This could 
> take some time since I'm not familiar with the code for fhandler_termios or any 
> of its derived classes, nor do I have any idea how to test sending that kind of fd.

I've now written and tested code for sending pty slave descriptors.  This is the 
first case I've dealt with in which the fhandler uses an archetype, and I'm not 
completely sure that my approach is right (though I can't think of an alternative).

Suppose a process wants to send a pty slave descriptor for /dev/ptyN.  The 
receiving process checks whether it already has an archetype for that device. 
If so, it uses it.  If not, it creates a new one by duplicating handles from the 
sending process.

The first case (in which the receiving process already has an archetype) bothers 
me, because it means that deserialization uses no information about the fhandler 
it receives other than the device number.  That seems wrong, somehow, though I 
can't really say why.

If you want to see exactly what I've done, it's in commit c605ea0d on the 
topic/af_unix branch.

Ken

Re: AF_UNIX status report

[Corinna Vinschen]

On Dec 15 12:33, Ken Brown via Cygwin-developers wrote:
> On 11/26/2020 12:06 PM, Ken Brown via Cygwin-developers wrote:
> > I took a quick glance at the openssh code, and I think I see places
> > where pty/tty descriptors are sent.  For example, I see calls like
> > mm_send_fd(sock, s->ttyfd).  So maybe I need to try to add support for
> > that next.  This could take some time since I'm not familiar with the
> > code for fhandler_termios or any of its derived classes, nor do I have
> > any idea how to test sending that kind of fd.
> 
> I've now written and tested code for sending pty slave descriptors.  This is
> the first case I've dealt with in which the fhandler uses an archetype, and
> I'm not completely sure that my approach is right (though I can't think of
> an alternative).
> 
> Suppose a process wants to send a pty slave descriptor for /dev/ptyN.  The
> receiving process checks whether it already has an archetype for that
> device. If so, it uses it.  If not, it creates a new one by duplicating
> handles from the sending process.
> 
> The first case (in which the receiving process already has an archetype)
> bothers me, because it means that deserialization uses no information about
> the fhandler it receives other than the device number.  That seems wrong,
> somehow, though I can't really say why.
> 
> If you want to see exactly what I've done, it's in commit c605ea0d on the
> topic/af_unix branch.

I think it should be ok to use the archetype if it's available.  The
important tty data is shared.  The handles and stuff in the fhandler is
mostly connecting handles.  Permissions are not exactly taken into
account anyway.

The connection to the pseudo console could be a problem, but reusing
the existing archetype may workaround that.  Otherwise the process
would be connected to two pseudo consoles, and I'm not sure that's ok.
I hope so, but...


Corinna

Re: AF_UNIX status report

[Ken Brown]

On 12/16/2020 4:29 AM, Corinna Vinschen via Cygwin-developers wrote:
> On Dec 15 12:33, Ken Brown via Cygwin-developers wrote:
>> On 11/26/2020 12:06 PM, Ken Brown via Cygwin-developers wrote:
>>> I took a quick glance at the openssh code, and I think I see places
>>> where pty/tty descriptors are sent.  For example, I see calls like
>>> mm_send_fd(sock, s->ttyfd).  So maybe I need to try to add support for
>>> that next.  This could take some time since I'm not familiar with the
>>> code for fhandler_termios or any of its derived classes, nor do I have
>>> any idea how to test sending that kind of fd.
>>
>> I've now written and tested code for sending pty slave descriptors.  This is
>> the first case I've dealt with in which the fhandler uses an archetype, and
>> I'm not completely sure that my approach is right (though I can't think of
>> an alternative).
>>
>> Suppose a process wants to send a pty slave descriptor for /dev/ptyN.  The
>> receiving process checks whether it already has an archetype for that
>> device. If so, it uses it.  If not, it creates a new one by duplicating
>> handles from the sending process.
>>
>> The first case (in which the receiving process already has an archetype)
>> bothers me, because it means that deserialization uses no information about
>> the fhandler it receives other than the device number.  That seems wrong,
>> somehow, though I can't really say why.
>>
>> If you want to see exactly what I've done, it's in commit c605ea0d on the
>> topic/af_unix branch.
> 
> I think it should be ok to use the archetype if it's available.  The
> important tty data is shared.  The handles and stuff in the fhandler is
> mostly connecting handles.  Permissions are not exactly taken into
> account anyway.
> 
> The connection to the pseudo console could be a problem, but reusing
> the existing archetype may workaround that.  Otherwise the process
> would be connected to two pseudo consoles, and I'm not sure that's ok.
> I hope so, but...

I completely overlooked the pseudo console.  So I guess the 
serialization/deserialization has to deal with get_ttyp()->h_pseudo_console 
somehow.  My guess (based on only a very brief look at the pseudo console code) 
is that this should be non-NULL only for the process that called 
CreatePseudoConsole.  Does that sound right?  If so, then there shouldn't be 
much to do.  I'll keep poking around, but please let me know if you see 
something that I'm missing something.

(Takashi, if you happen to see this, please do the same.)

Ken

Re: AF_UNIX status report

[Ken Brown]

On 12/16/2020 4:09 PM, Ken Brown via Cygwin-developers wrote:
> On 12/16/2020 4:29 AM, Corinna Vinschen via Cygwin-developers wrote:
>> On Dec 15 12:33, Ken Brown via Cygwin-developers wrote:
>>> On 11/26/2020 12:06 PM, Ken Brown via Cygwin-developers wrote:
>>>> I took a quick glance at the openssh code, and I think I see places
>>>> where pty/tty descriptors are sent.  For example, I see calls like
>>>> mm_send_fd(sock, s->ttyfd).  So maybe I need to try to add support for
>>>> that next.  This could take some time since I'm not familiar with the
>>>> code for fhandler_termios or any of its derived classes, nor do I have
>>>> any idea how to test sending that kind of fd.
>>>
>>> I've now written and tested code for sending pty slave descriptors.  This is
>>> the first case I've dealt with in which the fhandler uses an archetype, and
>>> I'm not completely sure that my approach is right (though I can't think of
>>> an alternative).
>>>
>>> Suppose a process wants to send a pty slave descriptor for /dev/ptyN.  The
>>> receiving process checks whether it already has an archetype for that
>>> device. If so, it uses it.  If not, it creates a new one by duplicating
>>> handles from the sending process.
>>>
>>> The first case (in which the receiving process already has an archetype)
>>> bothers me, because it means that deserialization uses no information about
>>> the fhandler it receives other than the device number.  That seems wrong,
>>> somehow, though I can't really say why.
>>>
>>> If you want to see exactly what I've done, it's in commit c605ea0d on the
>>> topic/af_unix branch.
>>
>> I think it should be ok to use the archetype if it's available.  The
>> important tty data is shared.  The handles and stuff in the fhandler is
>> mostly connecting handles.  Permissions are not exactly taken into
>> account anyway.
>>
>> The connection to the pseudo console could be a problem, but reusing
>> the existing archetype may workaround that.  Otherwise the process
>> would be connected to two pseudo consoles, and I'm not sure that's ok.
>> I hope so, but...
> 
> I completely overlooked the pseudo console.  So I guess the 
> serialization/deserialization has to deal with get_ttyp()->h_pseudo_console 
> somehow.  My guess (based on only a very brief look at the pseudo console code) 
> is that this should be non-NULL only for the process that called 
> CreatePseudoConsole.  Does that sound right?  If so, then there shouldn't be 
> much to do.  I'll keep poking around, but please let me know if you see 
> something that I'm missing something.
> 
> (Takashi, if you happen to see this, please do the same.)

Answering my own question, yes, I was missing something.  I wasn't taking into 
account the fact that tty data is shared.  You said it, but it hadn't sunk in. 
I need to rethink this.

Ken