Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Can't unlock with physlock and xsecurelock #7

Closed
x70b1 opened this issue Oct 17, 2019 · 11 comments
Closed

Can't unlock with physlock and xsecurelock #7

x70b1 opened this issue Oct 17, 2019 · 11 comments

Comments

@x70b1
Copy link

x70b1 commented Oct 17, 2019

I'm not sure what project the problem is.

I have installed and configured pam-gnupg. It works with i3lock. But if I create files in /etc/pam.d/ for physlock or xsecurelock, I can't unlock the key.

auth     optional  pam_gnupg.so
session  optional  pam_gnupg.so

I tested it also with:

auth      required   pam_gnome_keyring.so   auto_start
session   optional   pam_gnome_keyring.so   auto_start

The GNOME Keyring works.

@cruegge
Copy link
Owner

cruegge commented Oct 19, 2019

It looks like physlock and xsecurelock call pam_authenticate(3), but not pam_setcred(3), while i3lock does both (also explaining why in a comment). The current logic is to unlock gpg from pam_setcred, since that will only be called after successful authentication, when we have a valid passphrase, while pam_authenticate is called on every attempt, even with incorrect passphrases.

If I remember correctly, presetting an invalid passphrase could lead to gpg failing and not even asking for a passphrase using the usual graphical dialog on some versions. I'll have to check whether that's actually the case, or whether it would be possible to move unlocking to pam_authenticate without triggering weird behaviour. However, a failed login attempt on a VT would then effectively lock gpg for the graphical session.

Also, I'm not sure whether physlock and xsecurelock should call pam_setcred. The man page says

It should be called to set the credentials after a user has been authenticated and before a session is opened

which does not explain what to do when a user is authenticated, but we do not want to open a session. As I said, I'm somewhat fuzzy on the details of pam. Maybe I should open issues for physlock and xsecurelock, asking if they'd implement pam_setcred.

In the meantime, I'm probably going to add an option to enable unlocking from pam_authenticate as a workaround. I'm slightly busy currently, so it might take a couple of days.

physlock looks quite nice btw, thanks for pointing me to it.

@x70b1
Copy link
Author

x70b1 commented Oct 21, 2019

Sounds good! 😎

@x70b1
Copy link
Author

x70b1 commented Dec 16, 2019

Your PR is merged. I guess this issue is fixed.
But I had no chance to test it as I can't build the latest code of physlock at the moment.

I will test it as soon as possible and close this issue after that.

Thanks for your support and help!

@cruegge
Copy link
Owner

cruegge commented Dec 16, 2019

physlock should work now, but maybe leave the issue open to remind me that I still want to write a similar PR for xsecurelock. Also, I still want to implement some workaround options in case other screen lockers have similar problems.

@x70b1
Copy link
Author

x70b1 commented Dec 16, 2019

Nice plan!

@x70b1
Copy link
Author

x70b1 commented Dec 17, 2019

It works for me now.
It is up to you to close this issue. 😃

@Barbaross93
Copy link

As of google/xsecurelock@4f438fd, xsecurelock now calls pam_setcred. This issue can be closed

@cruegge
Copy link
Owner

cruegge commented Jun 7, 2021

Thanks for the info. Ignoring the issue for 1.5 years worked out nicely :)

@cruegge cruegge closed this as completed Jun 7, 2021
@x70b1
Copy link
Author

x70b1 commented Jun 7, 2021

Thanks anyway! 😄

@Barbaross93
Copy link

Looks like I spoke a little too early...

I'm trying to use slock with the pam-auth patch . The patch itself doesnt call pam_setcred, so naturally as with any suckless tool, I'm trying to implement it myself. Judging from the physlock and xsecurelock PRs, it looks like it should be a simple addition. So, I thought this would work:

			switch (ksym) {
			case XK_Return:
				passwd[len] = '\0';
				errno = 0;
				#if PAMAUTH_PATCH
				retval = pam_start(pam_service, hash, &pamc, &pamh);
				color = PAM;
				for (screen = 0; screen < nscreens; screen++) {
					XSetWindowBackground(dpy, locks[screen]->win, locks[screen]->colors[color]);
					XClearWindow(dpy, locks[screen]->win);
					XRaiseWindow(dpy, locks[screen]->win);
				}
				XSync(dpy, False);

				if (retval == PAM_SUCCESS)
					retval = pam_authenticate(pamh, 0);
				if (retval == PAM_SUCCESS)
					retval = pam_acct_mgmt(pamh, 0);

				running = 1;
				if (retval == PAM_SUCCESS) {
					pam_setcred(pamh, PAM_REFRESH_CRED);
					running = 0;
				}
				else
					fprintf(stderr, "slock: %s\n", pam_strerror(pamh, retval));
				pam_end(pamh, retval);

But it doesn't seem to be the case. Is there something else that needs to be called before pam_setcred?

@Barbaross93
Copy link

@cruegge Do you want me to make a separate issue for this?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants