intetsim review (Re: ITP: inetsim)
Lukas Schwaighofer
lukas at schwaighofer.name
Wed Oct 4 20:37:32 UTC 2017
Hi,
Adding pkg-security back to CC. We may get additional feedback that
way and it may be useful to one of the DDs who might sponsor your
package eventually.
As promised, I took a look at your repository at
https://github.com/Z-Y00/inetsim . Feedback:
Gerneral:
* Please make one git commit for each change. That makes reviewing
changes much easier.
* I have no idea if there is a convention regarding debian/changelog
for kali imports. I know I've seen kali versions in Debian changelogs
before… Maybe someone here can tell us if the changelog should be
preserved. I guess you should at least mention that the packaging is
based on kali…
Packaging:
* please update to debhelper compatibility level 10
* the most recent standards-version at the moment is 4.1.1
* debian/control:
- use ${perl:Depends} for the perl dependency (generated by dh_perl)
- unless you need a specific version of libdigest-sha-perl, I think
you can drop it from the dependencies as it is also present as a
core modules (see output of `corelist -a Digest::SHA`); I'm not sure
about this though since I'm not really familiar with perl and perl
packaging.
* helper script:
- use /bin/sh instead of /bin/bash as interpreter
- use "exec" to execute inetsim after the cd, otherwise the shell
process will be kept alive while inetsim runs (needing some
resources)
* maintainer scripts:
- inetsimp.postinst: ugly mix of tabs/spaces, please unify :)
- inetstim.postrm: I noticed it does not remove the inetsim group, but
this might be ok (I didn't find something in the policy to
explicitly request the removal, and there is an old wiki page [1]
which apparently allows both).
- inetsim.preinst: I would propose to drop it. It contains a fix for
a previously broken kali maintainer script. Since this has never
been part of any Debian release it's safe for Debian to drop it.
Also, the fix is in Kali since 2014, so it shouldn't be a problem
for them either.
Looking at the contents of the binary package, I don't like the look of
the /usr/share/inetsim/contrib directory:
* it contains a windows binary and some source files which probably
are not needed
* it contains an init script; should this be installed?
* in any case, the contents of this directory and its use should be
explained (also for filter-logs.pl and gen_control.pl); I would
suggest creating a README.Debian for that
That's all I have for now. I hope the pointers are useful for
improving the package. If you have any questions please ask :) .
Regards
Lukas
[1] https://wiki.debian.org/AccountHandlingInMaintainerScripts
More information about the Pkg-security-team
mailing list