GNOME Bugzilla – Bug 675085
Add --with-default-pam-config option
Last modified: 2016-01-21 20:25:23 UTC
The PAM files that ship with GDM are really specific to Red Hat's historical fork of pam. For example, the "system-auth" file still lives in the Fedora 17 "pam" git. Add new PAM files (which may still not work for everybody, but hey, it's a starting point) that should be somewhat useful for people who build with "Linux PAM" upstream, which uses "common-*" prefixes. The default is still to use the Red Hat PAM files for backwards compatibility.
Created attachment 213052 [details] [review] Add --with-default-pam-config option
I think i'd be more okay with one of 1) changing the stock ones to be this without the configure option 2) having per distro pam configs
(In reply to comment #2) > I think i'd be more okay with one of > > 1) changing the stock ones to be this without the configure option You mean change the default to --with-default-pam-config=linux, and change the Fedora/RHEL spec files to do %configure --with-default-pam-config=redhat? I'm totally fine with that - it's really your call obviously as the maintainer of both =) > 2) having per distro pam configs Can you elaborate on this? That's basically what my patch is doing, although the reason I called the other one "linux" is because there does exist this thing called "Linux-PAM", and I'm using pretty much the stock version. So we could add --with-default-pam-config=ubuntu too. That's totally possible now, but we should contact the Debian/Ubuntu gdm maintainers.
basically i commented before reading your patch very well, and you're actually already doing what I wanted. I'd just default to linux and not fedora, or try to detect like network manager does based on release files.
Created attachment 213148 [details] [review] Add --with-default-pam-config option, autodetect from /etc/foo-release files Now more similar to NM; autodetect from e.g. /etc/redhat-release so we don't need to modify the Fedora/RHEL spec files.
Review of attachment 213148 [details] [review]: ah cool, you're certainly motivated. ::: configure.ac @@ +250,3 @@ +if test x$enable_split_authentication = xyes; then + AC_DEFINE(ENABLE_SPLIT_AUTHENTICATION, 1, [Define if split authentication is enabled]) +fi what's this stuff about split authentication getting added for? That does remind me that we don't actually need to install the gdm pam file if split authentication is enabled (likewise gdm-fingerprint/gdm-smartcard if split authentication is disabled). ::: data/Makefile.am @@ +98,2 @@ pamdir = $(PAM_PREFIX)/pam.d pam_DATA = gdm-fingerprint gdm-smartcard what about these guys? @@ +250,3 @@ + $(INSTALL_DATA) $(srcdir)/$$file $(DESTDIR)$(PAM_PREFIX)/pam.d/$$bn; \ + fi; \ + done; \ Maybe we should drop this stuff and just use the more standard pam_filesdir= $(PAM_PREFIX)/pam.d pam_files_DATA = $(pam_files) I'm not sure why we're doing it this way at the moment.
(In reply to comment #6) > Review of attachment 213148 [details] [review]: > > ah cool, you're certainly motivated. I accidentally undid my local pam patches while trying to rebase gdm, so having stuff upstream is obviously a lot less painful =) > what's this stuff about split authentication getting added for? Looks like an accidental copy/paste. Fixed. > That does > remind me that we don't actually need to install the gdm pam file if split > authentication is enabled (likewise gdm-fingerprint/gdm-smartcard if split > authentication is disabled). Oof...true, but this is going to turn into a 2x2 matrix =/ I'm not sure I want to add the possibility (yet) of --enable-split-authentication --with-default-pam-config=linux. So let's make that case an error for now. You know...actually looking at the Fedora spec file, am I crazy or does Fedora/RHEL gdm not even use the PAM files included in gdm git, but it overwrites them with what's in the spec file? Is it worth keeping a copy here? > @@ +250,3 @@ > + $(INSTALL_DATA) $(srcdir)/$$file $(DESTDIR)$(PAM_PREFIX)/pam.d/$$bn; > \ > + fi; \ > + done; \ > > Maybe we should drop this stuff and just use the more standard > > pam_filesdir= $(PAM_PREFIX)/pam.d > pam_files_DATA = $(pam_files) > > I'm not sure why we're doing it this way at the moment. Looks like the goal is to avoid overwriting the system copies if one does a bare "make install" (i.e. no DESTDIR=) as root.
(In reply to comment #7) > (In reply to comment #6) > > Review of attachment 213148 [details] [review] [details]: > > > > ah cool, you're certainly motivated. > > I accidentally undid my local pam patches while trying to rebase gdm, so having > stuff upstream is obviously a lot less painful =) > > > what's this stuff about split authentication getting added for? > > Looks like an accidental copy/paste. Fixed. > > > That does > > remind me that we don't actually need to install the gdm pam file if split > > authentication is enabled (likewise gdm-fingerprint/gdm-smartcard if split > > authentication is disabled). > > Oof...true, but this is going to turn into a 2x2 matrix =/ I'm not sure I want > to add the possibility (yet) of --enable-split-authentication > --with-default-pam-config=linux. So let's make that case an error for now. Well split-authentication is the preferred way these days, so I don't think making it error is right. Installing a few extra pam files that don't normally get used is probably okay though. > You know...actually looking at the Fedora spec file, am I crazy or does > Fedora/RHEL gdm not even use the PAM files included in gdm git, but it > overwrites them with what's in the spec file? > > Is it worth keeping a copy here? You're not crazy, but that's a bad thing! I like the idea of doing it upstream instead and minimizing the fedora spec file. not sure why we're doing it this way at the moment. > > Looks like the goal is to avoid overwriting the system copies if one does a > bare "make install" (i.e. no DESTDIR=) as root. I guess in a world where the upstream pam files aren't correct that makes sense, but your patches change the world, so overwriting is probably good and fine now.
(In reply to comment #8) > Well split-authentication is the preferred way these days, so I don't think > making it error is right. Installing a few extra pam files that don't normally > get used is probably okay though. Yeah, I know split-authentication makes sense, but in jhbuild right now (i.e. the official GNOME modulesets), we don't have fprintd nor smartcard stuff. Where possible I'm trying to match what we have in GNOME officially at the moment, rather than blindly pulling stuff from distributions into the OSTree GNOME manifest. > You're not crazy, but that's a bad thing! I like the idea of doing it upstream > instead and minimizing the fedora spec file. > not sure why we're doing it this way at the moment. So...step 1) then is to drain the Fedora spec file stuff into gdm git. One issue with this is that it might conceivably break Red Hat Linux-derived forks who are relying on the current ones. > > > > Looks like the goal is to avoid overwriting the system copies if one does a > > bare "make install" (i.e. no DESTDIR=) as root. > I guess in a world where the upstream pam files aren't correct that makes > sense, but your patches change the world, so overwriting is probably good and > fine now. Ok, it would certainly significantly simplify the code.
Created attachment 213231 [details] [review] Add --with-default-pam-config option, autodetect from /etc/foo-release files Minor cleanup to this one
Created attachment 213232 [details] [review] Move gdm-fingerprint/gdm-smartcard into pam-redhat These are also Red Hat specific PAM files, so unify them with the rest.
Created attachment 213233 [details] [review] Respect split-authentication build setting for Red Hat PAM files If split authentication is disabled, install "pam.d/gdm", otherwise install the smartcard and fingerprint ones.
Created attachment 213234 [details] [review] Merge in PAM files from Fedora git into pam-redhat No sense having broken copies in here; since we now have machinery to maintain OS-specific PAM files in gdm git, let's drain the Fedora version here.
Created attachment 213235 [details] [review] Patch for fedora gdm git Note: this patch is for Fedora gdm git. Tested with 'fedpkg local' and installing the resulting RPMs in a rawhide VM.
Review of attachment 213232 [details] [review]: what about gdm-password ?
(In reply to comment #15) > Review of attachment 213232 [details] [review]: > > what about gdm-password ? That comes in the last patch; there wasn't a copy of it in GDM git. I structured the patches roughly as: 1) Add configure option 2) Cleanups to the internal build 3) Merge in Fedora
looks like i neglected to move it from ./gui/simple-greeter/extensions/password/gdm-password in commit 0e0aca600da17cb952525617f79ee3cadc028a8a
Created attachment 213241 [details] [review] Add --with-default-pam-config option, autodetect from /etc/foo-release files The PAM files that ship with GDM are really specific to Red Hat's historical fork of pam. For example, the "system-auth" file still lives in the Fedora 17 "pam" git. A long while back, Debian hit the same problem, and of course the difference is the naming; common-auth/common-password etc. OpenEmbedded then picked up Debian's PAM fork. Since for OSTree-GNOME we're using Poky/OpenEmbedded, let's add an option to integrate with their PAM. We use code similar to what NetworkManager has, so we should keep using the Red Hat files on systems with /etc/redhat-release or /etc/fedora-release.
Created attachment 213242 [details] [review] Move gdm-fingerprint/gdm-smartcard into pam-redhat Rebased
Created attachment 213243 [details] [review] Respect split-authentication build setting for Red Hat PAM files Just rebased
Created attachment 213244 [details] [review] Delete gdm-password from simple-greeter/extensions It's going to be obsoleted by the next patch. See commit 0e0aca600da17cb952525617f79ee3cadc028a8a which should have moved it.
Created attachment 213245 [details] [review] Merge in PAM files from Fedora git into pam-redhat Rebased
Note the first patch talks about OpenEmbedded now, not "Linux-PAM", since as Ray pointed out there really isn't anything useful upstream here (it's Red Hat=system, Debian=common). Comment 21 includes a new patch.
Hi, Ray asked me to take a look at this bug; I guess he would have liked patches to add a data/pam-debian/ baseline for Debian/Ubuntu. I lack the time to look at this immediately, but the idea of the patches seems good to me (less delta with distros), and I've checked the GDM packaging in Debian/Ubuntu for its PAM handling: * Debian has a patch to change the PAM service name from gdm and gdm-autologin to a configure-time string; I guess this was to support parallel installation of gdm2 and gdm3 * there are some differences between Debian's and Ubuntu's PAM configs, mostly in handling of environment setting (e.g. Debian has pam_env in session and Ubuntu in auth) * Ubuntu uses LightDM as the default rather than GDM This is Ubuntu's gdm.pam: #%PAM-1.0 auth requisite pam_nologin.so auth required pam_env.so readenv=1 auth required pam_env.so readenv=1 envfile=/etc/default/locale auth sufficient pam_succeed_if.so user ingroup nopasswdlogin @include common-auth auth optional pam_gnome_keyring.so @include common-account session [success=ok ignore=ignore module_unknown=ignore default=bad] pam_selinux.so close session required pam_limits.so @include common-session session [success=ok ignore=ignore module_unknown=ignore default=bad] pam_selinux.so open session optional pam_gnome_keyring.so auto_start @include common-password and Debian's gdm3.pam: #%PAM-1.0 auth requisite pam_nologin.so auth required pam_succeed_if.so user != root quiet_success @include common-auth auth optional pam_gnome_keyring.so @include common-account # SELinux needs to be the first session rule. This ensures that any # lingering context has been cleared. Without out this it is possible # that a module could execute code in the wrong domain. session [success=ok ignore=ignore module_unknown=ignore default=bad] pam_selinux.so close session required pam_limits.so session required pam_env.so readenv=1 session required pam_env.so readenv=1 envfile=/etc/default/locale @include common-session # SELinux needs to intervene at login time to ensure that the process # starts in the proper default security context. Only sessions which are # intended to run in the user's context should be run after this. session [success=ok ignore=ignore module_unknown=ignore default=bad] pam_selinux.so open session optional pam_gnome_keyring.so auto_start @include common-password There are also autologin files; here's Debian's gdm3-autologin: #%PAM-1.0 auth requisite pam_nologin.so auth required pam_succeed_if.so user != root quiet_success auth required pam_permit.so @include common-account # SELinux needs to be the first session rule. This ensures that any # lingering context has been cleared. Without out this it is possible # that a module could execute code in the wrong domain. session [success=ok ignore=ignore module_unknown=ignore default=bad] pam_selinux.so close session required pam_limits.so session required pam_env.so readenv=1 session required pam_env.so readenv=1 envfile=/etc/default/locale @include common-session # SELinux needs to intervene at login time to ensure that the process # starts in the proper default security context. Only sessions which are # intended to run in the user's context should be run after this. session [success=ok ignore=ignore module_unknown=ignore default=bad] pam_selinux.so open @include common-password and Ubuntu's gdm-autologin.pam: #%PAM-1.0 auth requisite pam_nologin.so auth required pam_env.so readenv=1 auth required pam_env.so readenv=1 envfile=/etc/default/locale auth required pam_permit.so @include common-account session [success=ok ignore=ignore module_unknown=ignore default=bad] pam_selinux.so close session required pam_limits.so @include common-session session [success=ok ignore=ignore module_unknown=ignore default=bad] pam_selinux.so open @include common-password Hope this helps,
Arch Linux has custom pam files as well and they are located here: https://projects.archlinux.org/svntogit/packages.git/tree/trunk?h=packages/gdm
(In reply to comment #24) > Hi, > > Ray asked me to take a look at this bug; I guess he would have liked patches to > add a data/pam-debian/ baseline for Debian/Ubuntu. Yeah, I think it'd be useful. > * Debian has a patch to change the PAM service name from gdm and gdm-autologin > to a configure-time string; I guess this was to support parallel installation > of gdm2 and gdm3 Hmm. Seems like a configure option like --with-pam-service-suffix=3 then? Although do you really care about gdm2 anymore?
Would you be kind to preserve the "login without password" option that upstream PAM files currently suggest, and that e.g. Ubuntu uses? This is the line: auth sufficient pam_succeed_if.so user ingroup nopasswdlogin (See bug 675780)
So...what about these patches?
Incidentally, polkit has similar PAM/distribution abstraction: http://cgit.freedesktop.org/polkit/tree/configure.ac#n395
(In reply to comment #28) > So...what about these patches? well they still need work to integrate the distros that have chimed in (though you don't necessarily have to do that work, but one of us needs to finish it before we push it.
We(ArchLinux) are reworking our pam modules and we are using pambase from gentoo. https://projects.archlinux.org/svntogit/packages.git/tree/trunk?h=packages/pambase I believe now we can use your stock pam modules but didn't had time to check it.
(In reply to comment #30) > (In reply to comment #28) > > So...what about these patches? > > well they still need work to integrate the distros that have chimed in (though > you don't necessarily have to do that work, but one of us needs to finish it > before we push it. Hmmm...all of them? At least one besides Fedora and the OpenEmbedded ones? This seems like a case where it'd be easier for us to land, and they can do it afterwards. Probably the main one would be Debian... (In reply to comment #31) > We(ArchLinux) are reworking our pam modules and we are using pambase from > gentoo. > > https://projects.archlinux.org/svntogit/packages.git/tree/trunk?h=packages/pambase You guys just imported it into a new git repository and lost the Gentoo history? Not even crediting the original people who made it? Or even just linking to where you got the code? Nice. Although, looking through the Gentoo page, I can't find the actual $#@$ing source code in git repository form, though the .gitignore in the tarball implies it exists... Anyways, yes, it does look like you naming the PAM bits with system- to match the Red Hat Linux ones will mostly just work with what exists in GDM now.
We should also consider adding session required pam_lastlog.so silent nowtmp to the default config, as proposed here: https://bugzilla.redhat.com/show_bug.cgi?id=812853
Still looking for what the necessary preconditions for merging are on this one. It's an annoying stack to rebase across gdm configure.ac/Makefile.am changes.
Created attachment 218309 [details] [review] Clean up PAM build/install rules The build system was inconsistent in its handling of pam files. The multistack files had names ending in .pam, which we copied to an unsuffixed file, and installed via pam_DATA. The non-multistack files had unsuffixed filenames in the source, which we installed manually via install-data-local. Unify on the latter method, because we might as well keep the weird custom bits to only install if they don't already exist; who knows, maybe someone is actually using this crappy implementation of a configuration management scheme. Also, the hack to copy the files at build time is unnecessary; the only copy should be made when we put them in the install root, and we can just as easily rename them then.
I'll rebase the other patches on top of this one.
Created attachment 218313 [details] [review] Clean up PAM build/install rules; move to pam-redhat This patch also moves the files to pam-redhat; makes the following patch easier.
Created attachment 218314 [details] [review] Add --with-default-pam-config option, autodetect from /etc/foo-release files The PAM files that ship with GDM are really specific to Red Hat's historical fork of pam. For example, the "system-auth" file still lives in the Fedora 17 "pam" git. A long while back, Debian hit the same problem, and of course the difference is the naming; common-auth/common-password etc. OpenEmbedded then picked up Debian's PAM fork. Since for OSTree-GNOME we're using Poky/OpenEmbedded, let's add an option to integrate with their PAM. We use code similar to what NetworkManager has, so we should keep using the Red Hat files on systems with /etc/redhat-release or /etc/fedora-release.
Feel free to push, but leave the bug open so we can get the other mentioned changes in later.
Attachment 218313 [details] pushed as f42e685 - Clean up PAM build/install rules; move to pam-redhat Attachment 218314 [details] pushed as 295d0bc - Add --with-default-pam-config option, autodetect from /etc/foo-release files
(In reply to comment #39) > Feel free to push, but leave the bug open so we can get the other mentioned > changes in later. Ok. I'll be on point for regressions. I didn't go through the hoops to 'make dist' and put the result in an RPM yet; if someone runs into trouble with that, ping me.
Has the change from Comment #33 been applied? On Fedora-19, GDM logins are still not recorded in lastlog.
Hi, any chance toa pply the arch configuration files? The patch is located here: https://projects.archlinux.org/svntogit/packages.git/plain/trunk/0001-Add-Arch-Linux-PAM-config-files.patch?h=packages/gdm
yea feel free to push that.
(In reply to Ray Strode [halfline] from comment #44) > yea feel free to push that. Pushed in 7caae964d1634d179c6936ddeb46af8172c4e348