Glad to see you're submitting this, will review.  Some initial comments from 
skimming the specfile, not actually tested yet: 
  
- pam_pkcs11 0.5.2 is out (and renamed to pam_pkcs11 upstream), see  
http://oasis.dit.upm.es/~jantonio/pam-pkcs11/downloads/ , maybe update?  
 
- License seems to be LGPL, not GPL 
 
- Package is self-obsoleting, suggesting changing the obsoletes to "<" (or 
dropping them altogether), also in -tools 
 
- I wonder if automake is needed for building (not checked), I don't see it 
called and no patches are applied etc 
 
- Unowned %{_sysconfdir}/pkcs11, %{_libdir}/pam_pkcs11 and 
%{_datadir}/pkcs11_login dirs 
 
- Shouldn't the /etc stuff be somewhere below /etc/pki? 
 
- configuring with --disable-static could be cleaner (untested) 
 
Then, a related note: pam_opensc will most likely be dropped in the next 
upstream opensc release, with this being the replacement (unbundled, I hope).  
At that time, I think I would be good to respin this one and add Obsoletes: 
pam_opensc < $somever here instead of the opensc package.  But that's in the 
future, I don't think we should rush that yet. 

New SRPM: http://www.auroralinux.org/people/spot/review/pam_pkcs11-0.5.2-1.src.rpm
New SPEC:
http://www.auroralinux.org/people/spot/review/pam_pkcs11.spec

I don't see any problem with it Obsoleting pam_opensc when the time comes. 
--disable-static didn't work, it broke the compile. All other changes were made.

Still just poking around, I have nothing to test this with right now (but will  
have later this week), assorted notes (some of them cosmetic): 
  
- I would personally just nuke the whole %{_datadir}/pam_pkcs11 dir, the 
  same files are already installed in /etc/pki/pkcs11 as config files. 
  Exception: pam.d_login.example could be added to main package's %doc, it 
  doesn't seem to be available elsewhere. 
 
- Maybe move the default location of the "default" module to somewhere 
  below %{_libdir} too, /lib/security appears somewhat kind of 
  reserved for pam modules and pkcs11_module.so there seems slightly 
  misplaced to me.  This wouldn't cause any /lib vs /usr/lib mountedness 
  problems; pam_pkcs11.so won't function anyway if /usr is not mounted. 
  Upstream seems to have changed to /usr/lib/pam_pkcs11/pkcs11_module.so in 
  svn, http://www.opensc.org/pam_pkcs11/file/trunk/etc/pam_pkcs11.conf.example 
 
- Hardcoded /lib/security in src/pam_pkcs11/Makefile* will probably break on 
  x86_64 and friends.  Maybe "%ifarch x86_64 ppc64 sparc64 ia64" (dunno what's 
  the exact list of potentially affected archs) and just move the module to 
  /lib64/security if so. 
 
- Ditto, hardcoded /usr/lib/pkcs11/opensc-pkcs11.so for the opensc PKCS #11 
  module location and /usr/lib/pam_pkcs11 for the mappers will cause 
  problems on the above archs. 
 
- %changelog mentions --disable-static, but it's not used 
 
- %defattr missing from -tools 
 
- Doubly-owned %{_sysconfdir}/pki/pkcs11 in both main package and -tools, 
  doesn't really hurt, though. 
 
With the above taken care of, go ahead and commit to CVS, the rest can be 
worked out there before the first build.  I'll recheck a bit later when I have 
access to a smart card reader again. 

ping :) 

OK. -2 with all those fixes is checked into CVS.

More findings, this time actually testing the module:    
    
- Path to pam_pkcs11.conf in the pki patch is wrong, should     
  be /etc/pki/pkcs11, not /etc/pki/pam_pkcs11.    
    
- pam_pkcs11.conf mentions $HOME/.ssh/authorized_keys for the opensc mapper,    
  that's almost certainly wrong and should probably be     
  $HOME/.eid/authorized_certificates    
    
- Security: all /etc/pki/pkcs11/*_mappers contain _enabled_ example entries,    
  they should be commented out or removed.    
    
- Undefined symbols: trying to use the opensc mapper (with debug on) barfs:    
[...]    
DEBUG:mapper_mgr.c:77: dlopen failed for module:  opensc    
path: /usr/lib/pam_pkcs11/opensc_mapper.so    
Error: /usr/lib/pam_pkcs11/opensc_mapper.so: undefined symbol: is_empty_str    
DEBUG:mapper_mgr.c:77: dlopen failed for module:  null    
path: /usr/lib/pam_pkcs11/null_mapper.so    
Error: /usr/lib/pam_pkcs11/null_mapper.so: undefined symbol: mapper_find_user    
[...]    
    
- Note: FC-3 doesn't have /etc/pki; you decide whether that matters and   
  whether to do something about it in this package.   
    
- Cosmetics from comment 3 (feel free to ignore, won't mention these again):  
  0.5.2-1 changelog still mentions --disable-static being used, but it's not.   
  Also, the double ownership of the /etc/pki/pkcs11 dir remains in main and 
  -tools %files. 
 
- /usr/bin/make_hash_link.sh smells a bit too generic IMHO, could consider 
  moving it eg. to /etc/pki/pkcs11, you decide. 

Hmm, actually it seems that /etc/pki/pam_pkcs11 is used  consistently  
throughout the pki patch, but the specfile installs stuff to /etc/pki/pkcs11.  
Maybe fix the specfile instead of the patch. 

Re: opensc mapper:   
http://opensc.org/pipermail/opensc-devel/2005-August/006936.html  
Drop it and the config file part for now?  The mapper is not useful for  
anything AFAICT, see its code...  
  
More fun, digest mapper bombs after verifying the CA cert, let me know if you 
want a backtrace: 
 
*** glibc detected *** su: free(): invalid next size (fast): 0x0913ae08 *** 
======= Backtrace: ========= 
/lib/libc.so.6[0x5eb124] 
/lib/libc.so.6(__libc_free+0x77)[0x5eb65f] 
/usr/lib/pam_pkcs11/digest_mapper.so(get_mapent+0x22)[0xeec135] 
/usr/lib/pam_pkcs11/digest_mapper.so(mapfile_find+0x120)[0xeec476] 
/usr/lib/pam_pkcs11/digest_mapper.so(mapfile_match+0x30)[0xeec5a6] 
/usr/lib/pam_pkcs11/digest_mapper.so[0xeec7c7] 
/lib/security/pam_pkcs11.so(match_user+0x73)[0xa66f6f] 
/lib/security/pam_pkcs11.so(pam_sm_authenticate+0x7d4)[0xa661d1] 
/lib/libpam.so.0(_pam_dispatch+0x28c)[0xda8d8c] 
/lib/libpam.so.0(pam_authenticate+0x47)[0xdaae9b] 
su(main+0x30c)[0xd0682f] 
/lib/libc.so.6(__libc_start_main+0xdf)[0x59cd5f] 
su[0xd05941] 
 
Doesn't look too good :( 

What do you think about similar packages pamusb (http://pamusb.sourceforge.net)
and pam-x509 (http://pam-x509.sourceforge.net) ?
Whether has sense to add them to FE too?

I'm not familiar with either of those, but they seem useful and substantially 
different from pam_pkcs11.  OTOH, the two seem to have pretty much equivalent 
functionality so maybe pick what you (or whoever packages them) considers 
better, and submit it first.  FWIW, the last release of pam-x509 was almost 2 
years ago and for pam_usb some 9 months ago, and both are listed as 
"Production/Stable". 

I'm Juan Antonio Martínez, co-author and current maintainer of pam_pkcs11
at http://www.opensc.org/pam_pkcs11

First of all, I'm glad to see interest on our PAM module, but what about
emailing us and notifying that some work in porting to Fedora was in
progress?. tsk, tsk, nettiquette please. ( Mantainers of other distros
already contacted us )

Perhaps you'd better to test and port our new incomming pam_pkcs11-0.5.3
package, actually in test stage. Release is scheduled at Sept 12 2005

You'll notice that most of bugs described here are solved. 0.5.2 is
really a obsolete version. In particular neither opensc, nor openssh,
nor ldap mappers works, and there are known bugs in mapfile handling

btw Incomming 0.5.3 is intensively used in some production environments,
and is full docummented (man pages, html docs, doxygen API and so )

I'm really interested in rpm packaging (I work in a FC4 environment).
So perhaps you'll find better working together

Please take a look at svn tree, and send comments, suggestions and patches
to jonsito _at_ teleline _dot_ es, or even better to OpenSC development
list opensc-devel _at_ lists _dot_ opensc _dot_ org

Juan Antonio

(In reply to comment #11)  
> what about  
> emailing us and notifying that some work in porting to Fedora was in  
> progress?. tsk, tsk, nettiquette please. 
 
Heh, I _did_ notify the opensc list about this submission half an hour ago ;) 

Umm, because I'm swamped? :)

If you're interested in rpm packaging, perhaps you should maintain this package?
I'm more than willing to let the upstream maintainer own this.

"If you're interested in rpm packaging, perhaps you should maintain this package?"

Mmmh. Not sure:

I don't really know the intrinsics of Fedora Team: release politics, Filesystem
structure, and so. In fact the OpenSC team philosophy is not to distribute
binaries, just sources. ( "Let the coders code and packagers package", or as we
say in Spain "Zapatero a tus zapatos" :-)

For Pam_pkcs11, I only provide a "reference .spec file", not a FC4 .spec one. 
Of course, i've tested it under FC4, but certainly sure that is not the correct
one. I lack of Fedora RPM HOWTO knowledge...

Anyway, I'm interested in:
- Knowing progress on pam_pkcs11 packaging on several distros (AFAIK pam_pkcs11
is distributed by Mandriva, Debian, and Fedora)
- Get feedback from packagers, and perhaps include a "dist_patches" directory
into OpenSC svn. Not really interested in mantain several trees.
- Receive notices, suggestions, bug-reports, contribs and so from Fedora channels

As I told Ville Skyttä, I think pam_pkcs11 should not be "Fedora oriented". So
let me stand as neutral as possible. But let me being informed too :-)

Juan Antonio

BTW: where can I find Fedora packaging and FHS guidelines?

Packaging guidelines can be found at:
http://fedoraproject.org/wiki/PackagingGuidelines

There is also a useful checklist at:
http://fedoraproject.org/wiki/PackageReviewGuidelines

There are also work-in-progress guides:
http://fedoraproject.org/wiki/Docs/Drafts/BuildingPackagesGuide
http://koti.welho.com/vskytta/packagers-handbook/packagers-handbook.html

I don't know of any Fedora FHS guidelines yet, so I try to look at what other
packages do.

I don't think there is such a thing as "Fedora FHS guidelines", there's just 
the FHS, http://www.pathname.com/fhs/ 
 
Once this package is in, it's trivial to add your email address to the initial 
Cc list so you'd be Cc'd for all Bugzilla traffic related to this package, if 
that's what you want. 

FYI: pam_pkcs11-0.5.3 just released. 
It fixes all the problems related here, plus adding lots of features

http://www.opensc.org/files/pam_pkcs11-0.5.3.tar.gz

I'm in the progress of updating this submission to 0.5.3. and I'm wondering  
whether the "badstatic" patch is still required.  0.5.3 appears to compile  
fine without it; not yet runtime-tested with nor without. 
  
Tom, could you clarify what is it for, and Juan, could you have a look at the  
patch?  It doesn't apply as-is to 0.5.3, but the intention should be clear.  
http://cvs.fedora.redhat.com/viewcvs/rpms/pam_pkcs11/devel/pam_pkcs11-0.5.2-badstatic.patch?root=extras&rev=1.1&view=auto 
 
BTW, in the process of updating, I submitted a couple of new patches upstream: 
http://www.opensc.org/pam_pkcs11/ticket/9 
http://www.opensc.org/pam_pkcs11/ticket/10 
http://www.opensc.org/pam_pkcs11/ticket/11 

Tom, Juan, could you clarify the status of the "badstatic" patch, see comment  
18? 

OpenSC 0.10.0 is out, as expected without the pam module, so I made this 
obsolete pam_opensc < 0.10.0 in CVS (and this bug is now blocking my OpenSC 
update). 
 
Still waiting for comments about what to do with the "badstatic" patch, see 
comment 18.  Tom, Juan, anyone? 

Sorry for the delay... I'm a bit busy :-(

In release 0.5.3 FIND_USER, MATCH_USER, FIND_ENTRIES and MODULE_END _must_ be
declared static, as they may be statically linked, to avoid "duplicate symbols"

MODULE_INIT should never be used, unless included in dynamically loaded mappers

Anyway, in current tree, "match", "find", and "entries" are used only by null
mapper, that is to be statically linked. So let it remain "static" to get sure
that these functions will not be duplicated

In fact, my idea is remove remove these macros on next release, as they were 
a dirty patch for non-complete mappers

Juan Antonio

PS: in a few weeks I'll restart work on pam_pkcs11.

Okay, thanks for the comments, Juan.  
  
I have now committed my local changes to CVS.  Tom, please note that there are  
differences between devel and FC[34]; devel is tuned for OpenSC 0.10.0 and  
FC[34] for 0.9.6 (paths, Obsoletes), and I intend to keep OpenSC that way for 
the foreseeable future, so please don't do blind branch syncs in this 
package :)  
  
There's one more thing that would be good to address before releasing: I think  
make_hash_link.sh is a too generic name for a script to be included  
in /usr/bin.  Tom, feel free to address (or not) this the way you see fit; 
as far as I'm concerned, this is approved. 

ping?

pong :-)

About make_hash_link.sh . My last svn commit to opensc.org site makes pam_pkcs11
now handle either hash_dir (old behaviour) or cacert files. In the later case,
there is no real need to use make_hash_link.sh script anymore. My plan is move
this script to /usr/lib/pam_pkcs11 (or whatever site according to LSB)

Just starting several "semi-cosmetic" changes to code and preparing a new 0.6
release

Juan Antonio

Thanks for the update.  I'll let spot decide whether to wait for 0.6 or 
publish the current stuff in Extras CVS. 

I'm going to wait for 0.6. Juan, if you'd be so kind as to notify me when it
releases, I would be grateful. :)

Any news from 0.6 (I can't find www.opensc.org) ?

No idea, but the website is (temporarily?) at http://www.opensc-project.org/

I'm temporally off. My Idea is to fix some minor issues on current svn version
and release it as 0.6 on April, as soon as I return to my "normal" life

So it's time for fixes and patches. No more features planned at this moment

Hi all, I didn't realize we had a proposed pam_pkcs11 package up.

We need this for smart card login.
I have some rpm with 0.5.3 and some changes so it can use NSS rather than raw
pkcs_11.

Package is here:
The latest versions appear to be at:
http://directory.fedora.redhat.com/built/rpm_review/rrelyea/pam_pkcs11.spec
http://directory.fedora.redhat.com/built/rpm_review/rrelyea/pam_pkcs11-0.5.3-3.src.rpm

thanks.

bob

I am happily handing this over to Bob for ownership.

This should be Core, not Extras.

Hi Bob,

- I'm not sure what our policy on Epoch is (whether to explicitly list it or not
when it's 0), but you might want to check that.

- You might want to %define macros for the version numbers in your dependencies

- You require automake but you don't use it

- You might want to follow the more standard %patchN -p1 -b .description
convention for patch lines

- Should we standardize on an nss dir? maybe /etc/pki/nss or something?  I think
there is some precedent here, but I don't know what it is.

- make_hash_link.sh is a horrible name for something in %{_bindir}, can we
rename it or move it to libexecdir or something?

Maybe mark the example files as %doc instead of shipping them in datadir?

If epoch is 0, don't list it explicitly.

Thanks. I'll make those changes.
Hmm a common nssdir for system apps does sound like a good idea. /etc/pki/nss
sounds reasonable.

Users should have their own, obviously.

bob

Hope I'm not stepping on any toes but I've tried to update the SRPMS based on
Ray's comments.

http://acs.pha.jhu.edu/~mccannwj/pam_pkcs11/pam_pkcs11-0.5.3-3.src.rpm
http://acs.pha.jhu.edu/~mccannwj/pam_pkcs11/pam_pkcs11.spec

For now I've disabled building the ldap mapping because it fails to compile with
the following message:
http://acs.pha.jhu.edu/~mccannwj/pam_pkcs11/pam-pkcs11-ldap-failure.txt

Suggestions are quite welcome! even in the form of a complete source RPM.;)

I've taken your updated spec and created a new one, however. The rpm includes
the proper Makefile.in, so I've removed the automake requires rather than
accually use automake. Your spec contained some ideas i've incorporated into my
new spec file.

New files are available at:
http://directory.fedora.redhat.com/built/rpm_review/rrelyea/pam_pkcs11.spec
http://directory.fedora.redhat.com/built/rpm_review/rrelyea/pam_pkcs11-0.5.3-3.src.rpm


thanks,

bob

Why do you prefer to patch Makefile.in rather than Makefile.am ?  Isn't patching
a generated file rather fragile?

I'm patching both.

I patch the .am file, then I run automake (carefully using the same version of
automake as the original source) to generate the .in file, then patching both.

This means builds can be made without automake, but someone can take the source
rpm, make their own changes to .am and run automake themselves.

bob

THis is now under Core review, need to get this review finished so the rest of
the smart card stuff can come in.

The packages are looking pretty good to me.

Let's get them in.  Bill what do you think?

Works for me.

Accepting.

Please let me know when you build this into rawhide and I'll make the comps
changes for this package set.

pam_pkcs11 has now been imported.

bob

Built into rawhide.