Bug 691894 - (pyrit) Review Request: pyrit - A GPGPU-driven WPA/WPA2-PSK key cracker
Review Request: pyrit - A GPGPU-driven WPA/WPA2-PSK key cracker
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Garrett Holmstrom
Fedora Extras Quality Assurance
:
: 530806 (view as bug list)
Depends On:
Blocks: FE-SECLAB
  Show dependency treegraph
 
Reported: 2011-03-29 16:05 EDT by Tom "spot" Callaway
Modified: 2011-09-30 14:31 EDT (History)
8 users (show)

See Also:
Fixed In Version: pyrit-0.4.0-4.fc16
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2011-09-14 18:32:41 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
gholms: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Tom "spot" Callaway 2011-03-29 16:05:43 EDT
Spec URL: http://spot.fedorapeople.org/pyrit.spec
SRPM URL: http://spot.fedorapeople.org/pyrit-0.4.0-1.fc14.src.rpm
Description: 
Pyrit exploits the computational power of many-core- and GPGPU-platforms to
create massive databases, pre-computing part of the WPA/WPA2-PSK authentication
phase in a space-time trade-off. It is a powerful attack against one of the
world's most used security-protocols.

Koji Scratch Build (dist-f15): http://koji.fedoraproject.org/koji/taskinfo?taskID=2957971
Comment 1 Elad Alfassa 2011-03-30 02:03:31 EDT
I'll do an unofficial review.
Comment 2 Elad Alfassa 2011-03-30 02:30:47 EDT
+ = OK
- = NA
? = issue

+ Package meets naming guidelines
+ Spec file matches base package name.
+ Spec has consistent macro usage.
+ Meets Packaging Guidelines.
? License
+ License field in spec matches
+ License file included in package
+ Spec in American English
+ Spec is legible.
- Sources match upstream md5sum:

- Package needs ExcludeArch
+ BuildRequires correct
- Spec handles locales/find_lang
- Package is relocatable and has a reason to be.
+ Package has %defattr and permissions on files is good.
- Package has a correct %clean section.
- Package has correct buildroot
%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
+ Package is code or permissible content.
- Doc subpackage needed/used.
- Packages %doc files don't affect runtime.

- Headers/static libs in -devel subpackage.
- Spec has needed ldconfig in post and postun
- .pc files in -devel subpackage/requires pkgconfig
- .so files in -devel subpackage.
- -devel package Requires: %{name} = %{version}-%{release}
- .la files are removed.

- Package is a GUI app and has a .desktop file

+ Package compiles and builds on at least one arch.
+ Package has no duplicate files in %files.
+ Package doesn't own any directories other packages own.
+ Package owns all the directories it creates.
+ No rpmlint output. (looks like bogus output to me):
pyrit.x86_64: W: spelling-error %description -l en_US pre -> per, ore, pee
pyrit.x86_64: W: private-shared-object-provides /usr/lib64/python2.7/site-packages/cpyrit/_cpyrit_cpu.so _cpyrit_cpu.so()(64bit)
pyrit.x86_64: W: no-manual-page-for-binary pyrit
2 packages and 0 specfiles checked; 0 errors, 3 warnings.

+ final provides and requires are sane

SHOULD Items:

- Should build in mock.
- Should build on all supported archs
- Should function as described.
- Should have sane scriptlets.
- Should have subpackages require base package with fully versioned depend.
+ Should have dist tag
+ Should package latest version
- check for outstanding bugs on package. (For core merge reviews)

Issues:

1.I'm not sure, probably I'm wrong, but GPLv3 with openssl exception is not listed here: https://fedoraproject.org/wiki/Licensing

Not an issue, but I think you should fix it:
The hyphen after "many-core" in the description is unnecessary. Also I think you should mention that this package only provide CPU support, and not GPUGPU (there are CUDA and OpenCL support sub-packages available upstream, but can not be used in fedora because no free driver currently implements those features)
Comment 3 Fabian Affolter 2011-04-04 11:26:56 EDT
*** Bug 530806 has been marked as a duplicate of this bug. ***
Comment 4 Garrett Holmstrom 2011-06-08 17:01:35 EDT
Review of pyrit-0.4.0-1.fc15:

The dump files and dictionary in pyrit-0.4.0/test are considered content, but the license under which they are distributable is unclear.  I recommend asking upstream for clarification.  If the license is acceptable, is the content itself considered acceptable for inclusion in a Fedora SRPM?

Packaging-wise, please fix the permissions and RPM Provides of _cpyrit_cpu.so.  AFAIK the build also needs to either honor Fedora's CFLAGS or justify its failure to do so.  Note that distutils respects the CFLAGS environment variable.  The remaining issues, specifically man pages and translations, are optional.  See below for a full review.

Mandatory review guidelines:
NO - rpmlint output
     pyrit.src: W: spelling-error %description -l en_US pre -> per, ore, pee
     pyrit.src: W: invalid-url Source0: http://pyrit.googlecode.com/files/pyrit-0.4.0.tar.gz HTTP Error 404: Not Found
     pyrit.x86_64: W: spelling-error %description -l en_US pre -> per, ore, pee
     pyrit.x86_64: W: private-shared-object-provides /usr/lib64/python2.7/site-packages/cpyrit/_cpyrit_cpu.so _cpyrit_cpu.so()(64bit)
     pyrit.x86_64: E: non-standard-executable-perm /usr/lib64/python2.7/site-packages/cpyrit/_cpyrit_cpu.so 0775L
     pyrit.x86_64: W: no-manual-page-for-binary pyrit
     --
     The spelling-error and invalid-url complaints appear to be bogus.
ok - Package meets naming guidelines
ok - Spec file name matches base package name
ok - License is acceptable (GPLv3+ with exceptions)
ok - License field in spec is correct
     The file "pyrit" has no license header; assuming GPLv3+
ok - License files included in package %docs or not included in upstream source
ok - License files installed when any subpackage combination is installed
ok - Spec written in American English
ok - Spec is legible
ok - Sources match upstream unless altered to fix permissibility issues
     Upstream MD5:  7258b6f3dacfb09736ddeed2a379df2d  pyrit-0.4.0.tar.gz
     Your MD5:      7258b6f3dacfb09736ddeed2a379df2d  pyrit-0.4.0.tar.gz
ok - Build succeeds on at least one supported platform
ok - Build succeeds on all supported platforms or has ExcludeArch + bugs filed
ok - BuildRequires correct
-- - Package handles locales with %find_lang
-- - %post, %postun call ldconfig if package contains shared .so files
ok - No bundled system libs
-- - Relocatability is justified
ok - Package owns all directories it creates
ok - Package requires other packages for directories it uses but does not own
ok - No duplicate files in %files unless necessary for license files
NO - File permissions are sane
     -rwxrwxr-x /usr/lib64/python2.7/site-packages/cpyrit/_cpyrit_cpu.so
ok - Each %files section contains %defattr
ok - Consistent use of macros
NO - Sources contain only permissible code or content
     test/*.gz has no associated content license.
-- - Large documentation files go in -doc package
ok - Missing %doc files do not affect runtime
-- - Headers go in -devel package
-- - Static libs go in -static package
-- - Unversioned .so files go in -devel package
-- - Devel packages require base with fully-versioned dependency
ok - Package contains no .la files
-- - GUI app installs .desktop file w/desktop-file-install or has justification
ok - Package's files and directories don't conflict with others' or justified
ok - File names are valid UTF-8

Optional review guidelines:
no - Query upstream about including license files
     No license for test/*.gz
no - Translations of description, Summary
ok - Builds in mock
ok - Builds on all supported platforms
ok - Functions as described
-- - Scriptlets are sane
-- - Non-devel subpackage Requires are sane
-- - .pc files go in -devel unless main package is a development tool
ok - No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin
no - Man pages included for all executables
ok - Package with test-suite executes it in %check section

Packaging guidelines:
ok - Has dist tag
ok - Useful without external bits
ok - Package obeys FHS, except libexecdir, /usr/target, /run
-- - Programs launched before FS mounting use /run instead of /var/run
-- - Binaries in /bin, /sbin do not depend on files in /usr
ok - Changelog in prescribed format
ok - Spec file lacks Packager, Vendor, PreReq tags
no - Correct BuildRoot tag on < F10/EL6
     This prevents building for epel-5.
no - Correct %clean section on < F13/EL6
     This prevents building for epel-5.
ok - Requires correct, justified where necessary
ok - Summary, description do not use trademarks incorrectly
ok - All relevant documentation is packaged, tagged appropriately
ok - Documentation files do not have executable permissions
NO - %build honors applicable compiler flags or justifies otherwise
     Note that distutils respects the CFLAGS environment variable.
-- - Package with .pc files Requires pkgconfig on < EL6
ok - Useful -debuginfo package or disabled and justified
ok - No static executables
ok - Rpath absent or only used for internal libs
-- - Config files marked with %config
-- - %config files marked noreplace or justified
-- - No %config files under /usr
-- - SysV-style init script
ok - Spec uses macros instead of hard-coded directory names when appropriate
ok - Spec uses macros for executables only when configurability is needed
-- - %makeinstall used only when ``make install DESTDIR=...'' doesn't work
ok - Macros in Summary, %description expandable at SRPM build time
-- - Spec uses %{SOURCE#} instead of $RPM_SOURCE_DIR or %{sourcedir}
ok - %global instead of %define where appropriate
-- - Package containing translations BuildRequires gettext
-- - File timestamps preserved by file ops
-- - Parallel make
ok - Spec does not use Requires(pre,post) notation
-- - User, group creation handled correctly (See Packaging:UsersAndGroups)
-- - Web app files go in /usr/share/%{name}, not /var/www
-- - Conflicts are justified
ok - No external kernel modules
ok - No files in /srv
ok - One project per package
-- - Patches link to upstream bugs/comments/lists or are otherwise justified
-- - Packages needing dirs in /var/run or /var/lock use tmpfiles.d on >= F15

Python guidelines:
ok - Runtime Requires correct
ok - Python macros declared on < F13/EL6
ok - All .py files packaged with .pyc, .pyo counterparts
ok - Includes .egg-info files/directories when generated
NO - Provides/Requires properly filtered
     _cpyrit_cpu.so must be filtered from Provides.
-- - Code that invokes gtk.gdk.get_pixels_array() Requires numpy
Comment 5 Lukas Lueg 2011-06-09 14:35:35 EDT
Upstream here.

The license-issue regarding the dump-files has also been discussed for inclusion in Debian6. To resolve, I've contacted the original owners of these files and have written permission to include them in Pyrit's distribution (under the terms of the GPLv3+). The README refers to that in the License-section. The file dict.gz is not mentioned but falls under then same agreement as wpa2psk-linksys.dump.gz and wpapsk-linksys.dump.gz
Comment 6 Tom "spot" Callaway 2011-08-08 17:03:35 EDT
Added filtering, optflags, description cleanup:

Spec URL: http://spot.fedorapeople.org/pyrit.spec
SRPM URL: http://spot.fedorapeople.org/pyrit-0.4.0-2.fc15.src.rpm
Comment 7 Garrett Holmstrom 2011-08-18 14:29:32 EDT
Most everything looks good.  Just fix the permissions on _cpyrit_cpu.so and that should do it.

Review of pyrit-0.4.0-2.fc15:

Mandatory review guidelines:
NO - rpmlint output
     pyrit.src: W: spelling-error %description -l en_US pre -> per, ore, pee
     pyrit.x86_64: W: spelling-error %description -l en_US pre -> per, ore, pee
     pyrit.x86_64: E: non-standard-executable-perm /usr/lib64/python2.7/site-packages/cpyrit/_cpyrit_cpu.so 0775L
     pyrit.x86_64: W: no-manual-page-for-binary pyrit
     --
     The spelling-error warnings are bogus.  Man pages are optional.
ok - Package meets naming guidelines
ok - Spec file name matches base package name
ok - License is acceptable (GPLv3+ with exceptions)
ok - License field in spec is correct
     The file "pyrit" has no license header; assuming GPLv3+
ok - License files included in package %docs or not included in upstream source
ok - License files installed when any subpackage combination is installed
ok - Spec written in American English
ok - Spec is legible
ok - Sources match upstream unless altered to fix permissibility issues
     Upstream MD5:  7258b6f3dacfb09736ddeed2a379df2d  pyrit-0.4.0.tar.gz
     Your MD5:      7258b6f3dacfb09736ddeed2a379df2d  pyrit-0.4.0.tar.gz
ok - Build succeeds on at least one supported platform
ok - Build succeeds on all supported platforms or has ExcludeArch + bugs filed
ok - BuildRequires correct
-- - Package handles locales with %find_lang
-- - %post, %postun call ldconfig if package contains shared .so files
ok - No bundled system libs
-- - Relocatability is justified
ok - Package owns all directories it creates
ok - Package requires other packages for directories it uses but does not own
ok - No duplicate files in %files unless necessary for license files
NO - File permissions are sane
     -rwxrwxr-x /usr/lib64/python2.7/site-packages/cpyrit/_cpyrit_cpu.so
ok - Each %files section contains %defattr on EL4
ok - Consistent use of macros
ok - Sources contain only permissible code or content
-- - Large documentation files go in -doc package
ok - Missing %doc files do not affect runtime
-- - Headers go in -devel package
-- - Static libs go in -static package
-- - Unversioned .so files go in -devel package
-- - Devel packages require base with fully-versioned dependency
ok - Package contains no .la files
-- - GUI app installs .desktop file w/desktop-file-install or has justification
ok - Package's files and directories don't conflict with others' or justified
ok - File names are valid UTF-8

Optional review guidelines:
ok - Query upstream about including license files
no - Translations of description, Summary
ok - Builds in mock
ok - Builds on all supported platforms
ok - Functions as described
-- - Scriptlets are sane
-- - Non-devel subpackage Requires are sane
-- - .pc files go in -devel unless main package is a development tool
ok - No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin
no - Man pages included for all executables
ok - Package with test-suite executes it in %check section

Packaging guidelines:
ok - Has dist tag
ok - Useful without external bits
ok - Package obeys FHS, except libexecdir, /usr/target, /run
-- - Programs launched before FS mounting use /run instead of /var/run
-- - Binaries in /bin, /sbin do not depend on files in /usr
ok - Changelog in prescribed format
ok - Spec file lacks Packager, Vendor, PreReq tags
no - Correct BuildRoot tag on < F10/EL6
     This prevents building for epel-5
no - Correct %clean section on < F13/EL6
     This prevents building for epel-5
ok - Requires correct, justified where necessary
ok - Summary, description do not use trademarks incorrectly
ok - All relevant documentation is packaged, tagged appropriately
ok - Documentation files do not have executable permissions
ok - %build honors applicable compiler flags or justifies otherwise
-- - Package with .pc files Requires pkgconfig on < EL6
ok - Useful -debuginfo package or disabled and justified
ok - No static executables
ok - Rpath absent or only used for internal libs
-- - Config files marked with %config
-- - %config files marked noreplace or justified
-- - No %config files under /usr
-- - SysV-style init script
ok - Spec uses macros instead of hard-coded directory names where appropriate
ok - Spec uses macros for executables only when configurability is needed
-- - %makeinstall used only when ``make install DESTDIR=...'' doesn't work
ok - Macros in Summary, %description expandable at SRPM build time
-- - Spec uses %{SOURCE#} instead of $RPM_SOURCE_DIR or %{sourcedir}
ok - %global instead of %define where appropriate
-- - Package containing translations BuildRequires gettext
-- - File timestamps preserved by file ops
-- - Parallel make
ok - Spec does not use Requires(pre,post) notation
-- - User, group creation handled correctly (See Packaging:UsersAndGroups)
-- - Web app files go in /usr/share/%{name}, not /var/www
-- - Conflicts are justified
ok - No external kernel modules
ok - No files in /srv
ok - One project per package
-- - Patches link to upstream bugs/comments/lists or are otherwise justified
-- - Packages needing dirs in /var/run or /var/lock use tmpfiles.d on >= F15

Python guidelines:
ok - Runtime Requires correct
ok - Python macros declared on < F13/EL6
ok - All .py files packaged with .pyc, .pyo counterparts
ok - Includes .egg-info files/directories when generated
ok - Provides/Requires properly filtered
-- - Code that invokes gtk.gdk.get_pixels_array() Requires numpy
Comment 8 Lukas Lueg 2011-08-21 09:24:11 EDT
Upstream here. Please notice that the test-suite will run tests on Pyrit's sql-backend if python-sqlalchemy is available. This package should therefor also be in BuildRequires
Comment 9 Tom "spot" Callaway 2011-08-23 17:05:09 EDT
Fixed perms on library file (0755)
Added python-sqlalchemy to BR for tests

Spec URL: http://spot.fedorapeople.org/pyrit.spec
SRPM URL: http://spot.fedorapeople.org/pyrit-0.4.0-3.fc15.src.rpm
Comment 10 Garrett Holmstrom 2011-09-01 16:45:28 EDT
Looks good to me.  Enjoy!

Review of pyrit-0.4.0-3.fc15:

Mandatory review guidelines:
ok - rpmlint output
     pyrit.src: W: spelling-error %description -l en_US pre -> per, ore, pee
     pyrit.x86_64: W: spelling-error %description -l en_US pre -> per, ore, pee
     pyrit.x86_64: W: no-manual-page-for-binary pyrit
     --
     The spelling-error warnings are bogus.  Man pages are optional.
ok - Package meets naming guidelines
ok - Spec file name matches base package name
ok - License is acceptable (GPLv3+ with exceptions)
ok - License field in spec is correct
     The file "pyrit" has no license header; assuming GPLv3+
ok - License files included in package %docs or not included in upstream source
ok - License files installed when any subpackage combination is installed
ok - Spec written in American English
ok - Spec is legible
ok - Sources match upstream unless altered to fix permissibility issues
     Upstream MD5:  7258b6f3dacfb09736ddeed2a379df2d  pyrit-0.4.0.tar.gz
     Your MD5:      7258b6f3dacfb09736ddeed2a379df2d  pyrit-0.4.0.tar.gz
ok - Build succeeds on at least one supported platform
ok - Build succeeds on all supported platforms or has ExcludeArch + bugs filed
ok - BuildRequires correct
-- - Package handles locales with %find_lang
-- - %post, %postun call ldconfig if package contains shared .so files
ok - No bundled system libs
-- - Relocatability is justified
ok - Package owns all directories it creates
ok - Package requires other packages for directories it uses but does not own
ok - No duplicate files in %files unless necessary for license files
ok - File permissions are sane
ok - Each %files section contains %defattr on EL4
ok - Consistent use of macros
ok - Sources contain only permissible code or content
-- - Large documentation files go in -doc package
ok - Missing %doc files do not affect runtime
-- - Headers go in -devel package
-- - Static libs go in -static package
-- - Unversioned .so files go in -devel package
-- - Devel packages require base with fully-versioned dependency
ok - Package contains no .la files
-- - GUI app installs .desktop file w/desktop-file-install or has justification
ok - Package's files and directories don't conflict with others' or justified
ok - File names are valid UTF-8

Optional review guidelines:
ok - Query upstream about including license files
no - Translations of description, Summary
ok - Builds in mock
ok - Builds on all supported platforms
ok - Functions as described
-- - Scriptlets are sane
-- - Non-devel subpackage Requires are sane
-- - .pc files go in -devel unless main package is a development tool
ok - No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin
no - Man pages included for all executables
ok - Package with test-suite executes it in %check section

Packaging guidelines:
ok - Has dist tag
ok - Useful without external bits
ok - Package obeys FHS, except libexecdir, /usr/target, /run
-- - Programs launched before FS mounting use /run instead of /var/run
-- - Binaries in /bin, /sbin do not depend on files in /usr
ok - Changelog in prescribed format
ok - Spec file lacks Packager, Vendor, PreReq tags
no - Correct BuildRoot tag on < F10/EL6
     This prevents building for epel-5
no - Correct %clean section on < F13/EL6
     This prevents building for epel-5
ok - Requires correct, justified where necessary
ok - Summary, description do not use trademarks incorrectly
ok - All relevant documentation is packaged, tagged appropriately
ok - Documentation files do not have executable permissions
ok - %build honors applicable compiler flags or justifies otherwise
-- - Package with .pc files Requires pkgconfig on < EL6
ok - Useful -debuginfo package or disabled and justified
ok - No static executables
ok - Rpath absent or only used for internal libs
-- - Config files marked with %config
-- - %config files marked noreplace or justified
-- - No %config files under /usr
-- - SysV-style init script
ok - Spec uses macros instead of hard-coded directory names where appropriate
ok - Spec uses macros for executables only when configurability is needed
-- - %makeinstall used only when ``make install DESTDIR=...'' doesn't work
ok - Macros in Summary, %description expandable at SRPM build time
-- - Spec uses %{SOURCE#} instead of $RPM_SOURCE_DIR or %{sourcedir}
ok - %global instead of %define where appropriate
-- - Package containing translations BuildRequires gettext
-- - File timestamps preserved by file ops
-- - Parallel make
ok - Spec does not use Requires(pre,post) notation
-- - User, group creation handled correctly (See Packaging:UsersAndGroups)
-- - Web app files go in /usr/share/%{name}, not /var/www
-- - Conflicts are justified
ok - No external kernel modules
ok - No files in /srv
ok - One project per package
-- - Patches link to upstream bugs/comments/lists or are otherwise justified
-- - Packages needing dirs in /var/run or /var/lock use tmpfiles.d on >= F15

Python guidelines:
ok - Runtime Requires correct
ok - Python macros declared on < F13/EL6
ok - All .py files packaged with .pyc, .pyo counterparts
ok - Includes .egg-info files/directories when generated
ok - Provides/Requires properly filtered
-- - Code that invokes gtk.gdk.get_pixels_array() Requires numpy
Comment 11 Tom "spot" Callaway 2011-09-06 10:45:52 EDT
New Package SCM Request
=======================
Package Name: pyrit
Short Description: A GPGPU-driven WPA/WPA2-PSK key cracker
Owners: spot
Branches: f14 f15 f16 
InitialCC:
Comment 12 Jon Ciesla 2011-09-06 10:51:23 EDT
Git done (by process-git-requests).

Garrett, in the future, please take ownership of review BZs.  Thanks!
Comment 13 Garrett Holmstrom 2011-09-06 11:30:24 EDT
(In reply to comment #12)
> Garrett, in the future, please take ownership of review BZs.  Thanks!

Bah, looks like I forgot.  Sorry about that.
Comment 14 Fedora Update System 2011-09-06 16:15:41 EDT
pyrit-0.4.0-4.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/pyrit-0.4.0-4.fc14
Comment 15 Fedora Update System 2011-09-06 16:15:50 EDT
pyrit-0.4.0-4.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/pyrit-0.4.0-4.fc15
Comment 16 Fedora Update System 2011-09-06 16:16:00 EDT
pyrit-0.4.0-4.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/pyrit-0.4.0-4.fc16
Comment 17 Fedora Update System 2011-09-06 23:00:13 EDT
pyrit-0.4.0-4.fc16 has been pushed to the Fedora 16 testing repository.
Comment 18 Fedora Update System 2011-09-14 18:32:33 EDT
pyrit-0.4.0-4.fc15 has been pushed to the Fedora 15 stable repository.
Comment 19 Fedora Update System 2011-09-15 21:58:32 EDT
pyrit-0.4.0-4.fc14 has been pushed to the Fedora 14 stable repository.
Comment 20 Fedora Update System 2011-09-30 14:31:36 EDT
pyrit-0.4.0-4.fc16 has been pushed to the Fedora 16 stable repository.

Note You need to log in before you can comment on or make changes to this bug.