Discussion:
[Thinkfinger-devel] Thinkfinger will be in Fedora Extras really soon, and a few proposals
l***@poczta.onet.pl
2007-02-20 10:36:36 UTC
Permalink
Hello. I have packaged thinkfinger for Fedora Extras. It has already passed the review, so users should be able to install it with yum install thinkfinger really soon. I have some suggestions as well.
1. Could you please drop a patch that removes executable permissions from pam_thinkfinger.so? It causes rpm not to strip the library, and as a result some rpmlint warnings.
2. The reviewer has suggested the following: “Would it be worth filing a RFE against pam to include the line needed in /etc/pam.d/system-auth? It should ignore it if the module isn't available I think, and will only work for users that setup a fingerprint. Perhaps something to think about when this package is more mature.”
3. I have made a quick browse through SVN and I haven't noticed that the patch for 1654013 buffer overflow is included. Please include it, for me the overflow is not only possible, it is actually happening. Patch fixes the problem. Apart from that, thinkfinger works very well in Fedora. I'm attaching the patch for your convenience (to avoid tabs/spaces pain).

Regards,
JS
Timo Hoenig
2007-02-21 09:17:06 UTC
Permalink
Hi JS,
Post by l***@poczta.onet.pl
1. Could you please drop a patch that removes executable permissions
from pam_thinkfinger.so? It causes rpm not to strip the library, and
as a result some rpmlint warnings.
I've added that on behalf on Luca. He can probably comment on that.
When building RPMs for openSUSE and SUSE Linux Enterprise Server/Desktop
I did not run into issues with the +x flag.
Post by l***@poczta.onet.pl
2. The reviewer has suggested the following: “Would it be worth
filing a RFE against pam to include the line needed
in /etc/pam.d/system-auth? It should ignore it if the module isn't
available I think, and will only work for users that setup a
fingerprint. Perhaps something to think about when this package is
more mature.”
RFE?

PAM (upstream) is not required to know about ThinkFinger. Upon
installation of ThinkFinger it is up to the distribution to set up the
PAM configuration.

Other than that: pam_thinkfinger already hops off if a user logs in
for whom there is no biometric identification record being stored. I
suspect that the reviewer did not look very close at the code. The
package is probably "more mature" than he though?
Post by l***@poczta.onet.pl
3. I have made a quick browse through SVN and I haven't noticed that
the patch for 1654013 buffer overflow is included. Please include it,
for me the overflow is not only possible, it is actually happening.
Patch fixes the problem. Apart from that, thinkfinger works very well
in Fedora. I'm attaching the patch for your convenience (to avoid
tabs/spaces pain).
Yup, it's in.

Thanks,

Timo
Timo Hoenig
2007-02-21 09:18:55 UTC
Permalink
Almost forgot...
Post by l***@poczta.onet.pl
I have packaged thinkfinger for Fedora Extras.
Thanks for that! Very much appreciated!

Timo
Julian Sikorski
2007-02-21 12:02:13 UTC
Permalink
Post by Timo Hoenig
Hi JS,
Post by l***@poczta.onet.pl
1. Could you please drop a patch that removes executable permissions
from pam_thinkfinger.so? It causes rpm not to strip the library, and
as a result some rpmlint warnings.
I've added that on behalf on Luca. He can probably comment on that.
When building RPMs for openSUSE and SUSE Linux Enterprise Server/Desktop
I did not run into issues with the +x flag.
Well, RPM is quite likely to be “a bit” different across the distros. The only problem is that the script that strips binaries does not recognise the lib as a one that needs its attention (I think). Anyway, re-adding the +x bit is like a one line in spec, so it is not a big deal.
Post by Timo Hoenig
Post by l***@poczta.onet.pl
2. The reviewer has suggested the following: “Would it be worth
filing a RFE against pam to include the line needed
in /etc/pam.d/system-auth? It should ignore it if the module isn't
available I think, and will only work for users that setup a
fingerprint. Perhaps something to think about when this package is
more mature.”
RFE?
PAM (upstream) is not required to know about ThinkFinger. Upon
installation of ThinkFinger it is up to the distribution to set up the
PAM configuration.
Other than that: pam_thinkfinger already hops off if a user logs in
for whom there is no biometric identification record being stored. I
suspect that the reviewer did not look very close at the code. The
package is probably "more mature" than he though?
Post by l***@poczta.onet.pl
3. I have made a quick browse through SVN and I haven't noticed that
the patch for 1654013 buffer overflow is included. Please include it,
for me the overflow is not only possible, it is actually happening.
Patch fixes the problem. Apart from that, thinkfinger works very well
in Fedora. I'm attaching the patch for your convenience (to avoid
tabs/spaces pain).
Yup, it's in.
Thanks,
Timo
Timo Hoenig
2007-02-21 15:02:06 UTC
Permalink
Hi Julian,
Post by Julian Sikorski
Well, RPM is quite likely to be “a bit” different across
the distros. The only problem is that the script that strips binaries
does not recognise the lib as a one that needs its attention (I
think). Anyway, re-adding the +x bit is like a one line in spec, so it
is not a big deal.
I'll have a look at other packages which come with PAM modules. If
there's something different I'm happy to adopt that for ThinkFinger.

Thanks,

Timo
Timo Hoenig
2007-02-21 15:06:23 UTC
Permalink
If there's something different I'm happy to adopt that for
ThinkFinger.
<wearing my packager hat>

Only few things are more annoying than carrying around distribution
specific patches which could be avoided.

Timo
Luca Capello
2007-02-21 22:19:05 UTC
Permalink
Hello!
Post by Timo Hoenig
Post by Julian Sikorski
Well, RPM is quite likely to be &#8220;a bit&#8221; different
across the distros. The only problem is that the script that strips
binaries does not recognise the lib as a one that needs its
attention (I think). Anyway, re-adding the +x bit is like a one
line in spec, so it is not a big deal.
I'll have a look at other packages which come with PAM modules. If
there's something different I'm happy to adopt that for ThinkFinger.
OK, my fault about permissions... I just checked pam-unix2 [1] and it
installs the module with write permission. I was biased by the other
modules I've on my Debian. Thus, sorry Timo, I think my patch at [2]
should be reverted.

Thx, bye,
Gismo / Luca

Footnotes:
[1] ftp://ftp.suse.com/pub/people/kukuk/pam/pam_unix2/
[2] http://thread.gmane.org/gmane.linux.drivers.thinkfinger/110
Timo Hoenig
2007-02-22 07:34:58 UTC
Permalink
Hi Luca,
Post by Luca Capello
OK, my fault about permissions... I just checked pam-unix2 [1] and it
installs the module with write permission. I was biased by the other
modules I've on my Debian. Thus, sorry Timo, I think my patch at [2]
should be reverted.
Nevermind, patch now is reverted.

Thanks,

Timo

Loading...