FS#28876 - [cdemu-daemon] 1.5.0-2 has multiple issues

Attached to Project: Community Packages
Opened by Alexander (AlexanderR) - Monday, 12 March 2012, 11:41 GMT
Last edited by Ray Rashif (schivmeister) - Tuesday, 03 April 2012, 17:36 GMT
Task Type Bug Report
Category Packages: Testing
Status Closed
Assigned To Ray Rashif (schivmeister)
Architecture All
Severity High
Priority Normal
Reported Version
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 2
Private No

Details

Description:
1) Package adds files to /usr/lib/modules-load.d to trigger module autoload (something that is usually left to user's choise in Arch)
3) Value "system" should be hard-coded in /etc/rc.d/cdemud, this option should be removed from /etc/conf.d/cdemud (/etc/rc.d daemon, running early with root rights should generally use system message bus)
4) Add alsa-lib and libpulse to optdepends and remove message regarding alsa-lib from post_install
5) Blank cdemu-daemon-system.sh and/or replace it's contents with check for daemon state (via "rc.d list") printing appropriate error message in case daemon is not launched.
6) Supress libao message about missing libs by adding something like "&>/dev/null" to cdemud startup line in init-script
7) libdaemon is not a dependency
This task depends upon

Closed by  Ray Rashif (schivmeister)
Tuesday, 03 April 2012, 17:36 GMT
Reason for closing:  Fixed
Comment by Henrik S. (hstokset) - Monday, 12 March 2012, 15:43 GMT
I'd like to add something.

The dbus-glib dependency can be dropped as well.

The vhba udev rules should be moved to the vhba package where it belongs. Also the module _should_ be loaded on boot (disagree on pt.1).

The daemon is also auto-started by dbus when a client application tries to talk to it. So it is not necessary to load it via an init-script. The kernel module will still need to be loaded though. You might be able to cook up a better solution, but keep in mind there's some delay before the module completely initializes.

I also noticed you use the "system bus" mode. This has implications for security (more info in README file) so for normal operation you'd typically want to use the more secure "session bus".
Comment by Alexander (AlexanderR) - Monday, 12 March 2012, 21:19 GMT
2Henrik Stokseth (hstokset)
>> The daemon is also auto-started by dbus when a client application tries to talk to it. So it is not necessary to load it via an init-script.
>> I also noticed you use the "system bus" mode.
You seems to misunderstand something important about present solution. Current /etc/rc.d script can not and should not be more "secure" than session-bus daemon launching, it is here just for convenience of those who prefer to configure everything via /etc/rc.conf. By default daemon is already launched by dbus automatically in session mode (at least by cdemu-client) with configuration in "~/.cdemu-daemon"; this fact should be just documented somewhere.

>> Also the module _should_ be loaded on boot
Yes it should be loaded on boot - by user's explicit decision. I do not want to remove something from /usr/lib/modules-load.d on every package update just to disable automatic loading of module that is not used most of time.
Comment by Henrik S. (hstokset) - Monday, 12 March 2012, 23:34 GMT
@Alexander: Thanks for the clarification, I am obviously not familiar with Arch, just wanted to give a few tips on packaging.

Regarding init-scripts, it would _simplify_ the packaging if you just dropped it. You can still keep a system-wide configuration file in /etc, and source it from either of the wrapper scripts. In the case of session bus, you can allow local config file to override pretty easy.

test -f /etc/cdemud.conf && . /etc/cdemud.conf
test -f ~/.cdemud.conf && . ~/.cdemud.conf

Use-cases for system bus are rare, and its use is discouraged for security reasons. One case worth mentioning is where you are unable or unwilling to run X, since the dbus session bus is IIRC often tied to a user's X login session. If you want to use the daemon in an odd way, it's better to just launch the daemon "manually" and get a warning message. And it would still be possible to auto-start a system bus instance of cdemu via dbus (if it's configured correctly and not disabled), so the init-script isn't really needed either way. If you supply an init script with the package, the user might use it, not knowing it is unsafe.

And you might consider changing from group 'cdemu' to the generic group 'cdrom' assuming it's used on Arch.
Comment by Ray Rashif (schivmeister) - Sunday, 25 March 2012, 08:14 GMT
1. Apparently this is how it is for systemd (you are not affected if you don't use systemd). But since I've made the change I'm not going to revert unless someone requests for it.

2. No need. Let the user change the bus if they want to, even if it's useless.

3. Done. Will be adding libpulse to dep and pulseaudio to optdep.

4. No need. Let it remain there. It _can_ be used if cdemud is run directly with the system bus.

5. No need. This happens when you build against the library and then remove it. We're keeping libpulse.

6. Done. Removed.

7. Will be deprecating the 'cdemu' group in favour of 'optical'.

8. Will be moving the rules to the vhba package.

9. Will be dropping libdaemon and dbus-glib from deps as there's no significant trace of them in the sources.

10. Not going to drop the init script as that's an Arch Way. Will add some info in post-install.
Comment by Alexander (AlexanderR) - Sunday, 25 March 2012, 09:05 GMT
1) Please do not patch packages when it is avoidable.
3) Thanks to libao modularity libpulse is optdep not dep. Cdemu starts without it and it can work without it.
5) Again package works just fine without libpulse. Why should it be dependency?
7) Are these two groups equal? I thought that vhba misuse (e.g. loading of malicious image to gain root access) is potentially bigger vulnerability than access to physical drive.
Comment by Henrik S. (hstokset) - Sunday, 25 March 2012, 10:47 GMT
3 & 5. already said this: it's the LIBAO package that should 1) depend on low-level alsa drivers and 2) optdepend on pulse as well as other audio backends. if the daemon is configured to use the default driver whatever libao is configured to use goes. cdemu-daemon should only depend on libao.

7. well on debian i set up the package to use session bus only and disabled system bus, so using a generic group made more sense. but you're right Alexander, if the package maintainer insists on keeping the initscript it does make sense to keep an exclusive group which no user is member of by default. the reason is that (using system bus + root) any group member can snoop on other user's files including the system administrator's files. i'm not sure it can be used for privilege escalation, but it's bad enough.
Comment by Ray Rashif (schivmeister) - Sunday, 25 March 2012, 11:33 GMT
1) Are you a systemd user?

3) Then optdep should still be pulseaudio and not libpulse. Removing libpulse.

5) There will be no libpulse dep in any form in light of (3).

7) No. You bring up a good point against Henrik's recommmendation. Not changing the group then.

But this is a problem: "any group member can snoop on other user's files including the system administrator's files"

So for multi-user systems we'll recommend skipping the init script, or using the session bus with the init script, whichever works.

See new update in testing after a few hours.
Comment by Henrik S. (hstokset) - Sunday, 25 March 2012, 22:01 GMT
Starting to look good.

- Looks like you accidentally removed the glib2 dependency.
- The patch in vhba-module was merged upstream 2011.02.27 so it can be deleted.

noticed there's no gcdemu package, but I spotted two in AUR. any reason it's not in the 'Community' repository yet?
edit: ok, gcdemu's dependency list is an unholy mess. but kidoz' package looks the sanest.
it should depend on python2-gobject and the GIR (gobject introspection repository) stuff for glib2, gtk3, notify. not sure how it's packaged on arch.
it should not depend on python-notify, dbus-python, python-gconf, python-rsvg, python2-gobject2, pygtk.
and at least on debian depending on the GIR stuff makes it indirectly depend on gobject, gtk3, glib2 and notify, so on that distro they don't show up in the dependency list.

Comment by Ray Rashif (schivmeister) - Monday, 26 March 2012, 09:01 GMT
glib2 is provided for by many of the dependencies, including libmirage.

Yes, the patch is not being applied. It's probably a leftover.

gcdemu is not in the repos because no packager has taken an interest yet.

Anyway, I'm going to revert the change to /usr/lib/modules-load.d and put back the systemd autoloading. According to systemd maintainers this should be the way towards a proper systemd infrastructure. If it exists, load it. I'm not a systemd user so you may want to question the systemd guys on the public lists if it bothers you too much.
Comment by Alexander (AlexanderR) - Monday, 02 April 2012, 03:14 GMT
Add "cdemu" group in post_install with "groupadd cdemu" and remove it in post_remove.
Comment by Ray Rashif (schivmeister) - Monday, 02 April 2012, 05:51 GMT
The group is created by vhba-module now. If there's no other issue I'll close this.
Comment by Alexander (AlexanderR) - Monday, 02 April 2012, 10:50 GMT
>> The group is created by vhba-module now
I am currently not on testing so I did not notice. Please close this issue and if possible move appropriate packages to stable repository.
Comment by Ray Rashif (schivmeister) - Tuesday, 03 April 2012, 17:36 GMT
The change was made in -5, which has been in extra for many days already. [1] -6 was pushed to testing as rebuild for linux 3.3 only. Anyway, thanks to everyone. Closing.

[1] http://projects.archlinux.org/svntogit/community.git/commit/trunk?h=packages/vhba-module&id=efdcfa3bf0bbc676b52d775018ab47748a5ac217

Loading...