aluzzardi / pam_usb

Hardware authentication for Linux using ordinary USB Flash Drives.
GNU General Public License v2.0
151 stars 82 forks source link

port to udisks2 #31

Open luka-n opened 8 years ago

luka-n commented 8 years ago

re #18

frederikmoellers commented 8 years ago

There seems to be a bug with (at least) xscreensaver under Debian stretch. When using pam_usb for authentication only (no agent functionality like automatic screen unlock), it sometimes hangs during udisks_client_get_object_manager in device.c:41. I'm still trying to figure out what exactly goes wrong there, just FYI.

nazar-pc commented 7 years ago

@luka-n, can you respond to comments in this PR so that we can get it merged and (possibly) even back to Ubuntu repository?

drmfinlay commented 7 years ago

Hey. I was looking into this and I noticed that another fork of this repo has most of these issues fixed. Plus it doesn't appear to have the problem with xscreensaver that @frederikmoellers mentioned above, which was an issue for me too, at least under Linux Mint 18. Perhaps a pull request from that fork's master branch would be a better idea. I could fix the problems @aluzzardi commented on above that are not fixed in that fork and make a new pull request.

aluzzardi commented 7 years ago

Please do - happy to review & merge

andrey-utkin commented 6 years ago

it doesn't appear to have the problem with xscreensaver that @frederikmoellers mentioned above

In your PR there is still such problem.

It works first time, but hangs a second time.

Sep 25 14:39:36 starlite pam_usb[11787]: pam_usb v0.5.0
Sep 25 14:39:36 starlite pam_usb[11787]: Authentication request for user "j" (xscreensaver)
Sep 25 14:39:36 starlite pam_usb[11787]: deny_remote is disabled. Skipping local check.
Sep 25 14:39:36 starlite pam_usb[11787]: Searching for "/dev/sdb2" in the hardware database...
Sep 25 14:39:36 starlite pam_usb[11787]: Device "/dev/sdb2" is not connected (bad).
Sep 25 14:39:36 starlite pam_usb[11787]: Access denied.
Sep 25 14:39:50 starlite pam_usb[11787]: pam_usb v0.5.0
Sep 25 14:39:50 starlite pam_usb[11787]: Authentication request for user "j" (xscreensaver)
Sep 25 14:39:50 starlite pam_usb[11787]: deny_remote is disabled. Skipping local check.

apparently after last line, "Searching for ..." should appear, but this never happens.

 $ gdb -p 16259
Password:
GNU gdb (Gentoo 8.0 vanilla) 8.0
Copyright (C) 2017 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-pc-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<https://bugs.gentoo.org/>.
Find the GDB manual and other documentation resources online at:
<http://www.gnu.org/software/gdb/documentation/>.
For help, type "help".
Type "apropos word" to search for commands related to "word".
Attaching to process 16259
[New LWP 16294]
[New LWP 16295]
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
0x00007f15050fa829 in syscall () from /lib64/libc.so.6
(gdb) info threads
  Id   Target Id         Frame
* 1    Thread 0x7f1506b93780 (LWP 16259) "xscreensaver" 0x00007f15050fa829 in syscall () from /lib64/libc.so.6
  2    Thread 0x7f14fdbb9700 (LWP 16294) "gmain" 0x00007f15050f48dd in poll () from /lib64/libc.so.6
  3    Thread 0x7f14fd3b8700 (LWP 16295) "gdbus" 0x00007f15050f48dd in poll () from /lib64/libc.so.6
(gdb) bt
#0  0x00007f15050fa829 in syscall () from /lib64/libc.so.6
#1  0x00007f1501d4ea7d in g_cond_wait () from /usr/lib64/libglib-2.0.so.0
#2  0x00007f1501d384c4 in g_once_init_enter () from /usr/lib64/libglib-2.0.so.0
#3  0x00007f15025c7492 in udisks_client_get_type () from /usr/lib64/libudisks2.so.0
#4  0x00007f15025c7a3a in udisks_client_get_object_manager () from /usr/lib64/libudisks2.so.0
#5  0x00007f1502b1dc10 in pusb_device_check () from /lib64/security/pam_usb.so
#6  0x00007f1502b1df62 in pam_sm_authenticate () from /lib64/security/pam_usb.so
#7  0x00007f15055f2ebe in ?? () from /lib64/libpam.so.0
#8  0x00007f15055f27be in pam_authenticate () from /lib64/libpam.so.0
#9  0x0000563df3de1514 in ?? ()
#10 0x0000563df3de0d0a in ?? ()
#11 0x0000563df3de083f in ?? ()
#12 0x0000563df3dcca7d in ?? ()
#13 0x00007f1505027521 in __libc_start_main () from /lib64/libc.so.6
#14 0x0000563df3dcccaa in ?? ()
(gdb) 

I couldn't find any refcounting bug in src/device.c. Making manager allocation one-time with making it static doesn't seem to work, too.

andrey-utkin commented 6 years ago

Published a small improvement https://github.com/IGP/pam_usb/pull/4 , but it doesn't help the bug described above. I have worked around my issue by calling pam_exec instead:

auth            sufficient      pam_exec.so quiet /usr/bin/pamusb-check my-user-name
auth    include     system-auth

Sorry for hijacking this thread. I'm done now.

drmfinlay commented 6 years ago

Ah yes. Sorry, I noticed that bug was still there some time after writing that and forgot to mention it I guess. I see what you mean about 'Searching for...' not appearing. I might look into that tomorrow. Thanks for the workaround though. It works for me, but I'm not really 100% on why it works.

andrey-utkin commented 6 years ago

The workaround works apparently becasue spawned pamusb-check process opens udisks resources just one in its lifetime, and exits. While in case of calling pam_usb through PAM API, same xscreensaver process does it again. Looks like resources recycling bug somewhere, maybe even udisks, because i reviewed src/device.c extensively and it looks fine in regard to refcounting etc.

drmfinlay commented 6 years ago

Ah right, that makes sense. I had thought the bug was external to pam_usb. If it is a udisks issue, then I'm wondering if it's fixed in more recent versions. I'm running udisks 2.1.8 (a little dated) on Debian 9. I'll try testing with a more recent version and see if I can confirm this.

By the way, have you given any thought to using an alternative to udisks2 (re: #36)?

I'm now also sorry for hijacking this thread.. Should we continue this elsewhere..?