[Bug 970009] New: Review Request: stoken - Token code generator compatible with RSA SecurID 128-bit (AES) token
https://bugzilla.redhat.com/show_bug.cgi?id=970009
Bug ID: 970009 Summary: Review Request: stoken - Token code generator compatible with RSA SecurID 128-bit (AES) token Product: Fedora Version: rawhide Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: negativo17@gmail.com QA Contact: extras-qa@fedoraproject.org CC: notting@redhat.com, package-review@lists.fedoraproject.org
Spec URL: http://slaanesh.fedorapeople.org/stoken.spec SRPM URL: http://slaanesh.fedorapeople.org/stoken-0.2-1.fc19.src.rpm Description: Software Token for Linux/UNIX. It's a token code generator compatible with RSA SecurID 128-bit (AES) tokens. It is a hobbyist project, not affiliated with or endorsed by RSA Security. Fedora Account System Username: slaanesh
https://bugzilla.redhat.com/show_bug.cgi?id=970009
Simone Caronni negativo17@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Depends On| |970002
https://bugzilla.redhat.com/show_bug.cgi?id=970009
--- Comment #1 from Simone Caronni negativo17@gmail.com --- Note:
This package is required for enabling RSA software token support in openconnect, the Cisco AnyConnect VPN client.
https://bugzilla.redhat.com/show_bug.cgi?id=970009
David Woodhouse dwmw2@infradead.org changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |cernekee@gmail.com, | |dwmw2@infradead.org
--- Comment #2 from David Woodhouse dwmw2@infradead.org --- I could commit the libtomcrypt changes for you in the short term, as a provenpackager. But I think we'd be better off ditching libtomcrypt and using something else like gnutls if possible.
https://bugzilla.redhat.com/show_bug.cgi?id=970009
--- Comment #3 from Simone Caronni negativo17@gmail.com --- (In reply to David Woodhouse from comment #2)
I could commit the libtomcrypt changes for you in the short term, as a provenpackager. But I think we'd be better off ditching libtomcrypt and using something else like gnutls if possible.
Well, for me is the same; I would simply like to have stoken in Fedora. The code needs to be patched for gnutls support. though.
https://bugzilla.redhat.com/show_bug.cgi?id=970009
--- Comment #4 from Simone Caronni negativo17@gmail.com --- Woah, installed the token with stoken, rebuilt openconnect with stoken-devel, launch from the command line with "--token-mode=rsa" and now I'm asked the pin instead of the passcode (one number for another... don't know if it's a big achievement or not... :D).
I assume that for enabling the graphial interface there are additional changes required in NetworkManager-openconnect.
https://bugzilla.redhat.com/show_bug.cgi?id=970009
--- Comment #5 from David Woodhouse dwmw2@infradead.org --- Well, it might be one number for another, but at least it's now a *constant* number so it can be scripted. And with NetworkManager-openconnect, it can be stored in the keyring. Yes, some changes will be required in NetworkManager-openconnect but I think they're relatively simple. And I think Kevin had already done them but I was slacking and hadn't yet merged them, or something like that?
https://bugzilla.redhat.com/show_bug.cgi?id=970009
--- Comment #6 from David Woodhouse dwmw2@infradead.org --- Ah, no. The NM-openconnect parts should all be there already. Now you have a version of libopenconnect which admits to having stoken support, you should see the corresponding options in the configuration GUI.
https://bugzilla.redhat.com/show_bug.cgi?id=970009
--- Comment #7 from David Woodhouse dwmw2@infradead.org --- Why set CFLAGS=$RPM_OPT_FLAGS in %build? That's done by default anyway. Is that a leftover from an experiment with *changing* CFLAGS?
Perhaps you did that when you noticed the errant -O0 that you get by adding --enable-debug? We'll want upstream to give us a way to build with -O2 -ggdb which is our normal CFLAGS, rather than forcibly either overriding it to -O0 or stripping the debug info.
If you apply the patch I just sent you for using GnuTLS, you'll need to run autoreconf. And then you don't need to hack the libtool script with sed to fix the stupid rpath brokenness. Is that something you (should) have reported to the upstream maintainer, to make sure it's not present in the next release?
https://bugzilla.redhat.com/show_bug.cgi?id=970009
--- Comment #8 from Simone Caronni negativo17@gmail.com --- (In reply to David Woodhouse from comment #6)
Ah, no. The NM-openconnect parts should all be there already. Now you have a version of libopenconnect which admits to having stoken support, you should see the corresponding options in the configuration GUI.
Nope, I don't have them. I see support comes from the 0.9.8 release; though I don't know if NetworkManager-openconnect-0.9.7.0-2.git20120918.fc19 is a snapshot of 0.9.8 or not.
Can you update the current git snapshot in Fedora 18+ to 0.9.8?
======================================================= network-manager-openconnect-0.9.8 Overview of changes since network-manager-openconnect-0.9.6.2 =======================================================
This is a new stable release of network-manager-openconnect. Notable changes include:
* Updated translations * Builds against the GNOME 3.8 versions of GLib and Gtk+ * Software Tokens are now supported (if libopenconnect has support for them) * Fixed typos in several messages * nm-openconnect-service now returns translated error messages
(In reply to David Woodhouse from comment #7)
Perhaps you did that when you noticed the errant -O0 that you get by adding --enable-debug? We'll want upstream to give us a way to build with -O2 -ggdb which is our normal CFLAGS, rather than forcibly either overriding it to -O0 or stripping the debug info.
Exactly for this actually.
If you apply the patch I just sent you for using GnuTLS, you'll need to run autoreconf. And then you don't need to hack the libtool script with sed to fix the stupid rpath brokenness.
I was planning to make the minimum changes, but since the GnuTLS requires autoreconf to be rerun I removed the rpath script and CFLAGS override as well.
Is that something you (should) have reported to the upstream maintainer, to make sure it's not present in the next release?
Before doing anything I would have liked to see all the changes that are required coming from the review.
Is removing entirely the "enable debugging option" (lines 25-39) from configure.ac the best approach?
Thanks, --Simone
https://bugzilla.redhat.com/show_bug.cgi?id=970009
--- Comment #9 from David Woodhouse dwmw2@infradead.org --- NM-openconnect needs to be built against openconnect 5.00 or newer in order to get stoken support. I've updated it in rawhide and will update f19 once https://admin.fedoraproject.org/updates/openconnect-5.01-1.fc19 makes it to stable (and hence the buildroots).
We should probably look at enabling OATH support at the same time...
https://bugzilla.redhat.com/show_bug.cgi?id=970009
--- Comment #10 from Simone Caronni negativo17@gmail.com --- Spec URL: http://slaanesh.fedorapeople.org/stoken.spec SRPM URL: http://slaanesh.fedorapeople.org/stoken-0.2-2.fc19.src.rpm
Intermediate package until upstream replies to your mail about GnuTLS and removal of the conditional CFLAGS.
- Removed CFLAGS override - Removed rpath sed hacks - Added GnuTLS patch - Patch away "debug" section in configure.ac
Btw, as you said I rebuilt NetworkManager-openconnect-0.9.8 on openconnect-5.01-1 and now I have the token settings on Fedora 19.
Thanks.
https://bugzilla.redhat.com/show_bug.cgi?id=970009
--- Comment #11 from Kevin Cernekee cernekee@gmail.com ---
Woah, installed the token with stoken, rebuilt openconnect with stoken-devel, launch from the command line with "--token-mode=rsa" and now I'm asked the pin instead of the passcode (one number for another... don't know if it's a big achievement or not... :D).
You can use "stoken setpin" to cache the pin in ~/.stokenrc, allowing fully unattended operation.
Depending on your account lockout policies, you may be able to run openconnect in a loop to auto-reconnect on error:
while :; do openconnect -u user --non-inter --token-mode=rsa vpn.example.com; sleep 5; done
NM-openconnect needs to be built against openconnect 5.00 or newer in order to get stoken support. I've updated it in rawhide and will update f19 once https://admin.fedoraproject.org/updates/openconnect-5.01-1.fc19 makes it to stable (and hence the buildroots).
We should probably look at enabling OATH support at the same time...
The current NM-openconnect head of tree supports RSA but not OATH. I submitted OATH patches here (needs testing):
http://lists.infradead.org/pipermail/openconnect-devel/2013-March/001007.htm... http://lists.infradead.org/pipermail/openconnect-devel/2013-March/001006.htm... http://lists.infradead.org/pipermail/openconnect-devel/2013-March/001008.htm...
Intermediate package until upstream replies to your mail about GnuTLS and removal of the conditional CFLAGS.
Replied privately with a couple of follow-on questions.
https://bugzilla.redhat.com/show_bug.cgi?id=970009
--- Comment #12 from Kevin Cernekee cernekee@gmail.com --- I installed the FC19 beta tonight and made some additional progress:
stoken only really needs a tiny piece of libtomcrypt, so I copied the necessary libtomcrypt files right into the stoken tree. autoconf will still look for a shared version (since presumably it will get bugfixes and updates) but lacking that, it will use the local copy. Users who do have libtomcrypt installed can bypass it using "configure --without-libtomcrypt".
Incorporating the ~5 files from libtomcrypt allowed us to drop the GnuTLS dependency and the various concurrency / initialization problems that arose.
If all of this works OK, and doesn't completely break the Debian builds, I'll make it a formal v0.3 release.
Unfortunately I still wasn't able to get rpmbuild to stop complaining about the /usr/lib64 rpath, even after running autogen.sh on a much newer Linux installation than usual. So the autoreconf line remains in the spec file.
My latest SRPM: https://dl.dropboxusercontent.com/u/169702767/stoken/stoken-0.3-1.fc19.src.r...
Feature branch: https://github.com/cernekee/stoken/commits/fedora-v1
https://bugzilla.redhat.com/show_bug.cgi?id=970009
--- Comment #13 from Simone Caronni negativo17@gmail.com --- (In reply to Kevin Cernekee from comment #12)
stoken only really needs a tiny piece of libtomcrypt, so I copied the necessary libtomcrypt files right into the stoken tree. autoconf will still look for a shared version (since presumably it will get bugfixes and updates) but lacking that, it will use the local copy. Users who do have libtomcrypt installed can bypass it using "configure --without-libtomcrypt".
Incorporating the ~5 files from libtomcrypt allowed us to drop the GnuTLS dependency and the various concurrency / initialization problems that arose.
Unfortunately, the packaging guidelines forbid the inclusion of bundled libraries in the code; so at least in the Fedora case we have to use the libtomcrypt packages:
https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries
Spec URL: http://slaanesh.fedorapeople.org/stoken.spec SRPM URL: http://slaanesh.fedorapeople.org/stoken-0.3-2.fc19.src.rpm
- Re-enabled libtomcrypt system library
Thanks, --Simone
https://bugzilla.redhat.com/show_bug.cgi?id=970009
--- Comment #14 from David Woodhouse dwmw2@infradead.org --- If run on a machine with the Intel AES-NI instructions, does this make use of them? I'd like to make sure it does. Give me a SSH public key and a preferred username if you need an account on a suitable machine.
https://bugzilla.redhat.com/show_bug.cgi?id=970009
--- Comment #15 from David Woodhouse dwmw2@infradead.org --- Hm, you don't *really* need to update libtomcrypt; you could just configure with TOMCRYPT_CFLAGS=-I%{_includedir}/tomcrypt.
Well, that *was* true with the 0.2 tarball. Now it seems to define LOCAL_TOMCRYPT if there's no pkg-config package, and it doesn't fall back to using the system libtomcrypt unless there's a corresponding .pc file.
https://bugzilla.redhat.com/show_bug.cgi?id=970009
--- Comment #16 from Simone Caronni negativo17@gmail.com --- (In reply to David Woodhouse from comment #15)
Hm, you don't *really* need to update libtomcrypt; you could just configure with TOMCRYPT_CFLAGS=-I%{_includedir}/tomcrypt.
Well, that *was* true with the 0.2 tarball. Now it seems to define LOCAL_TOMCRYPT if there's no pkg-config package, and it doesn't fall back to using the system libtomcrypt unless there's a corresponding .pc file.
Yep. So it still needs the update. Actually I preferred the GnuTLS patch, including the 5 files from libtomcrypt makes things more complicated when packaging.
https://bugzilla.redhat.com/show_bug.cgi?id=970009
Bug 970009 depends on bug 970002, which changed state.
Bug 970002 Summary: Package seems abandoned https://bugzilla.redhat.com/show_bug.cgi?id=970002
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution|--- |CURRENTRELEASE
https://bugzilla.redhat.com/show_bug.cgi?id=970009
--- Comment #17 from David Woodhouse dwmw2@infradead.org --- The GnuTLS patch needs to be fixed to call gnutls_cipher_deinit(), and you need to call gnutls_global_init() somewhere.
And you need to *not* care about the tiny possibility of a race condition with multiple libraries in different threads each calling gnutls_global_init() at the same time. Which I suppose is OKish.
https://bugzilla.redhat.com/show_bug.cgi?id=970009
--- Comment #18 from David Woodhouse dwmw2@infradead.org --- [!]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. Note: rm -rf %{buildroot} present but not required
[!]: update-desktop-database is invoked when required Note: desktop file(s) in stoken-gui
[!]: Package installs a %{name}.desktop using desktop-file-install if there is such a file.
[!]: Sources can be downloaded from URI in Source: tag Note: Could not download Source0
https://bugzilla.redhat.com/show_bug.cgi?id=970009
--- Comment #19 from Simone Caronni negativo17@gmail.com --- (In reply to David Woodhouse from comment #18)
[!]: Package do,es not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. Note: rm -rf %{buildroot} present but not required
Fixed this, it is created by rpmdev-newspec even on new (rpm 4.11) templates.
[!]: update-desktop-database is invoked when required Note: desktop file(s) in stoken-gui
This is no mime type installed, so this should not be required [1].
[!]: Package installs a %{name}.desktop using desktop-file-install if there is such a file.
SPEC file already calls desktop-file-validate, as specified by the packaging guidelines desktop-file-install should be run only if the package does not install the desktop file on its own [2].
[!]: Sources can be downloaded from URI in Source: tag Note: Could not download Source0
I will revert to version 0.2, since it's released and the changes introduced still do not allow us to avoid libtomcrypt. I don't see any benefit now for using version 0.3; unless it's released before the end of the review.
I would also have to use the SourceURL for github prereleases according to the guidelines [3] which I would rather avoid.
[1] http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#desktop-database
[2] http://fedoraproject.org/wiki/Packaging:Guidelines#desktop-file-install_usag...
[3] https://fedoraproject.org/wiki/Packaging:SourceURL#Github
https://bugzilla.redhat.com/show_bug.cgi?id=970009
--- Comment #20 from Simone Caronni negativo17@gmail.com --- Here is the updated package. Version is 0.2, I updated the changelog accordingly.
No rpath sed hacks, proper CFLAGS. Requires proper libtomcrypt version.
Spec URL: http://slaanesh.fedorapeople.org/stoken.spec SRPM URL: http://slaanesh.fedorapeople.org/stoken-0.2-3.fc19.src.rpm
https://bugzilla.redhat.com/show_bug.cgi?id=970009
--- Comment #21 from Kevin Cernekee cernekee@gmail.com --- (In reply to Simone Caronni from comment #13)
Unfortunately, the packaging guidelines forbid the inclusion of bundled libraries in the code; so at least in the Fedora case we have to use the libtomcrypt packages
If you do manage to revive the libtomcrypt package, it might be a good idea to see if dropbear (which currently bundles libtomcrypt) can use it. If dropbear is set up as an internet-facing sshd, it could be vulnerable to library problems like this:
https://github.com/libtom/libtomcrypt/commit/2cd666f2849d62b11469fc876f51e07...
or the various side-channel attacks that people keep finding in crypto libraries.
Actually I preferred the GnuTLS patch, including the 5 files from libtomcrypt makes things more complicated when packaging.
OK - I don't have a strong opinion one way or the other. The "5 files" code was experimental and I can just kill off that branch.
(In reply to David Woodhouse from comment #14)
If run on a machine with the Intel AES-NI instructions, does this make use of them?
I don't see any architecture-specific optimizations in libtomcrypt.
It would be nice to have AES-NI support but from a practical standpoint it probably doesn't matter much for such an infrequent calculation.
https://bugzilla.redhat.com/show_bug.cgi?id=970009
--- Comment #22 from Kevin Cernekee cernekee@gmail.com --- BTW - related question about x86 crypto features: what is the preferred way to utilize the Ivy Bridge RDRAND instruction from an application?
Should the GnuTLS/libtomcrypt/OpenSSL/... library authors assume that the user is already running rngd in the background to seed the kernel's /dev/random pool? Or would it be better to integrate RDRAND support right into the library?
Fedora's rng-tools package is up to date, but Debian and Ubuntu are using an old fork which lacks RDRAND support.
https://bugzilla.redhat.com/show_bug.cgi?id=970009
--- Comment #23 from David Woodhouse dwmw2@infradead.org --- Hm.
For an application the question is a bit simpler. If the app needs to generate random numbers often and fast, then using rdrand directly is the way to go. Otherwise, just use the library and don't worry about it.
However, it's a bit harder for the library. I suppose it wants to be optional, and the distribution packager needs to do the right thing. Or perhaps this is just another case of Debian and Ubuntu being a bit behind the curve. We update the libraries to assume that rngd is doing it, and when we finally have current software in .deb form it'll all work out OK.
Getting back vaguely on-topic... it occurs to me that stoken could probably just open /dev/random and read from it to get a random number. You don't do this often, right? It's only for 'stoken import --random'? And thus it is /dev/random not /dev/urandom that you need?
In that case I could have just used nettle instead of gnutls, and not had to worry about the library initialisation issues.
https://bugzilla.redhat.com/show_bug.cgi?id=970009
--- Comment #24 from Simone Caronni negativo17@gmail.com --- (In reply to Kevin Cernekee from comment #21)
(In reply to Simone Caronni from comment #13) If you do manage to revive the libtomcrypt package, it might be a good idea to see if dropbear (which currently bundles libtomcrypt) can use it.
True, I will file a bug on it.
Actually I preferred the GnuTLS patch, including the 5 files from libtomcrypt makes things more complicated when packaging.
OK - I don't have a strong opinion one way or the other. The "5 files" code was experimental and I can just kill off that branch.
Actually libtomcrypt is used now only by 1 package (2 if we count dropbear not using the bundled library):
# repoquery --whatrequires "libtomcrypt.so.0()(64bit)" libtomcrypt-devel-0:1.17-14.fc18.x86_64 libtomcrypt-devel-0:1.17-15.fc18.x86_64 olpc-os-builder-0:6.0.0-1.fc19.x86_64
https://bugzilla.redhat.com/show_bug.cgi?id=970009
David Woodhouse dwmw2@infradead.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Assignee|nobody@fedoraproject.org |dwmw2@infradead.org Flags| |fedora-review+
--- Comment #25 from David Woodhouse dwmw2@infradead.org ---
Package Review ==============
Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed
===== MUST items =====
C/C++: [x]: Package does not contain kernel modules. [x]: Package contains no static executables. [x]: Header files in -devel subpackage, if present. [x]: ldconfig called in %post and %postun if required. [x]: Package does not contain any libtool archives (.la) [x]: Rpath absent or only used for internal libs. [x]: Development (unversioned) .so files in -devel subpackage, if present.
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. [x]: %build honors applicable compiler flags or justifies otherwise. [x]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [x]: Development files must be in a -devel package [x]: Package requires other packages for directories it uses. [x]: Package uses nothing in %doc for runtime. [x]: Package is not known to require ExcludeArch. [x]: Fully versioned dependency in subpackages, if present. Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in stoken- devel , stoken-libs , stoken-cli , stoken-gui [x]: Package complies to the Packaging Guidelines [x]: License field in the package spec file matches the actual license. [x]: License file installed when any subpackage combination is installed. [x]: Package consistently uses macro is (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. [x]: If the package is a rename of another package, proper Obsoletes and Provides are present. [x]: Package must own all directories that it creates. [x]: Package does not own files or directories owned by other packages. [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]: update-desktop-database is invoked when required [x]: Useful -debuginfo package or justification otherwise. [x]: Large documentation must go in a -doc subpackage. [x]: All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines. [x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. [x]: Each %files section contains %defattr if rpm < 4.4 [x]: Macros in Summary, %description expandable at SRPM build time. [x]: Package contains desktop file if it is a GUI application. [x]: Package installs a %{name}.desktop using desktop-file-install if there is such a file. [x]: Package does not contain duplicates in %files. [x]: Permissions on files are set properly. [x]: 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 %doc. [x]: Package use %makeinstall only when make install' ' DESTDIR=... doesn't work. [x]: Package is named using only allowed ASCII characters. [x]: Package do not use a name that already exist [x]: Package is not relocatable. [x]: Sources used to build the package match the upstream source, as provided in the spec URL. [x]: Spec file name must match the spec package %{name}, in the format %{name}.spec. [x]: File names are valid UTF-8. [x]: Packages must not store files under /srv, /opt or /usr/local [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.
I would prefer to see BuildRequires: pkgconfig(libtomcrypt) pkgconfig(gtk+-2.0) instead of the package names libtomcrypt-devel and gtk2-devel.
In fact, I'd much rather see pkgconfig(gtk+-3.0) and it doesn't look particularly hard, but neither of those observations make it a review failure.
Review passed; thanks for contributing to Fedora.
https://bugzilla.redhat.com/show_bug.cgi?id=970009
--- Comment #26 from Simone Caronni negativo17@gmail.com --- (In reply to David Woodhouse from comment #25)
Package Review
Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed
===== MUST items =====
C/C++: [x]: Package does not contain kernel modules. [x]: Package contains no static executables. [x]: Header files in -devel subpackage, if present. [x]: ldconfig called in %post and %postun if required. [x]: Package does not contain any libtool archives (.la) [x]: Rpath absent or only used for internal libs. [x]: Development (unversioned) .so files in -devel subpackage, if present.
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. [x]: %build honors applicable compiler flags or justifies otherwise. [x]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [x]: Development files must be in a -devel package [x]: Package requires other packages for directories it uses. [x]: Package uses nothing in %doc for runtime. [x]: Package is not known to require ExcludeArch. [x]: Fully versioned dependency in subpackages, if present. Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in stoken- devel , stoken-libs , stoken-cli , stoken-gui [x]: Package complies to the Packaging Guidelines [x]: License field in the package spec file matches the actual license. [x]: License file installed when any subpackage combination is installed. [x]: Package consistently uses macro is (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. [x]: If the package is a rename of another package, proper Obsoletes and Provides are present. [x]: Package must own all directories that it creates. [x]: Package does not own files or directories owned by other packages. [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]: update-desktop-database is invoked when required [x]: Useful -debuginfo package or justification otherwise. [x]: Large documentation must go in a -doc subpackage. [x]: All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines. [x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. [x]: Each %files section contains %defattr if rpm < 4.4 [x]: Macros in Summary, %description expandable at SRPM build time. [x]: Package contains desktop file if it is a GUI application. [x]: Package installs a %{name}.desktop using desktop-file-install if there is such a file. [x]: Package does not contain duplicates in %files. [x]: Permissions on files are set properly. [x]: 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 %doc. [x]: Package use %makeinstall only when make install' ' DESTDIR=... doesn't work. [x]: Package is named using only allowed ASCII characters. [x]: Package do not use a name that already exist [x]: Package is not relocatable. [x]: Sources used to build the package match the upstream source, as provided in the spec URL. [x]: Spec file name must match the spec package %{name}, in the format %{name}.spec. [x]: File names are valid UTF-8. [x]: Packages must not store files under /srv, /opt or /usr/local [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.
I would prefer to see BuildRequires: pkgconfig(libtomcrypt) pkgconfig(gtk+-2.0) instead of the package names libtomcrypt-devel and gtk2-devel.
Done.
In fact, I'd much rather see pkgconfig(gtk+-3.0) and it doesn't look particularly hard, but neither of those observations make it a review failure.
Looking into it, is not my domain of expertise and could be good for learning on something simple like this gui.
I will post something here before asking for fedora-cvs?.
Review passed; thanks for contributing to Fedora.
Thank you very much as well for all your contributions :)
https://bugzilla.redhat.com/show_bug.cgi?id=970009
--- Comment #27 from David Woodhouse dwmw2@infradead.org --- (In reply to Simone Caronni from comment #26)
In fact, I'd much rather see pkgconfig(gtk+-3.0) and it doesn't look particularly hard, but neither of those observations make it a review failure.
Looking into it, is not my domain of expertise and could be good for learning on something simple like this gui.
I will post something here before asking for fedora-cvs?.
No, go ahead and ship it as-is, matching the upstream release. We can update to gtk3 later, when there's an upstream release that does so.
FWIW I think there's only one build *failure*; the rest is just warnings about using deprecated gtk functions that do still work anyway.
https://bugzilla.redhat.com/show_bug.cgi?id=970009
--- Comment #28 from Simone Caronni negativo17@gmail.com --- Spec URL: http://slaanesh.fedorapeople.org/stoken.spec SRPM URL: http://slaanesh.fedorapeople.org/stoken-0.2-4.fc19.src.rpm
- Change gtk and libtomcrypt build requirements (pkgconfig). - Remove useless "--with-libtomcrypt" parameter from %configure.
https://bugzilla.redhat.com/show_bug.cgi?id=970009
Simone Caronni negativo17@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-cvs?
--- Comment #29 from Simone Caronni negativo17@gmail.com --- New Package SCM Request ======================= Package Name: stoken Short Description: Token code generator compatible with RSA SecurID 128-bit (AES) token Owners: slaanesh Branches: f18 f19 el6 InitialCC:
https://bugzilla.redhat.com/show_bug.cgi?id=970009
Jon Ciesla limburgher@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-cvs? | Flags| |fedora-cvs+
https://bugzilla.redhat.com/show_bug.cgi?id=970009
--- Comment #30 from Jon Ciesla limburgher@gmail.com --- Git done (by process-git-requests).
https://bugzilla.redhat.com/show_bug.cgi?id=970009
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |MODIFIED
https://bugzilla.redhat.com/show_bug.cgi?id=970009
--- Comment #31 from Fedora Update System updates@fedoraproject.org --- stoken-0.2-4.fc18 has been submitted as an update for Fedora 18. https://admin.fedoraproject.org/updates/stoken-0.2-4.fc18
https://bugzilla.redhat.com/show_bug.cgi?id=970009
--- Comment #32 from Fedora Update System updates@fedoraproject.org --- stoken-0.2-4.fc19 has been submitted as an update for Fedora 19. https://admin.fedoraproject.org/updates/stoken-0.2-4.fc19
https://bugzilla.redhat.com/show_bug.cgi?id=970009
Simone Caronni negativo17@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-cvs+ | Flags| |fedora-cvs?
--- Comment #33 from Simone Caronni negativo17@gmail.com --- Package Change Request ====================== Package Name: stoken New Branches: f17 f18 Owners: slaanesh
https://bugzilla.redhat.com/show_bug.cgi?id=970009
--- Comment #34 from Simone Caronni negativo17@gmail.com --- Package Change Request ====================== Package Name: stoken New Branches: f17 Owners: slaanesh
https://bugzilla.redhat.com/show_bug.cgi?id=970009
Jon Ciesla limburgher@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-cvs? | Flags| |fedora-cvs+
https://bugzilla.redhat.com/show_bug.cgi?id=970009
--- Comment #35 from Jon Ciesla limburgher@gmail.com --- Git done (by process-git-requests).
https://bugzilla.redhat.com/show_bug.cgi?id=970009
--- Comment #36 from Fedora Update System updates@fedoraproject.org --- stoken-0.2-4.fc17 has been submitted as an update for Fedora 17. https://admin.fedoraproject.org/updates/stoken-0.2-4.fc17
https://bugzilla.redhat.com/show_bug.cgi?id=970009
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |ON_QA
--- Comment #37 from Fedora Update System updates@fedoraproject.org --- stoken-0.2-4.fc17 has been pushed to the Fedora 17 testing repository.
https://bugzilla.redhat.com/show_bug.cgi?id=970009
--- Comment #38 from Kevin Cernekee cernekee@gmail.com --- (In reply to David Woodhouse from comment #23)
For an application the question is a bit simpler. If the app needs to generate random numbers often and fast, then using rdrand directly is the way to go. Otherwise, just use the library and don't worry about it.
However, it's a bit harder for the library. I suppose it wants to be optional, and the distribution packager needs to do the right thing. Or perhaps this is just another case of Debian and Ubuntu being a bit behind the curve. We update the libraries to assume that rngd is doing it, and when we finally have current software in .deb form it'll all work out OK.
Well, this bothered me enough to try fixing the Debian rng-tools package:
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=692450
Getting back vaguely on-topic... it occurs to me that stoken could probably just open /dev/random and read from it to get a random number. You don't do this often, right? It's only for 'stoken import --random'? And thus it is /dev/random not /dev/urandom that you need?
/dev/random is really slow if there's not a lot of entropy saved up - to the point where "stoken-gui --random" lags for a minute or two unless I ask the user to furiously move his mouse around. Either way makes for a substandard user experience.
The RNG is used for two purposes in stoken:
1) Generating random tokens - per the man page this is for novelty purposes only. We can't convert our ctf seeds back into sdtid seeds yet, and even if we could, we can't generate the RSA signature needed to make the official SecurID server believe they are authentic. Maybe someday libstoken will be able to help perform service-side authentication, but we're not there yet so the random tokens are fairly useless except for testing.
2) Creating a random IV to encrypt the PIN, if the user elects to encrypt his seed + PIN in ~/.stokenrc.
At this point /dev/urandom is probably good enough for both use cases, and that is what libtomcrypt uses (at least on Debian).
In fact, I'd much rather see pkgconfig(gtk+-3.0) and it doesn't look particularly hard, but neither of those observations make it a review failure.
Will revisit this in the next few months - also planning to switch over to GtkBuilder and add a basic "import token" dialog to the GUI.
https://bugzilla.redhat.com/show_bug.cgi?id=970009
--- Comment #39 from Fedora Update System updates@fedoraproject.org --- stoken-0.2-4.fc19 has been pushed to the Fedora 19 stable repository.
https://bugzilla.redhat.com/show_bug.cgi?id=970009
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ON_QA |CLOSED Fixed In Version| |stoken-0.2-4.fc17 Resolution|--- |ERRATA Last Closed| |2013-06-16 01:32:40
--- Comment #40 from Fedora Update System updates@fedoraproject.org --- stoken-0.2-4.fc17 has been pushed to the Fedora 17 stable repository.
https://bugzilla.redhat.com/show_bug.cgi?id=970009
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Fixed In Version|stoken-0.2-4.fc17 |stoken-0.2-4.fc18
--- Comment #41 from Fedora Update System updates@fedoraproject.org --- stoken-0.2-4.fc18 has been pushed to the Fedora 18 stable repository.
https://bugzilla.redhat.com/show_bug.cgi?id=970009
Christopher Meng cickumqt@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |cickumqt@gmail.com
--- Comment #42 from Christopher Meng cickumqt@gmail.com --- I'm cleaning old emails and found this.
Seems dropbear has big issues, I'll handle this.
package-review@lists.fedoraproject.org
-
Red Hat Bugzilla