Bug 167714 - Review Request: pam_usb: PAM module for use with DSA key pairs and removable devices
Review Request: pam_usb: PAM module for use with DSA key pairs and removable...
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
All Linux
medium Severity medium
: ---
: ---
Assigned To: Wart
David Lawrence
Depends On:
  Show dependency treegraph
Reported: 2005-09-07 11:19 EDT by Dmitry Butskoy
Modified: 2007-11-30 17:11 EST (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2005-12-29 09:37:18 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
jwboyer: fedora‑cvs+

Attachments (Terms of Use)

  None (edit)
Description Dmitry Butskoy 2005-09-07 11:19:31 EDT
Spec Url: http://dmitry.butskoy.name/pam_usb/pam_usb.spec
SRPM Url: http://dmitry.butskoy.name/pam_usb/pam_usb-0.3.2-1.src.rpm

This PAM module provides authentication through DSA private/public keys.
Public keys placed on the target computer, whereas private keys are stored
on some removable device, including USB storages/flash drives, cdroms,
floppies, etc.

Any kind of mountable devices (not removable only) can be used.

Due to using of DSA key pairs, the passwordless authentication can be organized
(if a private key is stored not crypted on the media).

Additional info:

I separate hotplug stuff as an additional binary package, default not built (IMHO, it looks like not related to pam_usb, still not documented, etc.)

There is an article about pam_usb, a tip of the week at fedoranews site: http://fedoranews.org/mediawiki/index.php/Tip_of_the_Week_2005-08-08
Comment 1 Wart 2005-12-19 18:53:14 EST
I haven't been approved as a reviewer yet, so this remains an unofficial review.
 I'll take reviewing responsibility as soon as I have permission to do so.

+ Source matches upstream
+ GPL license ok.  Includes license text.  Matches upstream.
+ ChangeLog ok.
+ No tag violations
+ BuildRoot ok
+ No unnecessary Requires
+ No unnecessary BR
+ Summary and description ok.
+ Includes relevant documentation.
+ No desktop file needed.
+ Consistent macro usage.
+ Not relocatable
+ Code, not content
+ Package names contains an underscore, but so does upstream, so I'm willing to
ignore it.
+ Source matches upstream
+ 0wns its own directories.
+ Shared library not in default linker path; ldconfig not needed.
+ No duplicates in %files
+ permissions look ok
+ Builds in mock (for i386)

- Doesn't install on x86_64.  spec file correctly uses %{_lib}, but
  src/Makefile hardcodes $(DESTDIR)/lib/.
- Conditional compilation of hotplug:  Is this really necessary?  For FE I
  would think that you'd always want to build the hotplug module.  I would
  recomend either removing this conditonal completely, or changing it so
  that the hotplug package is built by default.

. Runs - I'll test that as soon as the x86_64 package works.

Additional comments:

rpmlint output seems harmless:
W: pam_usb unstripped-binary-or-object /lib/security/pam_usb.so

Commas are not necessary in BuildRequires.  My preferencs is to remove
them, but this is not a blocker.

It looks like 0.3.3 is available upstream.  Have you considered upgrading?
Comment 2 Dmitry Butskoy 2005-12-20 07:13:03 EST
> src/Makefile hardcodes $(DESTDIR)/lib/.
Add a patch for Makefiles for this and another issues.

> Conditional compilation of hotplug
Now just a separate package, compiled unconditionally

> pam_usb unstripped-binary-or-object /lib/security/pam_usb.so
Now stripped properly...

> It looks like 0.3.3 is available upstream. 

New SPEC: http://dmitry.butskoy.name/pam_usb/pam_usb.spec
New SRPM: http://dmitry.butskoy.name/pam_usb/pam_usb-0.3.3-1.src.rpm

Comment 3 Ville Skyttä 2005-12-20 10:45:24 EST
Note that hotplug agents are pretty much obsoleted by udev in FC5.  For an
example, see the openct package in Extras CVS (devel branch).
Comment 4 Dmitry Butskoy 2005-12-20 11:55:09 EST
Does it mean that pam_usb-hotplug will not work under FC5 completely?
In other words, should we do separate things for FC5 or not?
Comment 5 Wart 2005-12-21 01:00:03 EST
(In reply to comment #4)
> Does it mean that pam_usb-hotplug will not work under FC5 completely?
> In other words, should we do separate things for FC5 or not?

That's how I understand it.  When using udev on FC4 the -hotplug package isn't
needed either.  Depending on which older releases you want to package this for,
you could possibly get rid of the -hotplug package altogether.

btw, I finally got around to running this package and it works great.  :)

The only other suggestion that I would have is to include a couple of sample
pam.d configuration files in %doc that show how it should be used in Fedora Core
4.  For example, I had to add the following just before the pam_stack.so line in

auth  sufficient  pam_usb.so check_if_mounted mount_opts=ro quiet

but had to use the following in /etc/pam.d/gdm:

auth  sufficient  pam_usb.so check_if_mounted mount_opts=ro allow_remote

...and still haven't gotten /etc/pam.d/xscreensaver to work right.  In debug
mode the logs indicate that authentication succeeds, but it doesn't actually log
me back in.
Comment 6 Dmitry Butskoy 2005-12-22 10:31:42 EST
* add /etc/udev/rules.d file instead of /etc/hotplug.d

* add your pam.d examples in %doc

New SPEC: http://dmitry.butskoy.name/pam_usb/pam_usb.spec
New SRPM: http://dmitry.butskoy.name/pam_usb/pam_usb-0.3.3-2.src.rpm

> and still haven't gotten /etc/pam.d/xscreensaver to work right.
Perhaps it could chagnge after switch to udev ...

Comment 7 Wart 2005-12-28 18:36:20 EST
All needswork items have been fixed.  Package now compiles and runs on x86_64.

rpmlint has no complaints for the base package, and the following output for
pam_usb-hotplug, both of which can be ignored.

W: pam_usb-hotplug no-documentation
E: pam_usb-hotplug executable-marked-as-config-file /etc/pam_usb/handlers/xlock.sh

Comment 8 Dmitry Butskoy 2007-05-02 06:39:41 EDT
I'm orphaning this, please, change the initial owner in owners.list .

Note You need to log in before you can comment on or make changes to this bug.