[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control
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
[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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 beginning of %insta
[Bug 1310092] Review Request: cryptobone - Secure Communication Under Your Control
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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