[prev in list] [next in list] [prev in thread] [next in thread]
List: freedesktop-dbus
Subject: Re: RFC: adding fd-passing to win32
From: Marc-André Lureau <marcandre.lureau () gmail ! com>
Date: 2022-08-09 16:57:00
Message-ID: CAJ+F1CKPPZium+coxdRa=tvgD468HmkfpqM8-S3LWro4OWxOKQ () mail ! gmail ! com
[Download RAW message or body]
Hi
On Tue, Aug 9, 2022 at 7:15 PM Thiago Macieira <thiago@kde.org> wrote:
> On Tuesday, 9 August 2022 00:51:38 PDT Marc-André Lureau wrote:
> > If we are able to find solutions at D-Bus level, it's not impossible they
> > implement better low-level solutions in afunix.sys. See also cygwin
> > discussion pointed by Lawrence.
>
> You may want to think about widest compatibility too. I don't expect that
> afunix.sys will be as widespread, so implementing something that is able
> to
> work without it makes sense.
>
Honestly, I think it's sufficiently hard to do secure HANDLE passing than
trying to do it over TCP would be a mistake. And do we really consider
D-Bus over TCP secure anyway on Windows? I have doubts, because the way we
look up the peer process credentials is quite hackish.
> Compatibility with cygwin applications may not be your goal.
>
>
Why not?
> > My point is that 'h' is the right type to transfer FDs, not
> HANDLEs/SOCKET
> > etc.
>
> And you're still missing my point.
>
> The backend implementation is irrelevant. The fact that you get a file
> descriptor in the API is the important bit, and if on Windows you transfer
> a
> HANDLE but the API needs to do _open_osfhandle to return something to the
> user, so be it.
>
> Unless you tell us that you *can't* do that for SOCKETs, even if just as a
> holder so one can do _get_osfhandle later and get the HANDLE back.
>
>
We should avoid that mapping dance for native Windows HANDLE/SOCKETs. FDs
are often not helping on Windows, they often create extra issues (like
_close vs closesocket I mentioned).
> > > If you use 'H', you'll first need to start with a spec update and
> you'll
> > > need
> > > to convince people like Simon and myself, who don't have as extensive
> > > Windows
> > > knowledge, that it is needed (and this thread is pointing that we
> aren't
> > > getting convinced). You'll need code reviews throughout the
> implementation
> > > to
> >
> > Spec update will be needed for nego phase, details of HANDLE transfer
> etc.
> > Reference implementation changes are required anyway.
>
> Indeed, but those would be minor and focused. Not affecting the type
> system.
>
> > Frankly, adding 'H' is minor compared to the rest of the changes
> involved.
>
> No, it isn't.
>
Are we talking about the same thing? I did update the spec and I am working
on 3 different implementations, I deserve more credits.
> > Regarding compatibility, the 'H' type won't be used unless capability is
> > negotiated.
>
> But it'll exist.
>
Well, how did we introduce 'h' in the first place? It didn't exist before
0.13 or something. Was that so problematic, can you point me to related
issues?
>
> > SOCKET s = WSASocket(..)
> > int fd = _open_osfhandle(s,..)
> > dbus_message_append_args(.., DBUS_TYPE_UNIX_FD, &fd,..)
> >
> > In libdbus, or receiver
> > dbus_message_get_args(.., DBUS_TYPE_UNIX_FD, &fd)
> > HANDLE h = _get_osfhandle(fd)
> > if (is_socket(h)) /* see
> > https://gitlab.gnome.org/GNOME/glib/-/blob/main/glib/gwin32.c#L1457 for
> ex
> > */
> > {
> > SOCKET s = (SOCKET)h;
> > ..
> > WSADuplicateSocket(s, ) to target process for ex..
> > ..
> > closesocket(s) /* in theory better than _close(), but this also
> unclear!
> > */
> > } else {
> > DuplicateHandle(h, )..
> > _close(fd)
> > }
> >
> > If we expose HANDLE & SOCKETs directly instead (both in APIs and
> protocol),
> > we don't need to juggle with the fd back/forth mapping and potential
> > guesses & pitfalls.
>
> Thank you for more details.
>
> But this tells me that the API would need to split between HANDLE and
> SOCKET,
> so wouldn't you need two types instead?
>
Yes, as you can see from the example above, we need type information for
the "generic" HANDLE and SOCKET. I am handling that in the "handle array" I
talked about earlier.
I understand the desire to use 'h' FDs only, and in fact I started this way
in the gdbus implementation. But I have been convinced that the Windows API
need its own type to correctly deal with HANDLE vs SOCKET (or other more
marginals, and a bit less problematic such as Change Notification handle,
Event log handle, Find File handle, Update Resource handle, etc which must
also be closed with specific functions and whose type may not be easily
retrieved from FDs mapping).
Win32 FD mapping story is not a panacea, it looks full of pitfalls and
obscure cases. I think we should reserve it for the day AF_UNIX has
SCM_RIGHT with FDs support, and use the same code on Unix & Windows for it.
In the meantime, introduce the 'H' type for Windows native handles, and
provide native and safe handle APIs (not doing the FD mapping).
--
Marc-André Lureau
[Attachment #3 (text/html)]
<div dir="ltr"><div dir="ltr">Hi<br></div><br><div class="gmail_quote"><div dir="ltr" \
class="gmail_attr">On Tue, Aug 9, 2022 at 7:15 PM Thiago Macieira <<a \
href="mailto:thiago@kde.org">thiago@kde.org</a>> wrote:<br></div><blockquote \
class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid \
rgb(204,204,204);padding-left:1ex">On Tuesday, 9 August 2022 00:51:38 PDT Marc-André \
Lureau wrote:<br> > If we are able to find solutions at D-Bus level, it's not \
impossible they<br> > implement better low-level solutions in afunix.sys. See also \
cygwin<br> > discussion pointed by Lawrence.<br>
<br>
You may want to think about widest compatibility too. I don't expect that <br>
afunix.sys will be as widespread, so implementing something that is able to <br>
work without it makes sense.<br></blockquote><div><br></div><div>Honestly, I think \
it's sufficiently hard to do secure HANDLE passing than trying to do it over TCP \
would be a mistake. And do we really consider D-Bus over TCP secure anyway on \
Windows? I have doubts, because the way we look up the peer process credentials is \
quite hackish.<br></div><div> </div><blockquote class="gmail_quote" \
style="margin:0px 0px 0px 0.8ex;border-left:1px solid \
rgb(204,204,204);padding-left:1ex"> Compatibility with cygwin applications may not be \
your goal.<br> <br></blockquote><div><br></div><div>Why not?<br></div><div> \
</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px \
solid rgb(204,204,204);padding-left:1ex"> > My point is that 'h' is the \
right type to transfer FDs, not HANDLEs/SOCKET<br> > etc.<br>
<br>
And you're still missing my point.<br>
<br>
The backend implementation is irrelevant. The fact that you get a file <br>
descriptor in the API is the important bit, and if on Windows you transfer a <br>
HANDLE but the API needs to do _open_osfhandle to return something to the <br>
user, so be it.<br>
<br>
Unless you tell us that you *can't* do that for SOCKETs, even if just as a <br>
holder so one can do _get_osfhandle later and get the HANDLE back.<br>
<br></blockquote><div><br></div><div>We should avoid that mapping dance for native \
Windows HANDLE/SOCKETs. FDs are often not helping on Windows, they often create extra \
issues (like _close vs closesocket I mentioned).<br></div><div> </div><blockquote \
class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid \
rgb(204,204,204);padding-left:1ex"> > > If you use 'H', you'll \
first need to start with a spec update and you'll<br> > > need<br>
> > to convince people like Simon and myself, who don't have as \
extensive<br> > > Windows<br>
> > knowledge, that it is needed (and this thread is pointing that we \
aren't<br> > > getting convinced). You'll need code reviews throughout \
the implementation<br> > > to<br>
> <br>
> Spec update will be needed for nego phase, details of HANDLE transfer etc.<br>
> Reference implementation changes are required anyway.<br>
<br>
Indeed, but those would be minor and focused. Not affecting the type system.<br>
<br>
> Frankly, adding 'H' is minor compared to the rest of the changes \
involved.<br> <br>
No, it isn't.<br></blockquote><div><br></div><div>Are we talking about the same \
thing? I did update the spec and I am working on 3 different implementations, I \
deserve more credits.<br></div><div><br></div><blockquote class="gmail_quote" \
style="margin:0px 0px 0px 0.8ex;border-left:1px solid \
rgb(204,204,204);padding-left:1ex"> <br>
> Regarding compatibility, the 'H' type won't be used unless \
capability is<br> > negotiated.<br>
<br>
But it'll exist.<br></blockquote><div><br></div><div>Well, how did we introduce \
'h' in the first place? It didn't exist before 0.13 or something. Was \
that so problematic, can you point me to related issues?<br></div><div> \
<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px \
0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> <br>
> SOCKET s = WSASocket(..)<br>
> int fd = _open_osfhandle(s,..)<br>
> dbus_message_append_args(.., DBUS_TYPE_UNIX_FD, &fd,..)<br>
> <br>
> In libdbus, or receiver<br>
> dbus_message_get_args(.., DBUS_TYPE_UNIX_FD, &fd)<br>
> HANDLE h = _get_osfhandle(fd)<br>
> if (is_socket(h)) /* see<br>
> <a href="https://gitlab.gnome.org/GNOME/glib/-/blob/main/glib/gwin32.c#L1457" \
rel="noreferrer" target="_blank">https://gitlab.gnome.org/GNOME/glib/-/blob/main/glib/gwin32.c#L1457</a> \
for ex<br> > */<br>
> {<br>
> SOCKET s = (SOCKET)h;<br>
> ..<br>
> WSADuplicateSocket(s, ) to target process for ex..<br>
> ..<br>
> closesocket(s) /* in theory better than _close(), but this also unclear!<br>
> */<br>
> } else {<br>
> DuplicateHandle(h, )..<br>
> _close(fd)<br>
> }<br>
> <br>
> If we expose HANDLE & SOCKETs directly instead (both in APIs and \
protocol),<br> > we don't need to juggle with the fd back/forth mapping and \
potential<br> > guesses & pitfalls.<br>
<br>
Thank you for more details.<br>
<br>
But this tells me that the API would need to split between HANDLE and SOCKET, <br>
so wouldn't you need two types instead?<br></blockquote><div><br></div><div>Yes, \
as you can see from the example above, we need type information for the \
"generic" HANDLE and SOCKET. I am handling that in the "handle \
array" I talked about earlier.<br></div><div><br></div><div>I understand the \
desire to use 'h' FDs only, and in fact I started this way in the gdbus \
implementation. But I have been convinced that the Windows API need its own type to \
correctly deal with HANDLE vs SOCKET (or other more marginals, and a bit less \
problematic such as Change Notification handle, Event log handle, Find File handle, \
Update Resource handle, etc which must also be closed with specific functions and \
whose type may not be easily retrieved from FDs \
mapping).</div><div><br></div><div>Win32 FD mapping story is not a panacea, it looks \
full of pitfalls and obscure cases. I think we should reserve it for the day AF_UNIX \
has SCM_RIGHT with FDs support, and use the same code on Unix & Windows for \
it.<br></div></div><div class="gmail_quote"><br></div><div class="gmail_quote">In the \
meantime, introduce the 'H' type for Windows native handles, and provide \
native and safe handle APIs (not doing the FD mapping).<br clear="all"></div><br>-- \
<br><div dir="ltr" class="gmail_signature">Marc-André Lureau<br></div></div>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic