Re: Proposed changes to wesnoth-1.16 (fixes RC bug and autoremoval in boost1.83 transition)
On 2023-12-29 at 14:55, Simon McVittie wrote:
> On Fri, 29 Dec 2023 at 08:56:16 -0500, P. J. McDermott wrote:
> > In particular, I'd like a review of the systemd and init.d commit, to
> > confirm that games:games is the right user/group
>
> I don't think this is right, thanks for asking for review on this.
>
> The games group is defined in base-passwd and Policy as an appropriate
> group for making older Unix-oriented games setgid games, so that they can
> write to a system-wide high scores list or similar. I don't think running
> a dedicated server is really the same use-case.
>
> There is a games user in base-passwd, but no specific meaning is defined
> for it, which makes me concerned that people will be repurposing this
> username for purposes like "the user I log in as to run Steam".
>
> If running a Wesnoth dedicated server is something we want to support as a
> "first class citizen" use-case in the packaging system, as we do for some
> other games like OpenArena and the Quake series, then I think it should
> be using its own dedicated user/group pair, ideally _wesnoth:_wesnoth
> or something. That way, if there is an exploitable vulnerability in the
> Wesnoth server that lets an attacker run arbitrary code as the server user,
> the attacker will not be able to use that access to interfere with other
> games or other parts of the OS.
>
> OpenArena uses the Debian-openarena user ID, which is part of an older
> naming convention - if I was packaging OpenArena today, it would be running
> as _openarena instead. The relevant Policy wording is:
>
> When maintainers choose a new hardcoded or dynamically generated
> username for packages to use, they should start this username with
> an underscore
> — https://www.debian.org/doc/debian-policy/ch-opersys.html#users-and-groups
>
> (I have not done a more general review of this package and I am unlikely
> to be able to do so any time soon, so please don't block on me.)
Thanks Simon for reviewing this change and correcting my understanding
of Policy on this matter. I agree that would be the best solution. So
postinst/prerm should adduser/deluser.
I wonder how a systemd service file provided by an upstream source
archive is supposed to handle this then, since as I said I proposed a
similar change upstream (which thanks to you I now see should probably
not be merged as-is, though I don't think upstream's current use of
nobody:users is any better, since systemd will even warn about it).
I'll have to try to find some examples of what users/groups other
upstreams use in their provided systemd/init.d/etc. service files.
However, I guess none of my changes need to be reviewed now anyway.
Vincent, it looks like you ignored all of my proposed changes and
just uploaded a bare new upstream release? Granted, it's nice to get
something uploaded quickly since the package will be autoremoved from
testing on Tuesday, and a couple of my proposed changes are incorrect/
inappropriate. But I thought at least the upstream regression patch
backport, DEP-5 conversion and copyright update, removal of embedded JS
library code copies, deprecated debian/*.tmpfile update, and font
symlinks fix would be good to include.
--
Patrick "P. J." McDermott: http://www.pehjota.net/
Lead Developer, ProteanOS: http://www.proteanos.com/
Founder and CEO, Libiquity: http://www.libiquity.com/
Reply to: