[fprint] pam_fprint 0.2: some improvements

[Bastien Nocera]

On Fri, 2008-07-11 at 10:52 +0200, Diego Zuccato wrote:
> Pavel Herrmann ha scritto:
> 
> > Please don't think I don't appreciate your work on pam_fprint, but isn't 
> > it better to make patches against latest GIT version?
> Well, it really should. But I still have to learn to use GIT :-(
> Making it work with multiple fingers was a high priority work, for me.
> Now I can look at GIT to send another patch, maybe w/ some extra cleanup 
> (I left all the old code in place, "just in case").
> If others are interested, I can refiff against latest git, but if I'm 
> the only one interested...
> 
> Anyway, before proceeding, I'd like feedback on some assumptions I made:
> 1) it's ok that, if pam requires authenticating w/ X>Y prints and user 
> enrolled just Y prints (w/ available readers), access is DENIED

It should probably be unavailable actually, otherwise you won't be able
to stack the authentications. Ie. if you allow a fingerprint, and check
for a password when it's not available, using denied will mean that
they'll never get asked a password.

> 2) any user could have prints enrolled from multiple readers

Yes. But there's only one device being handled at a time in pam_fprint,
and fingerprint data from one reader can't be used on another one.

> 3) it is OK *not* to short-circuit a print recognition (so if X prints 
> are required, X prints are scanned, even if the first fails)

That would be fair enough if scanning fingerprints was as reliable as
entering a password, but it's not, and you'd get a few miffed off users
that wouldn't know which finger print they bodged the entry for.

> 4) it's OK to only return PAM_SUCCESS and PAM_AUTH_ERR (no more 
> PAM_AUTHINFO_UNAVAIL)

As mentioned above, unavail should still be used. Otherwise what would
happen when the fingerprint reader is already in use or not plugged in?

> 5) (obvious) there are no objections to the proposed options (and no 
> evident bugs, like in the first proposed patch)

The functionality sounds fine, although I didn't read the code. You
should really use git to generate it, or at the very least generate your
patch as a unified diff (diff -up). The current diff is unreadable by
humans.

> Another thing I'd like to implement is handling a "fixed fingers list" 
> so that different apps can require different fingers to be scanned. This 
>   could remove one implication (nfingers => randomlist), but adds 
> another (len(flist) >= nfingers, and if strictly greater the others are 
> chosen at random).

which applications would be using that? Different pam setups would
require a different pam config file.

Cheers