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

[Pekka Paalanen]

On Wed, 13 Jan 2016 10:31:58 +0800
Jonas Ådahl <jadahl at gmail.com> wrote:

> On Tue, Jan 12, 2016 at 10:58:16AM -0600, Derek Foreman wrote:
> > On 11/01/16 04:30 PM, Sung-Jin Park wrote:  
> > > This adds an API to get the socket fd for a client.
> > > The client socket fd can be used for a wayland compositor to validate a request
> > > from a client.
> > > For instance, this will be helpful in some linux distributions, in which SELinux
> > > or SMACK is enabled. In those environments, each file (including socket) will have
> > > each security context in its inode as xattr member variable. A wayland compositor
> > > can validate a client request by getting socket fd of the client and by checking
> > > the security context associated with the socket fd.
> > > 
> > > Signed-off-by: Sung-Jin Park <input.hacker at gmail.com>
> > > ---
> > >  src/wayland-server-core.h |  3 +++
> > >  src/wayland-server.c      | 15 +++++++++++++++
> > >  2 files changed, 18 insertions(+)
> > > 
> > > diff --git a/src/wayland-server-core.h b/src/wayland-server-core.h
> > > index 1700cd3..0d5fbc1 100644
> > > --- a/src/wayland-server-core.h
> > > +++ b/src/wayland-server-core.h
> > > @@ -182,6 +182,9 @@ void
> > >  wl_client_get_credentials(struct wl_client *client,
> > >  			  pid_t *pid, uid_t *uid, gid_t *gid);
> > >  
> > > +int
> > > +wl_client_get_socket_fd(struct wl_client *client);
> > > +
> > >  void
> > >  wl_client_add_destroy_listener(struct wl_client *client,
> > >  			       struct wl_listener *listener);
> > > diff --git a/src/wayland-server.c b/src/wayland-server.c
> > > index 55c0cf9..973a71c 100644
> > > --- a/src/wayland-server.c
> > > +++ b/src/wayland-server.c
> > > @@ -491,6 +491,21 @@ wl_client_get_credentials(struct wl_client *client,
> > >  		*gid = client->ucred.gid;
> > >  }
> > >  
> > > +/** Get the socket fd for the client  
> 
> s/fd/file descriptor/
> 
> > > + *
> > > + * \param client The display object
> > > + * \return fd The fd to use for the connection  
> > 
> > Should probably just be
> >  * \return The fd to use for the connection  
> 
> Maybe "file descriptor" here as well?
> 
> > 
> > Otherwise this looks good to me,
> > Reviewed-by: Derek Foreman <derekf at osg.samsung.com>  
> 
> I would also prefer the API to be "wl_client_get_fd()" as that is that
> is what all the other similar API looks like (be it a socket fd or a
> non-socket fd).
> 
> Other than those issues, this looks reasonable to me, so consider it
> Reviewed-by: Jonas Ådahl <jadahl at gmail.com>

Hi,

I agree with all of the above.

The fd may not always be from a socket file, it can also be from a call
to socketpair(2). 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().

One should probably also document, that the fd you get from this
function is not duplicated, and it remains owned and used by
libwayland-server. Therefore if the caller does anything to the fd that
changes its state, it will likely cause problems. Getting the fd is for
inspection only, like the use case described in the commit message.

It might even make sense to add the example use case to the doc.

With these documentation additions, you get
Acked-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>


Thanks,
pq

> >   
> > > + *
> > > + * This function returns the client socket fd for the given client.
> > > + *
> > > + * \memberof wl_client
> > > + */
> > > +WL_EXPORT int
> > > +wl_client_get_socket_fd(struct wl_client *client)
> > > +{
> > > +	return client->connection->fd;
> > > +}
> > > +
> > >  /** Look up an object in the client name space
> > >   *
> > >   * \param client The client object
> > >   
> > 
> > _______________________________________________
> > 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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20160113/3483158c/attachment-0001.sig>