Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 465372 - Review Request: chntpw - Change passwords in Windows SAM files
Summary: Review Request: chntpw - Change passwords in Windows SAM files
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Patrice Dumas
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: FE-Legal
TreeView+ depends on / blocked
 
Reported: 2008-10-02 22:15 UTC by Conrad Meyer
Modified: 2009-06-10 21:06 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-10-13 05:09:24 UTC
Type: ---
Embargoed:
pertusus: fedora-review+
j: fedora-cvs+


Attachments (Terms of Use)

Description Conrad Meyer 2008-10-02 22:15:22 UTC
Spec URL: http://konradm.fedorapeople.org/fedora/SPECS/chntpw.spec
SRPM URL: http://konradm.fedorapeople.org/fedora/SRPMS/chntpw-0.99.6-1.fc9.src.rpm
Description:
This is a utility to (re)set the password of any user that has a valid
(local) account on your Windows NT/2k/XP/Vista etc system. You do not
need to know the old password to set a new one. It works offline, that
is, you have to shutdown your computer and boot off a floppydisk or CD
or another system. Will detect and offer to unlock locked or disabled
out user accounts! There is also a registry editor and other registry
utilities that works under linux/unix, and can be used for other things
than password editing.

Comment 1 Patrice Dumas 2008-10-10 21:16:55 UTC
I can't see why you don't use the initial Makefile. Something along

make CC="%__cc" CFLAGS="$RPM_OPT_FLAGS" LIBS="-lcrpyto" chntpw cpnt reged

And use simple cp or install to install the resulting executables.

Using cmake while upstream doesn't seems too much to me.

openssl in requires is not useful, it is automatically found by rpm.

It is better to avoid using Fedora whenever possible, so please rename
README.Fedora to something more neutral like README.Dist

cp should be cp -p to keep timestamp.

Also I suggest using 
%{_mandir}/man8/%{name}.8*
to catch any kind of compression.

I am afraid that, if accepted, you'll need contact legal (through spot,
he is the contact) because of the crypto and export stuff.

Comment 2 Conrad Meyer 2008-10-11 00:04:41 UTC
> I can't see why you don't use the initial Makefile.

The initial Makefile seemed horribly broken enough to warrant replacement (-m32 among other things!). If it builds using correct Fedora flags with make like you suggest I don't see any problem using the original Makefile though.

> openssl in requires is not useful, it is automatically found by rpm.

Ok.

> It is better to avoid using Fedora whenever possible, so please rename
> README.Fedora to something more neutral like README.Dist
> cp should be cp -p to keep timestamp.
> Also I suggest using %{_mandir}/man8/%{name}.8* to catch any kind of 
> compression.

Ok.

> I am afraid that, if accepted, you'll need contact legal (through spot,
> he is the contact) because of the crypto and export stuff.

I'll contact him.

Comment 4 Patrice Dumas 2008-10-11 08:52:03 UTC
The files cannot be found?

Comment 5 Conrad Meyer 2008-10-11 08:58:01 UTC
Interesting. skvidal moved fedorapeople to a different machine earlier today, perhaps that had something to do with it. I'll try to upload them again.

Comment 6 Conrad Meyer 2008-10-11 09:00:33 UTC
Try it again now.

Comment 7 Patrice Dumas 2008-10-11 09:29:05 UTC
I'd prefer if the chntpw-README.Fedora was also renamed. The main aim of 
not having 'Fedora' is to be reused, so it shouldn't be anywhere:
https://fedoraproject.org/wiki/PackageMaintainers/Packaging_Tricks

The -p for keeping timestamps is notfor newly created executables, since
their timestamp is the one of the build anyway, but for the files that
may have their timestamps kept, here the man page and README file.

Just a suggestion, staticaly compiled executable should be kept, they 
don't do harm (and could even be used in-source), even though they are 
not of use in fedora.

I also suggest a comment in the spec file telling where the version comes
from since the source archive has another version string.

Comment 9 Patrice Dumas 2008-10-11 10:12:37 UTC
rpmlint says:

chntpw.i386: W: wrong-file-end-of-line-encoding /usr/share/doc/chntpw-0.99.6/WinReg.txt

I think that this should be fixed.

Otherwise everything seems ok.

Comment 11 Patrice Dumas 2008-10-12 09:32:51 UTC
* rpmlint is silent
* follow guidelines
* free software, license included
* match upstream:
09addfe7ae469677da39ed66d83858d3  chntpw-source-080526.zip
* %files section right


Just one suggestion, I think it is better to use sed for the
end of line, and to keep timestamp, like:

sed -e 's/\r$//' WinReg.txt > WinReg.txt.eol
touch -c -r WinReg.txt WinReg.txt.eol
mv WinReg.txt.eol WinReg.txt

This is only asuggestion, so

APPROVED

Comment 12 Conrad Meyer 2008-10-12 09:49:04 UTC
Your suggestions are greatly appreciated. Thanks much for the review!

New Package CVS Request
=======================
Package Name: chntpw
Short Description: Change passwords in Windows SAM files
Owners: konradm
Branches: F-9 F-10
InitialCC:

Comment 13 Kevin Fenzi 2008-10-13 01:53:11 UTC
cvs done.

Comment 14 Conrad Meyer 2008-10-13 05:09:24 UTC
http://koji.fedoraproject.org/koji/taskinfo?taskID=876482 <-- In rawhide.

Comment 15 Richard W.M. Jones 2009-06-09 09:23:45 UTC
Package Change Request
======================
Package Name: chntpw
New Branches: EL-5
Owners: rjones

Comment 16 Kevin Fenzi 2009-06-10 04:58:26 UTC
Does Conrad want to maintain this in EPEL? or doesn't care?

Comment 17 Conrad Meyer 2009-06-10 07:34:53 UTC
I have no interest in EPEL; Richard is totally free to take it.

Comment 18 Jason Tibbitts 2009-06-10 20:52:58 UTC
CVS done.

Comment 19 Richard W.M. Jones 2009-06-10 21:03:17 UTC
Thanks Jason.

Conrad - if you do need access to the EL-5 branch then just make
the acl change request through pkgdb and I'll approve it straight away.

Comment 20 Conrad Meyer 2009-06-10 21:06:37 UTC
Sure -- thanks.


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