[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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

Peter Robinson  changed:

   What|Removed |Added

 Status|ASSIGNED|CLOSED
 Resolution|--- |RAWHIDE
Last Closed||2016-04-28 05:24:04



--- Comment #49 from Peter Robinson  ---
built

-- 
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@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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



--- Comment #48 from Peter Robinson  ---
You should be good to go. Thanks for the help here, much appreciated!

-- 
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@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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



--- Comment #47 from Ralf Senderek  ---
(In reply to Tom "spot" Callaway from comment #46)
> Confirmed. New SRPM has no legal issues. Lifting FE-Legal, please unblock
> this package in git.

Of course when updated versions of cryptlib become available, I will apply
these restrictions and deletions to the updates as well and build new SRPMS.

-- 
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@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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

Tom "spot" Callaway  changed:

   What|Removed |Added

 Blocks|182235 (FE-Legal)   |



--- Comment #46 from Tom "spot" Callaway  ---
Confirmed. New SRPM has no legal issues. Lifting FE-Legal, please unblock this
package in git.


Referenced Bugs:

https://bugzilla.redhat.com/show_bug.cgi?id=182235
[Bug 182235] Fedora Legal Tracker
-- 
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@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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



--- Comment #45 from Ralf Senderek  ---
(In reply to Ralf Senderek from comment #44)
> (In reply to Ralf Senderek from comment #42)
> > (In reply to Peter Robinson from comment #37)
> > > I have concerns about the bundled cryptlib:

These are the new SRPM and spec file for cryptobone version 1.0.2

Included in the SRPM is the reduced cryptlib source code zip file, which
is permalinked for public inspection here:

cryptlib reduced ZIP:   
https://crypto-bone.com/fedora/cl343_beta_fedora.zip

SRPM: https://crypto-bone.com/fedora/cryptobone-1.0.2-1.fc23.src.rpm

Spec file (1.0.2 release 1): https://crypto-bone.com/fedora/cryptobone.spec


The SRPM does not include anything that has legal issues, so there is no
reason to block this version of the cryptobone.

Ralf Senderek

-- 
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@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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



--- Comment #44 from Ralf Senderek  ---
(In reply to Ralf Senderek from comment #42)
> (In reply to Peter Robinson from comment #37)
> > I have concerns about the bundled cryptlib:

In a private conversation with Peter Gutmann I have confirmed that

a) there is no legal problem with using the Sleepycat license for cryptlib
   as a library at all.

b) with reference to the authors of the brainpool curves, Peter Gutmann does
   not expect any restrictions of the use of brainpool curves.

Even though upstream sees no conflict with (alleged) unapproved brainpool curve
parameters with the Fedora legal team, I will make sure that brainpool curve
source code will not be included in the SRPMs used by Fedora. (see Comment #42) 

Ralf Senderek

-- 
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@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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



--- Comment #43 from Ralf Senderek  ---
(In reply to Peter Robinson from comment #37)
> I have concerns about the bundled cryptlib:

...

> Why was this classified as "Not applicable"? I don't see why cryptlib
> shouldn't be shipped separately [2]. Was there a FPC ticket for this
> exception?
> [-]: Package contains no bundled libraries without FPC exception.

I'd like to explain why cryptlib as a bundled library
does not violate the packaging guidelines and why 
Richard's original classification as "Not applicable"
is correct.

Under the heading "Bundling and Duplication of system libraries"
the packaging guidelines state the following:

   Fedora packages should make every effort to avoid having multiple,
   separate, upstream projects bundled together in a single package.

   All packages whose upstreams allow them to be built against system 
   libraries must be built against system libraries.

   All packages whose upstreams have no mechanism to build against
   system libraries may opt to carry bundled libraries, but if they do,
   they must include Provides: bundled() =  in their
   RPM spec file. In addition, packages whose upstreams have no mechanism
   to build against system libraries must be contacted publicly about a
   path to supporting system libraries. If upstream refuses, this must
   be recorded in the spec file, either in comments placed adjacent to
   the Provides: above, or in an additional file checked into the SCM
   and referenced by a comment placed adjacent to the Provides: above.

The cryptobone and cryptlib are not separable. Even if a separate
cryptlib package existed (which is not the fact) the cryptobone could
use it in principle, but as the cryptobone is based on only a very
small part of cryptlib, essentially the symmetric encryption enveloping
only, and because reduction of complexity is one of the cryptobone's
main goals, the whole point is to be able to link against a minimalistic
version of cryptlib, maybe under a different name like libcl-mini.so
instead of libcl.so.

Now, we don't have this separate cryptlib package yet, and the cryptobone
cannot be built against a system library. And if it could, the reduced
mini library is a better choice from a security point of view.
At the moment there is no alternative to bundle cryptlib with the 
cryptobone.

"In addition, packages whose ..." The cryptobone package has no mechanism
to build against a system library called libcl.so. I have made it clear 
(Comment #34 and devel list) that I (as the cryptobone upstream) want
to build a separate cryptlib, so that linking is possible. I don't refuse
to do that (as a packager). But when this library comes into existence
the mini version as a bundled library is the next thing to do and
to do it right takes time. 

If it helps I'll put this explanation as a comment into the 
spec file, but the cryptobone should not be blocked for this reason.

Ralf Senderek

-- 
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@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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



--- Comment #42 from Ralf Senderek  ---
(In reply to Peter Robinson from comment #37)
> I have concerns about the bundled cryptlib:

In order to resolve the legal issue that has led to the
blocking of the cryptobone package once and for all, I
will not include the original cryptlib source code file
in any future SRPM. Instead, I will store a much reduced
zip archive that has no reference to any brainpool curve
implementation or any reference to brainpool code at all.

For public inspection I will provide this new zip-file 
on the Crypto Bone web site as a permalink:

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

In addition to the work of eradicating brainbool code and
references from the original source code archive, for which
I thank Tom Callaway, I have deleted files that did
not contribute anything to the compilation of the library
for use on linux systems. These files are:

 Test32.vcxproj
 Test32.vcxproj.filters
 Test32.vcxproj.user
 cl32.dll
 cl32.lib
 cl64.dll
 cl64.lib
 crypt32.def
 crypt32.dsp
 crypt32.dsw
 crypt32.ico
 crypt32.rc
 crypt32.sln
 crypt32.vcxproj
 crypt32.vcxproj.filters
 crypt32.vcxproj.user
 crypt32ce.vcp
 crypt32ce.vcw
 test32.dsp
 test32.dsw
 test32.sln
 test32ce.vcp
 test32ce.vcw
 tools/GenPas.pl
 tools/GenPerl.pl
 tools/GenVB.pl

This greatly reduced zip-archive (cl343_beta_fedora.zip) will 
always be used in future versions of the cryptobone, starting
with version 1.0.2

Secondly, as Tom Callaway has pointed out in his Comment #40,
cryplib is licensed under the Sleepycat license, which is 
recognized as a good license for Fedora.

https://fedoraproject.org/wiki/Licensing:Main?rd=Licensing#Good_Licenses

As both objections have been addressed to avoid any possible
legal problem, I think the cryptobone package should be unblocked
once the new cryptobone version 1.0.2 is used to build.

Ralf Senderek

-- 
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@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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



--- Comment #41 from Ralf Senderek  ---
(In reply to Tom "spot" Callaway from comment #40)

> Well, this is a fun mess. Cryptlib is dual-licensed under the Sleepycat
> license or a closed-"commercial" license. Since we're fine with the
> Sleepycat license, Fedora can happily just use it under those terms.

And the Sleepycat license is exactly what's shipped with the cryptobone.

> The Brainpool curves are a problem. 
> I've made a cleaned zip file for the cryptlib source. You'll need to make a
> new tarball for cryptobone that either has no bundled cryptlib zip file, or
> replaces the one it has now with one that does not include brainpool.
> 
> https://spot.fedorapeople.org/cl343_beta-no-brainpool.zip

Thank you Tom for your help. I will change the upstream source code to make
sure that there will be no possibility of a legal issue. To indicate this 
change I will bump the version number to 1.0.2 and proceed after testing
next week.

-- 
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@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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

Tom "spot" Callaway  changed:

   What|Removed |Added

 CC||tcall...@redhat.com



--- Comment #40 from Tom "spot" Callaway  ---
(In reply to Peter Robinson from comment #37)
> I have concerns about the bundled cryptlib:
> * Some of the included ECC curves haven't been approved (see rhbz 1019390)
> by legal AFAICT: 
> - Brainpool p256r1
> - Brainpool p384r1
> - Brainpool p512r1
> * The license needs clarification as while it states
> (http://www.cryptlib.com/security-software/licensing) it's an opensource
> license it also states " All cryptlib users must have a valid software
> license. Please contact the cryptlib sales team for further details.". It
> also states in the COPYING file that the website takes precedence so it
> could change at any time without our knowledge and the version shipped would
> have legal issues.

Well, this is a fun mess. Cryptlib is dual-licensed under the Sleepycat license
or a closed-"commercial" license. Since we're fine with the Sleepycat license,
Fedora can happily just use it under those terms.

The Brainpool curves are a problem. They need to be cleaned out of cryptlib
before it can be included. Not just disabled, or not used, but the source
cleaned of implementations. Nothing in cryptobone seems to depend on them. I've
made a cleaned zip file for the cryptlib source. You'll need to make a new
tarball for cryptobone that either has no bundled cryptlib zip file, or
replaces the one it has now with one that does not include brainpool.

https://spot.fedorapeople.org/cl343_beta-no-brainpool.zip

-- 
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@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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

Patrick Uiterwijk  changed:

   What|Removed |Added

 CC||puiterw...@redhat.com



--- Comment #39 from Patrick Uiterwijk  ---
(In reply to Richard Shaw from comment #38)
> (In reply to Peter Robinson from comment #37)
> > Why was this classified as "Not applicable"? I don't see why cryptlib
> > shouldn't be shipped separately [2]. Was there a FPC ticket for this
> > exception?
> > [-]: Package contains no bundled libraries without FPC exception.
> 
> Unless things have changed again, the prohibition on bundled libraries were
> completely removed from Fedora but apparently FPR hasn't been updated in
> that regard.
> 
> https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries

Please note that the policy says:
"All packages whose upstreams have no mechanism to build against system
libraries must be contacted publicly about a path to supporting system
libraries. If upstream refuses, this must be recorded in the spec file using a
persistent mechanism to be clarified in the packaging guidelines. "

I can see no record of this request in the spec file, or any public question of
allowing to unbundle


Also, it says that in this case: "The FPC will maintain the list of bundled
provides, please consult with them when adding new provides or in case of
disputes.".

Nor can I see any FPC communication reporting this provides to them.

-- 
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@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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



--- Comment #38 from Richard Shaw  ---
(In reply to Peter Robinson from comment #37)
> Why was this classified as "Not applicable"? I don't see why cryptlib
> shouldn't be shipped separately [2]. Was there a FPC ticket for this
> exception?
> [-]: Package contains no bundled libraries without FPC exception.

Unless things have changed again, the prohibition on bundled libraries were
completely removed from Fedora but apparently FPR hasn't been updated in that
regard.

https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries

-- 
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@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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

Peter Robinson  changed:

   What|Removed |Added

 CC||pbrobin...@gmail.com
 Blocks||182235 (FE-Legal)
   Assignee|hobbes1...@gmail.com|pbrobin...@gmail.com



--- Comment #37 from Peter Robinson  ---
I have concerns about the bundled cryptlib:
* Some of the included ECC curves haven't been approved (see rhbz 1019390) by
legal AFAICT: 
- Brainpool p256r1
- Brainpool p384r1
- Brainpool p512r1
* The license needs clarification as while it states
(http://www.cryptlib.com/security-software/licensing) it's an opensource
license it also states " All cryptlib users must have a valid software license.
Please contact the cryptlib sales team for further details.". It also states in
the COPYING file that the website takes precedence so it could change at any
time without our knowledge and the version shipped would have legal issues.

Why was this classified as "Not applicable"? I don't see why cryptlib shouldn't
be shipped separately [2]. Was there a FPC ticket for this exception?
[-]: Package contains no bundled libraries without FPC exception.

A few other things to note:
* on x86_64 only. It explicitly states in the description "This external device
can be another Linux computer dedicated to this task or a Beagle Bone or a
Raspberry Pi. (https://crypto-bone.com)" which tells me it should be built on
at least ARMv7 otherwise the description is misleading. Also note the
architecture Packaging guidelines [1]. I don't see any reason to not build this
on all arches.
* It packages a zlib, that should be using the distro version
* I also, personally, believe it should be running non root as it's own user.
No "secure" application should have any need to run as root. But that is my
opinion.

So I'm going to:
* takeover this BZ review
* block the package while we confirm the legal details of the ECC curves and
license with legal.

[1] https://fedoraproject.org/wiki/Packaging:Guidelines#Architecture_Support
[2] https://fedoraproject.org/wiki/Bundled_Software_policy


Referenced Bugs:

https://bugzilla.redhat.com/show_bug.cgi?id=182235
[Bug 182235] Fedora Legal Tracker
-- 
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@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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

Richard Shaw  changed:

   What|Removed |Added

   Assignee|nob...@fedoraproject.org|hobbes1...@gmail.com



--- Comment #36 from Richard Shaw  ---
Too late but to make it correct, I'm assigning the bug to myself.

-- 
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@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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



--- Comment #35 from Jon Ciesla  ---
Package request has been approved:
https://admin.fedoraproject.org/pkgdb/package/rpms/cryptobone

-- 
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@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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



--- Comment #34 from Ralf Senderek  ---
(In reply to Richard Shaw from comment #31)
> Ok, I think everything has been addressed so the package is:
> 
> *** APPROVED ***
> 
> I'll need to sponsor you formally now. I'm not sure if you have interest
> packaging other pieces of software so let me know.

I have a few ideas for future packages, but for a couple of days I'll stay
with making me familiar with all the new stuff that comes with using SCM.
And over time I will work on extending the approved package to other arches
as to make it available everywhere.

Probably the next step would be to package the external cryptobone, that uses
a second linux system to store the encryption keys separately.
Then there was the idea of packaging Peter Gutmann's cryptlib as a system
library that could generally be used. But this would require some consultation
with Peter before I know exactly how to deal with the test suites.
But this is definitively on my radar.

>   Typically I would require
> review of other packages before sponsoring but as you are upstream on a
> pretty complicated project I was making an exception but I would like to
> review the next couple of package reviews you submit.

I'd like to contribute to and advance code review of security critical software
in the Fedora community. So I looked at the new review requests and did a
preliminary check on the git-evtag package (
https://bugzilla.redhat.com/show_bug.cgi?id=1323214 )

I think I'll follow the developments there first. I'll keep you informed.

Thanks again for your assistance and help.

Ralf.

-- 
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@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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



--- Comment #33 from Ralf Senderek  ---
(In reply to Richard Shaw from comment #32)
> Ok, you've been formally sponsored, use your powers for good!

I will, indeed. Thank you very much for sponsoring me and for your time you've
spent to help me improve this 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@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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

Richard Shaw  changed:

   What|Removed |Added

 Blocks|177841 (FE-NEEDSPONSOR) |



--- Comment #32 from Richard Shaw  ---
Ok, you've been formally sponsored, use your powers for good!


Referenced Bugs:

https://bugzilla.redhat.com/show_bug.cgi?id=177841
[Bug 177841] Tracker: Review requests from new Fedora packagers who need a
sponsor
-- 
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@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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

Richard Shaw  changed:

   What|Removed |Added

 Status|NEW |ASSIGNED
  Flags||fedora-review+



--- Comment #31 from Richard Shaw  ---
Ok, I think everything has been addressed so the package is:

*** APPROVED ***

I'll need to sponsor you formally now. I'm not sure if you have interest
packaging other pieces of software so let me know. Typically I would require
review of other packages before sponsoring but as you are upstream on a pretty
complicated project I was making an exception but I would like to review the
next couple of package reviews you submit.

-- 
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@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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



--- Comment #30 from Ralf Senderek  ---
(In reply to Richard Shaw from comment #25)

> I'm pretty sure the license file must be installed to /usr/share/license...

Anyway, we need to use the plural here. (revision 9)


Spec URL: https://crypto-bone.com/fedora/cryptobone.spec
SRPM URL: https://crypto-bone.com/fedora/cryptobone-1.0.1-9.fc23.src.rpm

-- 
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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



--- Comment #29 from Ralf Senderek  ---
(In reply to Ralf Senderek from comment #27)

> So here are the new SRPMS and spec file. (release 8)
> 
> Spec URL: https://crypto-bone.com/fedora/cryptobone.spec
> SRPM URL: https://crypto-bone.com/fedora/cryptobone-1.0.1-8.fc23.src.rpm

And the new KOJI build for rawhide:

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

-- 
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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



--- Comment #28 from Ralf Senderek  ---
(In reply to Richard Shaw from comment #25)

> I'm pretty sure the license file must be installed to /usr/share/license...
> The easiest solution would be to move them in %install after make install
> and update your paths in %files

This is changed in release 8.


> > For weeks I have been trying to find out what rpmlint thinks the problem
> > may be here, and I have found nothing substantial on the web since that 
> > could
> > shed some light on what's required. I suppose this is a false-positive.
> > I'm inclined to ignore this error.
> 
> Yeah, I just wanted to bring it up since we need to address it (even if the
> decision is to do nothing).

I would think to do nothing is the right thing to do, see my separate analysis
of the cryptlib source code.

-- 
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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



--- Comment #27 from Ralf Senderek  ---
(In reply to Richard Shaw from comment #26)
> Ok, I wish I had looked at the systemd file sooner :) 
> 
> Since it's basically calling the SysV script I think the best solution is to
> move it to another location. After reviewing FHS[1] and the Fedora packaging
> guidelines[2] I'm thinking /usr/libexec/cryptobone would be a good solution
> assuming you don't want to drop it in /usr/bin :)
> 
> The alternative would be /usr/lib64/cryptobone.
> 
> [1] https://refspecs.linuxfoundation.org/FHS_3.0/fhs/ch04.html
> [2] https://fedoraproject.org/wiki/Packaging:Guidelines#Libexecdir

The problem with relocating this script is that the cryptobone daemon is
coded in a way to reduce the attack surface, which means it would only work
if located in a certain directory (ie /etc/init.d). So I decided to finally
change the source code again and make sure that /etc/init.d is replaced by
/usr/lib/cryptobone/init.d (in the binary's source code).
Now everything is local to %(cryptobonedir) and tested to work as expected.

So here are the new SRPMS and spec file. (release 8)

Spec URL: https://crypto-bone.com/fedora/cryptobone.spec
SRPM URL: https://crypto-bone.com/fedora/cryptobone-1.0.1-8.fc23.src.rpm

All other issues, you mentioned have been addressed in the new spec file.
I've moved the two COPYING files to /usr/share/license/cryptobone and claimed
the ownership of this directory.

So, what's next?

-- 
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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



--- Comment #26 from Richard Shaw  ---
Ok, I wish I had looked at the systemd file sooner :) 

Since it's basically calling the SysV script I think the best solution is to
move it to another location. After reviewing FHS[1] and the Fedora packaging
guidelines[2] I'm thinking /usr/libexec/cryptobone would be a good solution
assuming you don't want to drop it in /usr/bin :)

The alternative would be /usr/lib64/cryptobone.

[1] https://refspecs.linuxfoundation.org/FHS_3.0/fhs/ch04.html
[2] https://fedoraproject.org/wiki/Packaging:Guidelines#Libexecdir

-- 
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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



--- Comment #25 from Richard Shaw  ---
(In reply to Ralf Senderek from comment #22)
> (In reply to Richard Shaw from comment #21)
> > 
> >   - If (and only if) the source package includes the text of the license(s)
> >   in its own file, then that file, containing the text of the license(s)
> >   for the package is included in %license.
> >   Note: License file COPYING is marked as %doc instead of %license
> >   See:
> >   http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text
> >   This is due to a Fedora specific guideline to put licenses in
> > /usr/share/license
> >   instead of /usr/share/doc to reduce install size for space limited targets
> > like arm.
> >   Might be best to remove the license stuff from your makefile and use
> > relative paths 
> >   instead.
> 
> To be honest, I don't know how to handle this. The COPYING file is already
> marked as %license. Would it be necessary to move them to 
> /usr/share/license and leave the mark as %license?

I'm pretty sure the license file must be installed to /usr/share/license... The
easiest solution would be to move them in %install after make install and
update your paths in %files


> >   
> >   - cryptobone.x86_64: E: missing-call-to-setgroups-before-setuid
> > /usr/lib/cryptobone/libcl.so.3.4.3
> >   $ rpmlint -I missing-call-to-setgroups-before-setuid
> >   missing-call-to-setgroups-before-setuid:
> >   This executable is calling setuid and setgid without setgroups or
> > initgroups.
> >   There is a high probability this means it didn't relinquish all groups, 
> > and
> >   this would be a potential security issue to be fixed. Seek POS36-C on the
> > web
> >   for details about the problem.
> 
> For weeks I have been trying to find out what rpmlint thinks the problem
> may be here, and I have found nothing substantial on the web since that could
> shed some light on what's required. I suppose this is a false-positive.
> I'm inclined to ignore this error.

Yeah, I just wanted to bring it up since we need to address it (even if the
decision is to do nothing).


> > [!]: Package must own all directories that it creates.
> >  Note: Directories without known owners: /etc/init.d,
> >  /usr/share/icons/default, /usr/share/doc/cryptobone\
> >  Do we need the init.d file since we have a systemd service file?
> 
> Well yes, we need /etc/init.d so I added 
> %dir /etc/init.d
> (see comments in the spec file (release 7))

Ok, I see your comment in the spec file... I'll need some time to digest this
but I'm pretty sure it's against the guidelines to provide both a systemd and
SysV startup file. 

Is the problem that the SysV files does a lot of things that the systemd file
does not?

I know there was a lot of pain during the SystemD migration because SysV was
supposed to be used for starting and stopping the daemon but because it used
shell scripting a lot of upstreams used (some would say abused) that fact to
have it do much more.

-- 
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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



--- Comment #24 from Ralf Senderek  ---
(In reply to Ralf Senderek from comment #23)
> (In reply to Richard Shaw from comment #21)
> > Package Review

URL:
https://www.securecoding.cert.org/confluence/display/c/POS36-C.+Observe+correct+revocation+order+while+relinquishing+privileges

-- 
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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



--- Comment #23 from Ralf Senderek  ---
(In reply to Richard Shaw from comment #21)
> Package Review

My analysis of the RPMLINT-issue:
  This executable is calling setuid and setgid without setgroups or initgroups.
  There is a high probability this means it didn't relinquish all groups, and
  this would be a potential security issue to be fixed. Seek POS36-C on the web
  for details about the problem.

The problem is described in the following URL:
https://www.securecoding.cert.org/confluence/display/c/POS36C.+Observe+correct+revocation+order+while+relinquishing+privileges

The relevant part of the source code file "random/unix.c" (lines 1258 and 1259)
uses setresuid to set all three uids (real, effective and saved) as __linux__
is set.
The author notes that dropping the privileges here occurs as a precautionary
measure.
I cannot see that there is any chance that there are supplementary groups set
that
will not be relinquished, so I don't see any reason to bother the author to
include
setgroups or initgroups to address this issue.


source code:
1215 /* If we're root, give up our permissions to make sure
that we don't
1216inadvertently read anything sensitive.  If the
getpwnam() fails
1217(this can happen if we're chrooted with no "nobody"
entry in the
1218local passwd file) we default to -1, which is usually
nobody.
1219The newer setreXid() and setresXid() calls use a
parameter value
1220of -1 to indicate "don't change this value" so this
isn't
1221possible any longer, but then there's not really much
else that
1222we can do at this point.
1223 
1224We don't check whether the change succeeds since it's
not a major
1225security problem but just a precaution (in theory an
attacker
1226could do something like fork()ing until RLIMIT_NPROC is
reached,
1227at which point it'd fail, but that doesn't really give
them
1228anything) */
1229 if( geteuid() == 0 )

1248 if( gathererUID != ( uid_t ) -1 )
1249 {
1250 #if 0   /* Not available on some OSes */
1251 ( void ) setuid( gathererUID );
1252 ( void ) seteuid( gathererUID );
1253 ( void ) setgid( gathererGID );
1254 ( void ) setegid( gathererGID );
1255 #else
1256   #if( defined( __linux__ ) || ( defined( __FreeBSD__ ) && OSVERSION >= 5
) || \
1257( defined( __hpux ) && OSVERSION >= 11 ) )
1258 ( void ) setresuid( gathererUID,
gathererUID, gathererUID );
1259 ( void ) setresgid( gathererGID,
gathererGID, gathererGID );
1260   #else
1261 ( void ) setreuid( gathererUID,
gathererUID );
1262 ( void ) setregid( gathererGID,
gathererGID );
1263   #endif /* OSses with setresXid() */
1264 #endif /* 0 */
1265 }
1266 }

-- 
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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



--- Comment #22 from Ralf Senderek  ---
(In reply to Richard Shaw from comment #21)
> Package Review
> 
> Issues:
> ===
>   - gtk-update-icon-cache is invoked in %postun and %posttrans if package
>   contains icons.
>   Note: icons in cryptobone
>   See: http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Icon_Cache

I have included the necessary update scripts in %post %postun and %posttrans
sections in the new (release 7) spec file


> 
>   - Permissions on files are set properly.
>   Note: See rpmlint output
>   See: http://fedoraproject.org/wiki/Packaging/Guidelines#FilePermissions
>   This is a special case I think we're good here.

OK.

> 
>   - If (and only if) the source package includes the text of the license(s)
>   in its own file, then that file, containing the text of the license(s)
>   for the package is included in %license.
>   Note: License file COPYING is marked as %doc instead of %license
>   See:
>   http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text
>   This is due to a Fedora specific guideline to put licenses in
> /usr/share/license
>   instead of /usr/share/doc to reduce install size for space limited targets
> like arm.
>   Might be best to remove the license stuff from your makefile and use
> relative paths 
>   instead.

To be honest, I don't know how to handle this. The COPYING file is already
marked as %license. Would it be necessary to move them to 
/usr/share/license and leave the mark as %license?

What else should be changed?

>   
>   - cryptobone.x86_64: E: missing-call-to-setgroups-before-setuid
> /usr/lib/cryptobone/libcl.so.3.4.3
>   $ rpmlint -I missing-call-to-setgroups-before-setuid
>   missing-call-to-setgroups-before-setuid:
>   This executable is calling setuid and setgid without setgroups or
> initgroups.
>   There is a high probability this means it didn't relinquish all groups, and
>   this would be a potential security issue to be fixed. Seek POS36-C on the
> web
>   for details about the problem.

For weeks I have been trying to find out what rpmlint thinks the problem
may be here, and I have found nothing substantial on the web since that could
shed some light on what's required. I suppose this is a false-positive.
I'm inclined to ignore this error.

>   
>   - Some files are licensed MIT:
> MIT/X11 (BSD like)
>   --
>   cryptobone-1.0.1/src/cryptoboned/b64.c
>   cryptobone-1.0.1/src/cryptoboned/b64.h
>   cryptobone-1.0.1/src/openpgp/b64.c
>   cryptobone-1.0.1/src/openpgp/b64.h
>   
>   I think just updating your license tag to "BSD and MIT" should be good
> enough here.

I've done that.



> = MUST items =

> [!]: License field in the package spec file matches the actual license.
>  Note: Checking patched sources after %prep for licenses. Licenses
>  found: "MIT/X11 (BSD like)", "BSD (2 clause)", "GPL (v3 or later)",
>  "Unknown or generated", "BSD (4 clause)". 12 files have unknown
>  license. Detailed output of licensecheck in /home/build/fedora-
>  review/1310092-cryptobone/licensecheck.txt

Should be good with the new updated license tag.

> [!]: Package requires other packages for directories it uses.
>  Note: No known owner of /usr/share/doc/cryptobone

I've added 
%dir   %{_docdir}/cryptobone


> [!]: Package must own all directories that it creates.
>  Note: Directories without known owners: /etc/init.d,
>  /usr/share/icons/default, /usr/share/doc/cryptobone\
>Do we need the init.d file since we have a systemd service file?

Well yes, we need /etc/init.d so I added 
%dir /etc/init.d
(see comments in the spec file (release 7))


>I think we can ignore all but /usr/share/doc/cryptobone which can be 
> added
> as:
>%dir %{_docdir}/cryptobone in %files

done.


This is the updated (release 7) spec file. No changes to the source.

https://crypto-bone.com/fedora/cryptobone.spec

Please let me know if I have to change anything else.

-- 
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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



--- Comment #21 from Richard Shaw  ---

Package Review
==

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed


Issues:
===
  - gtk-update-icon-cache is invoked in %postun and %posttrans if package
  contains icons.
  Note: icons in cryptobone
  See: http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Icon_Cache

  - Permissions on files are set properly.
  Note: See rpmlint output
  See: http://fedoraproject.org/wiki/Packaging/Guidelines#FilePermissions
  This is a special case I think we're good here.

  - If (and only if) the source package includes the text of the license(s)
  in its own file, then that file, containing the text of the license(s)
  for the package is included in %license.
  Note: License file COPYING is marked as %doc instead of %license
  See:
  http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text
  This is due to a Fedora specific guideline to put licenses in
/usr/share/license
  instead of /usr/share/doc to reduce install size for space limited targets
like arm.
  Might be best to remove the license stuff from your makefile and use relative
paths 
  instead.

  - cryptobone.x86_64: E: missing-call-to-setgroups-before-setuid
/usr/lib/cryptobone/libcl.so.3.4.3
  $ rpmlint -I missing-call-to-setgroups-before-setuid
  missing-call-to-setgroups-before-setuid:
  This executable is calling setuid and setgid without setgroups or initgroups.
  There is a high probability this means it didn't relinquish all groups, and
  this would be a potential security issue to be fixed. Seek POS36-C on the web
  for details about the problem.

  - Some files are licensed MIT:
  MIT/X11 (BSD like)
--
cryptobone-1.0.1/src/cryptoboned/b64.c
cryptobone-1.0.1/src/cryptoboned/b64.h
cryptobone-1.0.1/src/openpgp/b64.c
cryptobone-1.0.1/src/openpgp/b64.h

I think just updating your license tag to "BSD and MIT" should be good
enough here.




= MUST items =

C/C++:
[x]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.

Generic:
[x]: Package is licensed with an open-source compatible license and meets
 other legal requirements as defined in the legal section of Packaging
 Guidelines.
[!]: License field in the package spec file matches the actual license.
 Note: Checking patched sources after %prep for licenses. Licenses
 found: "MIT/X11 (BSD like)", "BSD (2 clause)", "GPL (v3 or later)",
 "Unknown or generated", "BSD (4 clause)". 12 files have unknown
 license. Detailed output of licensecheck in /home/build/fedora-
 review/1310092-cryptobone/licensecheck.txt
[!]: Package requires other packages for directories it uses.
 Note: No known owner of /usr/share/doc/cryptobone
[!]: Package must own all directories that it creates.
 Note: Directories without known owners: /etc/init.d,
 /usr/share/icons/default, /usr/share/doc/cryptobone\
 Do we need the init.d file since we have a systemd service file?
 I think we can ignore all but /usr/share/doc/cryptobone which can be added
as:
 %dir %{_docdir}/cryptobone in %files
[x]: %build honors applicable compiler flags or justifies otherwise.
[-]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[-]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[x]: Package consistently uses macros (instead of hard-coded directory
 names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[ x: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
 Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[x]: Package contains systemd file(s) if in need.
[x]: Useful -debuginfo package or justification otherwise.
[x]: Package is not known to require an ExcludeArch tag.
[-]: Large documentation must go in a -doc subpackage. Large could be size
 (~1MB) or number of files.
 Note: Documentation size is 102400 bytes in 5 files.
[x]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least
 one supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
 Note: There are rpmlint messages (see attachment).
[x]: Package does not own files or directories owned by other packages.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
  

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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



--- Comment #18 from Ralf Senderek  ---
(In reply to Richard Shaw from comment #17)

> > I have added a line "Requires=sshd.service" to the cryptoboned.service file
> > and removed the code above from the spec file.
> 
> OK.

In hindsight you've been quite right actually to complain about this feature.
I made a mistake by thinking that the ssh daemon is a requirement. On the
client side I really do need the ssh client to be present, the daemon would
be necessary on a Fedora system that implements the external Crypto Bone.
So I think I'd remove this requirement from the source code.

The next release (6) is in the pipeline, but needs further testing before 
it'll hit the road.

> >  The spec file now has
> > a %prosttrans section, which informs the user to run this script.
> > This can be done any time, as long as the user has knowledge of the 
> > root password, to set the sudoers.d/cbcontrol file and to activate the
> > deamon.
> 
> Ok, I may have to dig into this one a bit. There is actually a process to
> get permission to be enabled by default, I believe it requires an FPC ticket
> but really I don't for this kind of process that it's unreasonable to have
> them read a little documentation so they know why they're getting into and
> enable the daemon explicitly.
> 

I've already worked further on this problem, and you're right that explicit
user consent should be necessary to activate the daemon. So the best way
to make sure that everything needed is up and running is the GUI. I modified
the GUI to ask the user for permission to activate the Crypto Bone and to
define the login name of the user that should (exclusively) use this
Crypto Bone. It is essential that only a user who can acquire root permissions
is able to make this initial setup (via "sudogetuser"). All other users
must be blocked from changing anything. I think the GUI does that now. (rel 6)

> This is a pretty invasive package so I appreciate your patience with getting
> me up to speed and making all the requisite changes.

> I'll start on the full review as soon as I have a few moments.

Thank you for your time and effort.

PS: I know that rpmlint has a few issues with the rpm, but the non-standard
file permissions are all a result of security measures not ignorance.

-- 
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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



--- Comment #20 from Ralf Senderek  ---
(In reply to Richard Shaw from comment #19)
> Before you post a new spec and SRPM go ahead and remove the chkconfig stuff.
> No need to add it just to silence rpmlint. We have to review all rpmlint
> errors but it is sometimes wrong and we can choose to ignore it.

OK, here is the new (release 6) SRPM and spec file:

Spec URL: https://crypto-bone.com/fedora/cryptobone.spec
SRPM URL: https://crypto-bone.com/fedora/cryptobone-1.0.1-6.fc23.src.rpm

The new KOJI build is here:

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

-- 
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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



--- Comment #19 from Richard Shaw  ---
Before you post a new spec and SRPM go ahead and remove the chkconfig stuff. No
need to add it just to silence rpmlint. We have to review all rpmlint errors
but it is sometimes wrong and we can choose to ignore it.

-- 
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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



--- Comment #17 from Richard Shaw  ---
(In reply to Ralf Senderek from comment #16)
>> (In reply to Richard Shaw from comment #14)
> > I'm assuming the sudogetuser in %post creates an interactive prompt?
> > 
> > Unfortunately the guidelines strictly forbid interactive installs, it's one
> > of the biggest differences between Fedora/Redhat and Debian philosophies. 
> > 
> 
> OK, I've made the whole installation process non-interactive now!

Ok, good. While I understand why you wanted it, I was worried about gui based
installs, I'm not even sure what would happen.


> > 
> > Also, this is probably not compliant:
> > 
> > 
> >  if ! systemctl is-active sshd > /dev/null ; then
> >   systemctl enable sshd 
> >  fi
> 
> I have added a line "Requires=sshd.service" to the cryptoboned.service file
> and removed the code above from the spec file.

OK.


> > Some other script feedback:
> > 
> > Daemons are not allowed to be enabled on install unless they have been
> > approved to do so. You should be using the systemd macros which take care of
> > this for you:
> 
> OK, I have resolved these issues by transferring the activation of my daemons
> to the source code (/usr/lib/cryptobone/sudogetuser). The spec file now has
> a %prosttrans section, which informs the user to run this script.
> This can be done any time, as long as the user has knowledge of the 
> root password, to set the sudoers.d/cbcontrol file and to activate the
> deamon.

Ok, I may have to dig into this one a bit. There is actually a process to get
permission to be enabled by default, I believe it requires an FPC ticket but
really I don't for this kind of process that it's unreasonable to have them
read a little documentation so they know why they're getting into and enable
the daemon explicitly.

This is a pretty invasive package so I appreciate your patience with getting me
up to speed and making all the requisite changes.

I'll start on the full review as soon as I have a few moments.

-- 
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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



--- Comment #16 from Ralf Senderek  ---
I have changed the source code and updated the spec file (now release 5):

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



(In reply to Richard Shaw from comment #14)
> I'm assuming the sudogetuser in %post creates an interactive prompt?
> 
> Unfortunately the guidelines strictly forbid interactive installs, it's one
> of the biggest differences between Fedora/Redhat and Debian philosophies. 
> 

OK, I've made the whole installation process non-interactive now!


> 
> Also, this is probably not compliant:
> 
> 
>  if ! systemctl is-active sshd > /dev/null ; then
>   systemctl enable sshd 
>  fi

I have added a line "Requires=sshd.service" to the cryptoboned.service file
and removed the code above from the spec file.


> 
> Some other script feedback:
> 
> Daemons are not allowed to be enabled on install unless they have been
> approved to do so. You should be using the systemd macros which take care of
> this for you:

OK, I have resolved these issues by transferring the activation of my daemons
to the source code (/usr/lib/cryptobone/sudogetuser). The spec file now has
a %prosttrans section, which informs the user to run this script.
This can be done any time, as long as the user has knowledge of the 
root password, to set the sudoers.d/cbcontrol file and to activate the deamon.

Cron jobs are gone now in favour of a systemd timer, as you advised.

> 
> Other odds-and-ends:
> 
> # You want to use mkdir -p and always use the directory macros
> mkdir $RPM_BUILD_ROOT/usr/share/icons
> mkdir $RPM_BUILD_ROOT/usr/share/icons/default

done.

> 
> # I prefer %{buildroot} but either is acceptable:
> mkdir -p %{buildroot}%{_datadir}/default

done.

> 
> # Is a jpg the only one available? PNG/SVGs are preferred, usually XPM is
> the backup.
> cp $RPM_BUILD_ROOT%{cryptobonedir}/GUI/cryptobone.jpg
> $RPM_BUILD_ROOT/usr/share/icons/default

I've changed that to a PNG file instead.

> 
> # Is there a reason the desktop file needs to be executable? 

No, of course not, I've changed that to 0644 mode now.


I hope the cryptobone package is in an acceptable shape now.

Thank you for your review so far.

Ralf

-- 
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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



--- Comment #15 from Ralf Senderek  ---
(In reply to Richard Shaw from comment #14)
> I'm assuming the sudogetuser in %post creates an interactive prompt?
> 
> Unfortunately the guidelines strictly forbid interactive installs, it's one
> of the biggest differences between Fedora/Redhat and Debian philosophies. 
> 
> I haven't checked yet but there should be instructions on any setup required
> before running the daemon. 

I need to find a solution for this issue, because it is an essential point.

The reason why the installation is interactive at the moment is that there
can only be one single user for the cryptobone. This user must contact the
cryptobone daemon (/usr/lib/cryptobone/cryptoboned) using only a very limited
number of well-defined programs as the root user. I am using the sudo mechanism
to allow the owner of the cryptobone to execute /usr/lib/cryptobone/cbcontrol
as root. 

The usability of the system depends on this feature, that a user can have 
controlled access to the secret message key data base via the daemon without
having to know (and use) anything but his login password.

That's why I need to set the user name in the file /etc/sudoers.d/cbcontrol
when the software is being installed. That's why I chose a GUI to ask for the
user name.

I cannot come up with an alternative to this setup. Because if the installation
was silent, some other root process must write this crucial file.
I need to exit the GUI until the user has manually created the sudo file.
It seems much more natural to me to request the user name while the package
is being installed, because at that time, the user who is in control of the
machine can make the right choice. 

So if this is absolutely a no-go, what do you suggest?

-- 
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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



--- Comment #14 from Richard Shaw  ---
I'm assuming the sudogetuser in %post creates an interactive prompt?

Unfortunately the guidelines strictly forbid interactive installs, it's one of
the biggest differences between Fedora/Redhat and Debian philosophies. 

I haven't checked yet but there should be instructions on any setup required
before running the daemon. 

Also, this is probably not compliant:


 if ! systemctl is-active sshd > /dev/null ; then
  systemctl enable sshd 
 fi

However, as long as the dependencies are setup correctly in the service file
for cryptobone, it will start sshd unless the user has masked it. Disabling a
service only stops it from being started by default, if something requires it,
it will be started unless masked.

Some other script feedback:

Daemons are not allowed to be enabled on install unless they have been approved
to do so. You should be using the systemd macros which take care of this for
you:

$ rpm -E %systemd_post

if [ $1 -eq 1 ] ; then 
# Initial installation 
systemctl preset  >/dev/null 2>&1 || : 
fi 

$ rpm -E %systemd_preun

if [ $1 -eq 0 ] ; then 
# Package removal, not upgrade 
systemctl --no-reload disable  > /dev/null 2>&1 || : 
systemctl stop  > /dev/null 2>&1 || : 
fi 

$ rpm -E %systemd_postun

systemctl daemon-reload >/dev/null 2>&1 || : 


Other odds-and-ends:

# You want to use mkdir -p and always use the directory macros
mkdir $RPM_BUILD_ROOT/usr/share/icons
mkdir $RPM_BUILD_ROOT/usr/share/icons/default

# I prefer %{buildroot} but either is acceptable:
mkdir -p %{buildroot}%{_datadir}/default

# Is a jpg the only one available? PNG/SVGs are preferred, usually XPM is the
backup.
cp $RPM_BUILD_ROOT%{cryptobonedir}/GUI/cryptobone.jpg
$RPM_BUILD_ROOT/usr/share/icons/default

# Is there a reason the desktop file needs to be executable? 
desktop-file-install --dir $RPM_BUILD_ROOT/usr/share/applications -m 755
$RPM_BUILD_ROOT%{cryptobonedir}/GUI/cryptobone.desktop

-- 
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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



--- Comment #13 from Ralf Senderek  ---
To resolve the postfix issue, I changed the requirement to MTA.
On a system without an MTA installed the default "exim" will be installed
alongside the cryptobone package.
If postfix is already installed the dependency is satisfied.
Both exim and postfix are active after installation, so the cryptobone works
fine in both situations.

New spec file: https://crypto-bone.com/fedora/cryptobone.spec
New SRPM:  https://crypto-bone.com/fedora/cryptobone-1.0.1-4.fc23.src.rpm

And the KOJI build is here: 
http://koji.fedoraproject.org/koji/taskinfo?taskID=13188783

Please let me know if anything else is missing.

Ralf.

-- 
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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



--- Comment #12 from Upstream Release Monitoring 
 ---
senderek's scratch build of cryptobone-1.0.1-4.fc23.src.rpm for f23 completed
http://koji.fedoraproject.org/koji/taskinfo?taskID=13188782

-- 
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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



--- Comment #11 from Richard Shaw  ---
(In reply to Ralf Senderek from comment #9)
> (In reply to Richard Shaw from comment #8)
> > 1. Have you considered a systemd timer rather than a cron job?
> 
> Not yet, a cron job running every minute seemed a good trade-off to me, as I
> try
> to get new incoming messages fast without putting too much strain on the
> system.
> The user should see a new message popping up in the RAM disk automatically.
> I could have avoided the cron job if I burden the user with triggering the
> mail polling which I don't want to do. Switching to systemd timers might be
> a good idea if I wanted to increase the frequency of incoming email polling.
> 
> Do you have a reason to suggest switching to a systemd timer that has any
> advantage in this context?

Nothing other than like SysV -> Systemd, systemd timers are the "latest" way to
do it. I could help you develop the file if you like but we'll save that for
another day.


> > 3. It would be good to add a comment on why this is necessary:
> > 
> >  if ! systemctl is-active postfix > /dev/null ; then
> >   systemctl enable postfix
> >  fi
> 
> That is something I have thought about recently. The main reason is to make
> sure
> that email is sent out. I've decided to require postfix for this package,
> but anyone already using another MTA would not really need that and what's
> worse postfix might create a conflict with the installed alternative MTA.

Both sendmail and postfix provide the MTA capability (look at rpm -q --provies
) but that will only make sure it's installed, not enabled or started.
This is an interesting one.


> The best solution would be to require any MTA be it postfix or other.
> 
> If "Requires: sendmail" would work I'd be happy enough. But I need to test
> that
> before I can change this aspect of my spec file.
> 
> There's only one place in the source code where it is needed: 
> (line 67 in the script "sendmail")
> 
> 67   /usr/sbin/sendmail  -f "$SENDER" "$RECIPIENT" <
> /usr/lib/cryptobone/cryptobone/encryptedmessage.asc
> 
> No other dependency is required for outgoing messages.

You may want to ask about this on the devel mailing list. I'm sure there will
be plenty of opinions on the matter though :)


> > 6. echo "Please reboot your computer as the crypto bone daemon will start 
> > only
> > while booting."
> 
> The cryptobone daemon is unusual in the sense that it needs access to secret
> information that is only available for a tiny period while the system is
> booting, I tried to inform the user that a re-boot is necessary after
> installation. Users will find that the GUI alerts them if the cryptobone
> daemon does not run, which will also be the case, if it is stopped and
> restarted like ordinary daemons. 

Well when you submit bodhi updates it will give you the option to suggest
rebooting, but of course that only works with PackageKit, not direct dnf
install/updates.


> So the GUI will alert the user in any case. If the message can remain in the
> spec file it might be better to place it in a %posttrans script so that it
> gets displayed as the last line?

Doesn't make any different as far as the packaging guidelines go but probably
more appropriate overall.


> > 7. What is the purpose of this? Placing systemd unit files in /etc is 
> > intended
> > for the end user so they can override the files in /usr/lib/systemd/system. 
> > I'm
> > pretty sure mucking around with /etc/systemd/system is forbidden.
> > 
> >  if [ -f /etc/systemd/system/cryptoboned.service ] ; then
> >   # re-install symlinks
> >   systemctl disable cryptoboned
> >   rm -f /etc/systemd/system/cryptoboned.service 2> /dev/null
> >   systemctl enable cryptoboned
> >  fi
> 
> Yes, because I have previously (mistakenly) installed the unit file under
> /etc

You can fix this in %install as well. Basically anything that needs to happen
after "make install" to fix things should be done there. The script macros are
only for things that need to happen during install/uninstall/update/removal,
etc.

-- 
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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



--- Comment #10 from Ralf Senderek  ---
I have updated the spec file (now release 4) and the source code remains
unchanged for now.

https://crypto-bone.com/fedora/cryptobone.spec

When the postfix/sendmail issue is resolved, I'll post the new SRPM and KOJI
build.

-- 
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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



--- Comment #9 from Ralf Senderek  ---
(In reply to Richard Shaw from comment #8)
> Ok, another round of spec comments. You may have to do some explaining as this
> is not an area I'm familiar with but you're doing a lot of interesting things
> with this package.

I'd be glad to explain every aspect of the cryptobone to you. In order to make
things easy for the user and at the same time not compromising on security,
that
leads to a system with a few bells and whistles that need explanation.


> 1. Have you considered a systemd timer rather than a cron job?

Not yet, a cron job running every minute seemed a good trade-off to me, as I
try
to get new incoming messages fast without putting too much strain on the
system.
The user should see a new message popping up in the RAM disk automatically. I
could have avoided the cron job if I burden the user with triggering the mail
polling which I don't want to do. Switching to systemd timers might be a good
idea if I wanted to increase the frequency of incoming email polling.

Do you have a reason to suggest switching to a systemd timer that has any
advantage in this context?


> 2. %global cryptobonedir %{_prefix}/lib/%{name}
done.

> 3. It would be good to add a comment on why this is necessary:
> 
>  if ! systemctl is-active postfix > /dev/null ; then
>   systemctl enable postfix
>  fi

That is something I have thought about recently. The main reason is to make
sure
that email is sent out. I've decided to require postfix for this package, but
anyone already using another MTA would not really need that and what's worse
postfix might create a conflict with the installed alternative MTA.

The best solution would be to require any MTA be it postfix or other.

If "Requires: sendmail" would work I'd be happy enough. But I need to test that
before I can change this aspect of my spec file.

There's only one place in the source code where it is needed: 
(line 67 in the script "sendmail")

67   /usr/sbin/sendmail  -f "$SENDER" "$RECIPIENT" <
/usr/lib/cryptobone/cryptobone/encryptedmessage.asc

No other dependency is required for outgoing messages.


> 4. This should be done in %install
Of course.

> 5. chkconfig --add cryptoboned 
As the package now requires systemd, this is unnecessary.


> 6. echo "Please reboot your computer as the crypto bone daemon will start only
> while booting."

The cryptobone daemon is unusual in the sense that it needs access to secret
information that is only available for a tiny period while the system is
booting, I tried to inform the user that a re-boot is necessary after
installation. Users will find that the GUI alerts them if the cryptobone daemon
does not run, which will also be the case, if it is stopped and restarted like
ordinary daemons. 
So the GUI will alert the user in any case. If the message can remain in the
spec file it might be better to place it in a %posttrans script so that it gets
displayed as the last line?



> 7. What is the purpose of this? Placing systemd unit files in /etc is intended
> for the end user so they can override the files in /usr/lib/systemd/system. 
> I'm
> pretty sure mucking around with /etc/systemd/system is forbidden.
> 
>  if [ -f /etc/systemd/system/cryptoboned.service ] ; then
>   # re-install symlinks
>   systemctl disable cryptoboned
>   rm -f /etc/systemd/system/cryptoboned.service 2> /dev/null
>   systemctl enable cryptoboned
>  fi

Yes, because I have previously (mistakenly) installed the unit file under /etc
I included this code only to get a clean system after update. In future
versions I will delete this code as it is no longer necessary. be cause I use
%{_unitdir} now.


> 8. %files
done.

> I think that's enough for now!

Thanks for your helpful comments.

-- 
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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



--- Comment #8 from Richard Shaw  ---
Ok, another round of spec comments. You may have to do some explaining as this
is not an area I'm familiar with but you're doing a lot of interesting things
with this package.

1. Have you considered a systemd timer rather than a cron job?

2. %global cryptobonedir %{_prefix}/lib/%{name}

It's odd to place this between %description and %prep. I usually throw all this
stuff at the top of the spec file so it sticks out.

3. It would be good to add a comment on why this is necessary:

 if ! systemctl is-active postfix > /dev/null ; then
  systemctl enable postfix
 fi 

4. This should be done in %install

 cp %{cryptobonedir}/GUI/cryptobone.jpg /usr/share/icons/default
 desktop-file-install --dir /usr/share/applications -m 755
%{cryptobonedir}/GUI/cryptobone.desktop

5. chkconfig --add cryptoboned 

SysV init scripts are just redirected to systemd, I would think this would only
be useful on EL 6.

6. echo "Please reboot your computer as the crypto bone daemon will start only
while booting."

Generally output during install is discouraged and anyone using a GUI installer
won't see it.

7. What is the purpose of this? Placing systemd unit files in /etc is intended
for the end user so they can override the files in /usr/lib/systemd/system. I'm
pretty sure mucking around with /etc/systemd/system is forbidden.

 if [ -f /etc/systemd/system/cryptoboned.service ] ; then
  # re-install symlinks
  systemctl disable cryptoboned
  rm -f /etc/systemd/system/cryptoboned.service 2> /dev/null
  systemctl enable cryptoboned 
 fi

8. %files

/etc/init.d/cryptoboned -> %{_sysconfdir}/init.d/cryptoboned

/usr/share/man/man8/cryptoboned.8.gz -> %{_mandir}/man8/cryptoboned.8.gz

I think that's enough for now!

-- 
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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



--- Comment #7 from Upstream Release Monitoring 
 ---
senderek's scratch build of cryptobone-1.0.1-2.fc20.src.rpm for rawhide
completed http://koji.fedoraproject.org/koji/taskinfo?taskID=13068144

-- 
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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



--- Comment #6 from Ralf Senderek  ---
Richard, 

I have changed the installation process in the source code which led to a
substantial reduction in the spec file. Here are the updated SRPM and spec
file:

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

The new KOJI build is here:

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

Please let me know if the changes are ok.


Ralf.

-- 
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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



--- Comment #5 from Upstream Release Monitoring 
 ---
senderek's scratch build of cryptobone-1.0.1-2.fc20.src.rpm for f23 completed
http://koji.fedoraproject.org/koji/taskinfo?taskID=13066306

-- 
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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



--- Comment #4 from Ralf Senderek  ---
(In reply to Richard Shaw from comment #1)
> Here's a quick spec file review:

> - Why x86_64 only?

Richard, 

thank you for your spec file review. 

The permission issues you found may require a re-organisation of the
installation process (%make_install) and a change of the source code. 
I'm working on this.

Let me explain why some of these decisions are important for the cryptobone
package.

The numerous (not-standard) file permissions follow from the necessity to 
restrict the use of the crypto bone to the root user. Also the location in
/usr/lib where secrets are held is essential, because it is checked by the
binaries that use them, in particular the cryobone daemon. 

For now, I decided to only prepare a tar archive of the newly build software
that does not install but contains everything needed. And I did the fiddly work
in the spec file. I'm now trying to put as much as possible back into the 
"make install" in the source code that would clean up the spec file.

> - Why x86_64 only?

Well, in principle, this system should work on all architectures. But I decided 
to restrict it to x86_64 at the moment, because the full testing cycle on an
arch is quite expensive and I don't have access to all other arches to test the
final result there. For a security-critical system like the cryptobone it is 
indispensable to run these tests manually, and I simply cannot do this alone.
It should be possible to expand the scope to i386 or arm but I don't see me
testing
ppc and co. So let me stick to this restriction for the moment. 

I'll post new spec file and SRPM later.

Ralf.
~

-- 
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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



--- Comment #3 from Ralf Senderek  ---
(In reply to Corey Sheldon from comment #2)
> Running tests for functionality and ease..
> 
> 
> Will advise any function / design quarks / suggests within the next few days

Corey,

thank you for running the tests. Please keep in mind that someone who'd use the
RPM from the web site would probalbly know of the fundamental ideas behind the
Crypto Bone, because of the context given there. In particular they would have
read the installation page.

Maybe I have to include some of this information in the new Fedora 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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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

Corey Sheldon  changed:

   What|Removed |Added

 CC||sheldon.co...@gmail.com



--- Comment #2 from Corey Sheldon  ---
Running tests for functionality and ease..


Will advise any function / design quarks / suggests within the next few days

-- 
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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

Richard Shaw  changed:

   What|Removed |Added

 CC||hobbes1...@gmail.com



--- Comment #1 from Richard Shaw  ---
Here's a quick spec file review:

- Typographical: Space in name tag "Name :" 

- Why x86_64 only?

- The %prep section isn't empty as it contains %setup, just do:
%prep
%setup

- %pre is optional, just drop it if you don't need any scripts.

- Use of %post questions:
-- chmod in %post: Why can it not be installed in this mode? Or does the binary
modify this when run?
-- Why copy the binary in %post? Can't it just be installed in bin directly?
-- No installing to /usr/local, this package would be part of Fedora and should
install to the system location, using the proper macro, %{_bindir}.

- %clean shouldn't be needed unless you have multiple Source files.

- %attr use in %files: This is usually only used to set users/groups for
non-root. Setting the mode should be done correctly by %make_install.
Additionally it's more common to fix permissions in %install rather than in
%files.

- In the case where you really want /usr/lib on x86_64 the better method is to
use %{_prefix}/lib.

-- 
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review

[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control

[bugzilla]

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

Ralf Senderek  changed:

   What|Removed |Added

 Blocks||177841 (FE-NEEDSPONSOR)




Referenced Bugs:

https://bugzilla.redhat.com/show_bug.cgi?id=177841
[Bug 177841] Tracker: Review requests from new Fedora packagers who need a
sponsor
-- 
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@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review