Bug 523199

Summary: Review Request: ncrack - high-speed network authentication cracking tool
Product: [Fedora] Fedora Reporter: Steve Milner <smilner>
Component: Package ReviewAssignee: Martin Gieseking <martin.gieseking>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: a.badger, fedora-package-review, martin.gieseking, notting, opensource
Target Milestone: ---Flags: martin.gieseking: fedora-review+
kevin: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: 0.01-0.7.ALPHA.fc11 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2009-10-15 22:32:41 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
Attachments:
Description Flags
extends ncrack to look in /etc for configuration files none

Description Steve Milner 2009-09-14 13:15:01 UTC
Spec URL: http://stevemilner.org/images/ncrack.spec
SRPM URL: http://stevemilner.org/images/ncrack-0.01-1.ALPHA.fc11.src.rpm
Description: Ncrack is a high-speed network authentication cracking tool. It was
built to help companies secure their networks by proactively testing
all their hosts and networking devices for poor passwords. Security
professionals also rely on Ncrack when auditing their clients. Ncrack
was designed using a modular approach, a command-line syntax similar to
Nmap and a dynamic engine that can adapt its behaviour based on network
feedback. It allows for rapid, yet reliable large-scale auditing of
multiple hosts.

Comment 1 Martin Gieseking 2009-09-14 18:43:43 UTC
- according to the source file headers the license is GPLv2 only but with exceptions (the tag must probably be "GPLv2 with exceptions")

- You should preserve the timestamp of COPYING.OpenSSH. Also, replace ASCII by ISO-8859-1:

  iconv -f ISO-8859-1 -t UTF8 COPYING.OpenSSH -o COPYING.OpenSSH.new
  touch -r COPYING.OpenSSH COPYING.OpenSSH.new
  mv COPYING.OpenSSH.new COPYING.OpenSSH

Comment 2 Steve Milner 2009-09-15 18:39:09 UTC
Updated:
$ sha1sum ncrack-0.01-1.ALPHA.fc11.src.rpm ncrack.spec
1cb7b4ea93d56bf23d6f5e00e3bb0984ab266e3b  ncrack-0.01-1.ALPHA.fc11.src.rpm
9689d5cab38d70fb18b9811e44a96e6fbeb37965  ncrack.spec

Spec URL: http://stevemilner.org/images/ncrack.spec
SRPM URL: http://stevemilner.org/images/ncrack-0.01-1.ALPHA.fc11.src.rpm

Comment 3 Martin Gieseking 2009-09-15 19:38:18 UTC
Could you provide links to the updated files (release 2)? The URLs given above point to the initial ones.

Comment 4 Steve Milner 2009-09-15 20:04:03 UTC
$ sha1sum ncrack.spec ncrack-0.01-2.ALPHA.fc11.src.rpm
6c7acea31b496bfa4d997f4439f41fb669605565  ncrack.spec
e308b3e578998808879e89867c167ea8f85b6fbb  ncrack-0.01-2.ALPHA.fc11.src.rpm

Spec URL: http://stevemilner.org/images/ncrack.spec
SRPM URL: http://stevemilner.org/images/ncrack-0.01-2.ALPHA.fc11.src.rpm

Comment 5 Martin Gieseking 2009-09-15 21:49:14 UTC
* since this is a pre-release, the revision tag must be 0.X.ALPHA%{?dist} where X is the revision number to increment (see http://fedoraproject.org/wiki/Packaging/NamingGuidelines#NonNumericRelease)

* as far as I can see, Requires: openssl is not necessary

* remove COPYING.OpenSSL from %doc because it's not part of the package (openssl is bundled with the tarball for Windows builds)

* the changelog entries should contain information about the things that have been changed, so that the file history can be reproduced, e.g. 
  - updated license tag to GPLv2 with restrictions
  - preserve timestamp of file COPYING.OpenSSH

instead of 
  - Updated for package review

:)

Comment 6 Steve Milner 2009-09-16 14:55:35 UTC
Making the changes and rebuilding now ...

Comment 7 Steve Milner 2009-09-16 15:10:05 UTC
$ sha1sum ncrack-0.01-0.3.ALPHA.fc11.src.rpm ncrack.spec
5c70bc79bb891904b0f4ebb27164afd92651e573  ncrack-0.01-0.3.ALPHA.fc11.src.rpm
927ab719e67daddf67ddffb0ba8e28fb73c09a6f  ncrack.spec

Spec URL: http://stevemilner.org/images/ncrack.spec
SRPM URL: http://stevemilner.org/images/ncrack-0.01-0.3.ALPHA.fc11.src.rpm

Comment 8 Till Maas 2009-09-16 21:08:44 UTC
This
%{_mandir}/man1/ncrack.1.gz
should be
%{_mandir}/man1/ncrack.1*
because the compression of the manpage depends on the rpm configuration. 
%description contains an extra space as the last character of the last line, this is not needed.

Feel free to update this just when some other issues are discovered.

Comment 9 Martin Gieseking 2009-09-17 11:51:37 UTC
$ rpmlint /var/lib/mock/fedora-11-i386/result/ncrack-*
ncrack-debuginfo.i586: E: empty-debuginfo-package
3 packages and 0 specfiles checked; 1 errors, 0 warnings.

The debuginfo package is empty because the debugging information is stripped too early. To fix this issue, add

  sed -i '/\$(STRIP)/d' Makefile.in

to the %prep section.

Comment 10 Steve Milner 2009-09-21 16:07:18 UTC
a674c6c495ad24411bd2f809d300b23c2e3ead84  ncrack-0.01-0.4.ALPHA.fc11.src.rpm
943102e58844847163cd5c574d555fcde316d906  ncrack.spec

Spec URL: http://stevemilner.org/images/ncrack.spec
SRPM URL: http://stevemilner.org/images/ncrack-0.01-0.4.ALPHA.fc11.src.rpm

Comment 11 Martin Gieseking 2009-09-21 18:46:28 UTC
The package looks fine now. However, I'm not sure about file ncrack-services that is placed in /usr/share/ncrack by default. It looks as it could be a candidate for /etc because it's a kind of configuration file. I'm going to ask on fedora-devel.


rpmlint /var/lib/mock/fedora-11-x86_64/result/ncrack-*
3 packages and 0 specfiles checked; 0 errors, 0 warnings.


---------------------------------
keys used in following checklist:

[+] OK
[.] OK, not applicable
[X] needs work
---------------------------------

[+] MUST: The package must be named according to the Package Naming Guidelines.

[+] MUST: The spec file name must match the base package %{name}.

[X] MUST: The package must meet the Packaging Guidelines.
    - not sure whether /usr/share/ncrack/ncrack-services should go to /etc
      because it's a kind of admin configuration file 

[+] MUST: The package must be licensed with a Fedora approved license.
    - according to source headers GPLv2 with exceptions

[+] MUST: The License field in the package spec file must match the actual license.

[+] MUST: File containing the text of the license(s) for the package must be included in %doc.

[+] MUST: The spec file must be written in American English.

[+] MUST: The spec file for the package MUST be legible.

[+] MUST: The sources used to build the package must match the upstream source, as provided in the spec URL.
    $ sha1sum ncrack-0.01ALPHA.tar.gz*
    797f3274cfab33091330df6425c251c979a8bfb8  ncrack-0.01ALPHA.tar.gz
    797f3274cfab33091330df6425c251c979a8bfb8  ncrack-0.01ALPHA.tar.gz.1


[+] MUST: The package MUST successfully compile and build into binary rpms on at least one primary architecture.
    koji scratch build:
    https://koji.fedoraproject.org/koji/taskinfo?taskID=1695351

[.] MUST: If the package does not successfully compile,...
    - package builds for all targets

[+] MUST: All build dependencies must be listed in BuildRequires.

[.] MUST: The spec file MUST handle locales properly. This is done by using the %find_lang macro. Using %{_datadir}/locale/* is strictly forbidden.
    - no locales

[.] MUST: Packages containing shared library files must call ldconfig in %post and %postun.
    - no shared libs
  
[.] MUST: If the package is designed to be relocatable, ...
    - not relocatable

[+] MUST: A package must own all directories that it creates.

[+] MUST: A Fedora package must not list a file more than once in the spec file's %files listings.

[+] MUST: Permissions on files must be set properly.

[+] MUST: Each package must have a %clean section, which contains rm -rf %{buildroot}.

[+] MUST: Each package must consistently use macros.

[+] MUST: The package must contain code, or permissable content.

[.] MUST: Large documentation files must go in a -doc subpackage.
    - no large docs

[+] MUST: If a package includes something as %doc, it must not affect the runtime of the application.

[.] MUST: Header files must be in a -devel package.
    - no header files

[.] MUST: Static libraries must be in a -static package.
    - no static libs

[.] MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'
    - no pkgconfig files

[.] MUST: .so files must go in a -devel package.
    - no shared libs

[.] MUST: devel packages must require the base package using a fully versioned dependency: Requires: %{name} = %{version}-%{release}
    - no devel package

[.] MUST: Packages must NOT contain any .la libtool archives, these must be removed in the spec if they are built.
    - no .la files

[.] MUST: Packages containing GUI applications must include a %{name}.desktop file,... 
    - no GUI

[+] MUST: Packages must not own files or directories already owned by other packages.

[+] MUST: At the beginning of %install, each package MUST run rm -rf %{buildroot} (or $RPM_BUILD_ROOT).

[+] MUST: All filenames in rpm packages must be valid UTF-8.


[+] SHOULD: The reviewer should test that the package builds in mock.
    - builds in mock
  
[+] SHOULD: The package should compile and build into binary rpms on all supported architectures.
    - builds in koji

[+] SHOULD: The reviewer should test that the package functions as described.
    - made a short test, seems to work properly    

[.] SHOULD: If scriptlets are used, those scriptlets must be sane.
    - no scriptlets

[.] SHOULD: Usually, subpackages other than devel should require the base package using a fully versioned dependency.
    - no subpackages

[.] SHOULD: .pc files should be placed in a -devel pkg.
    - no .pc files

Comment 12 Steve Milner 2009-09-21 19:33:33 UTC
Thanks for the review!

It could go in /etc/ ... but the man page has this:

    --datadir directoryname (Specify custom Ncrack data file location) .
        Ncrack needs a file called ncrack-services to load a lookup-table of supported services/ports. This file shouldn´t be changed, unless you know what you are doing (e.g extending Ncrack for additional modules).

If you think I should move (or symlink) into sysconfdir I'll make the change :-).

I'm also fixing the man page which has a wrong path in it (it states /usr/local/share instead of /usr/share/).

Comment 13 Martin Gieseking 2009-09-21 19:45:47 UTC
(In reply to comment #12)
> Thanks for the review!
You're welcome.

 
> It could go in /etc/ ... but the man page has this: ...

Yes, I read the paragraph of the man page. I was just wondering whether there is some kind of Fedora policy that forces us to move the file to /etc. If there's no such demand, I can directly approve the package. Otherwise some patching is required first. :)

Comment 14 Martin Gieseking 2009-09-22 16:09:52 UTC
According to the replies I got from fedora-devel it's not necessary to move the file to /etc, so we can finish here.

------------------------
The package is APPROVED.
------------------------

Comment 15 Toshio Ernie Kuratomi 2009-09-22 18:03:07 UTC
Setting UNAPPROVED for now.

Config files go in %{_sysconfdir} (/etc on Fedora).  This is part of the FHS and there is a Fedora Guideline that we follow the FHS.

The question in this package is whether this file is a config file or a data file.  In your (Martin) last email you gave an example of when the local system admin might want to modify this file to suit the local site/machine.  If this is a valid use then this is a config file.  So it needs to be installed somewhere below /etc and the file marked %config or %config(noreplace) in the rpm spec file.

For reference, the message with the example https://www.redhat.com/archives/fedora-devel-list/2009-September/msg00887.html

Sorry people didn't make clear the real factors in deciding whether this needs to get moved to /etc.

Comment 16 Martin Gieseking 2009-09-22 18:22:15 UTC
OK, thanks for the clarification. It shouldn't be too complicated to adapt the sources. I'll have a look at it.

Comment 17 Martin Gieseking 2009-09-22 20:43:52 UTC
Created attachment 362136 [details]
extends ncrack to look in /etc for configuration files

I've attached a patch that extends the ncrack sources to also look in /etc for config files. Steve, you should ask upstream to apply it to the sources or add another mechanism that allows relocating file ncrack-services.

The spec file must also be modified:
- in the %setup section, apply the patch (with option -p1)

- add the following to %install:
  mkdir $RPM_BUILD_ROOT%{_sysconfdir}
  mv $RPM_BUILD_ROOT%{_datadir}/%{name}/ncrack-services $RPM_BUILD_ROOT%{_sysconfdir}

- add %config %{_sysconfdir}/ncrack-services to the %files section

This is just a suggestion. If necessary, feel free to modify it.


Toshio, is it required to put the config file in a subfolder of /etc, or can we place it directly in /etc?

Comment 18 Steve Milner 2009-09-23 17:41:40 UTC
Excellent! I'll make these changes, test, update this BZ and notify upstream of the patch. Thanks Martin!

Comment 19 Steve Milner 2009-09-23 20:32:09 UTC
eaca5f1b0ad2d776341cc7af8dfac834e8df7163  ncrack.spec
4575eee604f1c6ff01ee2cd03b5ee6e570b30a8a  ncrack-0.01-0.5.ALPHA.fc11.src.rpm

Spec URL: http://stevemilner.org/images/ncrack.spec
SRPM URL: http://stevemilner.org/images/ncrack-0.01-0.5.ALPHA.fc11.src.rpm

Comment 20 Steve Milner 2009-09-28 18:19:06 UTC
After speaking with the developers on the list it sounds like they don't recommend modifying ncrack-services as it is more of a data file ...

From Fyodor:
"""
I suppose people could modify it, but that is not recommmended.  If
they want to use a custom services file, it is usually better for them
to put it in ~/.ncrack or specify the location with the --datadir
command-line option or the NCRACKDIR environmental variable.
"""

You can read the full thread at http://seclists.org/nmap-dev/2009/q3/1059.html.

Comment 21 Toshio Ernie Kuratomi 2009-09-28 18:58:21 UTC
Yep.  If people are meant to override those defaults by specifying their own versions in a ~/.ncrack/ file then /usr/share is fine.

I don't know anything about the specifics of ncrack, though -- is it meant to be run by a user or setup to be run periodically by the system administrator?  If it's run ad hoc by a user then configuration in a ~/.ncrack file is fine.  If it's run on a schedule then it's very nice for system administrators to know that the configuration of services is all in /etc.

Note that many programs which are used as both services and invoked ad hoc (or even ones that are purely ad hoc, for instance, mutt) do store files like this in /etc for the system admin to configure while leaving the user to edit dot files in their home directories when they want to customize their ad hoc usage.

Comment 22 Steve Milner 2009-09-29 17:15:22 UTC
Someone *could* set it up to run periodically. It's usage is quite similar to nmap though ...

[steve@arc steve]$ rpm -qc nmap
[steve@arc steve]$ rpm -ql nmap | grep service
/usr/share/nmap/nmap-service-probes
/usr/share/nmap/nmap-services
[steve@arc steve]$

Comment 23 Toshio Ernie Kuratomi 2009-09-29 18:30:56 UTC
(In reply to comment #22)
> Someone *could* set it up to run periodically.

This is the part that needs to be evaluated.  While this::

> It's usage is quite similar to
> nmap though ...
> 
> [steve@arc steve]$ rpm -qc nmap
> [steve@arc steve]$ rpm -ql nmap | grep service
> /usr/share/nmap/nmap-service-probes
> /usr/share/nmap/nmap-services
> [steve@arc steve]$  

means that nmap potentially needs to move those files.

My position would be -- the system administrator should not have to configure things in /root/ in order to setup a service.  Hhat's what /etc/ is for.  However, if running this as a service is very esoteric (in most cases this is run at the command line by a user when they're evaluating the security of a network.  Very rarely does a system admin want to drop it in a cron job or otherwise use it to constantly monitor the security of their network) then it's okay to leave things in /usr/share/ and have the users put the configuration to run the program in their home directories.  In no case should the system administrator change files in /usr/ in order to configure the service.  Does that make sense?

Comment 24 Steve Milner 2009-09-29 18:59:14 UTC
Yes, it does. I think in this case it's OK as, in the few cases where an admin would want to run ncrack (or nmap) via cron they can use the command line switch to specify what config they want to use (or use the environment variable). They could place the file in /etc/ where system configs should be. In the rest of the (by far most common) cases they can use the data file, or override it with the same command line switches or environment variables.

Comment 25 Toshio Ernie Kuratomi 2009-09-29 19:45:23 UTC
Excellent.  I have no objection to that explanation.

Comment 26 Steve Milner 2009-09-29 20:12:49 UTC
e7433325e4df7a89f7509a5101af8c7ef459b166  ncrack.spec
76c3345d0e1b921d9cbcd4d9c79c4a3abe37c5ae  ncrack-0.01-0.6.ALPHA.fc11.src.rpm

Spec URL: http://stevemilner.org/images/ncrack.spec
SRPM URL: http://stevemilner.org/images/ncrack-0.01-0.6.ALPHA.fc11.src.rpm

Comment 27 Martin Gieseking 2009-09-30 16:34:50 UTC
The latest package release is identical to revision 4, so the review in comment #11 is still valid. I just had a look into the package and everything seems to be fine now. Since the default location of file ncrack-services turned out to be OK, I dare to approve the package again.

Comment 28 Steve Milner 2009-10-01 13:57:20 UTC
New Package CVS Request
=======================
Package Name: ncrack
Short Description: a high-speed network authentication cracking tool
Owners: smilner
Branches: F-11 F-12 EL-4 EL-5
InitialCC:

Comment 29 Kevin Fenzi 2009-10-03 21:24:01 UTC
cvs done.

Comment 30 Fedora Update System 2009-10-05 14:10:33 UTC
ncrack-0.01-0.6.ALPHA.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/ncrack-0.01-0.6.ALPHA.fc11

Comment 31 Fedora Update System 2009-10-07 03:11:10 UTC
ncrack-0.01-0.7.ALPHA.fc11 has been pushed to the Fedora 11 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update ncrack'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F11/FEDORA-2009-10297

Comment 32 Fedora Update System 2009-10-15 22:32:34 UTC
ncrack-0.01-0.7.ALPHA.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.