[Pkg-security-team] searching sponsor for the social engineering toolkit

[Gianfranco Costamagna]

Lets to a review:

>yes, i've noticed them too, a sponsor (or someone to collaborate with) 
>would be helpful for me to understand which of them actually need to be 
>fixed or if they can be just overridden

everything needs to be fixed.

lets see
1) you didn't push the tags
2) you pushed a master branch

3) changelog

7.3.12-2
wrong, one single entry targeting -1 and a "imported package from kali wording"

no need to changelog entries, and for sure not a -2 without a previous -1
4) control
you use dh-python but have no python:Depends

no-go, please learn how to use the tool

and please learn how to use install_requires and forward a patch upstream, instead
of hardcoding so many runtime dependencies

5) wrap-and-sort

6) 
"License: Other"

seriously? this is a simple BSD-3 clause license

(with some changes/additions)
7) copyright is referring many unexisting files

8) 
-rw-r--r-- root/root       912 2016-09-05 02:41 ./usr/share/set/README.md
this should be installed (if useful) with dh_installchangelogs

9)
override_dh_auto_configure:
override_dh_auto_clean:
override_dh_auto_build:
override_dh_auto_install:
# Setup.py is not usable

fixing it would be better.

10) 
set: extra-license-file usr/share/set/readme/LICENSE

remove it, or patch the code to not use it?
or open it from the debhelper copyright standard location

11)
patches:
Description: Edit the locations for ettercap, airbase-ng, and upx (patch imported from kali)
Author: Lorenzo "Palinuro" Faletra <palinuro at parrotsec.org>

so, you took a patch from kali, changed the authorship, and committed.
the patch is not even mentioning ettercap location.

12)
"src/sms/protectedapi.pyc: python 2.7 byte-compiled"

please get the source or remove it.

13)
Vcs-Git: git://anonscm.debian.org/pkg-security/set.git
wrong url. (non secure)

14)
-rwxr-xr-x root/root      5256 2016-09-06 13:08 ./usr/share/set/src/html/Signed_Update.jar.orig

source for this file?

15) many .c/cpp source files are installed, are them needed?
are you rebuilding them from scratch?

16) binaries into an arch:all package are source of segfaults.

17) all the lintian warnings/errors (I count 89 complains)
can be fixed and many of them *should* be fixed before uploading

18)
check-all-the-things review:

$ find .. -maxdepth 1 -type f -iwholename '../*.build' -exec grep -H -w E {} +

<lots>
$ find .. -maxdepth 1 -type f -iwholename '../*.build' -exec grep -H -w W {} +

<lots>
$ find -type f -iname '*.sh' -exec checkbashisms {} +

<lots>
$ env PERL5OPT=-m-lib=. cme check dpkg

<lots>
$ codespell --quiet-level=3
<lots>


$ grep -rF /proc/cpuinfo .
Binary file ./src/wireless/airbase-ng matches
Binary file ./src/payloads/ratte/ratteserver matches


sigh, other binaries?

# Please check if these README files belong to embedded code/data copies.
# Please remove any embedded copies from the upstream VCS and tarballs.
# https://wiki.debian.org/EmbeddedCodeCopies
$ find -mindepth 2 -iname '*README*'

<lots>

$ find \( -name .git -o -name .svn -o -name .bzr -o -name CVS -o -name .hg -o -name _darcs -o -name _FOSSIL_ -o -name .sgdrawer \) -prune -o -empty -print

<lots>

$ fdupes -q -r . | grep -vE '/(\.(git|svn|bzr|hg|sgdrawer)|_(darcs|FOSSIL_)|CVS)(/|$)' | cat -s

<lots>

$ grep -Er '/(home|srv|opt)(\W|$)' .

<lots>

$ flawfinder -Q -c .

<lots>

# check if these can be switched to https://
$ grep -rF http: .

<lots>

# This command checks style. While a consistent style
# is a good idea, people who have different style
# preferences will want to ignore some of the output.
# Do not bother adding non-upstreamable patches for this.
$ find -type f -iname '*.py' -exec pep8 --ignore W191 {} +

<lots>

$ find -type f -iname '*.py' -exec pyflakes {} +

<lots>

$ find -type f -iname '*.py' -exec pylint --rcfile=/dev/null --msg-template='{path}:{line}:{column}: [{category}:{symbol}] {obj}: {msg}' --reports=n {} +

<lots>

# Users of binary packages do not need install instructions.
$ find -type f -iname '*README*' -a ! \( -iname README.md -o -iname README.rst -o -iname README.install \) -exec grep --ignore-case --fixed-strings --with-filename install {} +

<lots>


sign.sh file
cp ../../html/unsigned/unsigned.jar Java_Exploit.jar
jar ufm Java_Exploit.jar manifest.mf
jarsigner -storetype pkcs12 -keystore /root/certs/MyCert.pfx Java_Exploit.jar "1"
cp Java_Exploit.jar Signed_Update.jar.orig
cp Java_Exploit.jar ../../html/Signed_Update.jar.orig


who does sign it?


$ find -type d \( -iname .bzr -o -iname .git -o -iname .hg -o -iname .svn -o -iname CVS -o -iname RCS -o -iname SCCS -o -iname _MTN -o -iname _darcs -o -iname .pc -o -iname .cabal-sandbox -o -iname .cdv -o -iname .metadata -o -iname CMakeFiles -o -iname _build -o -iname _sgbak -o -iname autom4te.cache -o -iname blib -o -iname cover_db -o -iname node_modules -o -iname '~.dep' -o -iname '~.dot' -o -iname '~.nib' -o -iname '~.plst' \) -prune -o -type f ! \( -iname '*.bak' -o -iname '*.swp' -o -iname '#.*' -o -iname '#*#' -o -iname 'core.*' -o -iname '*~' -o -iname '*.gif' -o -iname '*.jpg' -o -iname '*.jpeg' -o -iname '*.png' -o -iname '*.min.js' -o -iname '*.js.map' -o -iname '*.js.min' -o -iname '*.min.css' -o -iname '*.css.map' -o -iname '*.css.min' \) -exec env PERL5OPT=-m-lib=. spellintian --picky {} +
<lots>

$ suspicious-source
<lots>
$ grep -r '/tmp/' .
<lots>

$ grep -riE 'fixme|todo|hack|xxx+|broken' .
<lots>

Please add some upstream metadata: https://wiki.debian.org/UpstreamMetadata



> Currently lintian reports a bunch of errors on your package that must
> be fixed before uploading SET. Do you mind if i fix them* on the git
> repo or do you prefer that i point them out and you fix it yourself?
also this.

I'm stopping the review here, even the license extension from the BSD-3 might be non-dfsg to Debian.


"DISCLAIMER: This is only for testing purposes and can only be used where strict consent has been given. Do not use this for illegal purposes, period."

the package might be suitable for kali, but for Debian policy it is a no-go right now.
I guess you will need to do a lot of work if you really want to see this one reaching unstable.

also, there are a lot of embedded libraries, or jquery files, you need to remove them and patch the source
to use system libraries.

Hope this helps,

Gianfranco,
BackBox Developer and Repository maintainer,
who doesn't enjoy your *bad* attitude and behaviour against Debian forks in public forums.
(actually BackBox is giving Debian more than you will probably be able do in the near future)


Slightly unrelated note:
Debian policy welcomes forks, is not hiding things, and bad behaviour is not tolerated, like people who
"I just want to become DD because to send mails to my teacher with my @debian.org address"

this is something you will have to learn if you really want to join the community, but there will be time
for this, right now if you want to apply for Debian Maintainer you first need to learn
how to package and check a packaging for goodness, fix stuff, help even if you don't like the person you
are helping, and make the community a better place for everybody.
a Debian Maintainer knows the policy, and in this case before getting the package uploaded you need to learn:
- everything should be built from source, no binaries are allowed
(you can use prebuilt stuff under certain conditions, *but* you need to be able to rebuild it when needed,
with tools in Debian repositories in main).
- embedded code copies must/should be avoided whenever possible, source of security issues and other kind
of troubles
https://www.debian.org/doc/debian-policy/

might have something useful in this context, please read it :)

its all for now, a little suggestion might be to start from an easier tool, this one is really nice
but overcomplicated ;)

G.