Re: New paths for Wayland sockets (Re: Enabling Android-style per application user ids)

2017-11-09 Thread Matt Hoosier 
On Wed, Nov 8, 2017 at 2:16 AM, Pekka Paalanen  wrote:
> On Tue, 7 Nov 2017 19:42:42 +
> Daniel Stone  wrote:
>
>> Hi,
>>
>> On 3 November 2017 at 07:33, Pekka Paalanen  wrote:
>
>> > - 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.
>>
>> NAK. It's a surprising change in behaviour which could catch people
>> who could reasonably expect other behaviour. It doesn't play well with
>> each session expecting to be able to create a socket called
>> 'wayland-0', which is separated by $XDG_RUNTIME_DIR being different
>> per session.
>
> Hi all,
>
> obviously the /run/wayland option would be most useful for systems that
> will only ever run a single graphical session at a time, in other words,
> appliances, maybe tablets and phones, but not traditional/desktop
> computers.
>
> For the record, I'd never have libwayland-server automatically create
> the /run/wayland socket.
>
> It seems concensus is turning towards supporting absolute paths in
> WAYLAND_DISPLAY, with the /run/wayland proposal being more
> controversial. As the latter can be trivially realized with the former,
> I think we have a conclusion: absolute path support is good.
>
> I would hope someone will resurrect the patch and send it, I can review
> it for one, I'd like another maintainer Reviewed-by on it as well, and
> please do explicitly post your Acked-by's for the patch.

Done; see 
https://lists.freedesktop.org/archives/wayland-devel/2017-November/035745.html
.

>
> If the /run/wayland proposal is still really wanted, it can be raised
> again after we have the absolute path support in WAYLAND_DISPLAY.
>
>
> Thanks,
> pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: New paths for Wayland sockets (Re: Enabling Android-style per application user ids)

2017-11-08 Thread Jonas Ådahl 
I'd just like to point out that changing WAYLAND_DISPLAY to be able to
point to a socket using an absolute path is not completely backward
compatible. In the wl_display_connect() documentation it says that

  "The server socket must be placed in XDG_RUNTIME_DIR for
   this function to find it."

Some clients may have used this knowledge for processing before handing
anything over to libwayland-client.

One example of something that will break without further adaptations is
(although not a client) flatpak[0], which currently expose the socket
inside the sandbox by manually binding it.

So, in other words, if this feature is to be added, it must be made
clear what the guarantees regarding backward compatibility may be.


Jonas


[0] 
https://github.com/flatpak/flatpak/blob/a89de9dacc0139a8a9e19828a478a583dafc5c14/common/flatpak-run.c#L2009

On Tue, Nov 07, 2017 at 10:51:42AM -0600, Matt Hoosier wrote:
> Hi Pekka,
> 
> What do you think is a good amount of time to allow for people to
> respond to your call for acks/nacks?
> 
> On Fri, Nov 3, 2017 at 8:33 AM, Carsten Haitzler  wrote:
> > On Fri, 3 Nov 2017 12:47:39 +0200 Pekka Paalanen  said:
> >
> >> On Fri, 3 Nov 2017 11:04:27 +0100 (CET)
> >> Jan Engelhardt  wrote:
> >>
> >> > On Friday 2017-11-03 10:37, Pekka Paalanen wrote:
> >> > >
> >> > >> 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?
> >> >
> >> > Well because you cannot establish for certain that people have or have 
> >> > not
> >> > already used WAYLAND_DISPLAY=/newsock in the concatenation sense.
> >> >
> >> > (Depending on who you ask and how much weight they give to it,
> >> > breaking application interfaces is out of the question. That's all.)
> >>
> >> Ah, that, ok. I thought this was about the security stuff you referred
> >> to. But given the same rationale, we cannot forbid / in WAYLAND_DISPLAY
> >> either.
> >
> > security here i think is a red herring. i can effectively trick
> > libwayland-client to connect to an abs path by setting XDG_RUNTIME_DIR AND
> > WAYLAND_DISPLAY. so ... effectively same thing. it will force all xdg 
> > runtime
> > stuff to be in that same dir... but i think abs path for wl display
> > specifically being a security issue is a red herring, unless there is 
> > something
> > none of us can think of. then we have the problem already with runtime dir 
> > env
> > var and wl display too like above.
> >
> > --
> > - Codito, ergo sum - "I code, therefore I am" --
> > Carsten Haitzler - ras...@rasterman.com
> >
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: New paths for Wayland sockets (Re: Enabling Android-style per application user ids)

2017-11-08 Thread Pekka Paalanen 
On Tue, 7 Nov 2017 19:42:42 +
Daniel Stone  wrote:

> Hi,
> 
> On 3 November 2017 at 07:33, Pekka Paalanen  wrote:

> > - 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.  
> 
> NAK. It's a surprising change in behaviour which could catch people
> who could reasonably expect other behaviour. It doesn't play well with
> each session expecting to be able to create a socket called
> 'wayland-0', which is separated by $XDG_RUNTIME_DIR being different
> per session.

Hi all,

obviously the /run/wayland option would be most useful for systems that
will only ever run a single graphical session at a time, in other words,
appliances, maybe tablets and phones, but not traditional/desktop
computers.

For the record, I'd never have libwayland-server automatically create
the /run/wayland socket.

It seems concensus is turning towards supporting absolute paths in
WAYLAND_DISPLAY, with the /run/wayland proposal being more
controversial. As the latter can be trivially realized with the former,
I think we have a conclusion: absolute path support is good.

I would hope someone will resurrect the patch and send it, I can review
it for one, I'd like another maintainer Reviewed-by on it as well, and
please do explicitly post your Acked-by's for the patch.

If the /run/wayland proposal is still really wanted, it can be raised
again after we have the absolute path support in WAYLAND_DISPLAY.


Thanks,
pq


pgpkQuQIW3sgF.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: New paths for Wayland sockets (Re: Enabling Android-style per application user ids)

2017-11-07 Thread Daniel Stone 
Hi,

On 3 November 2017 at 07:33, Pekka Paalanen  wrote:
> People, give your acks/nacks for the two ideas, please:
>
> - modify WAYLAND_DISPLAY to support absolute paths which overrides
>   any search paths

ACK. It's a simple and straightforward extension to current behaviour.

> - 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.

NAK. It's a surprising change in behaviour which could catch people
who could reasonably expect other behaviour. It doesn't play well with
each session expecting to be able to create a socket called
'wayland-0', which is separated by $XDG_RUNTIME_DIR being different
per session.

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: New paths for Wayland sockets (Re: Enabling Android-style per application user ids)

2017-11-07 Thread Martin Flöser 

Am 2017-11-03 08:33, schrieb Pekka Paalanen:

People, give your acks/nacks for the two ideas, please:

- modify WAYLAND_DISPLAY to support absolute paths which overrides
  any search paths


+ 1 - like others expressed: I don't see any security issues with that. 
Allowing an absolute path makes sense IMHO.


- 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.


+1

Cheers
Martin
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: New paths for Wayland sockets (Re: Enabling Android-style per application user ids)

2017-11-07 Thread Matt Hoosier 
Hi Pekka,

What do you think is a good amount of time to allow for people to
respond to your call for acks/nacks?

On Fri, Nov 3, 2017 at 8:33 AM, Carsten Haitzler  wrote:
> On Fri, 3 Nov 2017 12:47:39 +0200 Pekka Paalanen  said:
>
>> On Fri, 3 Nov 2017 11:04:27 +0100 (CET)
>> Jan Engelhardt  wrote:
>>
>> > On Friday 2017-11-03 10:37, Pekka Paalanen wrote:
>> > >
>> > >> 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?
>> >
>> > Well because you cannot establish for certain that people have or have not
>> > already used WAYLAND_DISPLAY=/newsock in the concatenation sense.
>> >
>> > (Depending on who you ask and how much weight they give to it,
>> > breaking application interfaces is out of the question. That's all.)
>>
>> Ah, that, ok. I thought this was about the security stuff you referred
>> to. But given the same rationale, we cannot forbid / in WAYLAND_DISPLAY
>> either.
>
> security here i think is a red herring. i can effectively trick
> libwayland-client to connect to an abs path by setting XDG_RUNTIME_DIR AND
> WAYLAND_DISPLAY. so ... effectively same thing. it will force all xdg runtime
> stuff to be in that same dir... but i think abs path for wl display
> specifically being a security issue is a red herring, unless there is 
> something
> none of us can think of. then we have the problem already with runtime dir env
> var and wl display too like above.
>
> --
> - Codito, ergo sum - "I code, therefore I am" --
> Carsten Haitzler - ras...@rasterman.com
>
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: New paths for Wayland sockets (Re: Enabling Android-style per application user ids)

2017-11-03 Thread Carsten Haitzler 
On Fri, 3 Nov 2017 12:47:39 +0200 Pekka Paalanen  said:

> On Fri, 3 Nov 2017 11:04:27 +0100 (CET)
> Jan Engelhardt  wrote:
> 
> > On Friday 2017-11-03 10:37, Pekka Paalanen wrote:
> > >  
> > >> 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?  
> > 
> > Well because you cannot establish for certain that people have or have not
> > already used WAYLAND_DISPLAY=/newsock in the concatenation sense.
> > 
> > (Depending on who you ask and how much weight they give to it,
> > breaking application interfaces is out of the question. That's all.)
> 
> Ah, that, ok. I thought this was about the security stuff you referred
> to. But given the same rationale, we cannot forbid / in WAYLAND_DISPLAY
> either.

security here i think is a red herring. i can effectively trick
libwayland-client to connect to an abs path by setting XDG_RUNTIME_DIR AND
WAYLAND_DISPLAY. so ... effectively same thing. it will force all xdg runtime
stuff to be in that same dir... but i think abs path for wl display
specifically being a security issue is a red herring, unless there is something
none of us can think of. then we have the problem already with runtime dir env
var and wl display too like above.

-- 
- Codito, ergo sum - "I code, therefore I am" --
Carsten Haitzler - ras...@rasterman.com

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: New paths for Wayland sockets (Re: Enabling Android-style per application user ids)

2017-11-03 Thread Carsten Haitzler 
On Fri, 3 Nov 2017 11:04:27 +0100 (CET) Jan Engelhardt  said:

> 
> On Friday 2017-11-03 10:37, Pekka Paalanen wrote:
> >
> >> 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?
> 
> Well because you cannot establish for certain that people have or have not
> already used WAYLAND_DISPLAY=/newsock in the concatenation sense.

I think we can just call it a day on this and say anyone doing that has
fundamentally made a mistake. They likely dont set the env var at all in fact.
Wayland is still young and I think can tolerate this kind of change at this
point.

> (Depending on who you ask and how much weight they give to it,
> breaking application interfaces is out of the question. That's all.)
> 


-- 
- Codito, ergo sum - "I code, therefore I am" --
Carsten Haitzler - ras...@rasterman.com

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: New paths for Wayland sockets (Re: Enabling Android-style per application user ids)

2017-11-03 Thread Matt Hoosier 
On Fri, Nov 3, 2017 at 2:33 AM, Pekka Paalanen  wrote:

> On Thu, 2 Nov 2017 10:07:28 -0500
> Matt Hoosier  wrote:
>
> > On Thu, Nov 2, 2017 at 3:36 AM, Pekka Paalanen 
> wrote:
> >
> > > On Wed, 1 Nov 2017 13:44:50 -0500
> > > Matt Hoosier  wrote:
> > >
>
> > > >
> > > > I'd like to call attention to another difficult point in using
> Wayland
> > > for
> > > > a device that acts more like a smartphone than a desktop: the main
> logic
> > > in
> > > > wl_display_connect() always attempts to contact a server socket
> living at
> > > > ${XDG_RUNTIME_DIR}/${WAYLAND_DISPLAY}.
>
> Hi,
>
> I rewrote the subject with the hope to raise more interest, being more
> specific to the proposals.
>
> > > I can also imagine that it may not be feasible to create
> > > application-user specific sockets for everything, so there could be a
> > > need for a common socket file somewhere that is not tied to
> > > XDG_RUNTIME_DIR. With a good justification written down, I suspect that
> > > would be fine. We just need to figure out the details on how to do that
> > > exactly.
> > >
> > > 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///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.
>
> While this is true, I don't think it is a blocker unless we find out
> about tools or compositors doing so.
>
>
> > >
> > > The suggestion to automatically look for a fallback socket
> > > at /run/wayland/$WAYLAND_DISPLAY with WAYLAND_DISPLAY defaulting to
> > > "wayland-0" sounds reasonable to me, but I wouldn't feel comfortable
> > > making that review decision alone. I do know some people are eager to
> > > avoid mandatory environment variables if something can be found by
> > > convention.
> > >
> >
> > This would be my preference. The fewer tweaks to environment variables
> are
> > required, the better in my opinion.
>
> Right, so there is interest to both ideas, and I don't see them as
> mutually exclusive. I believe one can also write a good justification
> for each, so now I'd be looking for reasons why either might be
> unwanted and acks for each, so that we get enough buy-in to "bless" the
> behavioural change in libwayland-client.
>
> People, give your acks/nacks for the two ideas, please:
>
> - modify WAYLAND_DISPLAY to support absolute paths which overrides
>   any search paths
>

Ack


>
> - 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.
>

Ack


>
>
> The last proposed patch for absolute paths is probably
> https://lists.freedesktop.org/archives/wayland-devel/2015-
> August/023838.html
> which looks like it got ignored mostly because of a damaged submission
> and mixed-up (nowadays probably unwanted) server-side changes. The
> patch also lacks the rationale and justification in the commit message.
> This is FYI for anyone wanting to carry on that work.
>
>
> Thanks,
> pq
>
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: New paths for Wayland sockets (Re: Enabling Android-style per application user ids)

2017-11-03 Thread Jan Engelhardt 

On Friday 2017-11-03 10:37, Pekka Paalanen wrote:
>
>> 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?

Well because you cannot establish for certain that people have or have not
already used WAYLAND_DISPLAY=/newsock in the concatenation sense.

(Depending on who you ask and how much weight they give to it,
breaking application interfaces is out of the question. That's all.)
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: New paths for Wayland sockets (Re: Enabling Android-style per application user ids)

2017-11-03 Thread Pekka Paalanen 
On Fri, 3 Nov 2017 09:38:46 +0100 (CET)
Jan Engelhardt  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///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

Re: New paths for Wayland sockets (Re: Enabling Android-style per application user ids)

2017-11-03 Thread Carsten Haitzler 
On Fri, 3 Nov 2017 09:38:46 +0100 (CET) Jan Engelhardt  said:

> 
> 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///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.
> 
> 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.)
> 
> 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.

how would it be "attacked"? so if we're just talking regular user processes,
the user would be exploiting themselves. right? by telling the app to connect
to another socket and compositor (fake or real... does it matter?). if it's a
setuid process (privileged) then ... this env var will not exist when run (or
should not and any other environment that escalates privilege should set or
sanitize this env var). yes - they could let's say create a proxy cpmpositor
and listen to all i/o or even fake it... but they can do this withotu full
paths. just

export WAYLAND_DISPLAY=myproxy

and have $XDG_RUNTIME_DIR/myproxy be your "exploit" server proxy. it can be
done without full paths. xdg runtime dir is intended to be written and used by
the user... you could make this dir not writable by any user launching
processes... this would be entirely strange and probably break a lot of apps
that have ipc sockets. ?

i'm curious as to the attack vector.

> 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 '/'
> 
> >- 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
> 


-- 
- Codito, ergo sum - "I code, therefore I am" --
Carsten Haitzler - ras...@rasterman.com

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: New paths for Wayland sockets (Re: Enabling Android-style per application user ids)

2017-11-03 Thread Jan Engelhardt 

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///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.

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.)

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.


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 '/'

>- 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
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: New paths for Wayland sockets (Re: Enabling Android-style per application user ids)

2017-11-03 Thread Carsten Haitzler 
On Fri, 3 Nov 2017 09:33:47 +0200 Pekka Paalanen  said:

> The last proposed patch for absolute paths is probably
> https://lists.freedesktop.org/archives/wayland-devel/2015-August/023838.html
> which looks like it got ignored mostly because of a damaged submission
> and mixed-up (nowadays probably unwanted) server-side changes. The
> patch also lacks the rationale and justification in the commit message.
> This is FYI for anyone wanting to carry on that work.

I pretty much like that patch as-is. It's simple. It does the job.

-- 
- Codito, ergo sum - "I code, therefore I am" --
Carsten Haitzler - ras...@rasterman.com

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

RE: New paths for Wayland sockets (Re: Enabling Android-style per application user ids)

2017-11-03 Thread Ucan, Emre (ADITG/ESB) 
Hi,

I am for supporting absolute paths with WAYLAND_DISPLAY. Because this approach 
is more flexible than other proposal.

Best regards

Emre Ucan
Engineering Software Base (ADITG/ESB)

Tel. +49 5121 49 6937

> -Original Message-
> From: wayland-devel [mailto:wayland-devel-
> boun...@lists.freedesktop.org] On Behalf Of Pekka Paalanen
> Sent: Freitag, 3. November 2017 08:34
> To: Matt Hoosier; wayland mailing list
> Cc: Davide Bettio; Martin Flöser; Jonas Ådahl; ras...@rasterman.com
> Subject: New paths for Wayland sockets (Re: Enabling Android-style per
> application user ids)
> 
> On Thu, 2 Nov 2017 10:07:28 -0500
> Matt Hoosier <matt.hoos...@gmail.com> wrote:
> 
> > On Thu, Nov 2, 2017 at 3:36 AM, Pekka Paalanen <ppaala...@gmail.com>
> wrote:
> >
> > > On Wed, 1 Nov 2017 13:44:50 -0500
> > > Matt Hoosier <matt.hoos...@gmail.com> wrote:
> > >
> 
> > > >
> > > > I'd like to call attention to another difficult point in using Wayland
> > > for
> > > > a device that acts more like a smartphone than a desktop: the main
> logic
> > > in
> > > > wl_display_connect() always attempts to contact a server socket living
> at
> > > > ${XDG_RUNTIME_DIR}/${WAYLAND_DISPLAY}.
> 
> Hi,
> 
> I rewrote the subject with the hope to raise more interest, being more
> specific to the proposals.
> 
> > > I can also imagine that it may not be feasible to create
> > > application-user specific sockets for everything, so there could be a
> > > need for a common socket file somewhere that is not tied to
> > > XDG_RUNTIME_DIR. With a good justification written down, I suspect
> that
> > > would be fine. We just need to figure out the details on how to do that
> > > exactly.
> > >
> > > 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///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.
> 
> While this is true, I don't think it is a blocker unless we find out
> about tools or compositors doing so.
> 
> 
> > >
> > > The suggestion to automatically look for a fallback socket
> > > at /run/wayland/$WAYLAND_DISPLAY with WAYLAND_DISPLAY
> defaulting to
> > > "wayland-0" sounds reasonable to me, but I wouldn't feel comfortable
> > > making that review decision alone. I do know some people are eager to
> > > avoid mandatory environment variables if something can be found by
> > > convention.
> > >
> >
> > This would be my preference. The fewer tweaks to environment variables
> are
> > required, the better in my opinion.
> 
> Right, so there is interest to both ideas, and I don't see them as
> mutually exclusive. I believe one can also write a good justification
> for each, so now I'd be looking for reasons why either might be
> unwanted and acks for each, so that we get enough buy-in to "bless" the
> behavioural change in libwayland-client.
> 
> People, give your acks/nacks for the two ideas, please:
> 
> - modify WAYLAND_DISPLAY to support absolute paths which overrides
>   any search paths
> 
> - 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.
> 
> 
> The last proposed patch for absolute paths is probably
> https://lists.freedesktop.org/archives/wayland-devel/2015-
> August/023838.html
> which looks like it got ignored mostly because of a damaged submission
> and mixed-up (nowadays probably unwanted) server-side changes. The
> patch also lacks the rationale and justification in the commit message.
> This is FYI for anyone wanting to carry on that work.
> 
> 
> Thanks,
> pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

New paths for Wayland sockets (Re: Enabling Android-style per application user ids)

2017-11-03 Thread Pekka Paalanen 
On Thu, 2 Nov 2017 10:07:28 -0500
Matt Hoosier  wrote:

> On Thu, Nov 2, 2017 at 3:36 AM, Pekka Paalanen  wrote:
> 
> > On Wed, 1 Nov 2017 13:44:50 -0500
> > Matt Hoosier  wrote:
> >  

> > >
> > > I'd like to call attention to another difficult point in using Wayland  
> > for  
> > > a device that acts more like a smartphone than a desktop: the main logic  
> > in  
> > > wl_display_connect() always attempts to contact a server socket living at
> > > ${XDG_RUNTIME_DIR}/${WAYLAND_DISPLAY}.

Hi,

I rewrote the subject with the hope to raise more interest, being more
specific to the proposals.

> > I can also imagine that it may not be feasible to create
> > application-user specific sockets for everything, so there could be a
> > need for a common socket file somewhere that is not tied to
> > XDG_RUNTIME_DIR. With a good justification written down, I suspect that
> > would be fine. We just need to figure out the details on how to do that
> > exactly.
> >
> > 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///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.

While this is true, I don't think it is a blocker unless we find out
about tools or compositors doing so.


> >
> > The suggestion to automatically look for a fallback socket
> > at /run/wayland/$WAYLAND_DISPLAY with WAYLAND_DISPLAY defaulting to
> > "wayland-0" sounds reasonable to me, but I wouldn't feel comfortable
> > making that review decision alone. I do know some people are eager to
> > avoid mandatory environment variables if something can be found by
> > convention.
> >  
> 
> This would be my preference. The fewer tweaks to environment variables are
> required, the better in my opinion.

Right, so there is interest to both ideas, and I don't see them as
mutually exclusive. I believe one can also write a good justification
for each, so now I'd be looking for reasons why either might be
unwanted and acks for each, so that we get enough buy-in to "bless" the
behavioural change in libwayland-client.

People, give your acks/nacks for the two ideas, please:

- modify WAYLAND_DISPLAY to support absolute paths which overrides
  any search paths

- 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.


The last proposed patch for absolute paths is probably
https://lists.freedesktop.org/archives/wayland-devel/2015-August/023838.html
which looks like it got ignored mostly because of a damaged submission
and mixed-up (nowadays probably unwanted) server-side changes. The
patch also lacks the rationale and justification in the commit message.
This is FYI for anyone wanting to carry on that work.


Thanks,
pq


pgpt6FK6_f4gW.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel