Bug 1200389 - Review Request: caml-crush - PKCS#11 filtering proxy
Summary: Review Request: caml-crush - PKCS#11 filtering proxy
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Petr Pisar
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2015-03-10 13:15 UTC by Nikos Mavrogiannopoulos
Modified: 2015-04-21 18:40 UTC (History)
3 users (show)

Fixed In Version: caml-crush-1.0.4-6.fc22
Clone Of:
Environment:
Last Closed: 2015-03-26 07:38:01 UTC
Type: ---
Embargoed:
ppisar: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Nikos Mavrogiannopoulos 2015-03-10 13:15:19 UTC
Spec URL: http://people.redhat.com/nmavrogi/fedora/caml-crush.spec
SRPM URL: http://people.redhat.com/nmavrogi/fedora/caml-crush-1.0.4-1.fc21.src.rpm
Description: This software is a PKCS#11 proxy to softhsm allowing to store private keys  in an isolated environment in the system.
Fedora Account System Username: nmav

Comment 1 Richard W.M. Jones 2015-03-10 17:10:16 UTC
These should be deleted:

# Prevent RPM from stripping the binaries & debuginfo.
#
# NB: Only required because this package uses the obsolete -custom
# parameter and builds a bytecode 'ocamlrpcgen'.  I tried to fix the
# build to make a native code 'ocamlrpcgen' but the build system got
# the better of me.
%global debug_package %{nil}
%global __strip /bin/true

These should be deleted:

%global __ocaml_requires_opts -c -f '%{buildroot}%{_bindir}/ocamlrun %{buildroot}%{_bindir}/ocamlobjinfo'
%global __ocaml_provides_opts -f '%{buildroot}%{_bindir}/ocamlrun %{buildroot}%{_bindir}/ocamlobjinfo'

The name of the package may or may not meet the packaging guidelines.
I guess it isn't a library, so it doesn't need to be called 'ocaml-*'
It seems as if the program is really called 'pkcs11proxyd' and so that
should also be the name of the RPM.

Comment 2 Nikos Mavrogiannopoulos 2015-03-11 12:24:49 UTC
Thanks. The name is the same as the upstream name of this package (which is caml-crush). Other than that suggestions taken and package was updated:
Spec URL: http://people.redhat.com/nmavrogi/fedora/caml-crush.spec
SRPM URL: http://people.redhat.com/nmavrogi/fedora/caml-crush-1.0.4-2.fc21.src.rpm

Comment 3 Petr Pisar 2015-03-16 13:13:15 UTC
The spec file uses %{_unitdir} without dependencies on systemd <https://fedoraproject.org/wiki/Packaging:Systemd#Packaging>.

Comment 4 Nikos Mavrogiannopoulos 2015-03-16 14:58:00 UTC
Updated spec files to include systemd:
Spec URL: http://people.redhat.com/nmavrogi/fedora/caml-crush.spec
SRPM URL: http://people.redhat.com/nmavrogi/fedora/caml-crush-1.0.4-2.fc21.src.rpm

Comment 5 Petr Pisar 2015-03-18 11:52:58 UTC
URL is usable. Ok.

FIX: Source0 is invalid:

$ wget https://github.com/dlundquist/sniproxy/archive/v1.0.4.tar.gz
--2015-03-18 10:51:50--  https://github.com/dlundquist/sniproxy/archive/v1.0.4.tar.gz
Resolving github.com (github.com)... 192.30.252.130
Connecting to github.com (github.com)|192.30.252.130|:443... connected.
HTTP request sent, awaiting response... 302 Found
Location: https://codeload.github.com/dlundquist/sniproxy/tar.gz/v1.0.4 [following]
--2015-03-18 10:51:50--  https://codeload.github.com/dlundquist/sniproxy/tar.gz/v1.0.4
Resolving codeload.github.com (codeload.github.com)... 192.30.252.147
Connecting to codeload.github.com (codeload.github.com)|192.30.252.147|:443... connected.
HTTP request sent, awaiting response... 404 Not Found
2015-03-18 10:51:51 ERROR 404: Not Found.


FIX: Many Makefile.in files hard-code C compiler optimization level (CFLAGS_OPT variable). Distribution's CCFLAGS should be respected.

Just an orthogonal notice: Something is wrong with ocaml packages: Installing build-time dependencies pulls in half of desktop packages (pango, udisks2, libwayland-server, etc.). Either this spec file build-requires too much, or the ocaml is doomed.

TODO: The patches are prefixed `ocaml-crush', while the package is called `caml-crush'. Please rename the patches to start with `caml-crush-1.0.4'.

FIX: Do not bundle filter.conf (Source1). Patch src/pkcs11proxyd/filter.conf instead. The difference is 4 lines only.

TODO: Use printf(1) instead of echo(1) in pkcs11proxyd-init. Handling escape sequences is undefined by POSIX.

TODO: Consider using <https://fedoraproject.org/wiki/Packaging:Tmpfiles.d> instead of manual named socket handling by pkcs11proxyd-post.

TODO: Remove initial `a' from  Summary.

I will continue with the review after you provide fixed package.

Comment 6 Nikos Mavrogiannopoulos 2015-03-18 14:25:35 UTC
Updated package:
Spec URL: http://people.redhat.com/nmavrogi/fedora/caml-crush.spec
SRPM URL: http://people.redhat.com/nmavrogi/fedora/caml-crush-1.0.4-3.fc21.src.rpm

(In reply to Petr Pisar from comment #5)
> URL is usable. Ok.
> FIX: Source0 is invalid:
Done.

> FIX: Many Makefile.in files hard-code C compiler optimization level
> (CFLAGS_OPT variable). Distribution's CCFLAGS should be respected.

Done.

> Just an orthogonal notice: Something is wrong with ocaml packages:
> Installing build-time dependencies pulls in half of desktop packages (pango,
> udisks2, libwayland-server, etc.). Either this spec file build-requires too
> much, or the ocaml is doomed.

I know debian has ocaml-nox to avoid these dependencies. Maybe Richard can provide some info here on whether we can optimize that. We would certainly not want to depend on X to bring pkcs11proxyd.

> TODO: The patches are prefixed `ocaml-crush', while the package is called
> `caml-crush'. Please rename the patches to start with `caml-crush-1.0.4'.
Done.

> FIX: Do not bundle filter.conf (Source1). Patch src/pkcs11proxyd/filter.conf
> instead. The difference is 4 lines only.
Not done. I do want to keep a fedora config file which I control. Upstream has deviated a lot in the git repository and I wouldn't like to be surprised by updates.

> TODO: Use printf(1) instead of echo(1) in pkcs11proxyd-init. Handling escape
> sequences is undefined by POSIX.

Done.


> TODO: Consider using <https://fedoraproject.org/wiki/Packaging:Tmpfiles.d>
> instead of manual named socket handling by pkcs11proxyd-post.

My use case was simpler. Used Umask instead.

> TODO: Remove initial `a' from  Summary.

Done.

Comment 7 Richard W.M. Jones 2015-03-18 14:44:42 UTC
(In reply to Nikos Mavrogiannopoulos from comment #6)
> > Just an orthogonal notice: Something is wrong with ocaml packages:
> > Installing build-time dependencies pulls in half of desktop packages (pango,
> > udisks2, libwayland-server, etc.). Either this spec file build-requires too
> > much, or the ocaml is doomed.
> 
> I know debian has ocaml-nox to avoid these dependencies. Maybe Richard can
> provide some info here on whether we can optimize that. We would certainly
> not want to depend on X to bring pkcs11proxyd.

There is a bug filed about this somewhere, but I'll be damned if I
can find it right now ...

Comment 8 Richard W.M. Jones 2015-03-18 14:49:51 UTC
(In reply to Richard W.M. Jones from comment #7)
> (In reply to Nikos Mavrogiannopoulos from comment #6)
> > > Just an orthogonal notice: Something is wrong with ocaml packages:
> > > Installing build-time dependencies pulls in half of desktop packages (pango,
> > > udisks2, libwayland-server, etc.). Either this spec file build-requires too
> > > much, or the ocaml is doomed.
> > 
> > I know debian has ocaml-nox to avoid these dependencies. Maybe Richard can
> > provide some info here on whether we can optimize that. We would certainly
> > not want to depend on X to bring pkcs11proxyd.
> 
> There is a bug filed about this somewhere, but I'll be damned if I
> can find it right now ...

OK there wasn't a bug, instead another developer brought this
topic up in email but he didn't file a bug about it.  I have filed
one just now:

https://bugzilla.redhat.com/show_bug.cgi?id=1203313

Comment 9 Richard W.M. Jones 2015-03-18 14:51:32 UTC
BTW although *build time* dependencies include X, the final
program certainly should *not* depend on X.

eg:

$ ldd /usr/bin/virt-v2v|grep X
[no output]

Comment 10 Petr Pisar 2015-03-19 10:29:37 UTC
> FIX: Source0 is invalid:
Source archive is original (SHA-256: a9a248456f06de31475da8e3aeb673b1617bfa677fd4ee4869e4e67c3a0879a2).
Ok.

> FIX: Many Makefile.in files hard-code C compiler optimization level
> (CFLAGS_OPT variable). Distribution's CCFLAGS should be respected.

Following files still inject -O2:
src/bindings-pkcs11/Makefile.in:4 (this is followed by system CFLAGS)
src/pkcs11proxyd/Makefile.in:7

> TODO: The patches are prefixed `ocaml-crush', while the package is called
> `caml-crush'. Please rename the patches to start with `caml-crush-1.0.4'.
Ok.

> > FIX: Do not bundle filter.conf (Source1). Patch src/pkcs11proxyd/filter.conf
> > instead. The difference is 4 lines only.
> Not done. I do want to keep a fedora config file which I control.
Ok.

> TODO: Use printf(1) instead of echo(1) in pkcs11proxyd-init. Handling escape
> sequences is undefined by POSIX.
Not addressed. Ok.

> TODO: Consider using <https://fedoraproject.org/wiki/Packaging:Tmpfiles.d>
> instead of manual named socket handling by pkcs11proxyd-post.
Not addressed. Ok.

> TODO: Remove initial `a' from  Summary.
Ok.

Summary is Ok.
Description is Ok.

TODO: The caml-crush description reads `This software is a computer program' which sound a little bit redundant. I would remove the phrase.

FIX: License tag is wrong.

Found licenses:

CeCILL-B (src/rpc-pkcs11/rpc_helpers.ml, src/client-lib/modwrap_crpc.c etc.)
RSA??? (src/bindings-pkcs11/pkcs11t.h, src/bindings-pkcs11/original_pkcs11.h)
GPLv2+ (src/bindings-pkcs11/des.h)
GPLv3+ (config.guess)
CeCILL (LICENSE.txt, covers other files without a license statement probably. Actually this is the only file where this license is mentioned. Authors included wrong license text probably. Please raise a question.)

Please add this listing as a comment into spec file to document source code licenses in contrast to License tag which covers binary RPMs only.

Change License tag to `CeCILL and CeCILL-B and RSA???' for the caml-crush package, change License tag to `CeCILL and CeCILL-B' for caml-crush-softhsm
package:

config.guess is not installed.
src/bindings-pkcs11/des.h is not included because --with-aliasing is not passed to configure.
src/bindings-pkcs11/pkcs11t.h is included by src/bindings-pkcs11/pkcs11_functions.c and linked into pkcs11proxyd executable.

FIX: The src/bindings-pkcs11/pkcs11t.h file has unknown license <https://fedoraproject.org/wiki/Licensing>:

/* License to copy and use this software is granted provided that it is
 * identified as "RSA Security Inc. PKCS #11 Cryptographic Token Interface
 * (Cryptoki)" in all material mentioning or referencing this software.

 * License is also granted to make and use derivative works provided that
 * such works are identified as "derived from the RSA Security Inc. PKCS #11
 * Cryptographic Token Interface (Cryptoki)" in all material mentioning or
 * referencing the derived work.

 * RSA Security Inc. makes no representations concerning either the
 * merchantability of this software or the suitability of this software for
 * any particular purpose. It is provided "as is" without express or implied
 * warranty of any kind.
 */

Ask fedora-legal to identify it and assign an identifier. Then replace the RSA??? with it.

FIX: Build-require `systemd' for %{_unitdir} macro in the spec file.

TODO: Remove redundant configure arguments: --bindir, --libdir

FIX: Require(pre) `shadow-utils because of useradd in %pre section.
FIX: Add `exit 0' at the end of %pre section <https://fedoraproject.org/wiki/Packaging:UsersAndGroups#Dynamic_allocation>. RPM scriptlets are not allowed to fail in Fedora.

TODO: Run-require p11-kit by caml-cruh-softhsm pacakge for %{_datadir}/p11-kit/modules directory.

$ rpmlint  caml-crush.spec ../SRPMS/caml-crush-1.0.4-3.fc23.src.rpm ../RPMS/x86_64/caml-crush-*
caml-crush.src: W: strange-permission pkcs11proxyd-init 0755L
caml-crush.x86_64: W: no-manual-page-for-binary pkcs11proxyd
caml-crush-softhsm.x86_64: W: no-documentation
caml-crush-softhsm.x86_64: W: non-standard-uid /var/lib/pkcs11proxyd/softhsm.conf pkcs11proxyd
caml-crush-softhsm.x86_64: W: non-standard-gid /var/lib/pkcs11proxyd/softhsm.conf pkcs11proxyd
caml-crush-softhsm.x86_64: W: non-standard-uid /var/lib/pkcs11proxyd/.config/pkcs11 pkcs11proxyd
caml-crush-softhsm.x86_64: W: non-standard-gid /var/lib/pkcs11proxyd/.config/pkcs11 pkcs11proxyd
caml-crush-softhsm.x86_64: W: non-standard-uid /var/lib/pkcs11proxyd/.config/pkcs11/pkcs11.conf pkcs11proxyd
caml-crush-softhsm.x86_64: W: non-standard-gid /var/lib/pkcs11proxyd/.config/pkcs11/pkcs11.conf pkcs11proxyd
caml-crush-softhsm.x86_64: E: script-without-shebang /var/lib/pkcs11proxyd/.config/pkcs11/pkcs11.conf
caml-crush-softhsm.x86_64: W: non-conffile-in-etc /etc/pkcs11proxyd/filter-softhsm.conf
caml-crush-softhsm.x86_64: W: non-standard-uid /var/lib/pkcs11proxyd/.config/pkcs11/modules/softhsm.module pkcs11proxyd
caml-crush-softhsm.x86_64: W: non-standard-gid /var/lib/pkcs11proxyd/.config/pkcs11/modules/softhsm.module pkcs11proxyd
caml-crush-softhsm.x86_64: E: script-without-shebang /var/lib/pkcs11proxyd/.config/pkcs11/modules/softhsm.module
caml-crush-softhsm.x86_64: W: non-standard-uid /var/lib/pkcs11proxyd/.config pkcs11proxyd
caml-crush-softhsm.x86_64: W: non-standard-gid /var/lib/pkcs11proxyd/.config pkcs11proxyd
caml-crush-softhsm.x86_64: W: hidden-file-or-dir /var/lib/pkcs11proxyd/.config
caml-crush-softhsm.x86_64: W: hidden-file-or-dir /var/lib/pkcs11proxyd/.config
caml-crush-softhsm.x86_64: W: non-conffile-in-etc /etc/pkcs11proxyd/pkcs11proxyd-softhsm.conf
caml-crush-softhsm.x86_64: W: non-standard-uid /var/lib/pkcs11proxyd pkcs11proxyd
caml-crush-softhsm.x86_64: W: non-standard-gid /var/lib/pkcs11proxyd pkcs11proxyd
caml-crush-softhsm.x86_64: W: non-standard-uid /var/lib/pkcs11proxyd/.config/pkcs11/modules pkcs11proxyd
caml-crush-softhsm.x86_64: W: non-standard-gid /var/lib/pkcs11proxyd/.config/pkcs11/modules pkcs11proxyd
caml-crush-softhsm.x86_64: W: no-manual-page-for-binary pkcs11proxyd-init
4 packages and 1 specfiles checked; 2 errors, 22 warnings.

FIX: Remove executable bit from /var/lib/pkcs11proxyd/.config/pkcs11/pkcs11.conf and /var/lib/pkcs11proxyd/.config/pkcs11/modules/softhsm.module.
FIX: Mark /etc/pkcs11proxyd/filter-softhsm.conf and /etc/pkcs11proxyd/pkcs11proxyd-softhsm.conf as configuration file by `%config(noreplace)' in the %files section.

$ rpm -q -lv -p ../RPMS/x86_64/caml-crush-1.0.4-3.fc23.x86_64.rpm 
drwxr-xr-x    2 root    root                        0 Mar 19 09:37 /etc/pkcs11proxyd
-rw-r--r--    1 root    root                    10861 Mar 19 09:37 /etc/pkcs11proxyd/filter.conf
-rw-r--r--    1 root    root                     2356 Mar 19 07:41 /etc/pkcs11proxyd/pkcs11proxyd.conf
-rwxr-xr-x    1 root    root                   113992 Mar 19 09:37 /usr/lib64/pkcs11/libp11client.so
-rwxr-xr-x    1 root    root                  4067512 Mar 19 09:37 /usr/sbin/pkcs11proxyd
drwxr-xr-x    2 root    root                        0 Mar 19 09:37 /usr/share/doc/caml-crush
-rw-r--r--    1 root    root                     9425 Dec  2 09:41 /usr/share/doc/caml-crush/ISSUES.md
-rw-r--r--    1 root    root                     3645 Dec  2 09:41 /usr/share/doc/caml-crush/README.md
drwxr-xr-x    2 root    root                        0 Mar 19 09:37 /usr/share/licenses/caml-crush
-rw-r--r--    1 root    root                    21781 Dec  2 09:41 /usr/share/licenses/caml-crush/LICENSE.txt
File permissions and layout are Ok.

$ rpm -q -lv -p ../RPMS/x86_64/caml-crush-softhsm-1.0.4-3.fc23.x86_64.rpm 
-rw-r--r--    1 root    root                    10861 Mar 19 09:37 /etc/pkcs11proxyd/filter-softhsm.conf
-rw-r--r--    1 root    root                     2364 Mar 19 07:41 /etc/pkcs11proxyd/pkcs11proxyd-softhsm.conf
-rw-r--r--    1 root    root                      485 Mar 19 07:41 /usr/lib/systemd/system/pkcs11proxyd-softhsm.service
-rwxr-xr-x    1 root    root                   114000 Mar 19 09:37 /usr/lib64/pkcs11/libp11clientsofthsm.so
-rwxr-xr-x    1 root    root                      779 Mar 19 07:41 /usr/sbin/pkcs11proxyd-init
-rw-r--r--    1 root    root                      380 Mar 19 07:41 /usr/share/p11-kit/modules/pkcs11proxyd-softhsm.module
drwxr-xr-x    2 pkcs11prpkcs11pr                    0 Mar 19 09:37 /var/lib/pkcs11proxyd
drwxr-xr-x    2 pkcs11prpkcs11pr                    0 Mar 19 09:37 /var/lib/pkcs11proxyd/.config
drwxr-xr-x    2 pkcs11prpkcs11pr                    0 Mar 19 09:37 /var/lib/pkcs11proxyd/.config/pkcs11
drwxr-xr-x    2 pkcs11prpkcs11pr                    0 Mar 19 09:37 /var/lib/pkcs11proxyd/.config/pkcs11/modules
-rwxr-xr-x    1 pkcs11prpkcs11pr                   23 Mar 19 07:41 /var/lib/pkcs11proxyd/.config/pkcs11/modules/softhsm.module
-rwxr-xr-x    1 pkcs11prpkcs11pr                   18 Mar 19 07:41 /var/lib/pkcs11proxyd/.config/pkcs11/pkcs11.conf
-rw-r--r--    1 pkcs11prpkcs11pr                   79 Mar 19 07:41 /var/lib/pkcs11proxyd/softhsm.conf
FIX: Remove executable bit from /var/lib/pkcs11proxyd/.config/pkcs11/pkcs11.conf and /var/lib/pkcs11proxyd/.config/pkcs11/modules/softhsm.module.

$ rpm -q --requires -p ../RPMS/x86_64/caml-crush-1.0.4-3.fc23.x86_64.rpm | sort -f | uniq -c
      4 /bin/sh
      1 config(caml-crush) = 1.0.4-3.fc23
      1 libc.so.6()(64bit)
      1 libc.so.6(GLIBC_2.14)(64bit)
      1 libc.so.6(GLIBC_2.15)(64bit)
      1 libc.so.6(GLIBC_2.2.5)(64bit)
      1 libc.so.6(GLIBC_2.3)(64bit)
      1 libc.so.6(GLIBC_2.3.2)(64bit)
      1 libc.so.6(GLIBC_2.3.4)(64bit)
      1 libc.so.6(GLIBC_2.4)(64bit)
      1 libc.so.6(GLIBC_2.7)(64bit)
      1 libc.so.6(GLIBC_2.8)(64bit)
      1 libdl.so.2()(64bit)
      1 libdl.so.2(GLIBC_2.2.5)(64bit)
      1 libm.so.6()(64bit)
      1 libm.so.6(GLIBC_2.2.5)(64bit)
      1 libpthread.so.0()(64bit)
      1 libpthread.so.0(GLIBC_2.2.5)(64bit)
      1 librt.so.1()(64bit)
      1 librt.so.1(GLIBC_2.2.5)(64bit)
      1 librt.so.1(GLIBC_2.3.3)(64bit)
      1 rpmlib(CompressedFileNames) <= 3.0.4-1
      1 rpmlib(FileDigests) <= 4.6.0-1
      1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1
      1 rpmlib(PayloadIsXz) <= 5.2-1
      1 rtld(GNU_HASH)
Binary requires are Ok.

$ rpm -q --requires -p ../RPMS/x86_64/caml-crush-softhsm-1.0.4-3.fc23.x86_64.rpm | sort -f | uniq -c
      1 /bin/sh
      1 caml-crush(x86-64) = 1.0.4-3.fc23
      1 inotify-tools
      1 libc.so.6()(64bit)
      1 libc.so.6(GLIBC_2.14)(64bit)
      1 libc.so.6(GLIBC_2.2.5)(64bit)
      1 libc.so.6(GLIBC_2.3.4)(64bit)
      1 libc.so.6(GLIBC_2.4)(64bit)
      1 libpthread.so.0()(64bit)
      1 libpthread.so.0(GLIBC_2.2.5)(64bit)
      1 rpmlib(CompressedFileNames) <= 3.0.4-1
      1 rpmlib(FileDigests) <= 4.6.0-1
      1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1
      1 rpmlib(PayloadIsXz) <= 5.2-1
      1 rtld(GNU_HASH)
      1 softhsm
      3 systemd
      1 util-linux
Binary provides are Ok.

$ rpm -q --provides -p ../RPMS/x86_64/caml-crush-1.0.4-3.fc23.x86_64.rpm | sort -f | uniq -c
      1 caml-crush = 1.0.4-3.fc23
      1 caml-crush(x86-64) = 1.0.4-3.fc23
      1 config(caml-crush) = 1.0.4-3.fc23
      1 libp11client.so()(64bit)
Fix: Do not provide `libp11client.so()(64bit)'. It's a private library in non-standard path (/usr/lib64/pkcs11/libp11client.so).

$ rpm -q --provides -p ../RPMS/x86_64/caml-crush-softhsm-1.0.4-3.fc23.x86_64.rpm | sort -f | uniq -c
      1 caml-crush-softhsm = 1.0.4-3.fc23
      1 caml-crush-softhsm(x86-64) = 1.0.4-3.fc23
      1 libp11clientsofthsm.so()(64bit)
FIX: Do not provide `libp11clientsofthsm.so()(64bit)` because it's not in the standard path recognized by a linker.

$ resolvedeps rawhide ../RPMS/x86_64/caml-crush-1.0.4-3.fc23.x86_64.rpm ../RPMS/x86_64/caml-crush-softhsm-1.0.4-3.fc23.x86_64.rpm 
Binary dependencies resolvable. Ok.

Package builds in F23 (http://koji.fedoraproject.org/koji/taskinfo?taskID=9272455). Ok.

TODO: debuginfo generator complains on missing source files:

cpio: caml-crush-1.0.4/src/client-lib/modwrap_.c: Cannot stat: No such file or directory
cpio: caml-crush-1.0.4/src/client-lib/modwrap_softhsm.c: Cannot stat: No such file or directory

Otherwise the package is inline with Fedora packaging guidelines.

Please correct all `FIX' items, consider fixing `TODO' items, and provide new spec file.

Resolution: NOT approved.

Comment 11 Nikos Mavrogiannopoulos 2015-03-19 15:47:18 UTC
(In reply to Petr Pisar from comment #10)
> TODO: The caml-crush description reads `This software is a computer program'
> which sound a little bit redundant. I would remove the phrase.

Done.

> FIX: License tag is wrong.

Fixed in an alternative way. Use p11-kit's header to avoid dependency on RSA's license. Sent patch upstream as well.

> FIX: Build-require `systemd' for %{_unitdir} macro in the spec file.

Done.

> TODO: Remove redundant configure arguments: --bindir, --libdir

Done.

> FIX: Require(pre) `shadow-utils because of useradd in %pre section.

Done.

> FIX: Add `exit 0' at the end of %pre section

Done.

> TODO: Run-require p11-kit by caml-cruh-softhsm pacakge for
> %{_datadir}/p11-kit/modules directory.

Done.

> FIX: Remove executable bit from
> /var/lib/pkcs11proxyd/.config/pkcs11/pkcs11.conf and
> /var/lib/pkcs11proxyd/.config/pkcs11/modules/softhsm.module.

Done.

> FIX: Mark /etc/pkcs11proxyd/filter-softhsm.conf and
> /etc/pkcs11proxyd/pkcs11proxyd-softhsm.conf as configuration file by
> `%config(noreplace)' in the %files section.

These are intentionally not configuration files. They are supposed to change on upgraded. For configuring a module one can use the caml-crush package (instead of caml-crash-softhsm).

> $ rpm -q --provides -p ../RPMS/x86_64/caml-crush-1.0.4-3.fc23.x86_64.rpm |
> sort -f | uniq -c
>       1 caml-crush = 1.0.4-3.fc23
>       1 caml-crush(x86-64) = 1.0.4-3.fc23
>       1 config(caml-crush) = 1.0.4-3.fc23
>       1 libp11client.so()(64bit)
> Fix: Do not provide `libp11client.so()(64bit)'. It's a private library in
> non-standard path (/usr/lib64/pkcs11/libp11client.so).

This is not a private library but rather a PKCS #11 module, and that is the standard path for them. I'd prefer if this package provides this library in case some program directly links to it.

Comment 13 Petr Pisar 2015-03-20 09:21:52 UTC
Spec file changes:

--- caml-crush.spec.old 2015-03-18 15:22:00.000000000 +0100
+++ caml-crush.spec     2015-03-19 16:44:03.000000000 +0100
@@ -4,7 +4,7 @@
 Version:        1.0.4
 Release:        3%{?dist}
 Summary:        PKCS#11 filtering proxy
-License:        CeCILL
+License:        CeCILL + CeCILL-B + FSFUL

 URL:            https://github.com/ANSSI-FR/caml-crush
 Source0:        https://github.com/ANSSI-FR/caml-crush/archive/v%{version}.tar.gz
@@ -31,21 +31,29 @@
 BuildRequires:  ocaml-ocamlnet-devel
 BuildRequires:  ocaml-config-file-devel
 BuildRequires:  sed
+BuildRequires:  p11-kit-devel

 %package softhsm
+
+License:        CeCILL + CeCILL-B
 Summary: Deployment of caml-crush with softhsm
+
+BuildRequires: systemd
+
 Requires: %{name}%{?_isa} = %{version}-%{release}
 Requires:       softhsm
 Requires:       inotify-tools
 Requires:       util-linux
+Requires:       p11-kit
+Requires(pre): shadow-utils
 Requires(post):   systemd
 Requires(preun):  systemd
 Requires(postun): systemd


 %description
-This software is a computer program whose purpose is to implement a PKCS#11
-proxy as well as a PKCS#11 filter with security features in mind.
+This software implements a PKCS#11 proxy as well as a PKCS#11 filter with
+security features in mind.

 %description softhsm
 This software is a PKCS#11 proxy to softhsm allowing to store private keys
@@ -53,6 +61,10 @@

 %prep
 %setup -q -n caml-crush-%{version}
+rm -f src/bindings-pkcs11/des.h
+rm -f src/bindings-pkcs11/pkcs11t.h
+rm -f src/bindings-pkcs11/pkcs11h.h
+cp /usr/include/p11-kit-1/p11-kit/pkcs11.h src/bindings-pkcs11/original_pkcs11.h

 %patch1 -p1 -b .libname
 %patch2 -p1 -b .exit
@@ -63,8 +75,6 @@
 %build
 sh autogen.sh
 %configure \
-  --bindir=%{_bindir} \
-  --libdir=%{_libdir} \
   --datadir=%{_datadir}/%{name} \
   --with-rpcgen \
   --with-idlgen \
@@ -79,6 +89,7 @@
     /usr/sbin/useradd -r -g pkcs11proxyd -s /sbin/nologin -c pkcs11proxyd \
         -d %{_sharedstatedir}/pkcs11proxyd pkcs11proxyd
 getent group pkcs11proxy &>/dev/null || groupadd -r pkcs11proxy
+exit 0

 %post
 %systemd_post pkcs11proxyd-softhsm.service
@@ -107,8 +118,8 @@
 install -p -m 644 %{SOURCE4} %{buildroot}/%{_datadir}/p11-kit/modules/
 install -p -m 644 %{SOURCE5} %{buildroot}/%{_sharedstatedir}/pkcs11proxyd
 install -p -m 755 %{SOURCE6} %{buildroot}%{_sbindir}/
-install -p -m 755 %{SOURCE8} %{buildroot}%{_sharedstatedir}/pkcs11proxyd/.config/pkcs11/
-install -p -m 755 %{SOURCE9} %{buildroot}%{_sharedstatedir}/pkcs11proxyd/.config/pkcs11/modules
+install -p -m 644 %{SOURCE8} %{buildroot}%{_sharedstatedir}/pkcs11proxyd/.config/pkcs11/
+install -p -m 644 %{SOURCE9} %{buildroot}%{_sharedstatedir}/pkcs11proxyd/.config/pkcs11/modules

 %files
 %doc README.md ISSUES.md


> TODO: The caml-crush description reads `This software is a computer program'
> which sound a little bit redundant. I would remove the phrase.
-This software is a computer program whose purpose is to implement a PKCS#11
-proxy as well as a PKCS#11 filter with security features in mind.
+This software implements a PKCS#11 proxy as well as a PKCS#11 filter with
+security features in mind.
Ok.

> FIX: License tag is wrong.
-License:        CeCILL
+License:        CeCILL + CeCILL-B + FSFUL

 %package softhsm
+
+License:        CeCILL + CeCILL-B

FIX: The is invalid syntax. Use `and' instead of `+' <https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#Multiple_Licensing_Scenarios>.

> CeCILL-B (src/rpc-pkcs11/rpc_helpers.ml, src/client-lib/modwrap_crpc.c etc.)
> RSA??? (src/bindings-pkcs11/pkcs11t.h, src/bindings-pkcs11/original_pkcs11.h)
> GPLv2+ (src/bindings-pkcs11/des.h)
> GPLv3+ (config.guess)
> CeCILL (LICENSE.txt, covers other files without a license statement probably.
>
> Please add this listing as a comment into spec file to document source code
> licenses in contrast to License tag which covers binary RPMs only.
+rm -f src/bindings-pkcs11/des.h
+rm -f src/bindings-pkcs11/pkcs11t.h
+rm -f src/bindings-pkcs11/pkcs11h.h
+cp /usr/include/p11-kit-1/p11-kit/pkcs11.h src/bindings-pkcs11/original_pkcs11.h

FIX: Removing badly licensed files at build time does not remove them from source RPM package. Either repackage the source archive, or ask Fedora legal for help.

Notice: Also the copied /usr/include/p11-kit-1/p11-kit/pkcs11.h still is derivated work of the RSA-copyrighted work:

 /* This file is a modified implementation of the PKCS #11 standard by
   RSA Security Inc.  It is mostly a drop-in replacement, with the
   following change:

So you still have the RSA code there unless it was somehow relicensed. The only I could find is <https://fedoraproject.org/wiki/Licensing:FAQ?rd=Licensing/FAQ#What_about_the_RSA_license_on_their_MD5_implementation.3F_Isn.27t_that_GPL-incompatible.3F> about RSA's digest implementations.

> FIX: Build-require `systemd' for %{_unitdir} macro in the spec file.
+BuildRequires: systemd
Ok.

> TODO: Remove redundant configure arguments: --bindir, --libdir
 %configure \
-  --bindir=%{_bindir} \
-  --libdir=%{_libdir} \
Ok.

> FIX: Require(pre) `shadow-utils because of useradd in %pre section.
 %package softhsm
[...]
+Requires(pre): shadow-utils
FIX: The dependency should on the main package, not softhsm sub-package as this where the %pre section belongs to.

> FIX: Add `exit 0' at the end of %pre section
+exit 0
Ok.

> TODO: Run-require p11-kit by caml-cruh-softhsm pacakge for %{_datadir}/p11-kit/modules directory.
+Requires:       p11-kit
Ok.

> FIX: Remove executable bit from /var/lib/pkcs11proxyd/.config/pkcs11/pkcs11.conf and /var/lib/pkcs11proxyd/.config/pkcs11/modules/softhsm.module.
-install -p -m 755 %{SOURCE8} %{buildroot}%{_sharedstatedir}/pkcs11proxyd/.config/pkcs11/
-install -p -m 755 %{SOURCE9} %{buildroot}%{_sharedstatedir}/pkcs11proxyd/.config/pkcs11/modules
+install -p -m 644 %{SOURCE8} %{buildroot}%{_sharedstatedir}/pkcs11proxyd/.config/pkcs11/
+install -p -m 644 %{SOURCE9} %{buildroot}%{_sharedstatedir}/pkcs11proxyd/.config/pkcs11/modules

$ rpm -q -lv -p ../RPMS/x86_64/caml-crush-softhsm-1.0.4-3.fc23.x86_64.rpm 
-rw-r--r--    1 root    root                    10861 Mar 20 09:22 /etc/pkcs11proxyd/filter-softhsm.conf
-rw-r--r--    1 root    root                     2364 Mar 19 07:41 /etc/pkcs11proxyd/pkcs11proxyd-softhsm.conf
-rw-r--r--    1 root    root                      485 Mar 19 07:41 /usr/lib/systemd/system/pkcs11proxyd-softhsm.service
-rwxr-xr-x    1 root    root                   114000 Mar 20 09:23 /usr/lib64/pkcs11/libp11clientsofthsm.so
-rwxr-xr-x    1 root    root                      779 Mar 19 07:41 /usr/sbin/pkcs11proxyd-init
-rw-r--r--    1 root    root                      380 Mar 19 07:41 /usr/share/p11-kit/modules/pkcs11proxyd-softhsm.module
drwxr-xr-x    2 pkcs11prpkcs11pr                    0 Mar 20 09:23 /var/lib/pkcs11proxyd
drwxr-xr-x    2 pkcs11prpkcs11pr                    0 Mar 20 09:23 /var/lib/pkcs11proxyd/.config
drwxr-xr-x    2 pkcs11prpkcs11pr                    0 Mar 20 09:23 /var/lib/pkcs11proxyd/.config/pkcs11
drwxr-xr-x    2 pkcs11prpkcs11pr                    0 Mar 20 09:23 /var/lib/pkcs11proxyd/.config/pkcs11/modules
-rw-r--r--    1 pkcs11prpkcs11pr                   23 Mar 20 09:17 /var/lib/pkcs11proxyd/.config/pkcs11/modules/softhsm.module
-rw-r--r--    1 pkcs11prpkcs11pr                   18 Mar 20 09:17 /var/lib/pkcs11proxyd/.config/pkcs11/pkcs11.conf
-rw-r--r--    1 pkcs11prpkcs11pr                   79 Mar 19 07:41 /var/lib/pkcs11proxyd/softhsm.conf

File permissions are ok.

> > FIX: Mark /etc/pkcs11proxyd/filter-softhsm.conf and /etc/pkcs11proxyd/pkcs11proxyd-softhsm.conf as configuration file by `%config(noreplace)' in the %files section.
> These are intentionally not configuration files. They are supposed to change on
> upgraded. For configuring a module one can use the caml-crush package (instead
> of caml-crash-softhsm).
Then location under /etc is questionable.
Ok.

> > Fix: Do not provide `libp11client.so()(64bit)'. It's a private library in
> > non-standard path (/usr/lib64/pkcs11/libp11client.so).
> > FIX: Do not provide `libp11clientsofthsm.so()(64bit)` because it's not in the
> > standard path recognized by a linker.
> This is not a private library but rather a PKCS #11 module, and that is the
> standard path for them. I'd prefer if this package provides this library in
> case some program directly links to it.

If some program directly links to it, then it must be in standard library search path (/etc/ld.so.conf). Because it's not, it must not be provided.

FIX:  So either it's a public library, or it's a private library. See <https://fedoraproject.org/wiki/Packaging:AutoProvidesAndRequiresFiltering#Private_Libraries> for the second case, and <https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#Beware_of_Rpath> for the first case.

> TODO: debuginfo generator complains on missing source files:
Not addressed. Ok.


Please resolve the `FIX' items, and provide new package.

Comment 14 Nikos Mavrogiannopoulos 2015-03-23 08:55:27 UTC
(In reply to Petr Pisar from comment #13)
> FIX: Removing badly licensed files at build time does not remove them from
> source RPM package. Either repackage the source archive, or ask Fedora legal
> for help.

Correct. The source archive is now overriden.

> Notice: Also the copied /usr/include/p11-kit-1/p11-kit/pkcs11.h still is
> derivated work of the RSA-copyrighted work:
>  /* This file is a modified implementation of the PKCS #11 standard by
>    RSA Security Inc.  It is mostly a drop-in replacement, with the
>    following change:

This is not what the text above says. The header in p11-kit is a reimplementation of the PKCS #11 standard header. The license is the FSFUL.
 
> > FIX: Require(pre) `shadow-utils because of useradd in %pre section.
>  %package softhsm
> [...]
> +Requires(pre): shadow-utils
> FIX: The dependency should on the main package, not softhsm sub-package as
> this where the %pre section belongs to.

Thanks for the catch. Done.

> > > standard path recognized by a linker.
> > This is not a private library but rather a PKCS #11 module, and that is the
> > standard path for them. I'd prefer if this package provides this library in
> > case some program directly links to it.
> 
> If some program directly links to it, then it must be in standard library
> search path (/etc/ld.so.conf). Because it's not, it must not be provided.
> 
> FIX:  So either it's a public library, or it's a private library. See
> <https://fedoraproject.org/wiki/Packaging:
> AutoProvidesAndRequiresFiltering#Private_Libraries> for the second case, and
> <https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/
> Guidelines#Beware_of_Rpath> for the first case.

Done (no longer provide it)


New package:
Spec URL: http://people.redhat.com/nmavrogi/fedora/caml-crush.spec
SRPM URL: http://people.redhat.com/nmavrogi/fedora/caml-crush-1.0.4-4.fc21.src.rpm

Comment 15 Petr Pisar 2015-03-25 12:08:59 UTC
Spec file changes:

--- caml-crush.spec.old 2015-03-19 16:44:03.000000000 +0100
+++ caml-crush.spec     2015-03-23 09:54:24.000000000 +0100
@@ -2,12 +2,16 @@

 Name:           caml-crush
 Version:        1.0.4
-Release:        3%{?dist}
+Release:        4%{?dist}
 Summary:        PKCS#11 filtering proxy
-License:        CeCILL + CeCILL-B + FSFUL
+
+# The pkcs11proxyd server is under CeCILL, while the rest of the libraries are
+# under CeCILL-B. The pkcs11 bindings contain code  under GPLv2+ and the RSA
+# cryptoki license which we don't use.
+License:        CeCILL and CeCILL-B and FSFUL

 URL:            https://github.com/ANSSI-FR/caml-crush
-Source0:        https://github.com/ANSSI-FR/caml-crush/archive/v%{version}.tar.gz
+Source0:        v%{version}-hobbled.tar.gz
 Source1:        filter.conf
 Source2:        pkcs11proxyd.conf
 Source3:        pkcs11proxyd-softhsm.service
@@ -22,6 +26,7 @@
 Patch3:         caml-crush-better-msgs.patch
 Patch4:         caml-crush-honor-CFLAGS.patch

+Requires(pre): shadow-utils
 BuildRequires:  autoconf
 BuildRequires:  ocaml >= 4.00
 BuildRequires:  ocaml-findlib-devel
@@ -35,7 +40,7 @@

 %package softhsm

-License:        CeCILL + CeCILL-B
+License:        CeCILL and CeCILL-B
 Summary: Deployment of caml-crush with softhsm

 BuildRequires: systemd
@@ -45,7 +50,6 @@
 Requires:       inotify-tools
 Requires:       util-linux
 Requires:       p11-kit
-Requires(pre): shadow-utils
 Requires(post):   systemd
 Requires(preun):  systemd
 Requires(postun): systemd
@@ -61,10 +65,6 @@

 %prep
 %setup -q -n caml-crush-%{version}
-rm -f src/bindings-pkcs11/des.h
-rm -f src/bindings-pkcs11/pkcs11t.h
-rm -f src/bindings-pkcs11/pkcs11h.h
-cp /usr/include/p11-kit-1/p11-kit/pkcs11.h src/bindings-pkcs11/original_pkcs11.h

 %patch1 -p1 -b .libname
 %patch2 -p1 -b .exit
@@ -121,6 +121,8 @@
 install -p -m 644 %{SOURCE8} %{buildroot}%{_sharedstatedir}/pkcs11proxyd/.config/pkcs11/
 install -p -m 644 %{SOURCE9} %{buildroot}%{_sharedstatedir}/pkcs11proxyd/.config/pkcs11/modules

+%global __provides_filter_from ^%{_libdir}/pkcs11/.*\\.so$
+
 %files
 %doc README.md ISSUES.md
 %license LICENSE.txt
@@ -152,6 +154,9 @@


 %changelog
+* Mon Mar 23 2015 Nikos Mavrogiannopoulos <nmav> - 1.0.4-4
+- do not include the RSA's headers and GPLv3 code to simplify licensing
+
 * Wed Mar 18 2015 Nikos Mavrogiannopoulos <nmav> - 1.0.4-3
 - utilize global CFLAGS - suggested by Petr Pisar


> FIX: The is invalid syntax. Use `and' instead of `+'
> <https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#Multiple_Licensing_Scenarios>.

-License:        CeCILL + CeCILL-B + FSFUL
+
+# The pkcs11proxyd server is under CeCILL, while the rest of the libraries are
+# under CeCILL-B. The pkcs11 bindings contain code  under GPLv2+ and the RSA
+# cryptoki license which we don't use.
+License:        CeCILL and CeCILL-B and FSFUL
[...]
-License:        CeCILL + CeCILL-B
+License:        CeCILL and CeCILL-B
Ok.

> FIX: Removing badly licensed files at build time does not remove them from
> source RPM package. Either repackage the source archive, or ask Fedora legal
> for help.
-Source0:        https://github.com/ANSSI-FR/caml-crush/archive/v%{version}.tar.gz
+Source0:        v%{version}-hobbled.tar.gz
Some files were removed and original_pkcs11.h replaced.

FIX: There is still src/bindings-pkcs11/pkcs11f.h with the same RSA license. The file is not used. Remove it from the archive too.

> FIX: The dependency should on the main package, not softhsm sub-package as
> this where the %pre section belongs to.
+Requires(pre): shadow-utils
[...]
%package softhsm
[...]
-Requires(pre): shadow-utils
Ok.

> FIX:  So either it's a public library, or it's a private library. See
> <https://fedoraproject.org/wiki/Packaging:AutoProvidesAndRequiresFiltering#Private_Libraries>
> for the second case, and
> <https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#Beware_of_Rpath> for the first case.
+%global __provides_filter_from ^%{_libdir}/pkcs11/.*\\.so$
+

TODO: Move the macro definition right before %description section. It's the common place where to write the filters.

$ rpm -q --provides -p ../RPMS/x86_64/caml-crush-1.0.4-4.fc23.x86_64.rpm  | sort -f | uniq -c
      1 caml-crush = 1.0.4-4.fc23
      1 caml-crush(x86-64) = 1.0.4-4.fc23
      1 config(caml-crush) = 1.0.4-4.fc23
      1 libp11client.so()(64bit)

$ rpm -q --provides -p ../RPMS/x86_64/caml-crush-softhsm-1.0.4-4.fc23.x86_64.rpm  | sort -f | uniq -c
      1 caml-crush-softhsm = 1.0.4-4.fc23
      1 caml-crush-softhsm(x86-64) = 1.0.4-4.fc23
      1 libp11clientsofthsm.so()(64bit)

FIX: Your change did not removed the provides. The correct macro is name is `__provides_exclude_from'. See <https://fedoraproject.org/wiki/Packaging:AutoProvidesAndRequiresFiltering#Preventing_files.2Fdirectories_from_being_scanned_for_deps_.28pre-scan_filtering.29>.

TODO: Please append the expression to possible current %__provides_exclude_from value like this:

%global __provides_exclude_from %{?__provides_exclude_from:%__provides_exclude_from|}^%{_libdir}/pkcs11/.*\\.so$

It could happen the the value had already been defined somewhere else (macro files) and you could lose the old filter.


$ rpmlint caml-crush.spec ../SRPMS/caml-crush-1.0.4-4.fc23.src.rpm ../RPMS/x86_64/caml-crush-*
caml-crush.spec:29: W: mixed-use-of-spaces-and-tabs (spaces: line 3, tab: line 29)
caml-crush.spec: W: invalid-url Source0: v1.0.4-hobbled.tar.gz
caml-crush.src: W: strange-permission pkcs11proxyd-init 0755L
caml-crush.src:29: W: mixed-use-of-spaces-and-tabs (spaces: line 3, tab: line 29)
caml-crush.src: W: invalid-url Source0: v1.0.4-hobbled.tar.gz
caml-crush.x86_64: W: no-manual-page-for-binary pkcs11proxyd
caml-crush-softhsm.x86_64: W: no-documentation
caml-crush-softhsm.x86_64: W: non-standard-uid /var/lib/pkcs11proxyd/softhsm.conf pkcs11proxyd
caml-crush-softhsm.x86_64: W: non-standard-gid /var/lib/pkcs11proxyd/softhsm.conf pkcs11proxyd
caml-crush-softhsm.x86_64: W: non-standard-uid /var/lib/pkcs11proxyd/.config/pkcs11 pkcs11proxyd
caml-crush-softhsm.x86_64: W: non-standard-gid /var/lib/pkcs11proxyd/.config/pkcs11 pkcs11proxyd
caml-crush-softhsm.x86_64: W: non-standard-uid /var/lib/pkcs11proxyd/.config/pkcs11/pkcs11.conf pkcs11proxyd
caml-crush-softhsm.x86_64: W: non-standard-gid /var/lib/pkcs11proxyd/.config/pkcs11/pkcs11.conf pkcs11proxyd
caml-crush-softhsm.x86_64: W: non-conffile-in-etc /etc/pkcs11proxyd/filter-softhsm.conf
caml-crush-softhsm.x86_64: W: non-standard-uid /var/lib/pkcs11proxyd/.config/pkcs11/modules/softhsm.module pkcs11proxyd
caml-crush-softhsm.x86_64: W: non-standard-gid /var/lib/pkcs11proxyd/.config/pkcs11/modules/softhsm.module pkcs11proxyd
caml-crush-softhsm.x86_64: W: non-standard-uid /var/lib/pkcs11proxyd/.config pkcs11proxyd
caml-crush-softhsm.x86_64: W: non-standard-gid /var/lib/pkcs11proxyd/.config pkcs11proxyd
caml-crush-softhsm.x86_64: W: hidden-file-or-dir /var/lib/pkcs11proxyd/.config
caml-crush-softhsm.x86_64: W: hidden-file-or-dir /var/lib/pkcs11proxyd/.config
caml-crush-softhsm.x86_64: W: non-conffile-in-etc /etc/pkcs11proxyd/pkcs11proxyd-softhsm.conf
caml-crush-softhsm.x86_64: W: non-standard-uid /var/lib/pkcs11proxyd pkcs11proxyd
caml-crush-softhsm.x86_64: W: non-standard-gid /var/lib/pkcs11proxyd pkcs11proxyd
caml-crush-softhsm.x86_64: W: non-standard-uid /var/lib/pkcs11proxyd/.config/pkcs11/modules pkcs11proxyd
caml-crush-softhsm.x86_64: W: non-standard-gid /var/lib/pkcs11proxyd/.config/pkcs11/modules pkcs11proxyd
caml-crush-softhsm.x86_64: W: no-manual-page-for-binary pkcs11proxyd-init
4 packages and 1 specfiles checked; 0 errors, 26 warnings.

TODO: normalize the white spaces.

Package builds in F23 (http://koji.fedoraproject.org/koji/taskinfo?taskID=9319781). Ok.


Please correct all `FIX' items, consider fixing `TODO' items and provide new spec file.

Comment 16 Nikos Mavrogiannopoulos 2015-03-25 13:00:39 UTC
(In reply to Petr Pisar from comment #15)
> FIX: There is still src/bindings-pkcs11/pkcs11f.h with the same RSA license.
> The file is not used. Remove it from the archive too.
Done

> > FIX:  So either it's a public library, or it's a private library. See
> > <https://fedoraproject.org/wiki/Packaging:AutoProvidesAndRequiresFiltering#Private_Libraries>
> > for the second case, and
> > <https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#Beware_of_Rpath> for the first case.
> +%global __provides_filter_from ^%{_libdir}/pkcs11/.*\\.so$
> +

I used from the example in the same guidelines. Most probably a bug in the example.
https://fedoraproject.org/wiki/Packaging:AutoProvidesAndRequiresFiltering#Pidgin_plugin_package
 
> TODO: Move the macro definition right before %description section. It's the
> common place where to write the filters. 
> $ rpm -q --provides -p ../RPMS/x86_64/caml-crush-1.0.4-4.fc23.x86_64.rpm  |
> sort -f | uniq -c
>       1 caml-crush = 1.0.4-4.fc23
>       1 caml-crush(x86-64) = 1.0.4-4.fc23
>       1 config(caml-crush) = 1.0.4-4.fc23
>       1 libp11client.so()(64bit)

For some reason I don't see libp11client.so whether I add the filter or not. That's why I don't know whether the filter works or not.

> TODO: Please append the expression to possible current
> %__provides_exclude_from value like this:
> 
> %global __provides_exclude_from
> %{?__provides_exclude_from:%__provides_exclude_from|}^%{_libdir}/pkcs11/.*\\.
> so$
> It could happen the the value had already been defined somewhere else (macro
> files) and you could lose the old filter.

I'm not sure whether the previous provides should be replicated, this is not mentioned in the guidelines, and while it seems logical what you suggest, I'd 
be reluctant to use it.

Done.

> TODO: normalize the white spaces.

Done.

Spec URL: http://people.redhat.com/nmavrogi/fedora/caml-crush.spec
SRPM URL: http://people.redhat.com/nmavrogi/fedora/caml-crush-1.0.4-4.fc21.src.rpm

Comment 17 Petr Pisar 2015-03-25 14:40:19 UTC
Spec file changes:

--- caml-crush.spec.old 2015-03-23 09:54:24.000000000 +0100
+++ caml-crush.spec     2015-03-25 13:58:22.000000000 +0100
@@ -26,7 +26,7 @@
 Patch3:         caml-crush-better-msgs.patch
 Patch4:         caml-crush-honor-CFLAGS.patch

-Requires(pre): shadow-utils
+Requires(pre):  shadow-utils
 BuildRequires:  autoconf
 BuildRequires:  ocaml >= 4.00
 BuildRequires:  ocaml-findlib-devel
@@ -54,6 +54,7 @@
 Requires(preun):  systemd
 Requires(postun): systemd

+%global __provides_exclude_from ^%{_libdir}/pkcs11/.*\\.so$

 %description
 This software implements a PKCS#11 proxy as well as a PKCS#11 filter with
@@ -121,8 +122,6 @@
 install -p -m 644 %{SOURCE8} %{buildroot}%{_sharedstatedir}/pkcs11proxyd/.config/pkcs11/
 install -p -m 644 %{SOURCE9} %{buildroot}%{_sharedstatedir}/pkcs11proxyd/.config/pkcs11/modules

-%global __provides_filter_from ^%{_libdir}/pkcs11/.*\\.so$
-
 %files
 %doc README.md ISSUES.md
 %license LICENSE.txt


> FIX: There is still src/bindings-pkcs11/pkcs11f.h with the same RSA license.
> The file is not used. Remove it from the archive too.
The files has been removed. Ok.
Source archive (SHA-256:08d647041727d2f1d2d37b69a4d43eae2d76bcca0803dc2e9199676172971ea8) is Ok.

> TODO: Move the macro definition right before %description section. It's the
> common place where to write the filters.
Ok.

> > FIX: Your change did not removed the provides. The correct macro is name is
> > `__provides_exclude_from'. See
> > <https://fedoraproject.org/wiki/Packaging:AutoProvidesAndRequiresFiltering#Preventing_files.2Fdirectories_from_being_scanned_for_deps_.28pre-scan_filtering.29>.
> I used from the example in the same guidelines. Most probably a bug in the
> example.
> https://fedoraproject.org/wiki/Packaging:AutoProvidesAndRequiresFiltering#Pidgin_plugin_package

Yes, the example is wrong. I request a fix <https://fedorahosted.org/fpc/ticket/519>.

+%global __provides_exclude_from ^%{_libdir}/pkcs11/.*\\.so$
[...]
-%global __provides_filter_from ^%{_libdir}/pkcs11/.*\\.so$

$ rpm -q --provides -p ../RPMS/x86_64/caml-crush-1.0.4-4.fc23.x86_64.rpm | sort -f | uniq -c
      1 caml-crush = 1.0.4-4.fc23
      1 caml-crush(x86-64) = 1.0.4-4.fc23
      1 config(caml-crush) = 1.0.4-4.fc23
$ rpm -q --provides -p ../RPMS/x86_64/caml-crush-softhsm-1.0.4-4.fc23.x86_64.rpm | sort -f | uniq -c
      1 caml-crush-softhsm = 1.0.4-4.fc23
      1 caml-crush-softhsm(x86-64) = 1.0.4-4.fc23

Binary provides are Ok.

> TODO: Please append the expression to possible current %__provides_exclude_from
> value like this:
No addressed. Ok.

> TODO: normalize the white spaces.
$ rpmlint caml-crush.spec ../SRPMS/caml-crush-1.0.4-4.fc23.src.rpm ../RPMS/x86_64/caml-crush-*
caml-crush.spec:46: W: mixed-use-of-spaces-and-tabs (spaces: line 3, tab: line 46)
caml-crush.spec: W: invalid-url Source0: v1.0.4-hobbled.tar.gz
caml-crush.src: W: strange-permission pkcs11proxyd-init 0755L
caml-crush.src:46: W: mixed-use-of-spaces-and-tabs (spaces: line 3, tab: line 46)
caml-crush.src: W: invalid-url Source0: v1.0.4-hobbled.tar.gz
caml-crush.x86_64: W: no-manual-page-for-binary pkcs11proxyd
caml-crush-softhsm.x86_64: W: no-documentation
caml-crush-softhsm.x86_64: W: non-standard-uid /var/lib/pkcs11proxyd/softhsm.conf pkcs11proxyd
caml-crush-softhsm.x86_64: W: non-standard-gid /var/lib/pkcs11proxyd/softhsm.conf pkcs11proxyd
caml-crush-softhsm.x86_64: W: non-standard-uid /var/lib/pkcs11proxyd/.config/pkcs11 pkcs11proxyd
caml-crush-softhsm.x86_64: W: non-standard-gid /var/lib/pkcs11proxyd/.config/pkcs11 pkcs11proxyd
caml-crush-softhsm.x86_64: W: non-standard-uid /var/lib/pkcs11proxyd/.config/pkcs11/pkcs11.conf pkcs11proxyd
caml-crush-softhsm.x86_64: W: non-standard-gid /var/lib/pkcs11proxyd/.config/pkcs11/pkcs11.conf pkcs11proxyd
caml-crush-softhsm.x86_64: W: non-conffile-in-etc /etc/pkcs11proxyd/filter-softhsm.conf
caml-crush-softhsm.x86_64: W: non-standard-uid /var/lib/pkcs11proxyd/.config/pkcs11/modules/softhsm.module pkcs11proxyd
caml-crush-softhsm.x86_64: W: non-standard-gid /var/lib/pkcs11proxyd/.config/pkcs11/modules/softhsm.module pkcs11proxyd
caml-crush-softhsm.x86_64: W: non-standard-uid /var/lib/pkcs11proxyd/.config pkcs11proxyd
caml-crush-softhsm.x86_64: W: non-standard-gid /var/lib/pkcs11proxyd/.config pkcs11proxyd
caml-crush-softhsm.x86_64: W: hidden-file-or-dir /var/lib/pkcs11proxyd/.config
caml-crush-softhsm.x86_64: W: hidden-file-or-dir /var/lib/pkcs11proxyd/.config
caml-crush-softhsm.x86_64: W: non-conffile-in-etc /etc/pkcs11proxyd/pkcs11proxyd-softhsm.conf
caml-crush-softhsm.x86_64: W: non-standard-uid /var/lib/pkcs11proxyd pkcs11proxyd
caml-crush-softhsm.x86_64: W: non-standard-gid /var/lib/pkcs11proxyd pkcs11proxyd
caml-crush-softhsm.x86_64: W: non-standard-uid /var/lib/pkcs11proxyd/.config/pkcs11/modules pkcs11proxyd
caml-crush-softhsm.x86_64: W: non-standard-gid /var/lib/pkcs11proxyd/.config/pkcs11/modules pkcs11proxyd
caml-crush-softhsm.x86_64: W: no-manual-page-for-binary pkcs11proxyd-init
4 packages and 1 specfiles checked; 0 errors, 26 warnings.

rpmlint is Ok.

Package builds in F23 (http://koji.fedoraproject.org/koji/taskinfo?taskID=9321428). Ok.

Package is in line with Fedora packaging guidelines.

Resolution: Package APPROVED.

Comment 18 Nikos Mavrogiannopoulos 2015-03-25 15:14:45 UTC
New Package SCM Request
=======================
Package Name: caml-crush
Short Description: PKCS#11 filtering proxy
Owners: nmav
Branches: f21 f22 epel7
InitialCC:

Comment 19 Gwyn Ciesla 2015-03-25 17:27:19 UTC
Git done (by process-git-requests).

Comment 20 Fedora Update System 2015-04-02 11:28:12 UTC
caml-crush-1.0.4-6.fc22 has been submitted as an update for Fedora 22.
https://admin.fedoraproject.org/updates/caml-crush-1.0.4-6.fc22

Comment 21 Fedora Update System 2015-04-21 18:40:48 UTC
caml-crush-1.0.4-6.fc22 has been pushed to the Fedora 22 stable repository.


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