Bug 596138 - Review Request: nss-gui - A graphical user interface for NSS security databases
Summary: Review Request: nss-gui - A graphical user interface for NSS security databases
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jason Tibbitts
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 466626
TreeView+ depends on / blocked
 
Reported: 2010-05-26 11:55 UTC by Kai Engert (:kaie) (inactive account)
Modified: 2011-02-23 01:32 UTC (History)
5 users (show)

Fixed In Version: nss-gui-0.3.9-1.el6
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-02-09 18:24:56 UTC
Type: ---
Embargoed:
j: fedora-review+
notting: fedora-cvs+


Attachments (Terms of Use)

Description Kai Engert (:kaie) (inactive account) 2010-05-26 11:55:41 UTC
Spec URL: http://kuix.de/fedora/nss-gui/nss-gui.spec
SRPM URL: http://kuix.de/fedora/nss-gui/nss-gui-0.3.3-1.fc14.src.rpm
Description: Graphical user interface to manage the contents of an NSS
(Network Security Services) database, including registered CRLs and
registered security devices (PKCS#11). Based on Mozilla code.

Comment 1 Kai Engert (:kaie) (inactive account) 2010-05-26 15:39:12 UTC
Updated spec:

URL:              https://fedorahosted.org/nss-gui/

Comment 2 Bill Nottingham 2010-05-26 21:40:41 UTC
What's the usage case for this that isn't solved by just going through the Firefox (/thunderbird/xulrunner) dialogs directly?

Comment 3 Kai Engert (:kaie) (inactive account) 2010-05-26 21:48:41 UTC
(In reply to comment #2)
> What's the usage case for this that isn't solved by just going through the
> Firefox (/thunderbird/xulrunner) dialogs directly?    

Firefox will always edit the database associated with a profile,
but NSS-GUI can

- be used to edit the global systemwide NSS database located at
  /etc/pki/nssdb

- be used to edit a database located at a specific location, 
  which e.g. might be used for any other NSS based application,
  maybe related to some CA tool?

- people have asked for such a tool, see bug 466626

Comment 4 Bill Nottingham 2010-05-26 22:05:05 UTC
OK.

Some non-packaging comments:
- This tool is *extremely* jargon-heavy. I'm not sure how you'd explain it to someone not familiar with all the concepts already.
- Having a configuration tool that will require rebuilds at every xulrunner rebase is not nice.
- I don't see any real obvious use for the 'error console' and 'about:config' dialogs. Certainly 'about:config' seems superfluous; is there a reason that I need to be able to change nss-gui's cookie preferences?

Comment 5 Bill Nottingham 2010-05-26 22:05:40 UTC
(This also means that nss-gui, on startup, makes its own places sqlite database, and other things that are completely unrelated to what it actually needs to do.)

Comment 6 Kai Engert (:kaie) (inactive account) 2010-05-27 08:27:56 UTC
(In reply to comment #4)
> 
> Some non-packaging comments:
> - Having a configuration tool that will require rebuilds at every xulrunner
> rebase is not nice.

In 99% of xulrunner rebases it will NOT require a rebuild :)

There is no binary dependency between xulrunner and nss-gui, the integration works via dynamic runtime hooks. nss-gui asks to bring up dialogs by their internal URL-like addresses, and uses xml-like instructions to modify the contents of the UI.

Only on major version upgrades we'll need to test whether the UI hooks need to be adjusted, and the required changes are going to be small.


> - This tool is *extremely* jargon-heavy. I'm not sure how you'd explain it to
> someone not familiar with all the concepts already.

Agreed, but do you think this is different from the UI shown at the respective level of Firefox preferences?

I guess we don't need to make this tool a default install, so only people who need it will use it.


> - I don't see any real obvious use for the 'error console' and 'about:config'
> dialogs. Certainly 'about:config' seems superfluous; is there a reason that I
> need to be able to change nss-gui's cookie preferences?    

Yeah, these are not really necessary. I used them during development and testing.

I could use a xulrunner-level preference to switch those debug helpers on and off, make them off by default, offer a --debug switch for the nss-gui startup wrapper script, and have the wrapper script modify the pref before nss-gui xulrunner startup.

Comment 7 Kai Engert (:kaie) (inactive account) 2010-05-27 08:42:07 UTC
(In reply to comment #5)
> (This also means that nss-gui, on startup, makes its own places sqlite
> database, and other things that are completely unrelated to what it actually
> needs to do.)    

Yes, that's a small disadvantage, but the advantage is, it's possible to provide this tool with very little engineering work, the nss-gui package is very minimal in size (just some KB), and only people who actually need the nss-gui package will get the profile files.

Comment 8 Kai Engert (:kaie) (inactive account) 2010-07-31 20:23:25 UTC
No feedback within the last 2 months, did we miss Fedora 14 inclusion?

If I understand correctly, the complaint's were about properties of the implementation of the nss-gui tool, but is that a blocker for Fedora inclusion?

If I understand correctly, this bug is about reviewing the packaging, not the software itself?

Comment 9 Jason Tibbitts 2011-01-05 21:18:30 UTC
Just noticed this when going over old review tickets.  Perhaps I can answer a few questions:

The package can still be included in the Fedora 14 repositories; we won't stop that for quite some time yet.

Reviews are mostly about packaging, but the process does not entirely bypass all consideration of the software itself.  For example, it is important that it actually work and not open up massive security holes.

Even though it's a small package, it still needs a bit of work.  Did you ever run rpmlint over the srpm and resulting binary rpms?  It's expected that you do so.

  nss-gui.x86_64: W: no-documentation
  nss-gui.x86_64: W: no-manual-page-for-binary nss-gui
It might be nice to have some documentation.  It would also be nice to have some indication of the license.

  nss-gui-debuginfo.x86_64: E: debuginfo-without-sources
This generally indicates a build problem and indeed it seems that the compiler is not being called with the proper flags.  You'll need to figure out how to pass %optflags or $RPM_OPT_FLAGS.  Looks like this will involve Makefile patching as the included makefile doesn't seem to pay attention to anything.

The use of macros in the spec is inconsistent.  Generally you should just not use %{__rm} and such unless for some reason you really like pointless typing, but if you really do want to use those macros then you also need to use %{__cp} and %{__make} and such.

The description could use a bit less cryptic, I think.  If this is end-user software then some additional explanation is warranted.  Perhaps that's why you haven't attracted any reviewers; they can't figure out what it's supposed to do.

The software doesn't seem to work for me.  I built in mock for x86_64 rawhide and installed it on a rawhide VM; nss-gui fails to start:

Error: Platform version '2.0b7' is not compatible with
minVersion >= 1.9
maxVersion <= 1.9.2.*

At the least that indicates that some dependencies are off.

Comment 10 Kai Engert (:kaie) (inactive account) 2011-01-13 15:46:11 UTC
I don't agree that I'm a stalled submitter :)
Your comments came just one week ago, after I had waited more than 6 months for reviewer comments...

Comment 11 Jason Tibbitts 2011-01-13 15:51:51 UTC
That's how it works.  The tag will just be re-added if you don't respond to the commentary.

I'm sorry your package sat for so long; complaining to me about it when I'm the only person actually moving things forward doesn't make a whole lot of sense.

Comment 12 Kai Engert (:kaie) (inactive account) 2011-01-13 15:59:56 UTC
Jason, I'm very thankful for your review comments indeed. Not complaining at all, just saying I haven't yet switched back to this task, after having had to wait for it a long time.

I will address your comments shortly. Thanks again.

Comment 13 Kai Engert (:kaie) (inactive account) 2011-01-13 19:52:41 UTC
I think I adressed all your comments (besides the one that the purpose of the package is cryptic).

New spec uploaded to
http://kuix.de/fedora/nss-gui/nss-gui.spec

New SRPM uploaded to
http://kuix.de/fedora/nss-gui/nss-gui-0.3.5-1.fc14.src.rpm


Today I wrote a manpage, added a lincese file, fixed some dependencies, and made a new release.

I noticed a missing dependency, added:
    Requires:         boost-program-options

Also, I added the required dependencies necessary to build a manpage using the
AsciiDoc tool.


More replies inline:


> Even though it's a small package, it still needs a bit of work.  Did you ever
> run rpmlint over the srpm

    $ rpmlint SRPMS/nss-gui-0.3.4-1.fc14.src.rpm
    nss-gui.src: W: invalid-url Source0: nss-gui-0.3.4.tar.bz2
    1 packages and 0 specfiles checked; 0 errors, 1 warnings.

I've now changed Source0 to a real URL pointing to the released tarball.
The warning is gone.


> and resulting binary rpms?  It's expected that you do so.
>   nss-gui.x86_64: W: no-documentation
>   nss-gui.x86_64: W: no-manual-page-for-binary nss-gui

Thank your for reminding me.
Now I did, and I fixed the resulting mistakes.
Result for the new version is:

    $ rpmlint RPMS/i686/nss-gui-0.3.4-1.fc14.i686.rpm
    1 packages and 0 specfiles checked; 0 errors, 0 warnings.


> It might be nice to have some documentation.  It would also be nice to have
> some indication of the license.

The header in the spec file indicates that it's triple license as usually used
for Mozilla software: MPL/GPL/LGPL.

I added a LICENSE file that contains the MPL 1.3 license text.


>   nss-gui-debuginfo.x86_64: E: debuginfo-without-sources
> This generally indicates a build problem and indeed it seems that the compiler
> is not being called with the proper flags.  You'll need to figure out how to
> pass %optflags or $RPM_OPT_FLAGS.  Looks like this will involve Makefile
> patching as the included makefile doesn't seem to pay attention to anything.

Still using a minimal Makefile, but this is now fixed:

    $ rpmlint nss-gui-debuginfo-0.3.5.beta1-1.fc14.i686.rpm 
    1 packages and 0 specfiles checked; 0 errors, 0 warnings.


> The use of macros in the spec is inconsistent.  Generally you should just not
> use %{__rm} and such unless for some reason you really like pointless typing,
> but if you really do want to use those macros then you also need to use %{__cp}
> and %{__make} and such.

Fixed. No longer using macros for rm, install, mkdir.


> The description could use a bit less cryptic, I think.  If this is end-user
> software then some additional explanation is warranted.  Perhaps that's why you
> haven't attracted any reviewers; they can't figure out what it's supposed to
> do.

I can't do much about this one.
As of today, most users will not run it.

In the future, this app (or a replacement) might turn into the user's
central application for managing the user's private keys and certificates,
the CA certificates the user trusts, etc.

However, we're not there yet.
Today, users manage their data separately in each application, for example using the certificate manager available in Firefox or Thunderbird.

Before we can make this claim (this tool is universal), all applications
will have to change to using the shared NSS database for crypto objects.

Today this software is mostly for security experts and administrators.
However, I know of corporate deployments that are waiting for this tool to 
become available.

We need to make a first step.


> The software doesn't seem to work for me.  I built in mock for x86_64 rawhide
> and installed it on a rawhide VM; nss-gui fails to start:
> 
> Error: Platform version '2.0b7' is not compatible with
> minVersion >= 1.9
> maxVersion <= 1.9.2.*
> 
> At the least that indicates that some dependencies are off.

Sorry, it had bitrotted since I had submitted it.

Rawhide uses a newer XULRunner runtime environment.
It's still compatible, though.
I've updated the compatibility version, which should work.

I will find a rawhide machine to get this version string tested.

Comment 14 Jason Tibbitts 2011-01-14 17:39:55 UTC
The package indeed does look much better now; rpmlint is silent.  I built and installed it on my rawhide vm and while it doesn't bail with complaints about the version, it doesn't actually do anything.  strace shows that it's just timing out on a poll.  Occasionally it will access something, but it never opens a window.

Comment 15 Kai Engert (:kaie) (inactive account) 2011-01-14 23:20:44 UTC
Ok, thanks a lot for giving it a try. The package works fine on my F14 machine. I'll install rawhide and see what's failing.

Comment 16 Kai Engert (:kaie) (inactive account) 2011-01-18 12:16:50 UTC
Problems solved.

Rawhide/XulRunner 2.0 required a compatibility update of this software. Done now.

After you build and attempt to run nss-gui without parameters, you will get an error message that its default database does not yet exist.

This error is expected, because you most likely don't use other software yet that uses that path.

You can follow the instructions and create the given path.
In other words, do this:
  mkdir -p $HOME/.pki/nssdb

New files that have been tested on rawhide:
  http://kuix.de/fedora/nss-gui/nss-gui.spec
  http://kuix.de/fedora/nss-gui/nss-gui-0.3.6-1.fc14.src.rpm

FYI, when you run the software, you will be shown user interface that is similar to what's being offered by Firefox in advanced encryption preferences.

Thanks again for your help.

Comment 17 Jason Tibbitts 2011-01-27 17:35:20 UTC
Sorry for the wait; my time is limited and there are very many package review tickets that need attention.

Indeed, the 0.3.6 version starts in rawhide and seems to function well enough.

I note that there's only the MPL license text included.  It's not essential, but generally in that situation I'd ask you to ask upstream to consider adding the other valid license texts as well (GPL and LGPL).  Since you're upstream, though, I guess that skips a step.

I assume you want to push this to el5, since it appears to have a sufficiently new xulrunner.  If not, however, you can remove BuildRoot, %clean and the first line of %install.

It is not necessary for you to manually specify a dependency on boost-program-options; rpm finds that out on its own.

This needs a .desktop file.  http://fedoraproject.org/wiki/Packaging:Guidelines#Desktop_files


* source files match upstream.  sha256sum:
  907e3b93d17470fc0625376057e6b88ffd5213eea76aa3fb19820c9b03059ee4
   nss-gui-0.3.6.tar.bz2
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.
* description is OK.
* dist tag is present.
* license field matches the actual license.
* license is open source-compatible.
* license text included in package (though not all license texts are available).
* latest version is being packaged.
* BuildRequires are proper.
* compiler flags are appropriate.
* package builds in mock (rawhide, x86_64).
* package installs properly.
* debuginfo package looks complete.
* rpmlint is silent.
X final provides and requires:
   nss-gui = 0.3.6-1.fc15
   nss-gui(x86-64) = 0.3.6-1.fc15
  =
X  boost-program-options (unnecessary)
   libboost_program_options.so.1.44.0()(64bit)  
   libgcc_s.so.1()(64bit)  
   libgcc_s.so.1(GCC_3.0)(64bit)  
   libstdc++.so.6()(64bit)  
   libstdc++.so.6(CXXABI_1.3)(64bit)  
   libstdc++.so.6(GLIBCXX_3.4)(64bit)  
   xulrunner >= 1.9.2.2

* no bundled libraries.
* no shared libraries are added to the regular linker search paths.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no generically named files.
* code, not content.
* documentation is small, so no -doc subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* no headers.
* no pkgconfig files.
* no static libraries.
* no libtool .la files.
X This is a GUI application, but the .desktop file is missing.

Comment 18 Kai Engert (:kaie) (inactive account) 2011-01-31 19:42:04 UTC
Jason, thank you very much for your time, I highly appreciate it.

(In reply to comment #17)
> Indeed, the 0.3.6 version starts in rawhide and seems to function well enough.

glad to hear


> I note that there's only the MPL license text included.

I updated the LICENSE file to include a header, followed by all 3 license texts.
Released as 0.3.7


> I assume you want to push this to el5, since it appears to have a sufficiently
> new xulrunner.

Yes


> If not, however, you can remove BuildRoot, %clean and the first
> line of %install.

I've added reminder comments to the specfile, so this cleanup can be done later, after the RHEL 5 package has been branched.


> It is not necessary for you to manually specify a dependency on
> boost-program-options; rpm finds that out on its own.

Ok, thanks, removed.


> This needs a .desktop file. 
> http://fedoraproject.org/wiki/Packaging:Guidelines#Desktop_files

Ok. Added an icon and a desktop file.


New spec file
http://kuix.de/fedora/nss-gui/nss-gui.spec

New SRPM
http://kuix.de/fedora/nss-gui/nss-gui-0.3.7-1.fc14.src.rpm


I believe I have addressed all of your requests.
Thanks

Comment 19 Jason Tibbitts 2011-02-04 18:37:13 UTC
Sorry for taking so long; I have been sick for the past several days.

I believe everything looks fine, though I will admit to not installing and running this since I'm not in the office today.  Can't imagine that the changes would have broken anything, though.

APPROVED

Comment 20 Kai Engert (:kaie) (inactive account) 2011-02-08 13:03:44 UTC
(In reply to comment #19)
> Sorry for taking so long; I have been sick for the past several days.

Hope you're recovering quickly

> APPROVED

Thank you very much

Comment 21 Kai Engert (:kaie) (inactive account) 2011-02-08 13:49:45 UTC
New Package SCM Request
=======================
Package Name: nss-gui
Short Description: GUI to manage contents of an NSS database
Owners: kengert
Branches: f14 el6
InitialCC:

Comment 22 Bill Nottingham 2011-02-08 20:02:32 UTC
Git done (by process-git-requests).

Comment 23 Kai Engert (:kaie) (inactive account) 2011-02-09 18:24:56 UTC
I was able to commit and build.
Thanks again.

Closing.

Comment 24 Fedora Update System 2011-02-10 16:21:47 UTC
nss-gui-0.3.8-1.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/nss-gui-0.3.8-1.fc14

Comment 25 Fedora Update System 2011-02-10 16:24:49 UTC
nss-gui-0.3.8-1.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/nss-gui-0.3.8-1.el6

Comment 26 Fedora Update System 2011-02-10 21:23:29 UTC
nss-gui-0.3.8-1.fc14 has been pushed to the Fedora 14 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 27 Fedora Update System 2011-02-15 18:10:33 UTC
nss-gui-0.3.9-1.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/nss-gui-0.3.9-1.el6

Comment 28 Fedora Update System 2011-02-23 01:32:10 UTC
nss-gui-0.3.9-1.el6 has been pushed to the Fedora EPEL 6 stable repository.  If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.