Bug 254178
| Summary: | x11/xscreensaver: update to 6.02 | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | Ports & Packages | Reporter: | Steve Wills <swills> | ||||||||||||||||||||||||
| Component: | Individual Port(s) | Assignee: | freebsd-x11 (Nobody) <x11> | ||||||||||||||||||||||||
| Status: | Closed FIXED | ||||||||||||||||||||||||||
| Severity: | Affects Only Me | CC: | agh, arrowd, contact, cy, fluffy, jwb, kaltheat, olivierw1+bugzilla-freebsd, rene, scf, tcberner, trueos, vishwin, x11, zeising, zirias | ||||||||||||||||||||||||
| Priority: | --- | Flags: | zeising:
maintainer-feedback+
jbeich: maintainer-feedback? (x11) |
||||||||||||||||||||||||
| Version: | Latest | ||||||||||||||||||||||||||
| Hardware: | Any | ||||||||||||||||||||||||||
| OS: | Any | ||||||||||||||||||||||||||
| URL: | https://reviews.freebsd.org/D29196 | ||||||||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||||||||
Can you put the diff in phabricator, it's easier for me to review it there? Thanks! It looks like this patch was approved in the review. Currently, xscreensaver-5.44_1 is complaining that "This version of XScreenSaver is very old! Please upgrade!" Hopefully, this update will calm it a bit. :) On a related note, xscreensaver is now at v6.01. Created attachment 226958 [details]
patch for 6.01
(In reply to Steve Wills from comment #3) There's an issue with the tarball for 6.01 that I tried to work around in the patch, but it seems the tar in 12.2 behave differently and it didn't work there like it did on newer. I tried contacting the author about the tarball issue, I'll update if/when I hear back. I'm not 100% sure I understand the issue with tar. Is it just on 12.2? Could it be a bug in our tar (that has since been fixed?) It might be worth waiting a couple of days and see if upstream replies, but other than that I have no objections to the patch. (In reply to Niclas Zeising from comment #5) bsdtar is behaving as designed following the change in this issue: https://github.com/libarchive/libarchive/issues/1381 Since it is only skipping the hardlink to itself, which is nonsensical, it looks like the extraction is working correctly. There is no difference from the final extraction between gnutar and bsdtar other than the message and return status. I did find that there are two files missing from the tarball due to a couple of dangling symlinks. These both point to non-existent files. However, these are related to the Android port and not X11. xscreensaver-6.01/android/xscreensaver/assets/fonts/PxPlus_IBM_VGA8.ttf xscreensaver-6.01/android/xscreensaver/assets/fonts/YearlReg.ttf Created attachment 228092 [details]
direct upgrade to 6.01, rebased
It's a pity a broken tarball is holding this back – thanks a lot for the patches!
I tried to apply the upgrade locally (see attached patch, maybe I did something wrong?) and the result doesn't work correctly:
- Any screensaver just shows a red alert reading
"<name>" crashed with status -1
- I get the following on stderr from the daemon:
xscreensaver-gfx: 13:50:05: we're still running as root! Disaster!
Created attachment 228095 [details]
drop privileges in xscreensaver-gfx
I analyzed this myself, the problem seems to be that xscreensaver-gfx doesn't drop privileges, maybe an oversight upstream? The attached patch fixes the issue for me!
But then, dropping privileges is only necessary because bin/xscreensaver is installed suid root. In a quick test, it also worked fine for me without the suid bit (and without this patch). Is suid root really necessary? E.g. for using it without PAM?
(In reply to Steve Wills from comment #4) > I tried contacting the author about the tarball issue, I'll update if/when > I hear back. No feedback from upstream about this in 2 months? Would an EXTRACT_DEPENDS on gtar + some fiddling be a working and acceptable workaround? If so, I'd happily attempt to create a patch… (In reply to Felix Palmen from comment #9) Personally, I'd prefer star (archivers/star) over gtar, but sure. Ok thanks, I'll try to come up with something soon to finally solve this! Does anyone know more about the suid bit? I guess the patch I added should be passed to upstream… but maybe we should still remove the suid bit anyways if it's not needed? Created attachment 228500 [details]
workaround for broken tarball
Well, it can be done with archivers/star in gnutar compatible mode. Attached patch uses star only if OSVERSION < 1300139, otherwise the workaround just ignoring the error is kept. Tested it successfully in a 12.2 jail.
I hope this is (together with my patch to drop privileges) enough to finally commit the new version, although I'm still unsure whether the suid bit is really needed…
IIRC, the reason for the setuid bit in the XScreensaver 5 and lower was that it needed root priveleges on BSD to be able to read the password from getpwent() (assuming it wasn't built with PAM). It was detailed in the older man page: If you get an error message at startup like "couldn't get password of user" then this probably means that you're on a system in which the getpwent(3) library routine can only be effectively used by root. If this is the case, then xscreensaver must be installed as setuid to root in order for locking to work. Care has been taken to make this a safe thing to do. That was why the old version was setuid, and you used xscreensaver-command or xscreensaver-demo for interacting with it. I haven't looked much at the new architecture, and it could be that FreeBSD grew some feature that could get rid of the need for setuid that could be patched in (eventually, not right now). But that is at least why the old version was setuid. Hope this helps. I would like to see the update as well. (In reply to Trenton Schulz from comment #13) Thanks, what you write corresponds to what I assumed. So the main question would be: Does it still make sense to build xscreensaver without PAM support and would it work at all? If not, we could just remove the suid bit (that's a simple change in pkg-plist) and make PAM mandatory from now on. To get it straight, my patch (drop privileges) just makes sure xscreensaver runs correctly *when* it's installed suid root. It might be an upstream bug, but if suid root doesn't make sense any more, it's unnecessary to patch it. Maybe I just have to try to find out by experimentation whether xscreensaver still works without PAM… Side note, I tested the extract workaround with 14-CURRENT now and it works fine (without depending on archivers/star). Created attachment 228515 [details]
0001-x11-xscreensaver-update-to-6.01
Ok, I guess I finally got everything straight: I experimented with a locally created account and xscreensaver built without PAM support. This actually still works, but indeed, xscreensaver needs to be installed suid root for it (and therefore needs the patch to drop privileges in xscreensaver-gfx).
Therefore adding a patch for a single commit now, containing:
* all the cleanup as suggested by swills@
* update to 6.01 based on swills@' patch
* swills@' workaround ignoring the extraction error on FreeBSD >= 13
* otherwise, using archivers/star to extract
* only install suid root if PAM was disabled
* in that case, add extra patch for dropping privileges
Yep, that's quite a mess. I tested a lot and IMHO, everything works fine!
Attached patch can be applied with 'git am' (and includes a suggestion for a commit message)
Working great with PAM on 14-CURRENT (88c027338f182e2af56d0dbabd4a94fbca6f091a). xscreensaver-6.01 is a much needed update. Thanks. This looks OK, approved! I wonder if the whitespace/cleanup should be made to a separate commit, but it is not super important. @swills, can you get this into the tree? Created attachment 228661 [details] cleanup and update in 2 separate commits (In reply to Niclas Zeising from comment #17) > I wonder if the whitespace/cleanup should be made to a separate commit I think this would make a lot of sense to keep the git history readable, so I took the liberty to separate the commits here. If you apply this attachment with 'git am', you will get two commits. Comment on attachment 228661 [details] cleanup and update in 2 separate commits Additional info: I verified the end result of both commits is exactly the same as before and the first commit really only does the "cleanup" changes and allows for a successful 'poudriere testport' of xscreensaver 5.44_1. Maybe for review, it's easier to display the commits in my github repo: https://github.com/Zirias/zfbsd-ports/commits/local *** Bug 259272 has been marked as a duplicate of this bug. *** Created attachment 229118 [details]
x11-xscreensaver-cleanup-and-upgrade-to-6.02-2-commits
New upstream version is 6.02
* rebased swills' cleanups onto current main
* update directly to 6.02
* removed extract kludge, the new distfile seems fine
* dropping privileges patch without PAM still needed
Still two separate commits, the series can be applied with `git am`. Please review and commit ;)
(In reply to Felix Palmen from comment #21) Works great on 14-CURRENT (main-n250284-d524e370c4d). Ta very much! Ok, what's the blocker here? If there's anything wrong with my 2-commits patch, *please* let me know, I'll happily work on it! But seriously, it's about time this port is finally updated :o (In reply to Felix Palmen from comment #23) I don't have a commit bit, but I have noticed that using these patches that xscreensaver doesn't work with PAM for me. I haven't had a chance to debug my PAM set-up, though (although it is basically "default"). When I disabled PAM (i.e., back to setuid), it worked. Unfortunately, I hadn't built previous releases with PAM, so I have no idea if it's the patch here or from the previous revision. I don't think this explains the hold-up, but it could be one of the issues? (In reply to Trenton Schulz from comment #24) Interesting! The added local patch just makes sure xscreensaver still works when installed suid root, so that's not the issue here. But another thing I changed was to *only* install it suid root when PAM is disabled, expecting it's unnecessary with PAM. Maybe this was an error. Could you check whether it works for you when you build with PAM enabled and then manually set the suid root bit (chmod u+s /usr/local/bin/xscreensaver)? In this case, the local patch won't be added, so any actual screensaver will crash ... but locking/unlocking should work nevertheless. (In reply to Felix Palmen from comment #25) Did a couple of experiments this evening. 1. Built the current port of xscreensaver 5.44 with PAM support. Unlocking works. Removing the setuid bit from the xscreensaver binary causes unlocking not to work. A bit of a curveball, but OK. 2. Applied the patches to build a package for xscreensaver 6.02 with PAM support and reinstall Unlocking doesn't work. 3. Set the setuid bit on xscreensaver (chmod u+s $(which xscreensaver)). Unlocking does not work. In addition, you get a nice "don't login as root" screensaver, which I guess is what you suggested. This is a bit of a surprise for me. Especially since I was the one that pointed to the PAM sentence in the man page. I'm sorry I lead you astray and apologize that I haven't reported this earlier. 4. Compile 6.02 port without PAM and reinstall Unlocking works. In addition, it gives me a list of all the failed attempts earlier (when I killed xscreensaver to get back to the screen). May be the "extra-patch-drop-privileges" should just be dropped for now and could go in a later revision? Maybe dropping the "remove setuid patch" is sufficient for now? It might unstick the port at least. Hope this helps. ports @ (https://cgit.freebsd.org/ports/commit/?id=f7b61ce888d78d46a651b439d616dfff0bfbcd3e) src @ (https://cgit.freebsd.org/src/commit/?id=1b0602f2db94a4789da33302fbadfe5a57454277) xscreensaver-6.02 complied with: PAM : on SETUID_HACKS : off None of the xscreensaver binaries are setuid. xorg-server-1.20.13,1 complied with: SUID : off /usr/local/bin/Xorg is not setuid. xscreensaver works fine with no issues. (In reply to Alastair Hogge from comment #27) I use the same options, using xscreensaver under x11-tookits/motif (mwm) and under x11/cde (replacing CDE's screensaver with xscreensaver). Running 14-CURRENT. xscreensaver 6.02 works well. I think it's ready to commit. If there are any doubts commit the patch in this PR as x11/xscreensaver6. My only doubt after Trenton's latest comments is that there might be configurations where xscreensaver needs to be installed suid root, even if using PAM. This would be a simple change. There was some misconception about the local patch though: This patch *is* necessary, so xscreensaver 6 works at all when installed suid root. So, should I do another update and *always* install it suid root (as was the case in previous versions), just to be sure? (In reply to Felix Palmen from comment #29) I think adjusting the setuid stuff would be "safest" for the moment. That would at least get us a much newer xscreensaver in the ports tree. A second patch to fix the setuid issue (and making PAM the default) could then be added afterwards. Otherwise, we end up with one changing holding the other from getting committed. I don't rule out that I have my system misconfigured, and it may work with other configurations (My system was a stock 13-release with Plasma and only adjustments done in SDDM to open the GNOME keychain). I just know that with the previous configuration with a setuid binary and PAM not enabled by default, it worked, and it also works if I build with that configuration now as well. That would be my thoughts around it. I'm not a committer, though. So, I don't know how that helps. I'm going to give this a try. My xorg-server is SUID since I use startx exclusively. (In reply to Trenton Schulz from comment #30) I've been using it here since Dec 6. It is not setuid root. I use it with x11/cde (I replaced it in my DT configuration, bypassing CDE's screensaver). It works well, with one bug. The one bug is, windows that pop up by themselves while the screen is locked will display on the locked screen. After the windows pop down their image is still displayed on the screen. The other difference is that under 5.44 and 5.45 I could type in my password while the screen was still blanked out. Not anymore with 6.02. I need to hit a random key to make the password entry prompt display before it will accept any keystrokes. It's a small paradigm shift when unlocking. And probably why it shouldn't become the default xscreensaver -- see my discussion about how to deal with this two paragraphs below. Can either of these be mitigated with some kind of option? I don't know. I haven't looked into it at any real depth. Something worth looking into. Would it possible we maintain 5.45 as x11/xscreensaver5 and this update as xscreensaver6, with an x11/xscreensaver meta-port that selects one or the other based on an option the user selects. Allowing users with their own poudriere run poudriere-options, or build by hand, to default to one or the other. Just a thought. (In reply to Cy Schubert from comment #32) > The one bug is, windows that pop up by themselves while the screen is locked > will display on the locked screen. After the windows pop down their image is > still displayed on the screen. This is a security issue :o I never observed anything like that (using version 6 since submitting the first patch here...) Does it only happen with this specific wm? Does it only affect specific window types? Is upstream aware of that issue? > The other difference is that under 5.44 and 5.45 I could type in my password > while the screen was still blanked out. [...] I think that's not too relevant. Either behavior can be confusing if you expect the exact opposite. I personally think it makes more sense to accept input only when the password field is shown. > Would it possible we maintain 5.45 as x11/xscreensaver5 and this update as > xscreensaver6, with an x11/xscreensaver meta-port [...] Nothing wrong with that in general. But maybe also take into account that version 5 is showing a pretty prominent nag message, making it very clear it's considered EOL. So I'm not sure it should be kept. As for the SUID topic, it would be pretty simple to revert that one change to the port, so it's always installed suid, as it was before. Still I'd like to understand what exactly is the problem here (I can't reproduce any issue with non-suid install myself). Needlessly setting suid should IMHO be avoided. Keeping any and all older versions in the tree is a bad idea, not least because jwz considers anything but the latest version EOL and prone to security issues, and strongly enough that the nag screen exists. Until these final bugs can be resolved an update to 5.45 would be acceptable but an update to 6.02 is not. I ran the experiment from comment 26, albeit with my xorg-server setuid-enabled. Same results. Here is a note from jwz in driver/passwd-pam.c: Also note that FreeBSD's implementation of PAM requires the calling process to be running as root during the entire interactive PAM conversation: it can't ever disavow privileges. Linux's PAM implementation uses a setuid helper so that a non-root process can still authenticate, as is right and proper. Consequently, XScreenSaver does not support PAM on FreeBSD. Dear FreeBSD, get your shit together. This suggests that PAM support in 5.44 was either accidental (by way of setuiding bin/xscreensaver itself) or it fell back to something else. I'm going to try setuiding just bin/xscreensaver-hacks/xscreensaver-auth and see what happens. Additionally, from driver/xscreensaver-auth.c describing the Linux OOM killer mitigation: This program, "xscreensaver-auth", is the part of the XScreenSaver daemon that might need to be setuid for other reasons, so we handle the OOM killer here. We could instead handle OOM in the "xscreensaver" program, but then that program would *also* need to be setuid. Created attachment 231805 [details]
v8
No dice with bin/xscreensaver-hacks/xscreensaver-auth as setuid with PAM enabled. Most likely needs to interface with a pam_helper in order to work, so mark the option BROKEN and remove from DEFAULT_OPTIONS.
Includes previous two patches to apply using git-am(1).
(In reply to Charlie Li from comment #37) Please don't do that! PAM is not broken (and never was) for me, it works perfectly fine, without any suid bit. Maybe it depends on the actual PAM modules used, it's pam_winbind.so for me. What I don't really understand is, if pam_unix.so indeed requires being root during the PAM dialog, why doesn't it help to *keep* xscreensaver installed suid root? If you would remove PAM support, it needs to be installed suid root anyways to work correctly plus the patch I added is needed, otherwise the screen savers will crash... So maybe there's just another tiny additional patch missing to make PAM work with pam_unix.so (again?) (In reply to Charlie Li from comment #36) > This suggests that PAM support in 5.44 was either accidental (by way of > setuiding bin/xscreensaver itself) or it fell back to something else. The latter seems to be the case. I found this code in driver/passwd.c that didn't exist in 5.x yet: | /* Only try the first method that initialized properly. That means that | if PAM initialized correctly, we will never try pwent or Kerberos. | If we did, then typing an incorrect password at PAM would result in a | second password prompt that would only go to pwent. There's no | sensible way to re-use the password typed the first time, if there | even was one. With fingerprint readers or OTP fobs, there might have | been 0, 2, or more passwords entered. */ | break; So, that's most likely the reason for the problems, but it seems there are good reasons for that change as well... Analyzing the code some more, I found the "pwent" method gets hold of the password hash(es) *before* dropping privileges, something that can't work with PAM. I agree with jwz that a PAM authentication dialog shouldn't require root privileges. Now, my suggestion would be: * Keep the PAM option, but add a prominent warning that it won't work with pam_unix.so * Remove PAM from default options, of course * Have a look at base to solve the underlying problem, there should indeed be a suid helper for PAM Created attachment 231815 [details]
v9
Here's another attempt to getting this forward *without* breaking any pre-existing functionality. It's again two commits (apply with git-am), my second commit changed as follows:
* decouple suid-root installation from pam, make it a separate option enabled by default (otherwise, pwent authentication can never work).
* add an extra patch for pam enabled that adds an init function checking /etc/pam.d/system whether pam_unix.so is strictly required for authentication; if yes, fail. This should have the effect of the same observable behavior as in 5.x: a fallback to pwent authentication.
So, anyone who had issues before, please try that patch.
I'm aware it's a quite "hacky" workaround, the real solution should probably be a change in base to allow pam authentication unprivileged. But for now, I hope that works well enough...
pam_winbind isn't in the base system, but yeah, seems to very much depend on the modules used. pam_unix(8) requires privileges, usually root, to access the password database, which is where the setuid helper comes in. The helper that other programs, really other screensavers, have used is security/pam_helper. x11/mate-screensaver now uses security/mate-pam-helper. There is some commented/unused code and configure logic for an external helper (driver/passwd-helper.c) that I may try later. Remember, BROKEN, unlike IGNORE, does not preclude building entirely when TRYBROKEN is defined in the make environment. I'm running without extra-patch and PAM disabled, no crashes. The configure logic effectively has xscreensaver-auth setuid-enabled because shadow passwords are enabled by default. (In reply to Charlie Li from comment #41) > pam_winbind isn't in the base system Sure. Still, it worked just fine with xscreensaver 5.x (and did so with default options, IOW when installed from packages), so removing PAM support would be a regression. > I'm running without extra-patch and PAM disabled, no crashes. That's nice. Could you please also try with my updated patch? It works fine on my machine where PAM authentication doesn't require pam_unix.so, but *should* work (by falling back to pwent auth) as well otherwise. (In reply to Felix Palmen from comment #42) I have been using pam_krb5 with 6.02 on -CURRENT since the beginning of the year without any issues. (I have to use pam_krb5 since migrating from NIS to LDAP. When migrating to LDAP I did not migrate NIS passwords so, I rely entirely on krb5 for authentication.) Could the pam_winbind problem be a local problem? (In reply to Cy Schubert from comment #43) > Could the pam_winbind problem be a local problem? pam_winbind doesn't cause any problem, it was just the example what I am using. I assume that *only* pam_unix causes the issue, as it requires (root) privileges for the authentication dialog. That's why in my latest patch, I added an init function checking /etc/pam.d/system whether pam_unix is strictly required for auth, and if so, it fails, so xscreensaver doesn't consider pam for unlocking at all. Yes, that's a hack ;) (In reply to Felix Palmen from comment #42) pam_unix is still in the pam.conf(5) we ship, so it *has* to work. The existing logic is correct since PAM successfully initialises. The proper fix is still going through pam_helper regardless of module used, since most software that assume Linux-PAM will use the integrated helper. Created attachment 231850 [details]
v8+1
security/pam_helper successfully unlocks using pam_unix. I will notify jwz of this development, since the helper code is considered for removal.
(In reply to Charlie Li from comment #45) > pam_unix is still in the pam.conf(5) we ship, so it *has* to work. The > existing logic is correct since PAM successfully initialises. That wasn't the point, as I said, it's a hack just restoring more or less the functionality we had with 5.x: the idea is to check whether there *is* a path without pam_unix and try pam only then, while with 5.x, it just failed at unlock time and there was a fallback to pwent method. > The proper fix is still going through pam_helper regardless of module used, > since most software that assume Linux-PAM will use the integrated helper. I'd say this is a better workaround indeed, but still a workaround. On Linux, you don't need a helper, even with pam_unix. > security/pam_helper successfully unlocks using pam_unix. I will notify jwz of > this development, since the helper code is considered for removal. Judging from the code comment you cited, I assume he doesn't want to rely on some 3rd party helper. The original idea was to avoid having to install xscreensaver suid root, and it seems this is now achieved by making xscreensaver_auth a separate binary (btw good to know, this makes my other extra-patch obsolete, the problem was missing suid on this new binary). And IMHO, this makes sense: PAM authentication should not require privileges, and if a helper binary is needed for that, it should be under control of the PAM implementation. It's security critical after all. I'm already looking at OpenPAM code and might come up with a suggestion, but that will take some time and careful thought... BTW, I'd like to retract my latest patch as well, cause a solution not requiring any extra patches is of course preferred. Just can't find a way to do that without adding yet another attachment :( (In reply to Felix Palmen from comment #47) > I'd say this is a better workaround indeed, but still a workaround. On Linux, you don't need a helper, even with pam_unix. …because the helper for pam_unix, on Linux, is part of Linux-PAM as unix_chkpwd(8). security/pam_helper plays that role for us (for now). (In reply to Charlie Li from comment #49) > …because the helper for pam_unix, on Linux, is part of Linux-PAM as > unix_chkpwd(8). security/pam_helper plays that role for us (for now). Sure! I just say this IMHO makes sense, so the best way forward in the *long* term would be to add a similar helper to OpenPAM. For now, of course, if xscreensaver can be made to use a 3rd party helper, this is clearly the best workaround. Please kindly mark my workaround obsolete here, thanks :) (In reply to Felix Palmen from comment #33) I reached out to our upline regarding the displaying of new windows while the screen is locked. Apparently this has been a problem with mwm and cde since forever. Strangely I never had this problem with with any xscreensaver when using mwm or cde until 6.02. Having tested it out on Fedora the problem exists there as well. Our upstream has no intention of fixing it. I'll switch to using xlockmore with mwm and with cde (until I can figure out why cde doesn't work with pam). (In reply to Cy Schubert from comment #51) So if this isn't a new bug, I assume it shouldn't block the upgrade either? :) Just FYI, regarding the PAM issue, I'm now trying to tackle this in base: https://reviews.freebsd.org/D34322 But even if it's accepted eventually, a workaround for now is needed, and using a third-party helper sounds good! Maintainer reset. This seems to be extremely tricky. At its core, upstream's expectation is that you can authenticate with PAM as an unprivileged user (and I tend to share this view), therefore there's no plan to roll some suid-root helper with xscreensaver or use an existing 3rd-party one. Currently, authenticating with pam_unix.so requires root privileges. Linux PAM partially "solves" this for authenticating as the currently logged-in user with "unix_chkpwd". Allowing just authentication as yourself is much simpler to implement in a secure way than authenticating as any user. I submitted a stack of reviews mimicking this Linux solution here: https://reviews.freebsd.org/D34322. It was rejected without further comment, and although I see a partial solution is far from ideal, from my direct conversation with des@, I learned he doesn't even agree on the expectation that authentication should work without privileges. So, thinking about a better and more complete solution would probably be just time wasted. Therefore, to get this forward, I'm all in favor of having xscreensaver on FreeBSD use an existing helper as already suggested in this PR. jwz removed the helper code in 6.03 entirely, so I think for Unix user account-based authentication it will be easier to disable PAM by default (unless someone wants to implement a kludge). For now, just to have something updated in the tree, we can roll with the helper code's last hurrah until we start qualifying 6.03. mentors? (In reply to Charlie Li from comment #55) Yes, I'd say that 6.02 and a 3rd party helper is a way to go for now. (In reply to Gleb Popov from comment #56) This would need to be provided by a separate port which would also need to provide its own pam_unix.so. See the discussion at https://reviews.freebsd.org/D34322. A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=979995168c2f4dec4dd67cd617639c1abf67f16e commit 979995168c2f4dec4dd67cd617639c1abf67f16e Author: Felix Palmen <felix@palmen-it.de> AuthorDate: 2021-10-13 11:23:08 +0000 Commit: Charlie Li <vishwin@FreeBSD.org> CommitDate: 2022-05-02 18:41:08 +0000 x11/xscreensaver: update to 6.02 Only install xscreensaver suid root if option PAM is disabled. In this case, add an extra patch, so xscreensaver-gfx drops privileges (otherwise it doesn't show any screensaver). Fix setuid installs in the Makefile and remove extra patch. Respect upstream's USE_CSTD=gnu89. Remove now-unknown configure option and associated dependency. Remove old CONFLICTS. Use security/pam_helper for PAM support. This is the last release to support pam_unix as the next release removes external helper calling code. PR: 254178 Co-authored-by: swills, vishwin Approved by: arrowd (mentor), maintainer-timeout, maintainer-reset x11/xscreensaver/Makefile | 103 +++++++++++---------- x11/xscreensaver/distinfo | 6 +- x11/xscreensaver/files/patch-config.h.in | 10 +- x11/xscreensaver/files/patch-configure.ac (new) | 91 ++++++++++++++++++ x11/xscreensaver/files/patch-configure.in (gone) | 11 --- x11/xscreensaver/files/patch-driver_Makefile.in | 28 +++--- .../files/patch-driver_XScreenSaver.ad.in | 34 +++---- .../files/patch-driver_passwd-helper.c (new) | 11 +++ .../files/patch-hacks_glx_Makefile.in (new) | 11 +++ x11/xscreensaver/pkg-plist | 75 ++++++++++----- 10 files changed, 257 insertions(+), 123 deletions(-) Committed. Thank you everyone for your input and dogfooding. review D34322 is probably a better spot to continue discussing pam_unix specifically. |
Created attachment 223139 [details] patch to update and clean up port