Bug 247482 - Review Request: pwsafe - A unix command line utility that manages encrypted password databases
Review Request: pwsafe - A unix command line utility that manages encrypted p...
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jochen Schmitt
Fedora Extras Quality Assurance
: Reopened
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-07-09 11:49 EDT by Ralf Ertzinger
Modified: 2009-06-28 16:19 EDT (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-06-28 16:19:21 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
jochen: fedora‑review+
tibbs: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Ralf Ertzinger 2007-07-09 11:49:13 EDT
Spec URL: http://www.skytale.net/files/pwsafe/pwsafe.spec
SRPM URL: http://www.skytale.net/files/pwsafe/pwsafe-0.2.0-0.1.sky.src.rpm
Description:
pwsafe is a unix commandline program that manages encrypted password databases.
Comaptible with CounterPane's PasswordSafe Win32 program versions 2.x and 1.x.
Comment 1 Jochen Schmitt 2007-07-09 14:01:49 EDT
Good:
+ Packing Naming is ok.
+ Name of the SPEC File matches the package name
+ Package hast consitant macor usage.
+ Packsge contains %{?dist} tag
+ SPEC start to use GPL as an license
+ License is an open source license
+ License Tag matches with license test in the package
+ Package contains verbatim copy of the license text
+ Spec file is written in English
+ Tar ball matches with upstream
  (md5sum: 4bb36538a2772ecbf1a542bc7d4746c0)
+ Package has correct buildroot
+ package doesn't contains subpackages.
+ Buildroot will cleaned on %clean section an on the beginning of the %install
section
+ Package conatins proper* %defattr and file permissions
+ %doc section is small
* %doc section doen't affect runtime
+ Package doesn't contains duplicates file in the %file section
+ Package doesn't contains files or directories owned by ohter packages
+ Local build works fine
+ Rpmlint quite on Source rpm.
+ Rpmlint quite on Debuginfo rpm.
+ Local install and uninstall works fine
+ Mock build works fine for Devel and F-7 (x86_64, i386, PPC64, PPC)

Bad:
- Rpmlint has complaints for the binary package:
pmlint pwsafe-0.2.0-0.1.fc7.x86_64.rpm
W: pwsafe file-not-utf8 /usr/share/man/man1/pwsafe.1.gz
W: pwsafe incoherent-version-in-changelog 0.2.0-0.1.fc8 0.2.0-0.1.fc7
- When I call 'pwsafe --createdb' I will got the following message:
WARNING: pwsafe unable to use secure ram (need to be setuid root)









Comment 2 Ralf Ertzinger 2007-07-10 03:42:46 EDT
(In reply to comment #1)

> W: pwsafe file-not-utf8 /usr/share/man/man1/pwsafe.1.gz

Will fix.

> W: pwsafe incoherent-version-in-changelog 0.2.0-0.1.fc8 0.2.0-0.1.fc7

This will automagically go away when the package is built for rawhide.

> - When I call 'pwsafe --createdb' I will got the following message:
> WARNING: pwsafe unable to use secure ram (need to be setuid root)

Since I have not audited the code I'll not enable suid root for it.
Comment 4 Jochen Schmitt 2007-07-11 10:46:35 EDT
The UTF-8 issue seems to be fixed.

But I disagree agains you opion about the changelog entry.

You should write:

%changelog
* Sun Jul 08 2007 Ralf Ertzinger <ralf@skytale.net> 0.2.0-0.2
- Initial build for Fedora

Ths dist tag should not appear in the changelog entry.

As far as I understand the warning, I have complainted, the program try to use
an unswappable chunk of memory to store the password. It will be nice, if you
can examinate this, but his will not be a blocker for approving your package.
Comment 5 Ralf Ertzinger 2007-07-11 12:18:44 EDT
Leaving the dist tag out of the changelog entry does not make rpmlint happy,
either. It will complain as long as the E-V-R of the package and the E-V-R of
the latest changelog entry do not match.
As I said, the final build will not have this problem since both the package
E-V-R and the changelog E-V-R will be 0.2.0-1.fc8 (for the rawhide build)

The secure ram warning is about mlock() (marking a part of memory unswappable).
mlock() requires root privileges. This seems to be used all over the place in
pwsafe, and I do not feel comfortable at all about letting this effectively run
as root.
Comment 6 Jochen Schmitt 2007-07-11 12:46:22 EDT
(In reply to comment #5)
> Leaving the dist tag out of the changelog entry does not make rpmlint happy,
> either. It will complain as long as the E-V-R of the package and the E-V-R of
> the latest changelog entry do not match.

I have to disagree with your. For demonstration please look at:

http://www.herr-schmitt.de/pub/pwsafe/pwsafe-0.2.0-0.2.fc7.src.rpm

You will find out, that this source rpm makes rpmlint happy.
 
> The secure ram warning is about mlock() (marking a part of memory unswappable).
> mlock() requires root privileges. This seems to be used all over the place in
> pwsafe, and I do not feel comfortable at all about letting this effectively run
> as root.

That is ok. It may be nice, if you can contact the upstream to make your
improvements.
Comment 7 Ralf Ertzinger 2007-07-12 16:42:46 EDT
The changelog stuff is not really important, I'll drop the disttag.
Comment 8 Jochen Schmitt 2007-07-16 10:56:36 EDT
OK, If you have dropt the disttag from your package, I can APPROVE your package.
Comment 9 Ralf Ertzinger 2007-07-17 08:20:05 EDT
The disttag will be dropped in the final import.

Thank you for the review.
Comment 10 Ralf Ertzinger 2007-07-18 16:58:09 EDT
New Package CVS Request
=======================
Package Name: pwsafe
Short Description: A unix command line utility that manages encrypted password
databases
Owners: redhat-bugzilla@camperquake.de
Branches: F-7
InitialCC: 
Comment 11 Kevin Fenzi 2007-07-19 16:10:32 EDT
cvs done.
Comment 12 Jochen Schmitt 2007-08-14 14:48:15 EDT
I close this bug. But you should enter the bug number of the review on the
update process in bodhi.
Comment 13 Jonathan Steffan 2009-03-30 12:37:21 EDT
Package Change Request
======================
Package Name: pwsafe
New Branches: EL-4 EL-5
Comment 14 Dennis Gilmore 2009-04-01 12:35:48 EDT
CVS request denied.
The package maintainer for fedora should be making the request. 
if the maintainer doesnt wish to do EPEL then we need some indication that is the case  and then you need a full request as you would be the maintainer for the EPEL branches.
Comment 15 Ralf Ertzinger 2009-04-01 12:53:38 EDT
I have no interest in maintaining an EPEL branch. Are there some formalities to observe, or is this statement sufficient?
Comment 16 Jochen Schmitt 2009-04-23 13:28:06 EDT
The is no need to contribute for EPEL, so I wondering, that you have request ones?
Comment 17 Jochen Schmitt 2009-06-03 13:20:56 EDT
I have reread the last comments again. if Ralf has no interest for maintaining the EPEL part, put Jonathan may interested to see this package in EPEL, I want to suggest, that Ralf should create a CVSAdmin request for the EPEL branches.

In this Request Ralf can enter Jonathan as the owner of the EPEL branches.
Comment 18 Jonathan Steffan 2009-06-28 14:27:49 EDT
I'd maintain it. I'll go ahead and request access to the existing branches. Go ahead and also add me as the owner of the EL branches. This program is very useful.
Comment 19 Ralf Ertzinger 2009-06-28 15:09:42 EDT
Package Change Request
======================
Package Name: pwsafe
New Branches: EL-4 EL-5
Comment 20 Jason Tibbitts 2009-06-28 15:13:36 EDT
CVS done.

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