Bug 1341642 - Review Request: cryptlib - Security library and toolkit for encryption and authentication services
Summary: Review Request: cryptlib - Security library and toolkit for encryption and a...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Tom "spot" Callaway
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2016-06-01 12:00 UTC by Ralf Senderek
Modified: 2016-07-03 14:19 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2016-06-26 20:56:23 UTC
Type: ---
tcallawa: fedora-review+


Attachments (Terms of Use)

Description Ralf Senderek 2016-06-01 12:00:01 UTC
Spec URL: https://crypto-bone.com/fedora/cryptlib.spec
SRPM URL: https://crypto-bone.com/fedora/cryptlib-3.4.3-1.fc23.src.rpm

Description: 
Cryptlib is a powerful security tool kit that allows even inexperienced crypto
programmers to easily add encryption and authentication services to their
software. The high-level interface provides anyone with the ability to add
strong security capabilities to an application in as little as half an hour,
without needing to know any of the low-level details that make the encryption
or authentication work.  Because of this, cryptlib dramatically reduces the
cost involved in adding security to new or existing applications.

Fedora Account System Username: senderek (Ralf Senderek)

Comment 1 Ralf Senderek 2016-06-01 12:04:52 UTC
A successful KOJI build for f23 can be found here:

http://koji.fedoraproject.org/koji/taskinfo?taskID=14337244

Comment 2 Ralf Senderek 2016-06-01 12:13:25 UTC
There has already been a discussion about the necessity to exclude certain
code from the original cryptlib zip-file in RHBZ #1310092 (cryptobone).

Of course the same reduced source code zip file is used in this review request, to avoid any possible legal issues.

The reduced cryptlib source code zip file can be found here:

https://crypto-bone.com/fedora/cl343_fedora.zip



For more information, please refer to comment #47 of RHBZ #1310092 here:

https://bugzilla.redhat.com/show_bug.cgi?id=1310092#c47

Comment 3 Ralf Senderek 2016-06-03 15:34:07 UTC
I have made some improvements with the new release (release 2) of the cryptlib spec file:

 * I made all subpackages "noarch" that don't require or provide a shared lib.

 * I included a mandatory source code signature check that verifies the 
   file "cl343_fedora.zip"

 * I added subpackages for Perl and Javadoc

 * I disabled two tests from the native stestlib binary that are responsible
   for a failure due to a change on the openssh.org server that is contacted
   via the network during the cryptlib validation tests

New (release 2)

Spec URL: https://crypto-bone.com/fedora/cryptlib.spec
SRPM URL: https://crypto-bone.com/fedora/cryptlib-3.4.3-2.fc23.src.rpm

And the successful KOJI build for f23 here:

http://koji.fedoraproject.org/koji/taskinfo?taskID=14367434

Comment 4 Tom "spot" Callaway 2016-06-06 18:10:01 UTC
Two quick things:

* The devel package shouldn't be noarch.
* Don't include the libcl.so file in the java package. If the java package depends on it, handle that with Requires: cryptlib-devel.

Show me those items fixed and I'll finish off the review.

Comment 5 Ralf Senderek 2016-06-06 19:03:15 UTC
Thanks for the comments,

in the new release 3 the two things are fixed now:

New (release 3)

Spec URL: https://crypto-bone.com/fedora/cryptlib.spec
SRPM URL: https://crypto-bone.com/fedora/cryptlib-3.4.3-3.fc23.src.rpm

Comment 6 Ralf Senderek 2016-06-06 19:10:20 UTC
Peter Gutmann's excellent documentation for programmers is included in the doc package as "manual.pdf". This is the ultimate guide for everyone who will use cryptlib to enrich their code.

It has the following usage license:

   "You may print a reasonable number of copies of this work
    for personal use in conjunction with cryptlib software
    development provided that no fee is charged."

Obviously, it is Peter's intention not to impose any restriction on the manual's use except the one that he wants nobody to make money by providing his manual.
I think this is perfectly acceptable within the doc subpackage.

Comment 7 Tom "spot" Callaway 2016-06-09 15:38:30 UTC
Unfortunately, that license is non-free. That means that if someone makes a spin of Fedora that includes cryptlib, then sells it (even just to cover their costs), they're in violation.

Commercial-use restrictions make a license non Free (and non open source).

Please remove that file (or get the copyright holder to relicense it under a known FOSS license). Sorry. :/

Comment 8 Ralf Senderek 2016-06-09 17:45:35 UTC
So I have removed the doc subpackage altogether and I have created a 
file "README-manual" in which I point to the web site where users can 
download the manual.pdf.

This file contains the information about the usefulness of the PDF manual
which formerly was in the description of the doc package. 

This led to a new release (release 4)

Spec URL: https://crypto-bone.com/fedora/cryptlib.spec
SRPM URL: https://crypto-bone.com/fedora/cryptlib-3.4.3-4.fc23.src.rpm

I hope everything is OK now.





The new README-manual reads:

Peter Gutmann has writen an excellent 374 page manual for Cryptlib.

This manual provides code examples in C, Java, Python and other
languages and detailed descriptions of the cryptlib security
architecture, including the explanation of its High-, Medium-
and Low-level interface. All information needed to add security
services to existing applications is easily accessible with this
manual.

The manual has a very liberal copyright notice, that allows commercial
use under the condition that the manual isn't distributed for a fee.

Unfortunately, due to this use restriction it cannot be included in 
the Fedora distribution.

But the good news is, that you can download this excellent manual as a
PDF file from Peter's web page.

http://www.cypherpunks.to/~peter/manual.pdf

If you refer to the numerous code examples, you will be able to use
crpytlib in your own (commercial) projects very easily.

Comment 9 Tom "spot" Callaway 2016-06-13 18:14:19 UTC
These Provides shouldn't be necessary:

Provides: libcl.so.3 = 3.4.3
Provides: cryptlib_py.so

For the first one, it is autogenerated in a standard way that Fedora/EPEL packages use:
[spot@localhost SPECS]$ rpm -qp /home/spot/rpmbuild/RPMS/x86_64/cryptlib-3.4.3-4.fc24.x86_64.rpm --provides
cryptlib = 3.4.3-4.fc24
cryptlib(x86-64) = 3.4.3-4.fc24
libcl.so.3()(64bit)

For the python2 subpackage, nothing should depend on "cryptlib_py.so", so that Provides isn't useful.

* The %files entry for the -test subpackage is wrong. You're getting everything from the java, perl, and python packages too and duplicating it.

rpmlint output:

cryptlib.src: W: spelling-error %description -l en_US crypto -> crypt, crypts, crypt o
cryptlib.src:114: W: macro-in-comment %package
cryptlib.src:118: W: macro-in-comment %description
cryptlib.src:246: W: macro-in-comment %{buildroot}
cryptlib.src:246: W: macro-in-comment %{cryptlibdir}
cryptlib.src:297: W: macro-in-comment %files
cryptlib.src: W: invalid-url Source5: perlfiles.tar.gz
cryptlib.src: W: invalid-url Source4: cryptlib-tests.tar.gz
cryptlib.x86_64: W: spelling-error %description -l en_US crypto -> crypt, crypts, crypt o
cryptlib.x86_64: E: missing-call-to-setgroups-before-setuid /usr/lib64/libcl.so.3.4.3
cryptlib-devel.x86_64: W: only-non-binary-in-usr-lib
cryptlib-devel.x86_64: W: no-documentation
cryptlib-test.x86_64: W: unstripped-binary-or-object /usr/lib64/cryptlib/stestlib
cryptlib-test.x86_64: W: no-documentation
cryptlib-test.x86_64: W: devel-file-in-non-devel-package /usr/lib64/cryptlib/test/filename.h
cryptlib-test.x86_64: W: devel-file-in-non-devel-package /usr/lib64/cryptlib/c/cryptlib-test.c
cryptlib-test.x86_64: W: devel-file-in-non-devel-package /usr/lib64/cryptlib/test/test.h
cryptlib-java.x86_64: E: devel-dependency cryptlib-devel
cryptlib-java.x86_64: W: only-non-binary-in-usr-lib
cryptlib-java.x86_64: W: no-documentation
cryptlib-javadoc.noarch: W: wrong-file-end-of-line-encoding /usr/share/javadoc/cryptlib/META-INF/MANIFEST.MF
cryptlib-python2.x86_64: W: unstripped-binary-or-object /usr/lib/python2.7/site-packages/cryptlib_py.so
cryptlib-python2.x86_64: W: no-documentation
cryptlib-perl.x86_64: W: unstripped-binary-or-object /usr/lib64/perl5/auto/PerlCryptLib/PerlCryptLib.so
cryptlib-perl.x86_64: E: standard-dir-owned-by-package /usr/share/man/man3
cryptlib-perl.x86_64: E: non-standard-executable-perm /usr/lib64/perl5/auto/PerlCryptLib/PerlCryptLib.so 555
cryptlib-perl.x86_64: W: hidden-file-or-dir /usr/lib64/perl5/auto/PerlCryptLib/.packlist
cryptlib-perl.x86_64: W: perl-temp-file /usr/lib64/perl5/auto/PerlCryptLib/.packlist

To sum that up:
* You should add URLs for sources, unless they are not available anywhere online, and if that's the case, you need to add a comment describing how to construct that source tarball.
* Are the source files useful in the -test subpackage?
* Please fix the end-of-line encoding in the MANIFEST.MF
* Delete the perl5 hidden files (they're not useful post-build)
* Please remove macros from comments or replace "%" with "%%" to ensure they're not accidentally invoked. This is happening because you have the python3 bits commented out. An easier way to do it is to wrap those sections like this:

%if 0
%package python3
Summary:  Cryptlib bindings for python3
Group:    System Environment/Libraries
...
%endif

If you want to be fancy, you can do:

%global with_python3 0

%if %{with_python3}
%package python3
Summary:  Cryptlib bindings for python3
Group:    System Environment/Libraries
...
%endif

Comment 10 Ralf Senderek 2016-06-14 13:10:31 UTC
(In reply to Tom "spot" Callaway from comment #9)
> These Provides shouldn't be necessary:
> 
> Provides: libcl.so.3 = 3.4.3
> Provides: cryptlib_py.so

I have removed both.

> * The %files entry for the -test subpackage is wrong. You're getting
> everything from the java, perl, and python packages too and duplicating it.

No, the %files entry for the -test subpackage is good. All the numerous
test files are installed in this subpackage. In my last spec file the -java subpackage contained one file "/usr/lib64/cryptlib/java/cryptlib.jar" which
was a duplicate. I have now removed this file from the java subpackage, so
that everything in %{cryptlibdir} is in the -test package only.
All other subpackages have never contributed to %{cryptlibdir} in former 
spec files.


> To sum that up:
> * You should add URLs for sources, unless they are not available anywhere
> online, and if that's the case, you need to add a comment describing how to
> construct that source tarball.

I have put all sources on the web now, excluding the codesigning key, which
must be provided by distgit instead of an URL. See the discussion here:
https://fedorahosted.org/fpc/ticket/610 (comment #76)


> * Are the source files useful in the -test subpackage?

Yes, they are useful. The subpackage has a directory with a large number of
test files that includes two header files. These test files at their proper
location are used by the binary "stestlib" that checks the full functionality
of cryptlib and won't work without these header files, as they contain the file names of test data to be used by stestlib.



> * Please fix the end-of-line encoding in the MANIFEST.

Well I have removed this file after build, because it is not necessary for
the javadoc package.

> * Delete the perl5 hidden files (they're not useful post-build)

I've done that. And I put in a Requires: man to be able to use the standard
directory and fixed the permissions of the shared perl library.

> * Please remove macros from comments or replace "%" with "%%" to ensure
> they're not accidentally invoked. 

OK, I've opted for the fancy version.

Here are the new (release 5) SRPM and spec files and the KOJI build:


Spec URL: https://crypto-bone.com/fedora/cryptlib.spec
SRPM URL: https://crypto-bone.com/fedora/cryptlib-3.4.3-5.fc23.src.rpm


KOJI: http://koji.fedoraproject.org/koji/taskinfo?taskID=14485570


I think I have addressed all your comments.

Comment 11 Tom "spot" Callaway 2016-06-14 16:38:57 UTC
(In reply to Ralf Senderek from comment #10)

> I think I have addressed all your comments.

I think you have. Thanks for the quick turnaround.

= Review =

Good:

- rpmlint checks return:
cryptlib.src: W: spelling-error %description -l en_US crypto -> crypt, crypts, crypt o
cryptlib.x86_64: W: spelling-error %description -l en_US crypto -> crypt, crypts, crypt o
cryptlib.x86_64: E: missing-call-to-setgroups-before-setuid /usr/lib64/libcl.so.3.4.3
cryptlib-devel.x86_64: W: only-non-binary-in-usr-lib
cryptlib-devel.x86_64: W: no-documentation
cryptlib-test.x86_64: W: unstripped-binary-or-object /usr/lib64/cryptlib/stestlib
cryptlib-test.x86_64: W: no-documentation
cryptlib-test.x86_64: W: devel-file-in-non-devel-package /usr/lib64/cryptlib/c/cryptlib-test.c
cryptlib-test.x86_64: W: devel-file-in-non-devel-package /usr/lib64/cryptlib/test/filename.h
cryptlib-test.x86_64: W: devel-file-in-non-devel-package /usr/lib64/cryptlib/test/test.h
cryptlib-java.x86_64: W: only-non-binary-in-usr-lib
cryptlib-java.x86_64: W: no-documentation
cryptlib-python2.x86_64: W: unstripped-binary-or-object /usr/lib/python2.7/site-packages/cryptlib_py.so
cryptlib-python2.x86_64: W: no-documentation
cryptlib-perl.x86_64: W: unstripped-binary-or-object /usr/lib64/perl5/auto/PerlCryptLib/PerlCryptLib.so
8 packages and 1 specfiles checked; 1 errors, 14 warnings.

All safe to ignore (though, you might want to take that setuid/setgroups issue to the upstream).

- package meets naming guidelines
- package meets packaging guidelines
- license (Sleepycat) OK, text in %doc, matches source
- spec file legible, in am. english
- source matches upstream (df5fbae81bdda9e1f89afe7fb347332102327e48dca5dc0cbdf86d3c899d01a0)
- package compiles on devel (x86_64)
- no missing BR
- no unnecessary BR
- no locales
- not relocatable
- owns all directories that it creates
- no duplicate files
- permissions ok
- macro use consistent
- code, not content
- no need for -docs
- nothing in %doc affects runtime
- no need for .desktop file
- devel package ok
- no .la files
- post/postun ldconfig ok
- devel requires base package n-v-r 

APPROVED.

Comment 12 Ralf Senderek 2016-06-14 16:58:15 UTC
(In reply to Tom "spot" Callaway from comment #11)
> (In reply to Ralf Senderek from comment #10)

> - rpmlint checks return:
...
> All safe to ignore (though, you might want to take that setuid/setgroups
> issue to the upstream).

I will indeed discuss the setuid/setgroups issue with Peter.

Thank you very much for taking the time to help me improve the cryptlib
package.

Ralf.

Comment 13 Gwyn Ciesla 2016-06-14 17:25:13 UTC
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/cryptlib

Comment 14 Fedora Update System 2016-06-14 19:26:40 UTC
cryptlib-3.4.3-5.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2016-be8ddbb277

Comment 15 Fedora Update System 2016-06-14 19:26:51 UTC
cryptlib-3.4.3-5.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2016-c2d5cfd93a

Comment 16 Fedora Update System 2016-06-15 16:56:39 UTC
cryptlib-3.4.3-5.fc24 has been pushed to the Fedora 24 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-c2d5cfd93a

Comment 17 Fedora Update System 2016-06-15 17:04:47 UTC
cryptlib-3.4.3-5.el7 has been submitted as an update to Fedora EPEL 7. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2016-bd4a6c61df

Comment 18 Fedora Update System 2016-06-15 17:27:36 UTC
cryptlib-3.4.3-5.fc23 has been pushed to the Fedora 23 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-be8ddbb277

Comment 19 Fedora Update System 2016-06-16 17:04:59 UTC
cryptlib-3.4.3-6.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2016-5a6b08d677

Comment 20 Fedora Update System 2016-06-16 17:05:07 UTC
cryptlib-3.4.3-6.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2016-2c028207ff

Comment 21 Fedora Update System 2016-06-17 18:48:57 UTC
cryptlib-3.4.3-5.el7 has been pushed to the Fedora EPEL 7 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2016-bd4a6c61df

Comment 22 Fedora Update System 2016-06-18 05:24:51 UTC
cryptlib-3.4.3-6.fc23 has been pushed to the Fedora 23 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-5a6b08d677

Comment 23 Fedora Update System 2016-06-18 16:26:36 UTC
cryptlib-3.4.3-6.fc24 has been pushed to the Fedora 24 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-2c028207ff

Comment 24 Fedora Update System 2016-06-26 20:56:16 UTC
cryptlib-3.4.3-6.fc24 has been pushed to the Fedora 24 stable repository. If problems still persist, please make note of it in this bug report.

Comment 25 Fedora Update System 2016-06-27 00:52:39 UTC
cryptlib-3.4.3-6.fc23 has been pushed to the Fedora 23 stable repository. If problems still persist, please make note of it in this bug report.

Comment 26 Fedora Update System 2016-07-03 14:19:16 UTC
cryptlib-3.4.3-5.el7 has been pushed to the Fedora EPEL 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.