Bug#670176: RFS: kismet/2011.03.R2-1 [ITA]
reopen 670176
Hello Arno,
I just came back after a long period that I wasn't able to take a look
on the package.
I'm trying to address all the things you said.
2012/5/14 Arno Töll <arno@debian.org>:
> Hi Nick,
>
> this is a shallow review of your package. Unfortunately I am lacking
> experience with CDBS packages and your package is quite complex. Hence I
> am unsure if I will sponsor it eventually. That said, I'd love to see
> kismet in shape in Debian.
>
> * Please clarify the license of these files in your debian/copyright file:
> + apple80211.h: the header states: "From MacStumbler, which is under GPL"
I contacted upstream to clarify
> + extra/*: Lacks copyright header. Given all other files do, I suspect
> this is by purpose which means it is not known whether these files are
> distributable.
My guess is that it has the same copyright as the files in the root
folder (e.g. util.cc).
I asked upstream in order to clarify this
> + Likewise for packaging/dpkg* (I would also hope, this script is
> burned with fire)
Probably this folder will be removed altogether
> + Likewise for ruby/*. No fire this time, though
I asked upstream to add copyright/license info
> Consider repackaging the tarball if you cannot find any clarifcation
> regarding these files as they mostly seem irrelevant to me. Moreover,
> please note you are linking against OpenSSL but the package is licensed
> under the terms of the GPL-2. However, only a few files do have a
> OpenSSL linking exception. Note, the GPL is not compatible to the
> OpenSSL license [1]. Ask upstream for clarification on that matter.
I asked upstream and see what they tell me. I think there are only a
couple of plugins that need OpenSSL and if there is a license issue we
can just exclude them from the build.
> * Where does this package come from? Did you base your work upon the
> Ubuntu package? I'm asking because of "kismet (2008-05-R1-4.3build1)
> precise; urgency=low" for the previous upload. If the only difference to
> the Debian version is this Ubuntu binNMU, please remove this changelog
> stanza.
I think I took the latest ubuntu build, which indeed has only a minor
change wrt debian's version.
I dropped that changelog entry
> * What is debian/kismet-plugins-restricted.install? These refer to a
> binary package you don't build. Were they split to a non-free package
> previously? If yes, what did change to make this split unnecessary?
> License of the plugin-ptw/* stuff seems DFSG free to me (but it is
> listed in kismet-plugins-restricted as
> debian/tmp/usr/lib/kismet/aircrack-kismet.so I think).
I wanted to put it like that for the case that some of the plugins
needed openssl but didn't include the license exception.
I asked upstream to verify which plugins are these and then see if
they can all include the exception.
> * Please fill out copyright headers of debian/po/templates.pot properly.
Can I put automatic variables in there (e.g. package name/version/etc?)
> While you are it, the po translation files are not complete.
> Moreover,
> do not prompt in question style in debconf templates. Formulate the data
> you want to get from the user in an open way ("give me $foo:").
The only question I can find is: "Should Kismet be installed to run
with setuid privs?"
Any idea how I could rephrase it?
> * You do not depend on adduser, but you use it in your postinst
> maintainer script.
I thought adduser is part of the standard distribution, no?
> Same for libcap2-bin and setcap.
I thought libcap2-bin would get automatically installed as part of
shlibs:Depens part.
For setcap you are definitely right
> Moreover, the
> maintainer script does not look very sane. See [2] and policy §6.5 [3]
> to see how maintainer scripts are invoked and change it to something
> properly. I am referring to the way you are determining whether you are
> upgrading.
I am not sure I understand the question. The only extra thing I'm
doing in the maintainer scripts is to add(or remove) group that has
permission to use kismet
> Do also set -e in your maintainer scripts.
I think it is already there on the invocation line (sh -e), no?
> * Likewise you are blindly adding users provided by user input to
> usermod like this:
>
> for x in ${RET}; do
> usermod -a -G $GROUP $x
> done
>
> You should at least verify whether the supplied user(s) exists before
> passing random inputs to usermod.
Should I do that just after the input from the user (by debconf) or
before I add them to the group?
> * The postrm script looks acceptable, but note it is consensus not to
> remove users anymore once created. There is no safe and sane way to
> determine whether the site administrator hijacked the created group on
> purpose.
I do not remove users, just the group
> * Maybe you should bump the debhelper compatibility to v8.
Sure.
> * Consider writing man pages for your binaries lacking these.
I will ask upstream to see if they can provide those, since this issue
is not debian specific.
> * Lintian says:
> E: kismet: su-wrapper-without--c usr/share/applications/kismet.desktop
> su-to-root
> E: su-wrapper-without--c
> N:
> N: The menu item command or desktop file uses an su wrapper such as
> N: su-to-root without the -c flag. This is a syntax error.
> N:
> N: Refer to the su-to-root(1) manual page for details.
> N:
> N: Severity: important, Certainty: certain
> N:
> N: Check: menu-format, Type: binary
Fixed. Is it better to put "su-to-root -c" or gksudo ?
Thank you very much for your support
Nick
Reply to: