[PATCH] server: Add an API to get the socket fd for a client

[Bryce Harrington]

On Fri, Jan 15, 2016 at 02:15:51PM +0900, Sung-Jin Park wrote:
> Dear guys and pq,
> I updated the new version of patch with version 4 and modified its title to
> the following. Sorry for my mistake. :D
> 
> "[PATCH v4] server: Add an API to get the file descriptor for a client" (
> http://patchwork.freedesktop.org/patch/70475/)
> 
> Would you plz review it again whether it is better than before ? :)

Personally the wording seems fine either way.  I do like seeing the v4
indicated there with the PATCH tag, that's helpful when reviewing.

It seems to me this patch is nearly ready to land?  The actual code
looks fine; perhaps the doxygen comment could use some further
copyediting, but that shouldn't hold it up for inclusion in the
release.  Documentation changes would be fine to land post-alpha since
they're low risk, but the API change would be nice to get landed prior
to the alpha.

Bryce
 
> Thanks and regards,
> Sung-Jin Park
> 
> >sorry, that sounds completely bogus and I never implied anything like that.
> Oh, don't mention it. I misunderstood your comment. :)
> I'm also sorry for I mentioned UID in my previous email.
> I think that sounds stranges. I should have mentioned PID instead of UID.
> 
> >socketpair() is used for creating a connection before fork()'ing and
> >exec()'ing a client process, so that the process starts with an already
> >open connection. In that case, wl_client_get_credentials() provides
> >wrong information. Particularly the returned PID will be the
> >compositor's, not the client's. I do not know if the security context
> >you are interested in suffers from the same problem.
> Exactly. I understand what you mean and I won't suffer from the same
> problem.
> After comparing pids from compositor's and the client's, I'll bypass to
> check of client's privilege
> when the client equals to the compositor.
> 
> >Upstream Weston never sends requests to itself, FWIW. It does use
> >socketpair() when launching special clients, though.
> 
> >Yes, this is clear, assuming the security context information you
> >receive is actually always correct. I would assume it does not suffer
> >from the same caveat as getsockopt(SO_PEERCRED), but I don't know that.
> Yes, as I also think, the security context must always be correct when I
> pass the exact client's fd.
> The library to get the security contexts from the file descriptor must
> guarantee that
> the returned contexts are credible.
> 
> Thanks and regards,
> Sung-Jin Park
> 
> ------- Original Message -------
> Sender : Pekka Paalanen<ppaalanen at gmail.com>
> Date   : 2016-01-13 19:35 (GMT+09:00)
> Title  : Re: [PATCH] server: Add an API to get the socket fd for a client
> 
>  On Wed, 13 Jan 2016 10:02:18 +0000 (GMT)
> 박성진 <sj76.park at samsung.com> wrote:
> 
> > Samsung Enterprise Portal mySingle
> >
> > Pekka Paalanen, thank you for your review on this. :)
> >
> >
> >
> > >The fd may not always be from a socket file, it can also be from a call
> > >to socketpair(2).
> >
> > Yes, exactly.
> >
> > >Please refer to wl_client_get_credentials() for the
> > >caveat there, and evaluate whether it applies to your use case.
> > >wl_client_get_fd() doc should probably have a "see also
> > >wl_client_get_credentials()" so that someone reading the doc finds out
> > >about socketpair().
> >
> > I'll append "see also wl_client_get_credentials() to wl_client_get_fd()
> doc. :)
> >
> >
> >
> > Regarding your recommendation, as you meant, if I just need to
> distinguish between
> > the client's request and the request from compositor itself, it'll be
> better to use
> > wl_client_get_credentials() because comparing between the compositor's
> uid and
> > the uid from the function will be enough to make a decision for sth.
> 
> Hi,
> 
> sorry, that sounds completely bogus and I never implied anything like
> that.
> 
> socketpair() is used for creating a connection before fork()'ing and
> exec()'ing a client process, so that the process starts with an already
> open connection. In that case, wl_client_get_credentials() provides
> wrong information. Particularly the returned PID will be the
> compositor's, not the client's. I do not know if the security context
> you are interested in suffers from the same problem.
> 
> Upstream Weston never sends requests to itself, FWIW. It does use
> socketpair() when launching special clients, though.
> 
> > In my use case, I would like to get the client fd and check whether the
> client
> > has the needed privilege for doing sth with a request. The security
> context getting from
> > the client fd will be used to check the client's privilege.
> 
> Yes, this is clear, assuming the security context information you
> receive is actually always correct. I would assume it does not suffer
> from the same caveat as getsockopt(SO_PEERCRED), but I don't know that.
> 
> Thanks,
> pq
> <p> </p><p> </p>

> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel