Bug 553281 - (netsniff-ng) Review Request: netsniff-ng - high performance linux network sniffer for packet inspection
Review Request: netsniff-ng - high performance linux network sniffer for pack...
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Kevin Fenzi
Fedora Extras Quality Assurance
:
Depends On:
Blocks: FE-SECLAB
  Show dependency treegraph
 
Reported: 2010-01-07 10:13 EST by James Findley
Modified: 2010-03-24 13:58 EDT (History)
6 users (show)

See Also:
Fixed In Version: netsniff-ng-0.5.5.0-0.4.211svn.el5
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2010-03-03 19:20:42 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
kevin: fedora‑review+
tibbs: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description James Findley 2010-01-07 10:13:14 EST
Spec URL: http://sixy.myzen.co.uk/netsniff-ng.spec
SRPM URL: http://sixy.myzen.co.uk/netsniff-ng-0.5.4.1-1.fc12.src.rpm
Description: netsniff-ng is a high performance linux network sniffer for packet inspection. Basically, it is similar to tcpdump, but it doesn't
need one syscall per packet. Instead, it uses an memory mapped area within kernelspace for accessing packets without copying
them to userspace (zero-copy mechanism).

This tool is useful for debugging your network, measuring performance throughput or creating network statistics of incoming
packets on central network nodes like routers or firewalls.

By providing an unix domain socket client, you're able to integrate your statistics into the nagios framework.
Comment 1 James Findley 2010-01-07 10:14:53 EST
Note: First package submitted to fedora, so I need a sponsor.
Comment 2 Terje Røsten 2010-01-07 11:57:06 EST
# Useless, remove
%define name netsniff-ng
%define version 0.5.4.1
%define release 1

Name:		%{name}
Version:	%{version}
Release:	%{release}%{?dist}

# don't repeat name in summary
Summary:	netsniff-ng is a high performance network sniffer for packet inspection

# not valid
License:	GPL v2

# use %{version} macro here
Source0:	http://netsniff-ng.googlecode.com/files/netsniff-ng-0.5.4.1.tar.gz

# this is strange, do %setup -q -n %{name}_%{version}
%setup -q -n %{name}_%{version}/src

%build
# add pushd src here
make %{?_smp_mflags} CFLAGS="${RPM_OPT_FLAGS}"

# and these are not needed then
%{__cp} ../AUTHORS ../Changelog ../COPYING ../CREDITS ../README ../TODO .
%{__cp} -R ../examples .

%install
rm -rf $RPM_BUILD_ROOT
make install DESTDIR=$RPM_BUILD_ROOT

mkdir -p $RPM BINDIR_BUILD_ROOT%{_sbindir}

# add  BINDIR="%{_sbindir}" in %build and these will be fixed.
mv $RPM_BUILD_ROOT/sbin/netsniff-ng $RPM_BUILD_ROOT%{_sbindir}/netsniff-ng

%clean
rm -rf $RPM_BUILD_ROOT

%files
%defattr(-,root,root,-)
# one line is enough
%doc AUTHORS ChangeLog ....
%doc examples
# i would do:
# %doc is useless here
%doc %{_mandir}/*/*


# %config(noreplace) %{_sysconfdir}/netsniff-ng/*.bpf is fine.
# you forgot to own %{_sysconfdir}/netsniff-ng and
# %{_sysconfdir}/netsniff-ng/rules

%config(noreplace) %{_sysconfdir}/netsniff-ng/rules/all_traffic.bpf
%config(noreplace) %{_sysconfdir}/netsniff-ng/rules/arp.bpf
%config(noreplace) %{_sysconfdir}/netsniff-ng/rules/atalk.bpf
%config(noreplace) %{_sysconfdir}/netsniff-ng/rules/broadcast.bpf
%config(noreplace) %{_sysconfdir}/netsniff-ng/rules/ftp.bpf
%config(noreplace) %{_sysconfdir}/netsniff-ng/rules/http.bpf
%config(noreplace) %{_sysconfdir}/netsniff-ng/rules/icmp.bpf
%config(noreplace) %{_sysconfdir}/netsniff-ng/rules/imap.bpf
%config(noreplace) %{_sysconfdir}/netsniff-ng/rules/ip_broadcast.bpf
%config(noreplace) %{_sysconfdir}/netsniff-ng/rules/ip_multicast.bpf
%config(noreplace) %{_sysconfdir}/netsniff-ng/rules/multicast.bpf
%config(noreplace) %{_sysconfdir}/netsniff-ng/rules/pop3.bpf
%config(noreplace) %{_sysconfdir}/netsniff-ng/rules/rarp.bpf
%config(noreplace) %{_sysconfdir}/netsniff-ng/rules/rsync.bpf
%config(noreplace) %{_sysconfdir}/netsniff-ng/rules/skype_pre.bpf
%config(noreplace) %{_sysconfdir}/netsniff-ng/rules/smtp.bpf
%config(noreplace) %{_sysconfdir}/netsniff-ng/rules/ssh.bpf
%config(noreplace) %{_sysconfdir}/netsniff-ng/rules/vlan1000.bpf
%{_sbindir}/netsniff-ng
Comment 3 Terje Røsten 2010-01-07 12:11:33 EST
I have a srpm and spec file available, which is mostly rpmlint clean and adds
a init script for daemon mode. 

Have a look and feel free to use it if you want.

spec: http://terjeros.fedorapeople.org/netsniff-ng/netsniff-ng.spec
srpm: http://terjeros.fedorapeople.org/netsniff-ng/netsniff-ng-0.5.4.1-1.fc10.src.rpm
Comment 4 James Findley 2010-01-07 13:39:13 EST
Ok, I've used your %build and %install sections, and incorporated the other requirements.

I've ditched the patch in favour of just putting an %attr tag in the manpage line of %files - nice spot that it was 755, but a patch just to fix that seems overkill.

I've also fixed a typo in your initscript, and another in your sysconfig file.

spec: http://sixy.myzen.co.uk/netsniff-ng.spec
srpm: http://sixy.myzen.co.uk/netsniff-ng-0.5.4.1-2.fc12.src.rpm

It's still rpmlint clean.  Let me know if you find anything else wrong.
Comment 5 Terje Røsten 2010-01-07 14:02:27 EST
a) too long lines in %description (> 80)
b) mixing $RPM_BUILD_ROOT and %{buildroot}
c) no need to mark man pages as %doc, done by rpmbuild itself
d) I believe it's better to chmod 0644 man page last in %install than using %attr in %files. Better to save %attr for special cases.

In all, nice work. 

Do you have a Fedora account? If not create one, and try to do
a koji scratch build and post a koji link (makes it more easy for the reviewer)

More info here:

http://fedoraproject.org/wiki/PackageMaintainers/UsingKoji#Scratch_builds_2

koji build --scratch dist-f13 netsniff-ng-0.5.4.1-2.fc12.src.rpm

I created the patch so it could be sent upstream.
Comment 6 James Findley 2010-01-07 15:11:17 EST
a) fixed that
b) oops! those were the lines I shamelessly copied from you!  Thanks :)
c) didn't know that - updated.
d) ok, I've changed that.

I have an account but I admit I don't know koji very well (read: at all).
koji url: http://koji.fedoraproject.org/koji/taskinfo?taskID=1908133

as it doesn't seem to have the spec outside the srpm, a link to the spec is:
http://sixy.myzen.co.uk/netsniff-ng.spec
Comment 7 Terje Røsten 2010-01-07 15:37:12 EST
we are making progress, thanks for the koji build, its not possible
to be a maintainer without basic understanding in koji.

Spec file still has %doc %{_mandir}/man8/%{name}.8*

Remember to bump the release in %changelog, there are two "-2" lines. 

Now very pedantic, this is in %changelog too, add an extra space in dates < 10 
or add zero padding, change

* Thu Jan 7 2010 James Findley <james.findley@gmx.com> - 0.5.4.1-2
to
* Thu Jan  7 2010 James Findley <james.findley@gmx.com> - 0.5.4.1-2
or 
* Thu Jan 07 2010 James Findley <james.findley@gmx.com> - 0.5.4.1-2
Comment 8 James Findley 2010-01-07 17:00:49 EST
ok, fixed the above.

koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=1908283

spec: http://sixy.myzen.co.uk/netsniff-ng.spec

Thanks for the help!
Comment 9 James Findley 2010-01-08 05:44:32 EST
I noticed that gcc was throwing warnings on AMD64 builds, due to print %llu with a uint64_t var, and so added a -Wno-format to supress these, as they should be completely harmless.

koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=1908813
spec: http://sixy.myzen.co.uk/netsniff-ng.spec
Comment 10 James Findley 2010-02-02 14:20:15 EST
I've updated to a prerelease of 0.5.5.0, which adds a great deal of functionality, stability and performance, along with a good deal of bug fixes.

koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=1959464
spec: http://sixy.myzen.co.uk/netsniff-ng.spec

It now works correctly on older (e.g. RHEL 5.x) OSes, and I intend to submit the final 0.5.5.0 to epel, subject to this being approved.

Known issues: Not all new features are documented in the manpage yet, this will be fixed in a later update.

I'm adding the FE-NEEDSPONSOR tag to the bug, also.
Comment 11 James Findley 2010-02-17 07:56:28 EST
Further updates to do several things:
1) add BuildRequires and Requires that were missing
2) a patch to fix the output of netsniff-ng -h, so that features not yet implemented are marked with an * (in a similar way to versions of rndc in RHEL)
3) a patch to make the unix domain socket actually work
4) a client application to read data from the unix domain socket, based on code in the 0.5.3 sources.

Package is still rpmlint clean.

TODO: SELinux policy.  This will be implemented when the final version of 0.5.5.0 is released, as the current code is too much of a moving target.

koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=1993304
spec: http://sixy.myzen.co.uk/netsniff-ng.spec
srpm: http://sixy.myzen.co.uk/netsniff-ng-0.5.5.0-0.3.211svn.fc12.src.rpm
Comment 12 Kevin Fenzi 2010-02-20 16:23:37 EST
I'll go ahead and take this for review. 

James: Do you have any other packages to submit? Or can you do some 'pre-reviews' of some existing pending package reviews to show your understanding of the guidelines?

Look for a review here soon...
Comment 13 Kevin Fenzi 2010-02-22 20:04:41 EST
OK - Package meets naming and packaging guidelines
OK - Spec file matches base package name. 
OK - Spec has consistant macro usage. 
OK - Meets Packaging Guidelines. 
OK - License (GPLv2+)
OK - License field in spec matches
OK - License file included in package
OK - Spec in American English
OK - Spec is legible.
See below - Sources match upstream md5sum:

OK - BuildRequires correct
OK - Package has %defattr and permissions on files is good. 
OK - Package has a correct %clean section. 
OK - Package has correct buildroot
OK - Package is code or permissible content. 
OK - Packages %doc files don't affect runtime. 
OK - Package has rm -rf RPM_BUILD_ROOT at top of %install

OK - Package compiles and builds on at least one arch. 
OK - Package has no duplicate files in %files. 
OK - Package doesn't own any directories other packages own. 
OK - Package owns all the directories it creates. 
OK - Package obey's FHS standard (except for 2 exceptions)
See below - No rpmlint output. 
OK - final provides and requires are sane.

SHOULD Items:

OK - Should build in mock. 
OK - Should build on all supported archs
OK - Should have sane scriptlets. 
OK - Should have dist tag
OK - Should package latest version
OK - Should not use file requires outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin

Issues: 

1. For snapshots you shouldn't use the full URL in Source0 (since there isn't one). 
Instead include a comment or script that generates the tar.gz thats used. 

2. There shouldn't be any need for: 
BuildRequires:  gcc
Requires:       libc.so.6
Please remove

3. Why have the seperate check_packets tar? Are those Makefile changes going to be 
merged into upstream so this isn't needed moving forward?

4. Nit: the %{__rm} stuff isn't against any guideline, but I would drop all that and
just use 'rm' and 'make'. 

5. Our friend rpmlint says: 

netsniff-ng.src: W: spelling-error %description -l en_US linux -> Linux
netsniff-ng.src: W: spelling-error %description -l en_US tcpdump -> dumpster, Dumpster, dumpling
netsniff-ng.src: W: spelling-error %description -l en_US syscall -> scallop, systemically, scallion
netsniff-ng.src: W: spelling-error %description -l en_US kernelspace -> kernel space, kernel-space, kernels pace
netsniff-ng.src: W: spelling-error %description -l en_US userspace -> user space, user-space, users pace
netsniff-ng.src: W: spelling-error %description -l en_US unix -> UNIX, Unix, uni
netsniff-ng.src: W: spelling-error %description -l en_US nagios -> adagios, nagging, Nagoya
netsniff-ng.x86_64: W: spelling-error %description -l en_US linux -> Linux
netsniff-ng.x86_64: W: spelling-error %description -l en_US tcpdump -> dumpster, Dumpster, dumpling
netsniff-ng.x86_64: W: spelling-error %description -l en_US syscall -> scallop, systemically, scallion
netsniff-ng.x86_64: W: spelling-error %description -l en_US kernelspace -> kernel space, kernel-space, kernels pace
netsniff-ng.x86_64: W: spelling-error %description -l en_US userspace -> user space, user-space, users pace
netsniff-ng.x86_64: W: spelling-error %description -l en_US unix -> UNIX, Unix, uni
netsniff-ng.x86_64: W: spelling-error %description -l en_US nagios -> adagios, nagging, Nagoya

I think all those can be ignored. 

netsniff-ng.src: W: invalid-url Source0: http://netsniff-ng.googlecode.com/files/netsniff-ng-0.5.5.0.tar.gz HTTP Error 404: Not Found

This one would be fixed by moving to a comment about how to generate the
source checkout. (Or getting a final release of this version that has all the 
fixes you need).
Comment 14 James Findley 2010-02-23 12:32:24 EST
Thanks for the review!  I've changed the way the sources work some, in the hope of getting it to be a bit cleaner.

### Short background ###
Until very recently this app used a unix domain socket, to allow stats reporting in daemon mode (an essential part of daemon mode).  This is now a deprecated feature in the latest svn revisions, and is scheduled to be replaced by a better method, namely a netlink multicast group.  However this is not yet implemented, and so temporarily the latest upstream misses an essential part of daemon mode.  As we are building an unreleased svn version, to fix (many) issues with the latest stable, upstream aren't interested in keeping the unix domain socket around until the new method is functional.

So there are basically 3 options:
1) continue to ship the buggy 0.5.4 code
2) ship the latest upstream, and put up with daemon mode being not terribly useful
3) ship this svn revision until 0.5.5.0 is final, patching the UDS server code to make it work, and packaging the client to read statistics from an older version of the source.

I am proposing to do 3 - this svn revision is (after the patch) good code and has been stable across a wide range of test environments, so makes a good stop while I wait for 0.5.5.0.

So what I now do is svn export the correct revision, tar it up and use that as source 0.  I also use the 0.5.3 source to provide the client for the UDS.

I hope that explains the reasoning behind this spec madness, and why these patches aren't appropriate for upstream.

### Short background ###

Sources are fixed.

Requires/Buildrequires are also fixed.

I've got rid of the check_packets.tar.gz

I've stopped using the macros for rm, make and install.

However, rpmlint now produces some errors.

SPECS/netsniff-ng.spec: W: invalid-url Source3: http://netsniff-ng.googlecode.com/files/netsniff-ng-0.5.3.tar.gz HTTP Error 404: Not Found
SPECS/netsniff-ng.spec: W: invalid-url Source0: netsniff-ng-0.5.5.0.tar.gz

The first is an odd error.  Possibly a bug in rpmlint (I will investigate this more fully when I have time) as a wget of that URL works fine.
The second is explained by the comments above, in accordance with: http://fedoraproject.org/wiki/Packaging/SourceURL
Therefore I am proposing to ignore these warnings.
Comment 16 Kevin Fenzi 2010-02-26 15:00:22 EST
Yeah, I have no idea why rpmlint is complaining there. ;( 

I am seeing no further blockers, although if 0.5.5.0 is going to be released soon, you could just wait for that and the spec would be a good deal simpler. ;) 

This package is APPROVED. 
I will go ahead and sponsor you. 

If you have any questions at all with the process or steps, feel free to email me or catch me on irc. 
You can continue the process from: https://fedoraproject.org/wiki/PackageMaintainers/Join#Add_Package_to_CVS_and_Set_Owner
Comment 17 James Findley 2010-03-02 09:26:52 EST
New Package CVS Request
=======================
Package Name: netsniff-ng
Short Description: A high performance network sniffer for packet inspection
Owners: sixy
Branches: F-11 F-12 EL-5
InitialCC:
Comment 18 Jason Tibbitts 2010-03-02 17:51:42 EST
CVS done (by process-cvs-requests.py).

I added an F-13 branch as well since that seems to have been missed.
Comment 19 Fedora Update System 2010-03-03 06:32:13 EST
netsniff-ng-0.5.5.0-0.4.211svn.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/netsniff-ng-0.5.5.0-0.4.211svn.fc11
Comment 20 Fedora Update System 2010-03-03 06:32:21 EST
netsniff-ng-0.5.5.0-0.4.211svn.fc13 has been submitted as an update for Fedora 13.
http://admin.fedoraproject.org/updates/netsniff-ng-0.5.5.0-0.4.211svn.fc13
Comment 21 Fedora Update System 2010-03-03 06:32:26 EST
netsniff-ng-0.5.5.0-0.4.211svn.el5 has been submitted as an update for Fedora EPEL 5.
http://admin.fedoraproject.org/updates/netsniff-ng-0.5.5.0-0.4.211svn.el5
Comment 22 Fedora Update System 2010-03-03 06:32:31 EST
netsniff-ng-0.5.5.0-0.4.211svn.fc12 has been submitted as an update for Fedora 12.
http://admin.fedoraproject.org/updates/netsniff-ng-0.5.5.0-0.4.211svn.fc12
Comment 23 Fedora Update System 2010-03-03 06:33:57 EST
netsniff-ng-0.5.5.0-0.4.211svn.fc13 has been submitted as an update for Fedora 13.
http://admin.fedoraproject.org/updates/netsniff-ng-0.5.5.0-0.4.211svn.fc13
Comment 24 Fedora Update System 2010-03-03 19:08:22 EST
netsniff-ng-0.5.5.0-0.4.211svn.fc13 has been pushed to the Fedora 13 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 netsniff-ng'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F13/FEDORA-2010-3547
Comment 25 Fedora Update System 2010-03-03 19:20:37 EST
netsniff-ng-0.5.5.0-0.4.211svn.fc12 has been pushed to the Fedora 12 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 26 Fedora Update System 2010-03-03 19:23:29 EST
netsniff-ng-0.5.5.0-0.4.211svn.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 27 Fedora Update System 2010-03-04 13:40:11 EST
netsniff-ng-0.5.5.0-0.4.211svn.el5 has been submitted as an update for Fedora EPEL 5.
http://admin.fedoraproject.org/updates/netsniff-ng-0.5.5.0-0.4.211svn.el5
Comment 28 Fedora Update System 2010-03-22 22:12:32 EDT
netsniff-ng-0.5.5.0-0.4.211svn.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 29 Fedora Update System 2010-03-24 13:58:25 EDT
netsniff-ng-0.5.5.0-0.4.211svn.el5 has been pushed to the Fedora EPEL 5 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.