greeter user permissions are not enough to talk with seatd

Done

Details

3 participants

Liliana Marie Prikler
Liliana Marie Prikler
muradm

Owner: unassigned

Submitted by: muradm

Severity: normal

Blocked by

muradm wrote on 4 Aug 2022 11:45

Recipients:(address . bug-guix@gnu.org)

Message-ID:87czdg2unf.fsf@muradm.net

Hi,

As per discussion here:

https://lists.gnu.org/archive/html/guix-devel/2022-08/msg00020.html

Above change reduced permissions of greeter user.

While it is ok for greeters that do not talk to seatd,

greeters talking to seatd lost access to seatd socket.

As result, greeter (e.g. gtkgreet) requiring communication

with seatd is failing to start, causing "black screen"

behavior on active terminal (switching to the other non seatd

related terminal is possible, for manual permissions

adjustment as workaround).

To address this issue, we need more flexible control over

seatd user/group, which creates seatd.sock, and greeter user

which connects to seatd.sock.

Other distros (Arch for instance) introduced "seat" group.

So user which wants to login on system controlled by seatd

should be member of that group.

However, not all greeters require that, so I decided to make

more flexible. Propsed solutions consists of:

* 56690 - gnu: seatd-service-type: Should use seat group.

With this change, if seatd-service-type is present in the

system configuration, "seat" group will be added, and seatd

will run as root/seat. Group is configurable, but default is

"seat".

* 56699 - gnu: greetd-service-type: Add greeter-extra-groups

config field.

With this change, if user wants to use seatd-service-type with

greeter requiring seatd.sock, he can add "seat" group to

greeter-extra-groups field.

Thanks in advance,

muradm

Liliana Marie Prikler wrote on 4 Aug 2022 13:08

Recipients:(address . control@debbugs.gnu.org)

Message-ID:b5687a1a3eebc0cce2564634bc4e191cf7abd931.camel@ist.tugraz.at

block 56971 by 56690 56699

thanks

Hi muradm,

Am Donnerstag, dem 04.08.2022 um 12:45 +0300 schrieb muradm:

Toggle quote (9 lines)> [...] greeter (e.g. gtkgreet) requiring communication
> with seatd is failing to start, causing "black screen"
> behavior on active terminal (switching to the other non seatd
> related terminal is possible, for manual permissions
> adjustment as workaround).
> 
> To address this issue, we need more flexible control over
> seatd user/group, which creates seatd.sock, and greeter user
> which connects to seatd.sock.

Okay.

Toggle quote (2 lines)

> However, not all greeters require that, so I decided to make

> more flexible.

Flexibility for its own sake is not always the right solution. On the

other hand, looking at the two patches, it appears they are to be used

in combination?

Toggle quote (7 lines)>  Propsed solutions consists of:
> 
> * 56690 - gnu: seatd-service-type: Should use seat group.
> With this change, if seatd-service-type is present in the
> system configuration, "seat" group will be added, and seatd
> will run as root/seat. Group is configurable, but default is 
> "seat".

Why just the group and no user? Is it not possible to launch seatd as

non-root?

Toggle quote (5 lines)> * 56699 - gnu: greetd-service-type: Add greeter-extra-groups 
> � config field.
> With this change, if user wants to use seatd-service-type with
> greeter requiring seatd.sock, he can add "seat" group to
> greeter-extra-groups field.

Note that you still have a TODO on that patch.

Cheers

muradm wrote on 4 Aug 2022 14:52

Recipients:(name . Liliana Marie Prikler)(address . liliana.prikler@ist.tugraz.at)

Message-ID:874jys2m01.fsf@muradm.net

Liliana Marie Prikler <liliana.prikler@ist.tugraz.at> writes:

Toggle quote (4 lines)

> block 56971 by 56690 56699

> thanks

> Hi muradm,

Hi Liliana,

Toggle quote (20 lines)> Am Donnerstag, dem 04.08.2022 um 12:45 +0300 schrieb muradm:
>> [...] greeter (e.g. gtkgreet) requiring communication
>> with seatd is failing to start, causing "black screen"
>> behavior on active terminal (switching to the other non seatd
>> related terminal is possible, for manual permissions
>> adjustment as workaround).
>>
>> To address this issue, we need more flexible control over
>> seatd user/group, which creates seatd.sock, and greeter user
>> which connects to seatd.sock.
> Okay.
>
>> However, not all greeters require that, so I decided to make
>> more flexible.
> Flexibility for its own sake is not always the right solution. 
> On the
> other hand, looking at the two patches, it appears they are to 
> be used
> in combination?
>

No, technically they are not strongly dependent on each other,

could be applied one after another in no particular order.

After both are applied, in cooperation they address this issue.

Toggle quote (10 lines)>>  Propsed solutions consists of:
>>
>> * 56690 - gnu: seatd-service-type: Should use seat group.
>> With this change, if seatd-service-type is present in the
>> system configuration, "seat" group will be added, and seatd
>> will run as root/seat. Group is configurable, but default is
>> "seat".
> Why just the group and no user?  Is it not possible to launch 
> seatd as
> non-root?

seatd provides a way for display servers to access input/output 
devices
without having to be root. So seatd it self has to run as root.
When seatd opening socket as root/seat, all members of seat would
be able to communicate with it. Also socket could be opened with
seat/seat for instance, but there is no specific point in doing 
so.
Will be one more unused system user around.
Arch seems to follow similar way, root/seat is ok for socket.
Also will signal that seatd is running as root.

Toggle quote (6 lines)>> * 56699 - gnu: greetd-service-type: Add greeter-extra-groups
>> � config field.
>> With this change, if user wants to use seatd-service-type with
>> greeter requiring seatd.sock, he can add "seat" group to
>> greeter-extra-groups field.
> Note that you still have a TODO on that patch.

That TODO is from the initial commit, it is about cgroup file

system mounting, and totally out of scope of this issue.

Toggle quote (1 lines)

> Cheers

Thanks in advance

Liliana Marie Prikler wrote on 5 Aug 2022 08:11

Recipients:(name . muradm)(address . mail@muradm.net)

Message-ID:f87faa9307a2e57dd18cebfe93559b3261171695.camel@ist.tugraz.at

Am Donnerstag, dem 04.08.2022 um 15:52 +0300 schrieb muradm:

Toggle quote (9 lines)> 
> Liliana Marie Prikler <liliana.prikler@ist.tugraz.at> writes:
> 
> > [...] [L]ooking at the two patches, it appears they are to 
> > be used in combination?
> > 
> No, technically they are not strongly dependent on each other,
> could be applied one after another in no particular order.
> After both are applied, in cooperation they address this issue.

This is what I'm saying, albeit in different words. As far as I

understand, neither of these patches really accomplishes anything if

not put together. Thus, you more or less opened three issues to

address one.

Toggle quote (2 lines)

> >

> seatd it self has to run as root.

Okay.

Toggle quote (2 lines)

> That TODO is from the initial commit, it is about cgroup file

> system mounting, and totally out of scope of this issue.

I didn't mean your code, I meant a suggestion from a reviewer that you

haven't addressed yet (to my knowledge at least).

Cheers

muradm wrote on 5 Aug 2022 08:48

Recipients:(name . Liliana Marie Prikler)(address . liliana.prikler@ist.tugraz.at)

Message-ID:87r11v18mk.fsf@muradm.net

Liliana Marie Prikler <liliana.prikler@ist.tugraz.at> writes:

Toggle quote (15 lines)> Am Donnerstag, dem 04.08.2022 um 15:52 +0300 schrieb muradm:
>>
>> Liliana Marie Prikler <liliana.prikler@ist.tugraz.at> writes:
>>
>> > [...] [L]ooking at the two patches, it appears they are to
>> > be used in combination?
>> >
>> No, technically they are not strongly dependent on each other,
>> could be applied one after another in no particular order.
>> After both are applied, in cooperation they address this issue.
> This is what I'm saying, albeit in different words.  As far as I
> understand, neither of these patches really accomplishes 
> anything if
> not put together.  Thus, you more or less opened three issues to
> address one.

Really I don't know what to comment here else. My analysis showed
two independent issues, one is that seatd should have a declared
group so that users of it could join it. This issues is not 
specific
to greetd/greeter in any way. Any other greeting mechanism could
fall short on this. And second, greeter today required conditional
group to interact with seatd, or it could be any other group like
input, usb, modem or else depending on user setup.
Solutions are offered accordingly. Third issue, this bug I was
asked to open. I don't understand, is it a sin to have multiple
issues, or what is the problem here?

Toggle quote (10 lines)>
>> >
>> seatd it self has to run as root.
> Okay.
>
>> That TODO is from the initial commit, it is about cgroup file
>> system mounting, and totally out of scope of this issue.
> I didn't mean your code, I meant a suggestion from a reviewer 
> that you
> haven't addressed yet (to my knowledge at least).

done

Toggle quote (2 lines)

> Cheers

Liliana Marie Prikler wrote on 5 Aug 2022 10:04

Recipients:(name . muradm)(address . mail@muradm.net)(address . 56971@debbugs.gnu.org)

Message-ID:128b8b30d46a357318bad98c3f11bceb8adbb8ca.camel@ist.tugraz.at

Am Freitag, dem 05.08.2022 um 09:48 +0300 schrieb muradm:

Toggle quote (23 lines)> 
> Liliana Marie Prikler <liliana.prikler@ist.tugraz.at> writes:
> 
> > Am Donnerstag, dem 04.08.2022 um 15:52 +0300 schrieb muradm:
> > > 
> > > Liliana Marie Prikler <liliana.prikler@ist.tugraz.at> writes:
> > > 
> > > > [...] [L]ooking at the two patches, it appears they are to
> > > > be used in combination?
> > > > 
> > > No, technically they are not strongly dependent on each other,
> > > could be applied one after another in no particular order.
> > > After both are applied, in cooperation they address this issue.
> > This is what I'm saying, albeit in different words.� As far as I
> > understand, neither of these patches really accomplishes 
> > anything if
> > not put together.� Thus, you more or less opened three issues to
> > address one.
> Really I don't know what to comment here else.  My analysis showed
> two independent issues, one is that seatd should have a declared
> group so that users of it could join it.  This issues is not 
> specific to greetd/greeter in any way.  Any other greeting mechanism
> could fall short on this.

But it is greetd that does, no? If there are other greeting mechanisms

currently packaged in Guix falling short of this, please do tell.

Toggle quote (6 lines)> And second, greeter today required conditional group to interact with
> seatd, or it could be any other group like input, usb, modem or else
> depending on user setup.
> Solutions are offered accordingly. Third issue, this bug I was
> asked to open. I don't understand, is it a sin to have multiple
> issues, or what is the problem here?

It is not really a problem, but an observation.  I personally think a
single series that has both patches would have been more visible than
this thing split across three topics for two patches.  There is a small
overhead if you have to consider merges and blocks, even if debbugs
supports them.

Cheers

muradm wrote on 7 Aug 2022 22:48

Recipients:(name . Liliana Marie Prikler)(address . liliana.prikler@ist.tugraz.at)(address . 56971@debbugs.gnu.org)

Message-ID:878rnz22to.fsf@muradm.net

Liliana Marie Prikler <liliana.prikler@ist.tugraz.at> writes:

Toggle quote (37 lines)> Am Freitag, dem 05.08.2022 um 09:48 +0300 schrieb muradm:
>>
>> Liliana Marie Prikler <liliana.prikler@ist.tugraz.at> writes:
>>
>> > Am Donnerstag, dem 04.08.2022 um 15:52 +0300 schrieb muradm:
>> > >
>> > > Liliana Marie Prikler <liliana.prikler@ist.tugraz.at> 
>> > > writes:
>> > >
>> > > > [...] [L]ooking at the two patches, it appears they are 
>> > > > to
>> > > > be used in combination?
>> > > >
>> > > No, technically they are not strongly dependent on each 
>> > > other,
>> > > could be applied one after another in no particular order.
>> > > After both are applied, in cooperation they address this 
>> > > issue.
>> > This is what I'm saying, albeit in different words.� As far 
>> > as I
>> > understand, neither of these patches really accomplishes
>> > anything if
>> > not put together.� Thus, you more or less opened three issues 
>> > to
>> > address one.
>> Really I don't know what to comment here else.  My analysis 
>> showed
>> two independent issues, one is that seatd should have a 
>> declared
>> group so that users of it could join it.  This issues is not
>> specific to greetd/greeter in any way.  Any other greeting 
>> mechanism
>> could fall short on this.
> But it is greetd that does, no?  If there are other greeting 
> mechanisms
> currently packaged in Guix falling short of this, please do 
> tell.

Point is not that "there are any/others affected", the point is,

that

seatd is providing and interface, and currently it has a problem,

which is wrong permission.

Toggle quote (17 lines)>> And second, greeter today required conditional group to 
>> interact with
>> seatd, or it could be any other group like input, usb, modem or 
>> else
>> depending on user setup.
>> Solutions are offered accordingly. Third issue, this bug I was
>> asked to open. I don't understand, is it a sin to have multiple
>> issues, or what is the problem here?
> It is not really a problem, but an observation.  I personally 
> think a
> single series that has both patches would have been more visible 
> than
> this thing split across three topics for two patches.  There is 
> a small
> overhead if you have to consider merges and blocks, even if 
> debbugs
> supports them.

This is phylosophical topic, which I suppose is not in the scope.

Toggle quote (2 lines)

> Cheers

Liliana Marie Prikler wrote on 8 Aug 2022 07:54

Recipients:(name . muradm)(address . mail@muradm.net)(address . 56971@debbugs.gnu.org)

Message-ID:4ae0bdd4719993c471d2d58917fdddf63fcdf710.camel@ist.tugraz.at

Am Sonntag, dem 07.08.2022 um 23:48 +0300 schrieb muradm:

Toggle quote (3 lines)

> Point is not that "there are any/others affected", the point is,

> that seatd is providing [an] interface, and currently it has a

> problem, which is wrong permission.

But there are two sides in this permission issue.  Note that the
subject line of the email is the inverse of what you just said.  Note
further how this issue doesn't require a single, but two patches to
solve, which are currently marked as blockers.  These are obviously
related.

Cheers

Liliana Marie Prikler wrote on 26 Aug 2022 19:06

Recipients:(name . muradm)(address . mail@muradm.net)

Message-ID:400cf1fed0d340398da6e2e0e32bebdb8fd842ef.camel@gmail.com

Am Donnerstag, dem 04.08.2022 um 12:45 +0300 schrieb muradm:

Toggle quote (5 lines)> * 56690 - gnu: seatd-service-type: Should use seat group.
> With this change, if seatd-service-type is present in the
> system configuration, "seat" group will be added, and seatd
> will run as root/seat. Group is configurable, but default is 
> "seat".

I made it so that by default the sanitizer is used to turn the string

"seat" into a group and used (ice-9 match), reducing some needless

redundancy. I also reworded the manual to the best of my ability

following our conversations and adapted the commit message.

Toggle quote (5 lines)> * 56699 - gnu: greetd-service-type: Add greeter-extra-groups 
> � config field.
> With this change, if user wants to use seatd-service-type with
> greeter requiring seatd.sock, he can add "seat" group to
> greeter-extra-groups field.

I fixed some minor issue in the manual and reindented the marionette-

type in the tests, also reworded the commit message.

I didn't get the chance to run the system tests – some timeout causes

the marionette build to fail on my machine – but I verified

independently that at least the seatd socket has the right permissions.

I hope this will be enough for you to get gtkgreet running.

Cheers

Closed

Your comment

This issue is archived.

To comment on this conversation send an email to 56971@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it

mumi current 56971

Then, you may apply the latest patchset in this issue (with sign off)

mumi am -- -s

Or, compose a reply to this issue

mumi compose

Or, send patches to this issue

mumi send-email *.patch

is:open	open issues
is:done	closed issues
submitter:<who>	search issue submitter
author:<who>	search by message author
date:yesterday..now	search by issue date
mdate:3m..2d	search by message date