Bug 169378 - Review Request: shorewall
Review Request: shorewall
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
All Linux
medium Severity medium
: ---
: ---
Assigned To: Aurelien Bompard
David Lawrence
Depends On:
  Show dependency treegraph
Reported: 2005-09-27 14:18 EDT by Robert Marcano
Modified: 2010-08-10 07:24 EDT (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2005-10-11 15:02:02 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---

Attachments (Terms of Use)

  None (edit)
Description Robert Marcano 2005-09-27 14:18:56 EDT
Spec Name or Url: http://www.marcanoonline.com/downloads/fedora/package_submissions/shorewall/shorewall.spec
SRPM Name or Url: http://www.marcanoonline.com/downloads/fedora/package_submissions/shorewall/shorewall-2.4.4-2.src.rpm
Description: The Shoreline Firewall, more commonly known as "Shorewall", is a Netfilter
(iptables) based firewall that can be used on a dedicated firewall system,
a multi-function gateway/router/server or on a standalone GNU/Linux system.

It has been recently removed from extras, I updated it and I am trying to be the maintainer of this package
Comment 1 Aurelien Bompard 2005-09-27 18:44:13 EDT
Needs work:
* the conf files in /etc/shorewall should be set with flag noreplace
* /usr/share/shorewall dir should be 755 and the files in it should be readable,
since they are not conf files there is no point in making them 600.
* the service is enabled by default, please run a substitution on
/etc/init.d/shorewall to change the chkconfig line
* Requirements: missing dependancy on chkconfig in %post and %preun
* Scriptlets: missing "service" command in %preun and %postun 
  (wiki: ScriptletSnippets)
* In the files list, /sbin should be replaced with %{_sbindir}

* remove empty %build section
* %{_prefix}/share should be replaced with %{_datadir}
Comment 2 Ville Skyttä 2005-09-28 02:14:57 EDT
%{_sbindir} != /sbin 
Comment 3 Aurelien Bompard 2005-09-28 02:25:01 EDT
(In reply to comment #2)
> %{_sbindir} != /sbin 

Ooops, right :)
Please forget about this one.
Comment 4 Robert Marcano 2005-09-28 10:07:18 EDT
Thanks for the review. It has been updated... see


I disabled the service autostart using a patch and no a substitution. I think is
more easy to detect when the source init script is changed when the patch does
not applies anymore
Comment 5 Robert Marcano 2005-09-28 10:40:31 EDT
Well I still have a few warnings.. It is my first time running rpmlint :-(

> W: shorewall no-version-in-last-changelog
My mistake

> E: shorewall non-standard-dir-perm /var/lib/shorewall 0700
> E: shorewall non-readable /var/lib/shorewall/proxyarp 0600
> E: shorewall non-readable /var/lib/shorewall/nat 0600
> E: shorewall non-readable /var/lib/shorewall/chains 0600
> E: shorewall non-readable /var/lib/shorewall/zones 0600
> E: shorewall non-readable /var/lib/shorewall/restarted 0600
This files are generated by shorewall, and some are shell scripts that must be
hidden to non root users. When they do not exist, shorewall creates them with
0600. I think that the author of the original SPEC file add them empty in order
to be sure they are removed when uninstalled

> E: shorewall non-standard-executable-perm /sbin/shorewall 0554
Will be fixed to 0774

> E: shorewall non-executable-script /usr/share/shorewall/firewall 0644
> E: shorewall non-executable-script /usr/share/shorewall/functions 0644
> E: shorewall non-executable-script /usr/share/shorewall/help 0644
This scripts are sourced by other shell scripts, they are not called directly,
so I will let them non-executable

> W: shorewall no-reload-entry /etc/rc.d/init.d/shorewall
Will be fixed

> E: shorewall subsys-not-used /etc/rc.d/init.d/shorewall
What this means?
Comment 6 Aurelien Bompard 2005-09-28 11:30:41 EDT
(In reply to comment #5)
> > E: shorewall subsys-not-used /etc/rc.d/init.d/shorewall
> What this means?

Regular daemons should write their PID in a file in /var/lock/subsys. In
shorewall's case, you can ignore it.

Comment 8 Aurelien Bompard 2005-09-29 00:30:26 EDT
Review for release 4:
* RPM name is OK
* Source shorewall-2.4.4.tar.bz2 is the same as upstream
* This is the latest version
* Builds fine in mock
* rpmlint of shorewall looks OK
* File list of shorewall looks OK
* Works fine.
Comment 9 Robert Marcano 2005-10-09 15:37:09 EDT
Updated to 2.4.5
Comment 10 Aurelien Bompard 2005-10-09 16:08:03 EDT
Review for release 1:
* Changes are OK
* Source shorewall-2.4.5.tar.bz2 is the same as upstream
Comment 11 Robert Marcano 2005-10-11 15:02:02 EDT
Build logs:


is this the last step in order to push it to the repositories?
Comment 12 Aurelien Bompard 2005-10-12 01:35:31 EDT
Yes, the package is in the repos now. Thanks for maintaining it.
Comment 13 Jonathan Underwood 2010-08-07 16:36:48 EDT
Please create a EL6 branch.
Comment 14 Kevin Fenzi 2010-08-09 13:22:13 EDT
There is already a EL-6 branch. ;)
Comment 15 Jonathan Underwood 2010-08-10 07:24:15 EDT
Oh yes, so there is, apologies. I'll blame early onset Alzheimers.:)

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