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

[Bryce Harrington]

On Fri, Jan 15, 2016 at 01:36:21PM -0800, Bryce Harrington wrote:
> 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

I've gone ahead and landed this to trunk as eb52bb8e so it'll be
included in 1.10.

Thanks,
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
> 
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel