Bug 245760 - Review Request: engine_pkcs11 - A PKCS11 engine for use with OpenSSL
Summary: Review Request: engine_pkcs11 - A PKCS11 engine for use with OpenSSL
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Jason Tibbitts
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-06-26 14:50 UTC by Matt Anderson
Modified: 2007-11-30 22:12 UTC (History)
2 users (show)

Fixed In Version: 0.1.3-4.fc7
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2007-08-17 16:14:29 UTC
Type: ---
Embargoed:
j: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Matt Anderson 2007-06-26 14:50:57 UTC
Spec URL: http://free.linux.hp.com/~mra/rpms/engine_pkcs11/engine_pkcs11.spec
SRPM URL: http://free.linux.hp.com/~mra/rpms/engine_pkcs11/engine_pkcs11-0.1.3-1.fc7.src.rpm

Description: Engine_pkcs11 is an implementation of an engine for OpenSSL.  It is built upon libp11.  Engine_pkcs11 is meant to be used with smart cards and software for using smart cards in PKCS#11 format, such as OpenSC. Originaly this engine was a part of OpenSC, until OpenSC was split into several small projects for improved flexibility.

engine_pkcs11 has a underscore in its upstream name so it meets with the Naming Guidelines.

engine_pkcs11 claims to be licensed under the New BSD license.
http://www.opensc-project.org/engine_pkcs11/wiki/AuthorsAndCredits
although it is missing the endorsement clause.  For this initial release I have BSD listed in the spec file, but if there is a better keyword to use instead I will change it.

This rpm was built successfully on i386 and x86_64 and passes rpmlint with no errors or warnings.

Comment 1 Matt Anderson 2007-06-27 16:16:07 UTC
Based on the feedback michael provided to my libp11 package I've made
some changes to this spec file and srpm.

Spec URL: http://free.linux.hp.com/~mra/rpms/engine_pkcs11/engine_pkcs11.spec
SRPM URL:
http://free.linux.hp.com/~mra/rpms/engine_pkcs11/engine_pkcs11-0.1.3-2.fc7.src.rpm

This still lists OpenSSL as a requires since the whole point of this package is
to work with OpenSSL via their engine plug-in framework.

rpmlint has no issues with this rpm, and it has been tested on i386 and x86_64.

Comment 2 Jason Tibbitts 2007-06-28 20:38:12 UTC
rpmlint is truly silent on this one.

The Source0: issue applies here as well, and you probably want the same string:
   http://www.opensc-project.org/files/%{name}/%{name}-%{version}.tar.gz

"BSD" is fine for the license.  The two-clause variant is closer to the X11
license but either is fine.

There's no reason that I can see for the openssl dependency; rpm finds the
dependency on libcrypto.so.6 by itself.

This package places a file in /usr/lib/engines, but I don't see any package in
the distribution which owns that directory.

Review:
* source files match upstream:
   bf6f49203912cb77f92db55c146117312abf9244ba49e78649e4a7da22448e54  
   engine_pkcs11-0.1.3.tar.gz
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.
* description is OK.
* dist tag is present.
* build root is OK.
* license field matches the actual license.
* license is open source-compatible.
* license text included package.
* latest version is being packaged.
* BuildRequires are proper.
* compiler flags are appropriate.
* %clean is present.
* package builds in mock (development, x86_64).
* package installs properly
* debuginfo package looks complete.
* rpmlint is silent.
X final provides and requires:
   engine_pkcs11.so()(64bit)
   engine_pkcs11 = 0.1.3-2.fc8
  =
   libcrypto.so.6()(64bit)
   libp11.so.0()(64bit)
X  openssl

* %check is not present; no test suite upstream.  I have no means to test this 
   package.  (I don't really even understand what it does.)
* no shared libraries are added to the regular linker search paths.
X nothing owns /usr/lib/engines.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no scriptlets present.
* code, not content.
* documentation is relatively large, the the package is only 50K, so no -docs 
   subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* no headers.
* no pkgconfig files.
* no static libraries.
* no libtool .la files.

Comment 3 Matt Anderson 2007-06-28 22:50:12 UTC
I added the directory ownership and updated the Source0 as per your suggestions
and uploaded the updated files here:

Spec URL: http://free.linux.hp.com/~mra/rpms/engine_pkcs11/engine_pkcs11.spec
SRPM URL:
http://free.linux.hp.com/~mra/rpms/engine_pkcs11/engine_pkcs11-0.1.3-3.fc7.src.rpm

The only thing I didn't do was remove the openssl dependency.  I might be wrong
about this, but the library exists to interface with openssl via its engine
framework.  If a different SSL implementation was used instead could that meet
the libcrypto.so.6 dependency?

Comment 4 Jason Tibbitts 2007-07-06 02:02:34 UTC
OK, I can buy the openssl dependency.  However, when I look at openssl itself, I
note that it has several engines already present, but in
/usr/lib/openssl/engines, not /usr/lib/engines.  Perhaps this package should
place engine_pkcs11.so in the former directory (and not own it) rather than what
it's currently doing.

Comment 5 Matt Anderson 2007-07-09 17:10:28 UTC
I've updated the spec file to use the /usr/lib/openssl/engines directory, thanks
for catching that.  This also has a fixed Source0 with the /files folder added
to the URL.  Lastly I included > 0.9.6 in the OpenSSL requires, since that is
when the engine support was added.

The following spec file and srpm built cleanly on i386 and x86_64 and produce no
errors with rpmlint.

Spec URL: http://free.linux.hp.com/~mra/rpms/engine_pkcs11/engine_pkcs11.spec
SRPM URL:
http://free.linux.hp.com/~mra/rpms/engine_pkcs11/engine_pkcs11-0.1.3-4.fc7.src.rpm

Comment 6 Jason Tibbitts 2007-07-10 02:55:30 UTC
OK, this looks good to me.

APPROVED

Comment 7 Matt Anderson 2007-07-10 04:00:23 UTC
New Package CVS Request
=======================
Package Name: engine_pkcs11
Short Description: A PKCS11 engine for use with OpenSSL
Owners: mra
Branches: F-7
InitialCC: tibbs.edu

Comment 8 Kevin Fenzi 2007-07-10 16:19:13 UTC
cvs done. 
I talked with tibbs on IRC, and he didn't really want to get cc's on this
package, so I left that off. 

Comment 9 Jason Tibbitts 2007-07-27 22:12:19 UTC
Is there some reason this isn't in F-7 yet?  It looks like it failed to build
because a dependency had not yet been pushed to F-7, and indeed libp11 is in
testing and has not been pushed to the stable F-7 repository.  Do you need
assistance getting these packages built and pushed?

Comment 10 Fedora Update System 2007-08-08 15:28:46 UTC
engine_pkcs11-0.1.3-4.fc7 has been pushed to the Fedora 7 testing repository.  If problems still persist, please make note of it in this bug report.

Comment 11 Fedora Update System 2007-08-17 16:14:26 UTC
engine_pkcs11-0.1.3-4.fc7 has been pushed to the Fedora 7 stable repository.  If problems still persist, please make note of it in this bug report.


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