Created attachment 232795 [details] Patch to add SUID option for sysutils/screen, default off The sysutils/screen port installs the screen binary as setuid root. From my testing this does not appear necessary. For comparison, the debian package does not install the binary SUID. This patch adds a port option to keep the SUID bit if someone wants to do that. The default is a non-SUID install.
Using setuid screen is the only way to make screen multi-user (https://www.gnu.org/software/screen/manual/html_node/Multiuser-Session.html) work. And before telling me, who would use that feature, I have on multiple occasions, sharing a screen with a person or people in a telephone call or conference all who are located in other parts of this province. It is a handy feature. Setting suid default off is certainly a POLA violation but on the flip side of the coin it is a security issue. I think on balance it makes sense to disable it by default, and I'll update the patch to reflect the fact that it is needed for multi-user operation.
Created attachment 232798 [details] Multuser screen patch I haven't tested this but this is what will be committed later today. Hopefully it doesn't cause a POLA violation. If it does I will reverse the default setting.
(In reply to Cy Schubert from comment #2) LGTM
Looking at Red Hat 7 and Fedora 35 (Red Hat 8 has removed screen in lieu of tmux), screen is setgid screen, not setuid root. Their customer support article 2802851 provides an example but it only works when sharing to the root account -- yeah nice try, I think we can be as broken as they are and should people want full functionality they can either set the option themselves or simply flip the setuid bit. Their Bugzilla bug 580339 discusses that though this is a POLA violation it is more secure. Their customer wanted to use the multiuser feature but RH's conclusion was that the exposure wasn't worth the risk. What remains now is a commit log message that sufficiently explains this rationale.
Also, making screen setgid screen does absolutely nothing. I don't see why they did this.
This will also require an UPDATING entry.
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=8f528507e9ca0e4f9020269ac69fc7d87249417d commit 8f528507e9ca0e4f9020269ac69fc7d87249417d Author: Cy Schubert <cy@FreeBSD.org> AuthorDate: 2022-03-29 15:02:19 +0000 Commit: Cy Schubert <cy@FreeBSD.org> CommitDate: 2022-03-29 15:39:28 +0000 sysutils/screen: Disable multiuser mode by default Multiuser mode is a handy way to share a screen among people who do not reside in the same location. Unforutnately it requires that screen be setuid root. GNU screen has had a number of CVEs over the years. See https://www.cvedetails.com/vulnerability-list/vendor_id-72/\ product_id-1860/GNU-Screen.html. Removing the setuid bit mitigates this at the expense of breaking the multuser feature. Red Hat removed GNU screen's setuid bit over a dozen years ago. Their rationale is documented in their bugzilla bug 580339, where they stated that most users don't use the multiuser feature. (Personally, I'm the only person I know of who uses that feature.) Users who use the multuser feature should enable the MUILTUSER option prior to building screen or using poudriere-options. Alternatively, users can chmod the setuid bit on when needed. PR: 262903 Submitted by: david@isnic.is (mostly) Reported by: david@isnic.is sysutils/screen/Makefile | 7 +++++-- sysutils/screen/pkg-plist | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-)
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=60683a7bd52258b3f5e7e6681fac62dd7c88763b commit 60683a7bd52258b3f5e7e6681fac62dd7c88763b Author: Cy Schubert <cy@FreeBSD.org> AuthorDate: 2022-03-29 15:35:38 +0000 Commit: Cy Schubert <cy@FreeBSD.org> CommitDate: 2022-03-29 15:41:57 +0000 UPDATING: Chase sysutils/screen-4.9.0_3 Describe user impact as a result of sysutils/screen-4.9.0_3 which removes setuid root by default, disabling multiuser feature. The option is appropriately called MULTUSER. Users may enable the multiuser feature in three ways, as discsussed by the update to UPDATING. PR: 262903 Reported by: david@isnic.is UPDATING | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=36f516ad2ae4fe01c51f8055c87491d0dd2605e9 commit 36f516ad2ae4fe01c51f8055c87491d0dd2605e9 Author: Cy Schubert <cy@FreeBSD.org> AuthorDate: 2022-03-29 17:03:35 +0000 Commit: Cy Schubert <cy@FreeBSD.org> CommitDate: 2022-03-29 17:06:34 +0000 sysutils/screen: Fix plist PLIST doesn't like "_" in variable names. Fix by renaming MULTIUSER_SCREEN to MULTIUSER. PR: 262903 Reported by: sunpoet, Michael Butler <imb@protected-networks.net> Fixes: 8f528507e9ca Pointy hat to: cy sysutils/screen/Makefile | 4 ++-- sysutils/screen/pkg-plist | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)
Sadly, PLIST doesn't like MULTIUSER ether, which results in dropping bin/screen-4.9.0 from both installation and the package. Fix: use MULTIUSERSCREEN instead of MULTIUSER_SCREEN, not MULTIUSER. Also, there is a typo in Makefile:34 MULTUSER_PLIST_SUB_OFF should be MULTIUSER_PLIST_SUB_OFF
Make was getting confused by the same variable name being used as an argument, interpreting it for us when we didn't want that. Fixed.