[Pkg-security-team] Review of ncrack and t50

[Marcos Fouces]

El 21/07/16 a las 15:27, Raphael Hertzog escribió:
> Hello Marcos,
>
> I took a look at ncrack and t50. Here are my comments and questions. Please
> address them and I will upload the packages. Feel free to ask questions
> if you have any.
>
> For ncrack first:
> * why is there a Depends on python and Build-Depends on python-all-dev?
>    can we get rid of them?
Yes, as i said to you in a previous mail, this dependencies seemed a bit 
strange to me.
When i try "dpkg-depcheck -d ./configure" i did not see any of these.

The package builds correctly without them.I just left these fields 
unmodified as in Kali package because i was unsure.
> * same question for all other build dependencies except libssl-dev in
>    fact...
OK, now i just left libssl-dev, autotools-dev and debhelper (>= 9) as 
Build-Depends and no specific Depends.

> * there are remaining typos that can be fixed:
> I: ncrack: spelling-error-in-binary usr/bin/ncrack guage gauge
Fixed
> I: ncrack: spelling-error-in-binary usr/bin/ncrack addres address
This is a variable in source code: if (!strncmp(buf, "addres", 6))

I don't know if it should be fixed. Please, re-check it in the 
ncrack_input.cc file (line 198).

> * debian/control: use "optional" instead of "extra" as priority, extra
>    is only for packages that are alternatives for some other optional package
Fixed.
> * debian/copyright: the license is basically the GPL but with exceptions,
>    I wonder if we must mark it that way instead of "Other", it would probably
>    also make sense to add a sentence referring to /usr/share/common-licenses/GPL-2
Fixed

>    You should also update the list of copyright holders for "debian/*" to include
>    all persons who worked on the package.
I added all Kali repo commiters and myself.
> * please forward the typo patches to the upstream developers and mark
>    the patch as forwarded (using DEP-3 headers).
Fixed. I did a pull request on Github.

> For t50:
> * why do you override dh_strip in the way you do it?
Because the default behavior of dh_strip creates an extra  dbg_sym 
package at building time and Lintian complained about it. This way the 
binaries are also stripped (hopefully) and there is no extra debug package.

> * "dh $@ --with-autoreconf" should be "dh $@ --with autoreconf"
> * don't set DH_VERBOSE=1 by default
> * drop the boilerplate comments in debian/rules (line 1 to 7)
> * clean up debian/changelog, it contains unneeded "[ Marcos Fouces ]" and "[ Marcos ]"
>    since were the only one working on it
Fixed.
> * add DEP-3 headers to debian/patches/fix-spelling-errors.patch
>    and forward the patch upstream (likely with a pull request here:
>    https://github.com/fredericopissarra/t50/pulls)
Done.
> * debian/copyright: Source still mentions t50.sourceforge.net but everywhere
>    else you mentionnedhttps://github.com/fredericopissarra/t50
>    maybe use the same everywhere if sf.net is obsolete...
Done.
> * debian/copyright: update the copyright holders for the main code,
>    it seems to be "2010 - 2015 - T50 developers" everywhere now.
Fixed
> Cheers,
Thank you very much for your time.

Greetings. Marcos