Ubuntu

Intrepid regression: carriage-return required after finger scan

Reported by echo6 on 2008-08-09
316
This bug affects 43 people
Affects Status Importance Assigned to Milestone
HAL
Confirmed
Medium
thinkfinger (Ubuntu)
Medium
Unassigned
Nominated for Intrepid by Martin Rotter
Nominated for Jaunty by Martin Rotter
Nominated for Lucid by Nate Moore

Bug Description

0.3+r118-0ubuntu3

Was working prior to this version, but now requires carriage return both on login from gdm or for admin password via sudo or gksudo etc.

Patrick Hetu (patrick-hetu) wrote :

I confirm

echo6 (echo6-uk) wrote :

Thanks for the response. I've recently had occasion to do a full install of 8.04 followed by the installation of thinkfinger and this bug has not appeared, atm I can not repeat it.

rupa (rupa) wrote :

confirming this recently began happening on my intrepid install.

Jon Oberheide (jon-oberheide) wrote :

I can confirm this on Intrepid as well.

It seems to be related to libpam-thinkfinger's use of uinput. libpam-thinkfinger creates a uinput device to send a EV_KEY/KEY_ENTER press and release. This event _should_ emulate the enter key being pressed after a finger swipe so that the user does not have to press it manually.

I don't see any errors in the pam debug output about sending the enter event. Either libpam-thinkfinger is misusing uinput somehow or GDM is not processing the key event (maybe it's blocking focus on the password prompt textbox?).

Regards,
Jon Oberheide

Jon Oberheide (jon-oberheide) wrote :

After debugging for far too many hours, I have an apparent hack/fix.

Simply setting another KEYBIT along with our KEY_ENTER seems to fix the issue. The other KEYBIT can be any value and it doesn't seem to matter if the other KEYBIT is set before or after KEY_ENTER. If another KEYBIT is not set, it appears that the kernel is not sending the KEY_ENTER event.

I don't recommend committing this until we figure out what the root cause is, but if anyone else experiencing the problem could test it and verify that it resolves their issue, that'd be great.h

Jon Oberheide (jon-oberheide) wrote :

FYI, this issue of not receiving a key event when only one is set is reproducible in a standalone test program outside of thinkfinger-pam. So either it's a kernel issue or for some reason uinput wasn't designed to work with only a single KEYBIT set (which seems incorrect since it worked with earlier kernels).

Patrick Hetu (patrick-hetu) wrote :

I also notify that the bug doesn't happen when you are in a virtual terminal (ctrl-alt-f1) .

Jon Oberheide (jon-oberheide) wrote :

Ah, nice catch Patrick. Perhaps it is something in xorg/xinput/xorg-input-evdev. I'll try to bisect.

Jon Oberheide (jon-oberheide) wrote :

I can confirm that downgrading all my xorg packages to hardy fixes the problem.

akw (akw) wrote :

I also can confirm this issue. The problem does not happen on the console (ctrl-alt-f1), but in X.
I had no problems before upgrading to Intrepid.

Jon Oberheide (jon-oberheide) wrote :
Jon Oberheide (jon-oberheide) wrote :
Jon Oberheide (jon-oberheide) wrote :

I've attached two samples that show this behavior outside of thinkfinger.

They both create a uinput device and send a KEY_ENTER event. In bad.c, when the device is only set up with a KEY_ENTER key, the KEY_ENTER event is not observed in X. However, in good.c, if another supported key (say KEY_A) is added to the uinput device as well, the sending of the KEY_ENTER event works just fine.

Both of the testcases work as expected outside of X in the terminal which leads to me to believe something is getting munged in X's input layer.

Tom Jaeger (thjaeger) wrote :

My guess is that the new input hotplugging magic selects the wrong driver for the uinput device.

If I add a section (this might already be there, not sure anymore)

Section "InputDevice"
        Identifier "Generic Keyboard"
        Driver "kbd"
        Option "XkbRules" "xorg"
        Option "XkbModel" "pc105"
        Option "XkbLayout" "us"
EndSection

to my xorg.conf and then enable it by adding a line

        InputDevice "Generic Keyboard"

to the "ServerLayout" section, the fingerprint reader works as before.

Raja (rajajs) wrote :

That works great for me.
The InputDevice section was already there with the entry for the keyboard. But I had to add the line to the "ServerLayout" section. Once I restarted X, I could log in without following the swipe with a carriage return.

Jon Oberheide (jon-oberheide) wrote :

Attached is a patch to thinkfinger that should workaround the bug by adding a dummy key (KEY_A) to the uinput device.

It's certainly not a proper fix to why xorg is not correctly sending the KEY_ENTER but it's certainly an option for those of you who find hitting enter after you swipe annoying and/or don't have an xorg.conf to add the configuration workaround.

Tom's workaround worked for me. The keyboard input device and serverlayout reference were commented out claiming that HAL would deal with them after the upgrade. I uncommented and all works as before.

Shane Par-Due (shanepardue) wrote :

I don't see a "ServerLayout" section" on Intrepid Ibex using Envy to handle my xorg.conf.

Perhaps this should be reported upstream at freedesktop's bug tracker?

<quote>
to my xorg.conf and then enable it by adding a line

        InputDevice "Generic Keyboard"

to the "ServerLayout" section, the fingerprint reader works as before.
</quote>

Adding this to my xorg.conf causes unwanted side-effects, such as causing my volume control button next to my headphone jack to stop working, and bizarrely stopping the key combinations ctrl-<arrow key> to stop working also. So this is not an option for me.

Jon Oberheide (jon-oberheide) wrote :

Packages which resolve this issue are available in my PPA:

https://launchpad.net/~jon-oberheide/+archive

deb http://ppa.launchpad.net/jon-oberheide/ubuntu intrepid main
deb-src http://ppa.launchpad.net/jon-oberheide/ubuntu intrepid main

You are the man Jon Oberheide!!
Everything work just fine now.

will the fix be included in the main archive?

Jon Oberheide (jon-oberheide) wrote :

Unsure. It uses the workaround patch previously posted (adding a dummy KEY_A key capability to the uinput device). Without the workaround, the uinput device doesn't even show up in "xinput list". For some reason, a keyboard uinput device with only a KEY_ENTER is not sufficient to be registered.

So while it's more of a workaround than a fix, it does indeed resolve the issue and has absolutely zero side effects.

Noel J. Bergman (noeljb) wrote :

FWIW, neither Fedora 9 nor Fedora 10 (probably the more relevant of the two) exhibits the problem.

When creating a uinput device with only one key, hal incorrectly assigns it as a "button" rather than a "keyboard". This problem manifests itself in libpam-thinkfinger when a uinput device is creating to automatically send a carriage-return after the user swipes his fingerprint.

The problem lies in input_test_key() in hal's hald/linux/device.c. Since num_bits == 1 for a one-key keyboard, the device will be consider a button.

Created an attachment (id=20043)
failed uinput testcase

bad.c creates a one-key keyboard with KEY_ENTER and attempts to send the KEY_ENTER event. since the device is not recognized as a keyboard, the KEY_ENTER is not processed.

Created an attachment (id=20044)
workaround uinput testcase

bad.c creates a one-key keyboard with KEY_ENTER and attempts to send the KEY_ENTER event. by adding an extra dummy key (KEY_A), the uinput device is recognized as a keyboard and sends the KEY_ENTER event properly.

(In reply to comment #2)
> Created an attachment (id=20044) [details]
> workaround uinput testcase
>
> bad.c creates a one-key keyboard with KEY_ENTER and attempts to send the
> KEY_ENTER event. by adding an extra dummy key (KEY_A), the uinput device is
> recognized as a keyboard and sends the KEY_ENTER event properly.

whoops, s/bad.c/good.c in that last comment.

I had the same problem. I installed the packages from Jon Oberheide's PPA and it worked. Thanks Jon.

Here's the lsusb output for my fingerprint reader:

Bus 001 Device 004: ID 0483:2016 SGS Thomson Microelectronics Fingerprint Reader

I'm on a Thinkpad X61.

Jon Oberheide (jon-oberheide) wrote :
Jon Oberheide (jon-oberheide) wrote :
Jon Oberheide (jon-oberheide) wrote :

I've tracked down the source of this issue in hal's hald/linux/device.c. When a new input device is added, it goes through a series of checks to determine what kind of device it is (mouse, keyboard, etc).

In input_test_key(), hal will figure out the size of the bitmask of the devices (num_bits, aka the number of "buttons" this input device has). If num_bits == 1, hal will treat the device as a "button" (eg. power/sleep/hibernate button) rather than a keyboard. Otherwise, it will run further checks to see what kind of keyboard it is (input_test_keyboard(), input_test_keypad()).

This explains exactly why having only a single key on the uinput device (KEY_ENTER) fails to work and adding a second dummy key (KEY_A) fixes the problem. Without the dummy key, the uinput device is treated as a "button" instead of a "keyboard" and does not send the KEY_ENTER event when using libpam-thinkfinger.

I'll email upstream and figure out if there's a proper fix to allow one-button keyboards or if hacking around it is the best approach.

Changed in hal:
status: Unknown → Confirmed

Created an attachment (id=20094)
proposed patch

This patch implements what the inline TODO suggests and checks for a id/bustype of BUS_HOST instead of assuming that any 1-bit input devices are acpi buttons. The patch fixes the problem described in the bug as 1-bit key devices are now handled as "keyboards" instead of "buttons".

sachem_s (sachem-s) wrote :

Recently upgraded from ubuntu 8.04 to 8.10 on my Dell XPS 1330, the finger print module worked perfectly before the upgrade, but now .....
firstly I had to hit carriage return after the swipe, but this was later fixed after I installed packages from Jon Oberheide.
But there's something else I realized now, if I try to type my password (instead of swiping my finger) then it prompts me to retype the password, and it does this thrice.

Marius Gedminas (mgedmin) wrote :

sachem_s: please file a separate bug for your inability to log in by typing the password instead of swiping a finger.

Vreixo Formoso (metalpain2002) wrote :

sachem_s: please check if in /etc/pam.d/common-auth you have the "try_first_pass" option to the pam_unix.so module, something like this:

auth [success=1 default=ignore] pam_unix.so try_first_pass nullok_secure

otherwise that module won't use the password previously submitted to the thinkfinger module, and it will prompt you for another password.

Ghosty (ghosty.be) wrote :

Thx Jon Oberheide, this works great! Also nice bug tracking and description.

I added this fix to the thinkwiki descriptions for T61p (see: http://www.thinkwiki.org/wiki/Install_Ubuntu_Intrepid_Ibex_on_a_T61p#Fingerprint_Reader)

Marius Gedminas (mgedmin) wrote :

Jon: I've libpam-thinkfinger 0.3+r118-0ubuntu4~ppa1 from your PPA. It has the following problem: if you type in your password and press Enter at the 'Password or swipe finger:' prompt, it still sends that extra Enter keypress (twice!). So, for example, if you do sudo apt-get dist-upgrade, you get no choice to answer the first two apt's confirmation prompts -- they get answered for you.

If I swipe my finger, no spurious Enter keystrokes appear.

Noel J. Bergman (noeljb) wrote :

This regression still exists in Jaunty, too.

Michael Rooney (mrooney) on 2008-12-26
Changed in thinkfinger:
importance: Undecided → Medium
status: New → Confirmed
Changed in thinkfinger (Ubuntu):
status: Confirmed → Fix Released
37 comments hidden view all 117 comments
Vreixo Formoso (metalpain2002) wrote :

No problem for me, it works out-of-the-box in a fresh 64bit karmic install. Note that I have not updated from beta, I have installed the final release.

Raja (rajajs) wrote :

A reboot seems to have fixed the problem. Seems to have been a situation something like rupa. It appeared for some time, and seems to have got fixed with some updates.

I bisected myself through the xf86-input-evdev versions 2.2.5 to 2.3.1, checking whether my fingerprint reader would still be functioning without the necessity to append an extra Enter after the swipe.
The first bad commit was this one:

commit 1f641d75edba7394201c1c53938215bae696791b
Author: Oliver McFadden <email address hidden>
Date: Thu Jul 23 13:19:49 2009 +0300

    evdev: Only send the events at synchronization time.

    Instead of just posting the button/key press/release events to the
    server as soon as they arrive, add them to an internal queue and post
    them once we receive an EV_SYN synchronization event.

    The motion events are always sent first, followed by the queued events.
    There will be one motion event and possibly many queued button/key
    events posted every EV_SYN event.

    Note that the size of the event queue (EVDEV_MAXQUEUE) is arbitrary and
    you may change it. If we receive more events than the queue can handle,
    those events are dropped and a warning message printed.

    Tested on my Lenovo T400 using evdev for all input devices; keyboard,
    touchpad, and trackpoint.

    Signed-off-by: Peter Hutterer <email address hidden>

The versions before that commit send 3 events when swiping the finger over the reader:
Event. Dev: input11, Type: 1, Code: 28, Value: 1
Event. Dev: input11, Type: 1, Code: 28, Value: 0
Event. Dev: input11, Type: 0, Code: 0, Value: 1

whereas this version sends only 2 events:
Event. Dev: input12, Type: 1, Code: 28, Value: 1
Event. Dev: input12, Type: 1, Code: 28, Value: 0

the missing third event is only sent after Enter has been pressed on an external keyboard:

Event. Dev: input7, Type: 4, Code: 4, Value: 458792
Event. Dev: input7, Type: 1, Code: 28, Value: 1
Event. Dev: input7, Type: 0, Code: 0, Value: 0
Event. Dev: input12, Type: 0, Code: 0, Value: 1

Any idea what this commit has changed regarding the behavior of the fingerprint reader?

Thanks to Jon's input, issue fixed by sending an extra sync event in pam_thinkfinger.

(In reply to comment #8)
> Thanks to Jon's input, issue fixed by sending an extra sync event in
> pam_thinkfinger.

I guess it depended on the previous behavior which was to send events through as soon as we received them. This should be the correct solution. Sorry for any inconvenience caused; I didn't think to test fingerprint readers at the time.

Also the patch from comment #4 should go in (or at least via xorg-devel) if it hasn't already.

Pavel Rojtberg (rojtberg) wrote :

this seems to have come back in lucid...

xak (zach-xak) wrote :

Confirmed issue is back again on Lucid alpha3. Using package thinkfinger-tools 0.3+r118-0ubuntu4 on 64-bit Lucid alpha3 on Lenovo T61p.

Pelon (pelonicus) wrote :

Confirmed issue is back again on Ubuntu 9.10, thinkfinger 0.3+r118-0ubuntu4 on 32-bit, in graphics mode.
In console(CTRL+ALT+F1), work without problem login and sudo.

KERNEL=2.6.31-20-generic
GNOME=2.28.1

Noel J. Bergman (noeljb) wrote :

I'm not sure to which bug report it was posted, but Ubuntu has decided to no longer support Thinkfinger. fprint is the supported direction, despite its flaws, because no one has been working on Thinkfinger upstream and RedHat is funding fprint development.

Björn Torkelsson (torkel) wrote :

Checking the bug reports for the various fprint packages not much seems to be happening in Ubuntu and thinkfinger seems still to be the better option in most cases where the fingerprint reader is supported by thinkfinger.

Vreixo Formoso (metalpain2002) wrote :

I'm having the same problem on Lucid, in graphics mode. It works ok in console.

Ben (ben-lemire) wrote :

I'm also seeing this in Lucid. Everything seems to work, except I need to hit enter. This had been fixed in 9.10!

Vreixo Formoso (metalpain2002) wrote :

I have switched to fprint (Noel, thanks for the hint), it works pretty well on my DELL Vostro 1710, fingerprint is (from lsusb):

 0483:2016 SGS Thomson Microelectronics Fingerprint Reader

I have followed the instructions here: http://reactivated.net/fprint/wiki/Pam_fprint. Configuration is much like thinkfinger.

Once configured, it prompts you to scan your finger, no chance for entering password instead... After several failed attempts, however, it falls back to password authentication. I wonder if there is any way to change such behavior.

Vreixo Formoso (metalpain2002) wrote :

Update: I found an app fprint does not work pretty well with: gksu (see bug #347778),

Samuel Piau (lostnet) wrote :

This patch seems to fix the problem in Lucid :
http://sourceforge.net/tracker/?func=detail&atid=889697&aid=2921928&group_id=179573

I built the package with this patch and installed it.
Now I can use sudo in a gnome terminal without hitting the enter key.

Nate Moore (putermd) wrote :

Looks like this bug is back in 10.04. Exact same symptoms.

I tried adding Jon's PPA, but there's no packages for Lucid.

Anyone got any ideas? I'm not really keen on the X hack, although I'm more comfortable with it than the patch Samuel Piau wrote about, primarily because I understand what the X hack is doing, not what his pam patch is doing.

The patch linked to in comment #83 works for me.

I'm trying to build a version for my PPA... but I just need to wait for various build messes to complete... I tried doing something useful with the package versions...

Samuel Piau (lostnet) wrote :

Here is a more detailed explanation about this patch, it is not related to pam, but to device input event processing.

At some point xf86-input-evdev was changed to a newer version (2.2.5 to 2.3.1 ?), and they changed the events processing :

    evdev: Only send the events at synchronization time.

    Instead of just posting the button/key press/release events to the
    server as soon as they arrive, add them to an internal queue and post
    them once we receive an EV_SYN synchronization event.

    The motion events are always sent first, followed by the queued events.
    There will be one motion event and possibly many queued button/key
    events posted every EV_SYN event.
    [...]

What happens is the additional key code that was added in previous thinkfinger patch is not sent until a real keystroke happens (when you press enter). The patch here adds a synchronisation event after the key code, so that it is immediately posted.

See the comments from Jon Oberheide and Philipp Gortan (mephinet) in this Gentoo Bug report :
http://bugs.gentoo.org/show_bug.cgi?id=298459

Nate Moore (putermd) wrote :

I'm not at a level where I feel comfortable creating my own packages from source, which looks like something you did here. My previous attempts have failed spectacularly.

Are you in a position to share the package you built?

Ben (ben-lemire) wrote :

So is the patch works, what's the next step to get it accepted? It would be nice if this regression didn't happen in future distribution updates.

Packages containing the patch are in my PPA at:

  https://launchpad.net/~martin-meltin/+archive/ppa

A quick test seems to show that these packages fix the problem. If others can install, test and confirm then perhaps someone will build an official package for lucid....

Nate Moore (putermd) wrote :

Thanks Martin. The packages work.
For some reason they wouldn't install using APT, so I downloaded them and used dpkg to install them.

APT always came back and told me I had the latest version.

I suspect it's because I used to have Jon's PPA and his version number is higher than yours, but that's just a theory.

Good to hear that it works for you too, Nate.

Yeah, the documentation has changed its recommendation for modifying package versions for PPAs, so I followed the new recommendation. Older packagers from PPAs (such as Jon's will probably look newer). You should be able to install mine by doing something like:

  apt-get install libpam-thinkfinger=0.3+r118-0ubuntu4ppamartinmeltin5 \
    libthinkfinger0=0.3+r118-0ubuntu4ppamartinmeltin5 \
    thinkfinger-tools=0.3+r118-0ubuntu4ppamartinmeltin5

After that, if you remove the other PPA from your /etc/apt/sources.list.d/ (or wherever), future updates should work.

Bombenbach (bombenbach) wrote :

I confirm that Martin's version works on my Thinkpad Z61m (SGS Thomson Microelectronics Fingerprint Reader) with Lucid.

The packages from Joe's repo, however, gave me
 Warning: usb_bulk_read expected to read 0x40 (read 0x8 bytes).
when running "tf-tool --acquire" so that I couldn't use ThinkFinger at all.

ThinkFinger packages from Martin's repo work great, everything works as good as in Karmic and Jaunty.

I am confirming: Martin's version works fine here too.
(Got a SGS Thomson Microelectronics Fingerprint Reader).
Thanks!

Bombenbach (bombenbach) wrote :

Since we already have a working fix for thinkfinger why would nobody add it to the official repo? For example for business users working fingerprint authentication may be an important criterion to use or not to use Ubuntu and since thinkfinger did work in Hardy, Jaunty and Karmic, for a lot of people doing dist-upgrade to Lucid this bug might become a really bad surprise!

Thomas (t-hartwig) wrote :

This is still not fixed in Maverick. Is there any maintainer for this package?

Dimitri John Ledkov (xnox) wrote :

Ubuntu review team, please fast-forward this bug into archive =)

tags: added: patch
Bombenbach (bombenbach) wrote :

Before this bug will go into archive it should at least be f.i.x.e.d. I've even posted a message to the Ubuntu-motu maillist (http://tinyurl.com/358m7lq) since MOTU is listed as the official package maintainer and ... nothing as you see! Is it really that difficult to add a working patch to a package in the repository? So much for filling bugs and helping developers :(

Woonjas (woonjas) wrote :

fix please?

pretty PLEASE?

this really was a nasty surprise when I upgrded from Karmic.

Ben (ben-lemire) wrote :

Any word on when this is getting added to the repos? Someone, anyone?

Tom Jaeger (thjaeger) on 2010-07-24
Changed in thinkfinger (Ubuntu):
status: Fix Released → Confirmed
1 comments hidden view all 117 comments
Fabrice Coutadeur (fabricesp) wrote :

Hi,

For your information, thinkfinger has been removed from Debian almost one year ago because it was dead upstream (http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=546005).

I'll upload the fix, but except if someone adopt it, this kind of problems will appear from time to time, with sometime no solutions. My advice is that you upgrade to fprint, as indicated in the Debian bug I referenced.

Fabrice

Launchpad Janitor (janitor) wrote :

This bug was fixed in the package thinkfinger - 0.3+r118-0ubuntu5

---------------
thinkfinger (0.3+r118-0ubuntu5) maverick; urgency=low

  * pam/pam_thinkfinger-uinput.c: Send sync events to ensure key events are
    processed (LP: #256429). Patch taken from upstream bug tracker
    (http://sourceforge.net/tracker/?func=detail&atid=889697&aid=2921928&group_id=179573)
 -- Thomas Jaeger <email address hidden> Sat, 24 Jul 2010 10:19:07 +0200

Changed in thinkfinger (Ubuntu):
status: Confirmed → Fix Released

Thanks for uploading the package. fprint is just as dead upstream as
thinkfinger, and in addition does not allow the user to chose between
fingerprint and password authentication, so it's not a viable
alternative at this point.

On 07/27/2010 05:53 AM, Fabrice Coutadeur wrote:
> Hi,
>
> For your information, thinkfinger has been removed from Debian almost
> one year ago because it was dead upstream (http://bugs.debian.org/cgi-
> bin/bugreport.cgi?bug=546005).
>
> I'll upload the fix, but except if someone adopt it, this kind of
> problems will appear from time to time, with sometime no solutions. My
> advice is that you upgrade to fprint, as indicated in the Debian bug I
> referenced.
>
> Fabrice
>
> ** Bug watch added: Debian Bug tracker #546005
> http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=546005
>

Noel J. Bergman (noeljb) wrote :

If anyone is using thinkfinger with Maverick, please see Bug 609645. As noted there, I'm seeing the failure with the newly uploaded thinkfinger as well as current versions of fprint(d).

Bombenbach (bombenbach) wrote :

Excuse me, but why did you add it only to Maverick repos? What about Lucid?

Woonjas (woonjas) wrote :

Indeed, are there any plans to fix in Lucid?

Changed in hal:
importance: Unknown → Medium

I took the suggestion and upgraded to fprint for a couple of months. It is a downgrade. pam_thinkfinger at least prompts sensibly when doing the gsudo-type things. pam_fprint just sits there stupidly and you have to guess when something is waiting for a fingerprint scan or password. Combine that with not being able to type the password while a scan is being requested and you have a pretty frustrating piece of software... :-(

Changed in hal:
importance: Medium → Unknown
Changed in hal:
importance: Unknown → Medium
Displaying first 40 and last 40 comments. View all 117 comments or add a comment.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.