Bug 428823 - Review Request: sectool - A security audit system and intrusion detection system
Review Request: sectool - A security audit system and intrusion detection system
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Miloslav Trmač
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-01-15 09:21 EST by Peter Vrabec
Modified: 2008-02-28 13:16 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-02-28 13:16:36 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
mitr: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Peter Vrabec 2008-01-15 09:21:21 EST
Spec URL:
http://people.redhat.com/pvrabec/rpms/sectool/sectool.spec

SRPM URL:
http://people.redhat.com/pvrabec/rpms/sectool/sectool-0.1.0-1.fc7.src.rpm

Description:
sectool is a security tool that can be used both as a security audit
and intrusion detection system. It consists of set of tests, library
and command line inerface tool. Tests are sorted into groups and security
levels. Admins can run certain tests, groups or whole security levels.
The library and the tools are implemented in python and tests are
language independent.
Comment 1 Miloslav Trmač 2008-01-15 17:23:16 EST
* rpmlint output:
> sectool.noarch: E: non-executable-script
/usr/share/sectool/scheduler/scheduler.py 0644
> sectool.noarch: E: non-executable-script /usr/share/sectool/actions.py 0644
> sectool.noarch: E: non-executable-script
/usr/share/sectool/scheduler/__init__.py 0644
> sectool.noarch: E: non-executable-script /usr/share/sectool/scheduler/i18n.py 0644
> sectool.noarch: E: non-executable-script /usr/share/sectool/output.py 0644
> sectool.noarch: E: non-executable-script /usr/share/sectool/__init__.py 0644
> sectool.noarch: E: non-executable-script /usr/share/sectool/tuierrors.py 0644
> sectool.noarch: E: non-executable-script
/usr/share/sectool/scheduler/errors.py 0644
Need fixing (by removing the #!... line)
> sectool.noarch: E: non-executable-script
/usr/share/sectool/scheduler/selftest.py 0644
Needs fixing (make it executable, or don't ship it)
> sectool.noarch: E: non-standard-executable-perm
/usr/share/sectool/tests/netserv.sh 0744
> sectool.noarch: E: non-standard-executable-perm
/usr/share/sectool/tests/home_dirs.sh 0744
> sectool.noarch: E: non-standard-executable-perm
/usr/share/sectool/tests/selftest.sh 0744
> sectool.noarch: E: non-standard-executable-perm
/usr/share/sectool/tests/disc_usage.sh 0744
> sectool.noarch: E: non-standard-executable-perm
/usr/share/sectool/tests/suid.py 0744
> sectool.noarch: E: non-standard-executable-perm /usr/share/sectool/sectool.py 0744
> sectool.noarch: E: non-standard-executable-perm
/usr/share/sectool/tests/path.sh 0744
> sectool.noarch: E: non-standard-executable-perm
/usr/share/sectool/tests/home_files.sh 0744
> sectool.noarch: E: non-standard-executable-perm
/usr/share/sectool/tests/passwd.sh 0744
Need fixing - use 0755
> sectool.noarch: W: symlink-should-be-relative /usr/bin/sectool
/usr/share/sectool/sectool.py
Fixing this is not quite necessary IMHO, but it can't hurt.
> sectool.noarch: E: non-standard-executable-perm
/usr/share/sectool/tests/python_defs.py 0744
Needs fixing - use 0644
> sectool.noarch: W: incoherent-version-in-changelog 0.1.0 0.1.0-1.fc8
Use "0.1.0-1" in the %changelog version number
> sectool.noarch: W: dangerous-command-in-%post rm
> sectool.noarch: W: bogus-variable-use-in-%post $RPM_BUILD_ROOT
Definitely needs fixing:
* $RPM_BUILD_ROOT is no longer relevant in %post
* Why does that script remove the .py[co] files anyway?

> sectool-gui.noarch: W: no-documentation
OK
> sectool-gui.noarch: E: script-without-shebang
/etc/security/console.apps/sectool-gui
> sectool-gui.noarch: E: script-without-shebang /etc/pam.d/sectool-gui
These files should not be executable.
> sectool-gui.noarch: E: non-executable-script /usr/share/sectool/guiOutput.py 0644
> sectool-gui.noarch: E: non-executable-script /usr/share/sectool/guiRender.py 0644
Remove the #!... line.
> sectool-gui.noarch: E: non-standard-executable-perm
/usr/share/sectool/sectool-gui.py 0744
Use 0755
> sectool-gui.noarch: W: no-dependency-on usermode
Needs fixing
> sectool-gui.noarch: W: spelling-error-in-description extention extension
Needs fixing
Comment 2 Miloslav Trmač 2008-01-15 18:00:03 EST
* The source0: URL does not point to the tarball, the real URL is
https://hosted.fedoraproject.org/sectool/attachment/wiki/WikiStart/sectool-0.1.0.tar.bz2?format=raw
  Perhaps use only "%{name}-%{version}.tar.bz2" and put the full URL in a
  comment.
FIXME: source0: content in this case
* The uploaded tarball does not match the tarball in src.rpm!!
* License: GPL+ is probably correct, but do you _want_ it to be GPL+?
  (RH copyright guidelines say something else)
* ExclusiveOS: useless, just drop it
* Two source0 lines!
* sectool-gui should probably Requires: sectool = %{version}-%{release}
* Requires: gtk2 - You probably want pygtk2.
* desktop file: "HardwareSettings" isn't true
* Drop the (cat %{name}.lang)
* Add "%doc README:
* Use one of the recommended BuildRoot: values
* sectool.desktop should be shipped in sectool-gui
* sectool.desktop: Remove .png from Icon=
* Add --vendor=fedora to desktop-file-install
* A %defattr directive is missing in %files gui

* URL: perhaps use https://hosted.fedoraproject.org/sectool/wiki/WikiStart to
  avoid a certificate mismatch warning
* Consider using macros for paths; at least you can
  (make PREFIX=%{_prefix} ...and so on for other variables),
  perhaps add more variables to the Makefile to remove other hard-coded
  directory paths like /etc, /usr/bin and /var/lib/sectool and /usr/share
* Can you use (make %{?_smp_mflags}) ?
* %description: s/inerface/interface
* %description gui: To me this reads like "sectool provides a GUI";
  perhaps just "sectool-gui provides a GTK-based graphical user interface to
  sectool."?
Comment 3 Bill Nottingham 2008-01-15 22:31:43 EST
sectool seems to be a fairly popular name for a tool, judging by google. Not
that I have a better one.
Comment 5 Peter Vrabec 2008-01-16 12:28:20 EST
Update!
Spec URL:
http://people.redhat.com/pvrabec/rpms/sectool/sectool.spec

SRPM URL:
http://people.redhat.com/pvrabec/rpms/sectool/sectool-0.1.0-2.fc7.src.rpm

Note, I didn't update web pages(wiki). If new rpm pass review, I'll do that. I 
hope it will be much better now, rpmlint is quite.
Comment 6 Miloslav Trmač 2008-01-16 13:03:08 EST
* rpmlint output:
> sectool-gui.noarch: W: no-documentation
OK

* License: The field was changed to GPLv2+, but the code still does not contain
  copyright notices specifying the intended license.  The License: field might
  be sufficient - but just to be sure, please add a copyright header to each
  source file, per the RH copyright guidelines

* macros:
> Consider using macros for paths; at least you can
> (make PREFIX=%{_prefix} ...and so on for other variables),
> perhaps add more variables to the Makefile to remove other hard-coded
> directory paths like /etc, /usr/bin and /var/lib/sectool and /usr/share
-2 is not an improvement in this regard: macros are used in %files and
hard-coded paths in the Makefile: if the macro values change, the build will 
break.  For each path, either use a hard-coded path both in the Makefile and the
spec, or use a macro in the spec and pass the macro value to the Makefile somehow.

(If you decide this isn't worth it and revert to using hard-coded paths, I won't
protest.)


> Note, I didn't update web pages(wiki). If new rpm pass review, I'll do that.
OK.  (In the future, please don't change released tarballs like this - it might
become quite a mess if different distributions use different tarballs with the
same version number.)
Comment 7 Peter Vrabec 2008-01-17 09:43:28 EST
Update!
Spec URL:
http://people.redhat.com/pvrabec/rpms/sectool/sectool.spec

SRPM URL:
http://people.redhat.com/pvrabec/rpms/sectool/sectool-0.1.0-3.fc7.src.rpm

- license issues are fixed
- macros have been improved. For each macro in spec file is equivalent
Makefile macro. However spec file macros are not passed to Makefile. I don't 
expect that there will be any changes soon and if it happends there is easy 
fix. Lots of similar packages do it same way.

Comment 8 Miloslav Trmač 2008-01-17 12:06:27 EST
-3 doesn't build on f8:
> + /usr/lib/rpm/check-buildroot
> /var/tmp/sectool-0.1.0-root/usr/share/sectool/sectool.py:               
action="store", metavar="CONFIG",
default="/var/tmp/sectool-0.1.0-root/etc/sectool.conf",
> Found '/var/tmp/sectool-0.1.0-root' in installed files; aborting

At least these lines are incorrect:
>         sed -i
's,default=\".*\",default=\"$(DESTDIR)${SYSCONFDIR}/sectool\.conf\",'
$(DESTDIR)$(PKGDATADIR)/sectool.py
>         sed -i
's,scheduler\.SecToolConf(\".*\"),scheduler\.SecToolConf(\"$(DESTDIR)${SYSCONFDIR}/sectool\.conf\"),'
$(DESTDIR)$(PKGDATADIR)/sectool-gui.py

DESTDIR is "install under this root", not "at run-time, all files will be under
this prefix". 
Comment 10 Miloslav Trmač 2008-01-20 21:58:15 EST
> SRPM URL:
> http://people.redhat.com/pvrabec/rpms/sectool/sectool-0.1.0-4.fc8.src.rpm

Approved, UNDER THE CONDITION that sectool-0.1.0.tar.bz2 in the package and on
the wiki will match.  (And in the future, please don't ever change either of the
tarballs without incrementing a version number.)
Comment 11 Peter Vrabec 2008-01-21 04:36:45 EST
(In reply to comment #10)
> > SRPM URL:
> > http://people.redhat.com/pvrabec/rpms/sectool/sectool-0.1.0-4.fc8.src.rpm
> 
> Approved, UNDER THE CONDITION that sectool-0.1.0.tar.bz2 in the package and 
on
> the wiki will match.
done

Comment 12 Peter Vrabec 2008-01-21 04:39:10 EST
New Package CVS Request
=======================
Package Name: sectool
Short Description: - A security audit system and intrusion detection system
Owners: pvrabec
Branches:
InitialCC:
Cvsextras Commits: yes
Comment 13 Kevin Fenzi 2008-01-21 12:15:07 EST
cvs done.

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