Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wayland meta issue #134

Open
2 of 5 tasks
01micko opened this issue Oct 29, 2024 · 8 comments
Open
2 of 5 tasks

wayland meta issue #134

01micko opened this issue Oct 29, 2024 · 8 comments

Comments

@01micko
Copy link

01micko commented Oct 29, 2024

Just a TODO list for wayland stuffs

  • link for bunsenlabs-session-wayland - bunsenlabs-session-wayland -> bunsenlabs-session
  • line 15 in bunsen-configs-[base|lite].postinst should be GREETD="/etc/greetd"as inbunsen-configs.postinst` or was there a reason for the omission?
    MAN1='/usr/share/man/man1'
  • Where to put our wayland session desktop file? It has to end up in /usr/share/wayland-sessions
  • Populate wayland/skel - (my job) - then figure out how to incorporate it - I'm guessing @johnraff you have an idea for that.
  • How to handle the dependency on bunsen-configs | bunsen-configs-lite | bunsen-configs-base ?

If/when I think of more I'll add to this list.

@johnraff
Copy link
Member

link for bunsenlabs-session-wayland - bunsenlabs-session-wayland -> bunsenlabs-session

That will go in the addon package bunsen-configs-wayland. (I don't know yet if we will need separate lite/base package variants, but I'm hoping not.) We can either install a physical symlink - there are a few already in bunsen-configs - or put a line in debian/bunsen-configs-wayland.links: /usr/bin/bunsenlabs-session /usr/bin/bunsenlabs-wayland-session Maybe the latter?


line 15 in bunsen-configs-[base|lite].postinst should be GREETD="/etc/greetd"as in bunsen-configs.postinst`

It should indeed - thanks for catching the omission! Now fixed and pushed.
That section still needs attention though - lintian revealed a security issue with a recursive permissions change over a whole directory:

W: bunsen-configs: recursive-privilege-change "chown -R" [postinst:41]
N:
N: The named maintainer script appears to call chmod or chown with a
N: --recursive/-R argument, or it uses find(1) with similar intent.
N:
N: All such uses are vulnerable to hardlink attacks on mainline (i.e.
N: non-Debian) kernels that do not set fs.protected_hardlinks=1.
N:
N: The security risk arises when a non-privileged user set links to files
N: they do not own, such as such as /etc/shadow or files in /var/lib/dpkg/. A
N: superuser's recursive call to chown or chmod on behalf of a role user
N: account would then modify the non-owned files in ways that allow the
N: non-privileged user to manipulate them later.
N:
N: There are several ways to mitigate the issue in maintainer scripts:
N:
N: - For a static role user, please call chown at build time
N: and not during the installation.
N: - If that is too complicated, use runuser(1) in the
N: relevant build parts to create files with correct ownership.
N: - Given a static list of files to change, use non-recursive calls
N: for each file. (Please do not generate the list with find.)
N:
N: Please refer to Bug#895597, Bug#889060, Bug#889488, and the runuser(1)
N: manual page for details.

Do we know in advance the names of the files whose perms need changing? If so then this:

  • Given a static list of files to change, use non-recursive calls for each file.

looks like the way to go.


Where to put our wayland session desktop file? It has to end up in /usr/share/wayland-sessions

As with the bunsenlabs-session-wayland symlink, it will be installed by the (as yet nonexistent) package bunsen-configs-wayland. It should not be installed by bunsen-configs because that would cause a non-functioning boot menu item to appear for users without bunsen-configs-wayland.

As for its location in the source tree, I'm thinking of wayland/ as analagous with the root directory of the full source tree, so how about putting the .desktop file in wayland/ ? Its installed location will be determined by a line in debian/bunsen-configs-wayland.install so it could in principle go anywhere in the source.

The final organization of files in the source tree can be changed if it seems desirable for some reason. There are quite a lot of them though!

NOTE bunsen-configs-wayland will be an addon package that depends on bunsen-configs to provide most of the necessary files. Only the extra wayland-only files will be shipped in bunsen-configs-wayland.


Populate wayland/skel - (my job) - then figure out how to incorporate it - I'm guessing @johnraff you have an idea for that.

Yes, the files in wayland/skel can be installed by bunsen-configs-wayland into /usr/share/bunsen/skel, just like those from bunsen-configs. I don't see there being any files installed by both bunsen-configs-wayland and bunsen-configs?

@01micko
Copy link
Author

01micko commented Oct 29, 2024

I don't know yet if we will need separate lite/base package variants, but I'm hoping not.

Won't at all methinks. A meta package and a netinstall should both be fine dependent on any one of the parent bunsen-configs*

We can either install a physical symlink - there are a few already in bunsen-configs - or put a line in debian/bunsen-configs-wayland.links: /usr/bin/bunsenlabs-session /usr/bin/bunsenlabs-wayland-session Maybe the latter?

I prefer the latter.


line 15 in bunsen-configs-[base|lite].postinst should be GREETD="/etc/greetd"as in bunsen-configs.postinst

That section still needs attention though - lintian revealed a security issue with a recursive permissions change over a whole directory:

Right: should be enough to do the following:

    if [ -d "$GREETD" ] && id _greetd >/dev/null 2>&1
    then
        chown  _greetd:_greetd "$GREETD/greetd.conf"
        usermod -a -G video _greetd
    fi

or even a wildcard should do it: chown _greetd:_greetd "$GREETD/* but

Given a static list of files to change, use non-recursive calls for each file.

"each file" - probably forbidden.

IIRC - we have to do something similar with /var/cache/nwg-hello/cache.json too.


As with the bunsenlabs-session-wayland symlink, it will be installed by the (as yet nonexistent) package bunsen-configs-wayland. It should not be installed by bunsen-configs because that would cause a non-functioning boot menu item to appear for users without bunsen-configs-wayland.

And we don't want that. Ok, so I'll think of something for that.

The final organization of files in the source tree can be changed if it seems desirable for some reason. There are quite a lot of them though!

No need for that. Everthing wayland can go in wayland/ and the debian/bunsen-configs-wayland* files can sort it all out.

NOTE bunsen-configs-wayland will be an addon package that depends on bunsen-configs to provide most of the necessary files. Only the extra wayland-only files will be shipped in bunsen-configs-wayland.

Yes, that makes sense.


Yes, the files in wayland/skel can be installed by bunsen-configs-wayland into /usr/share/bunsen/skel, just like those from bunsen-configs. I don't see there being any files installed by both bunsen-configs-wayland and bunsen-configs?

Yes /usr/shar/bunsen/skel is the target - no problem. All files will have unique names, so no conflicts with parents. Happy children!

@johnraff
Copy link
Member

line 15 in bunsen-configs-[base|lite].postinst should be GREETD="/etc/greetd"as in bunsen-configs.postinst
That section still needs attention though - lintian revealed a security issue with a recursive permissions change over a whole directory:
Right: should be enough to do the following:

    if [ -d "$GREETD" ] && id _greetd >/dev/null 2>&1
    then
        chown  _greetd:_greetd "$GREETD/greetd.conf"
        usermod -a -G video _greetd
    fi

Sure, if greetd.conf is the only file that concerns us, that should be fine.

or even a wildcard should do it: chown _greetd:_greetd "$GREETD/* but

Given a static list of files to change, use non-recursive calls for each file.

"each file" - probably forbidden.

A wild card is effectively the same as the -R chown option. I think the issue is that arbitary files will have their permissions changed, so we need to explicitly name them.

IIRC - we have to do something similar with /var/cache/nwg-hello/cache.json too.

Yes I remember having to do that so the greeter could remember the previous login.
nwg-piotr/nwg-hello#16 (comment)

As with the bunsenlabs-session-wayland symlink, it will be installed by the (as yet nonexistent) package bunsen-configs-wayland. It should not be installed by bunsen-configs because that would cause a non-functioning boot menu item to appear for users without bunsen-configs-wayland.

And we don't want that. Ok, so I'll think of something for that.

I think it's OK. Let bunsen-configs-wayland install bunsenlabs-wayland.desktop in /usr/share/wayland-sessions. The file can go in wayland/ in the source tree.

The final organization of files in the source tree can be changed if it seems desirable for some reason. There are quite a lot of them though!
No need for that. Everthing wayland can go in wayland/ and the debian/bunsen-configs-wayland* files can sort it all out.

That's what I thought.

@johnraff
Copy link
Member

line 15 in bunsen-configs-[base|lite].postinst should be GREETD="/etc/greetd"as in bunsen-configs.postinst
That section still needs attention though - lintian revealed a security issue with a recursive permissions change over a whole directory:
Right: should be enough to do the following:

    if [ -d "$GREETD" ] && id _greetd >/dev/null 2>&1
    then
        chown  _greetd:_greetd "$GREETD/greetd.conf"
        usermod -a -G video _greetd
    fi

Sure, if greetd.conf is the only file that concerns us, that should be fine.

IIRC - we have to do something similar with /var/cache/nwg-hello/cache.json too.

Yes I remember having to do that so the greeter could remember the previous login. nwg-piotr/nwg-hello#16 (comment)

In fact, we ought to be testing for the existence of the file - not just the directory - greetd.conf before trying to operate on it. And if bunsen-configs happens to be installed before greetd then the file won't exist. Since greetd isn't a dependency of bunsen-configs (has no reason to be) then that isn't guaranteed at all. We need to think a bit more about this - at least move that postinst code from bunsen-configs to bunsen-configs-wayland and make greetd a Recommend of bunsen-configs-wayland. (It doesn't have to be a hard Depend because LightDM is an alternative.)

The same all applies to /var/cache/nwg-hello/cache.json too. Actually I don't know if that file is installed at all or created on the fly when running nwg-hello.

Ideally the Debian package maintainers of greetd and nwg-hello should handle these file permissions rather than us. Maybe that will happen in future releases?

@01micko
Copy link
Author

01micko commented Oct 30, 2024

^I was thinking that. And nwg-hello.

Actually I don't know if that file is installed at all or created on the fly when running nwg-hello.

I checked the package, and it is installed by the package.

The thing is, we are the ones who discovered these things, well you for nwg-hello cache and @nwg-piotr about the greetd.conf file.

About /etc/greetd/greetd.conf it is not shipped, it has to be created so it's not overwritten on upgrade. If you edit the original /etc/greetd/config.toml it works but I don't want to take the chance that it won't be overwritten. The debian maintainers can't know what greeter we will use for greetd so we have to handle that.

Since nwg-hwllo does have hard dependency on greetd then perhaps a bug should be filed with debian regarding the ownership of /var/cache/nwg-hello/cache.json

@johnraff
Copy link
Member

johnraff commented Oct 30, 2024

About greetd.conf, I was forgetting that it doesn't come with greetd, but as you say, has to be created. Presumably it will be installed by bunsen-configs-wayland, in which case it's our responsibility and no worries about whether greetd was previously installed or not. Still probably good practice to do a test for its existence before chowning it in .postinst though.

(/var/cache/nwg-hello/cache.json)

And nwg-hello.

Actually I don't know if that file is installed at all or created on the fly when running nwg-hello.

I checked the package, and it is installed by the package.
nwg-hwllo does have hard dependency on greetd

In that case, as you say, it sounds as if nwg-hello should apply the right permissions to cache.json.
Sounds like a Debian issue rather than upstream, since it's Debian who set the daemon name to _greetd.
If there's no sign of movement in the next month or two then yes maybe we should file a bug.

@01micko
Copy link
Author

01micko commented Oct 31, 2024

About greetd.conf, I was forgetting that it doesn't come with greetd, but as you say, has to be created. Presumably it will be installed by bunsen-configs-wayland, in which case it's our responsibility and no worries about whether greetd was previously installed or not. Still probably good practice to do a test for its existence before chowning it in .postinst though.

My emphasis added.

By that rationale, we should remove the greetd stuffs from bunsen-configs[-lite|-base].postinst and create a postinst for bunsen-configs-wayland.postinst. Makes sense anyway.


An aside: how in debian/control should we handle Depends: field when it can depend on bunsen-configs, bunsenconfigs-lite, bunsenconfigs-base? I'm note too concerned with the syntax, I believe its something like:

some_ dep, some_other, dep0 | dep1 | dep2

How does dpkg actually handle that at install time? Does user get a choice in the install process?

@johnraff
Copy link
Member

johnraff commented Oct 31, 2024

About greetd.conf, I was forgetting that it doesn't come with greetd, but as you say, has to be created. Presumably it will be installed by bunsen-configs-wayland, in which case it's our responsibility and no worries about whether greetd was previously installed or not. Still probably good practice to do a test for its existence before chowning it in .postinst though.

My emphasis added.

By that rationale, we should remove the greetd stuffs from bunsen-configs[-lite|-base].postinst and create a postinst for bunsen-configs-wayland.postinst. Makes sense anyway.

Yes, good question. I don't have much time today, but please let me think about that... I seem to remember there might have been a reason to put the greetd stuff in bunsen-configs & friends.


An aside: how in debian/control should we handle Depends: field when it can depend on bunsen-configs, bunsenconfigs-lite, bunsenconfigs-base? I'm note too concerned with the syntax, I believe its something like:

some_ dep, some_other, dep0 | dep1 | dep2

Looks right.

How does dpkg actually handle that at install time? Does user get a choice in the install process?

No user interaction. I think dpkg will check if any of the three is installed, and if not will install the first in the list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants