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 Description: 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
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. Good: + 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) Needswork: - 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. Check: . 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?
> 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. Upgraded. 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
Note that hotplug agents are pretty much obsoleted by udev in FC5. For an example, see the openct package in Extras CVS (devel branch).
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?
(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 /etc/pam.d/login: 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.
* 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 ...
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 APPROVED
I'm orphaning this, please, change the initial owner in owners.list .