[Bug 1200389] Review Request: caml-crush - a PKCS#11 filtering proxy

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



--- Comment #13 from Petr Pisar <ppisar@xxxxxxxxxx> ---
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.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review





[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]