On Fri, 3 Nov 2017 09:38:46 +0100 (CET) Jan Engelhardt <jeng...@inai.de> wrote:
> On Friday 2017-11-03 08:33, Pekka Paalanen wrote:
> >> > >
> >> > > wl_display_connect() always attempts to contact a server socket living
> >> > > at
> >> > > ${XDG_RUNTIME_DIR}/${WAYLAND_DISPLAY}.
> >
> >> > Modifying the meaning of WAYLAND_DISPLAY environment variable to
> >> > support also absolute paths has been proposed before IIRC. Maybe
> >> > resurrecting that work could be a way forward? Can anyone see a problem
> >> > with that?
> >>
> >> This would definitely work, so I don't object if this is the preference of
> >> other reviewers. I would prefer (for the reasons coming below) the
> >> /run/wayland/$WAYLAND_DISPLAY suggestion though.
> >>
> >> One note about this: this would contain a subtle change in behavior to
> >> existing users of $WAYLAND_DISPLAY. If somebody sets
> >> WAYLAND_DISPLAY="/wayland-0" currently, this works okay. The concatenation
> >> logic in wl_display_connect() results in a string
> >> "/run/user/<uid>//wayland-0", which is valid despite the duplicate '/'. If
> >> $WAYLAND_DISPLAY is now examined for absolute-pathiness, the logic would
> >> probably see "/wayland-0" as an absolute path reference, and the connection
> >> attempt would fail.
>
> So, for comparison, the X11 DISPLAY variable is an abstract identifier only.
> libX11 checks that $DISPLAY matches /^:[0-9]+$/ and rejects attempts to use
> e.g. "/../"-kind of injection attacks.
I'm with Carsten here, what attacks are you thinking of?
Wayland very much relies on file system access permissions to limit the
reachability of a socket. There are not and cannot ever be reliable
restrictions implemented in libwayland-client.
> And I would think that the intent of WAYLAND_DISPLAY was very similar in that
> its creators saw it as an identifier rather than a path component. (It
> certainly has that _ring_ to it, given its proximity to X11's "DISPLAY" name.)
I would agree with you, if it actually was an abstract socket, but it
is not. It is a socket file in the file system already.
> As such, WAYLAND_DISPLAY maybe should be hardened to reject any '/',
> and consequently, a new variable ("WAYLAND_SOCKET"?) be introduced that,
> if set, 1. overrides WAYLAND_DISPLAY, 2. decidedly allows path specs
> to the desired effect so as to use /run/wayland/wayland-N.
WAYLAND_SOCKET is already used. If set, it must be set to an open file
descriptor number for a pre-created Wayland connection, and then it
will override everything else once.
I also don't like introducing more environment variables and I haven't
seen a good reason to do that yet.
> Summary of (individual) proposals follows.
>
> >- modify WAYLAND_DISPLAY to support absolute paths which overrides
> > any search paths
>
> - introduce new WAYLAND_SOCKET
> - modify WAYLAND_DISPLAY to reject '/'
What would be the functional difference to WAYLAND_DISPLAY accepting
absolute paths? Why would a different environment variable make a
difference?
>
> >- when the current socket search fails, add one more place to look
> > in: /run/wayland/$WAYLAND_DISPLAY with the default "wayland-0" if
> > WAYLAND_DISPLAY is not set.
>
> - WAYLAND_SOCKET always overrides WAYLAND_DISPLAY
> - at most, automatic fallback if only if both WAYLAND_SOCKET and
> WAYLAND_DISPLAY are unset
Thanks,
pq
pgpn6Obbo_X2h.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
