beep will not work for unprivileged users from xterms and the likes. The manpage suggests installing the binary suid root to avoid this problem. As there is a security risk involved, and as perhaps not all users should be allowed to call beep freely on any system, this should not necessarily be the default. But a USE flag would be really nice, to avoid having to change permissions whenever beep might be updated. Some administrators might even want to restrict access to this suid root binary to a group, either a beep group specifically for this use, or some existing group like users. I'm trying to think of some way to keep the group settings and world execute permission when remerging. Everything I've come up so far is either really ugly or massive overkill. But if anyone has a good suggestion, let's hear it! Bug 14109 already requested and got 1.2.2 suid root, as far as I can see. It seems this was replaced by a patch from solar in -r1. This patch is called "beep-1.2.2-nosuid.patch" and a comment announces that "we don't have to be suid to work at all". However, as far as I can tell, all the patch does is echo \a without error output. There are a couple of scenarios where this will not work: - cronjobs - terminal emulators with visible bell - systems with deactivated X bell - systems with X bell using the sound card, and the speakers are off - remote users - use of the advanced beep features, like beeping some melody for 5 minutes So while this patch might make life a bit easier and the program a bit more useful on systems without the binary installed suid root, it is in many cases little better than a simple "echo -e \\a". It is in no way a full featured replacement for a suid root installation of beep. So I'd like to get the binary back to chmod 4711 or some such if USE=beep.
*** Bug 168205 has been marked as a duplicate of this bug. ***
Paste from Bug 168205, comment #0 The patch beep-1.2.2-nosuid.patch which is applied for app-misc/beep-1.2.2-r1 has two problems: close(console_fd); does not check console_fd is valid. This causes close to result in EBADF for users without access to /dev/console. Should be no problem, but it's the same situation as with the ioctl for stopping the beep. In both cases the call will fail with EBADF if executed, unchecked, and in both cases a check before the execution is cleaner. The stop ioctl does check, so I believe close should be preceded by an "if (console_fd >= 0)" as well. bell printed to stdout, not stderr. This can cause problems in scripts that use bell, expecting no sideeffects, and redirecting their standard output. Or put differently, using stderr instead would allow the fallback to work even when stdout is redirected. The upstream release has the same problem here, I'll report this upstream as well.
(In reply to comment #1) I'm not sure why Jakub duped this here. bug #168205 was a set of minor problems with the patch, with little real world impact. This here was intended as a feature request for making beep suid root, depending on USE flag configuration. This has little to do with the patch except that the patch is not and can never be a full replacement of a suid root binary.
What is the purpose of the patch Gentoo adds to upstream beep? It only makes things worse. From beep-1.2.2 to beep-1.2.2-r1, there is a regression. The patch removes the checks of ioctl. In my system, when running the program with no special privileges (nor sudo nor setuid): 1) The upstream program beeps and tells me that "ioctl: Operation not permitted" 2) The patched program does not beep or warn me. Probably because the program successfully opens /dev/console and then gets "permission denied" when trying to execute ioctl.
How about the "suid" USE flag making the binary mode 4750 owned by group "audio"? It is, after all, a program for playing audio. This seems to me to be the most sane policy: anyone who can make the sound card make noise can also make the PC speaker make noise, which is _not_ necessarily everyone (especially in a situation where you have a lot of SSHers).
Created attachment 175585 [details, diff] Introduce suid USE flag, restricting access to audio group (In reply to comment #5) > How about the "suid" USE flag making the binary mode 4750 owned by group > "audio"? Sounds good to me. To get some traction here, I attach a patch to the ebuild to implement this. I would think this ready for inclusion right now, as the default behaviour is USE=-suid, in which case nothing changes.
I think the nosuid patch should be applied in either case; it certainly doesn't hurt anything and I agree with the principle of the patch (though, as Jakub Moc points out, the implementation is not yet perfect). I think the name choice is rather poor though; it doesn't really have anything to do with SUID as such. It's more about making the repeat count and the intertone delay apply when permissions aren't available _for whatever reason_. That said, the rest of Martin's patch looks good to me.
(In reply to comment #7) > I think the nosuid patch should be applied in either case; it certainly > doesn't hurt anything The patch prevents diagnostic output which could be useful at times, as Jorge pointed out. In a non-suid setup, the most likely reason for the program to fail would be lack of privileges, in which case the diagnostics might be considered ugly (though I'd disagree even there). In a suid setup, that shouldn't be the case, so something else is causing trouble, and I'd want detailed information about this trouble without jumoing through hoops. Users not in the audio group wouldn't be granted permission to execute the binary in the first place and thus receive an error mesage from their command interpreter. > (though, as Jakub Moc points out, the implementation is not yet perfect). He was quoting my initial comment from another bug he duped here. Looking at the patch again, I found a third problem with it: When open succeeds but the ioctl fails, the original code would fallback to a bell and print diagnostic information, while the patched version does neither. > It's more about making the repeat count and the intertone delay apply when > permissions aren't available _for whatever reason_. That's one part of the patch, and I would even agree you are right there. The other part, however, is dropping lots of diagnostic output. That part is what I'd not like to loose when installing it suid. Creating a new patch to include the diagnostics and still do the repeats seems a bit overkill to me, though. And honouring the repeats without control over the lengths might be a bad idea as well, as you might end up with considerably longer run time than originally intended.
Created attachment 175910 [details, diff] Proposed new nosuid patch So you mean more something along the lines of this patch? (still dumps diagnostic output; writes bell char to stderr instead of stdout; guards all accesses to console_fd)
(In reply to comment #9) > So you mean more something along the lines of this patch? That's more like it, yes.
(In reply to comment #10) > (In reply to comment #9) > > So you mean more something along the lines of this patch? > > That's more like it, yes. > So, what is the consensus here? IUSE=suid, with suid patch??? I'm slightly lost on the specifics, can someone present a final ebuild patch for me?
Christopher Head's solution in comment #5 sounds the best to me. Although I'm no dev, I'd go with that one :)
I would vote for Martin's patch from #6 to just get this off the ground. The "nosuid" patch *should* have absolutely no effect when beep is SUID; whether that patch should be applied never, conditionally, or always seems a little less clear-cut (people seem to disagree on its costs and benefits and on whether the implementation is fully correct; however, since the patch is *already* being applied, it doesn't really matter for the sake of just getting the suid USE flag into the ebuild). It would be nice to see the USE flag pushed out, and then further discussion of the "nosuid" patch could happen separately.
It's been over two years. Is someone planning to apply this patch?
beep-1.3 has been around for a while, and it doesn't apply the “nosuid” patch. But neither does it install the binary suid root. So an unprivileged user on a graphical desktop will see this error message: Could not open /dev/tty0 or /dev/vc/0 for writing open: No such file or directory The second line likely refers to /dev/vc/0, while /dev/tty0 has mode 0620 and owner root:tty. Normal users doesn't have access to this, which I think makes sense; we shouldn't start adding users to the tty group. The beep README does discuss the possibility of installing it suid root, and perhaps restricting its access to a suitable group.
Created attachment 410544 [details, diff] Introduce suid USE flag, restricting access to audio group Since 1.3 doesn't do that “nosuid” patching any more, the discussion about when to apply that patch seems obsolete, as do the corresponding parts in my patch. For the rest I think we had a consensus: introduce a USE=suid flag, and change the group to audio if it is set. This patch to the ebuild reflects that. It's really simple.
* commit 88699eb | Author: Patrice Clement <monsieurp@gentoo.org> | Date: Fri Aug 28 23:28:57 2015 +0200 | | app-misc/beep: Add USE=suid flag courtesy of Martin von Gagern <Martin.vGagern@gmx.net>. Fixes bug 168201. | | Package-Manager: portage-2.2.20.1 | Signed-off-by: Patrice Clement <monsieurp@gentoo.org> | | create mode 100644 app-misc/beep/beep-1.3-r1.ebuild Thanks Martin.
Wow! After all this time, now so quickly after my patch resolved! That's great!
