Bug 321731

Summary: Review Request: Shorewall Version 4 - Iptables-based firewall - Review Tracker bug
Product: [Fedora] Fedora Reporter: Jonathan Underwood <jonathan.underwood>
Component: Package ReviewAssignee: Nobody's working on this, feel free to take it <nobody>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora-package-review, notting, robert
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: 4.0.5-1.fc8 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2007-11-20 18:10:25 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Bug Depends On: 321691, 321711, 321721    
Bug Blocks: 250641    

Description Jonathan Underwood 2007-10-07 01:03:40 UTC
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. 

The version 3 release series of Shorewall is already available in Fedora. With the release of version 4, upstream has added a new perl based rule compiler and completely changed the way the package is distributed. The shell-based and perl-based compilers are each distributed as individual tarballs, and files required to run shorewall with either compiler are packaged as a third tarball, shorewall-common. Version 3 however comprised only a single tarball, and so only a single package is required.

In order to provide a clean upgrade route, shorewall-common creates a shorewall subpackage which requires shorewall-common, shorewall-perl and shorewall-shell.

This is a tracking bug for the reviews of the other packages comprising version 4 of shorewall:

shorewall-common review is BZ #321691
shorewall-perl review is BZ #321711
shorewall-shell review is BZ #321721

There is also shorewall-lite, a light-weight Shorewall version that will run compiled firewall scripts generated on a system with one of the compiler packages installed - I'll package this up in the near future.

Comment 1 Jonathan Underwood 2007-10-07 01:06:11 UTC
Added current shorewall package owner to cc. 

Robert - I'm not trying to usurp your package here, but I thought that because
upstream has changed so much, and because Id done the packaging work for other
reasons, it would be useful to put them into BZ for review. I am more than happy
if you want to continue owning this package. Am also happy to co-maintain
shorewall with you, if you like.

Comment 2 Jonathan Underwood 2007-10-07 23:40:00 UTC
Following discussion with Robert, and also on #fedora-devel, consensus seems to
be that it is better to have a single package with all tarballs. Therefore, I'm
closing this review, and discussion of the multi-tarball package will continue in
this bugzilla entry.



Comment 3 Jonathan Underwood 2007-10-07 23:48:51 UTC
Spec URL: http://jgu.fedorapeople.org/shorewall.spec
SRPM URL: http://jgu.fedorapeople.org/shorewall-4.0.4-1.fc7.src.rpm

This is the combined tarball package. Plus it now contains the shorewall-lite
functionality. 

The main package (shorewall) simply Requires the sub-packages shorewall-common,
shorewall-perl and shorewall-shell. This is the pragmatic approach, aimed at not
breaking existing installations that require the legacy shell compiler. The
clued up user could install only shorewall-common and shorewall-perl or
shorewall-shell if she only wanted one compiler. There is also a sub-package for
shorewall-lite. 

Running rpmlint on the subpackages:
$ rpmlint ../RPMS/noarch/shorewall-common-4.0.4-1.fc7.noarch.rpm 
shorewall-common.noarch: W: service-default-enabled /etc/rc.d/init.d/shorewall
--> Bogus, as the Defaul-Start entry is empty

shorewall-common.noarch: E: subsys-not-used /etc/rc.d/init.d/shorewall
--> This is ok, as although the init file doesn't create a lockfile, shorewall
itself does.

shorewall-common.noarch: W: incoherent-init-script-name shorewall
--> Irrelevant

$ rpmlint ../RPMS/noarch/shorewall-perl-4.0.4-1.fc7.noarch.rpm
No warnings or errors

$ rpmlint ../RPMS/noarch/shorewall-shell-4.0.4-1.fc7.noarch.rpm 
shorewall-shell.noarch: E: non-executable-script
/usr/share/shorewall-shell/prog.header 0644
--> prog.header has a hash-bang at the top, but isn't executable as this file is
simply a template for shorewall to use when generating a shell script, and so it
shouldn't be executeable.

$ rpmlint ../RPMS/noarch/shorewall-lite-4.0.4-1.fc7.noarch.rpm 
shorewall-lite.noarch: W: non-conffile-in-etc /etc/shorewall-lite/Makefile
--> A weird one. that Makefile is used by shorewall-lite and is intentionally
put in /etc by upstream... not sure waht to do about this really.

shorewall-lite.noarch: W: service-default-enabled /etc/rc.d/init.d/shorewall-lite
--> bogus again - Default-Start is empty, Default-Stop has all runlevels.

shorewall-lite.noarch: E: subsys-not-used /etc/rc.d/init.d/shorewall-lite
--> Again, shorewall-lite handles lock file creation itself, there's no need for
the init script to create one.


Comment 4 Jonathan Underwood 2007-10-07 23:59:50 UTC
A remaining question is what to do about Ports.pm. Please see BZ #321711 for a
discussion about this. 

Synopsis: shorewall-perl requires the file:
/usr/share/shorewall-perl/Shorewall/Ports.pm

Ports.pm is built by buildports.pl which looks at entries in
/etc/{services,protocols} and builds Ports.pm accordingly. 

/etc/{services,protocols} are part of the setup package.

The current strategy in the spec file is that Ports.pm is created at package
build time by Build:requiring the setup package. However, the following issues
then arise:
A) What if the user has modified locally /etc/{services,protocols}
B) What if another package modifies /etc/{services,protocols} on package
installation (Ville recalled that the LTSP stuff might do this)

The only way around that would be to create Ports.pm on package installation.
This could be done in %post, and indeed the upstream sample spec does just that.

But, Ville also points out that creating files under /usr on package
installation fails if /usr is read-only (but then again, so will package
installation). So, then we'd have to do the building of Ports.pm under
/var/lib/shorewall-perl, and package a symlink there from
/usr/share/shorewall-perl. 

This is all do-able, but I can't help preferring the KISS approach of generating
Ports.pm at package build time using /etc/{services,protocols} from the setup
package. For case B above I would argue that the LTSP packager should have his
changes to /etc/{services,protocols} incoorporated into the setup package files.

... Thoughts?

Comment 5 Jonathan Underwood 2007-10-08 00:11:33 UTC
Oops - in Comment #2, please ignore the part that reads "I'm closing this review"!

Comment 6 Jonathan Underwood 2007-10-08 00:21:04 UTC
For background, the reason that Ports.pm is generated rather than direclty
parsing /etc/{services,protocols} is given by upstream:
3)  Previously, Shorewall-perl read /etc/protocols and /etc/services
    during compiler startup to build internal protocol and service
    tables. This had a fixed cost of up to one half second or more,
    depending on the speed of the system and the distribution
    (The /etc/services released with OpenSuSE 10.2 is over 14,000
    lines!!) These tables are now initialized by the Perl compiler
    which speeds up compilation considerably.

    During installation, Shorewall generates the Perl module
    /usr/share/shorewall-perl/Shorewall/Ports.pm, using your
    /etc/protocols and /etc/services as input.

    To re-generate the module from those two files:
        1. Backup your current /usr/share/shorewall-perl/Shorewall/Ports.pm
           file.
        2. /usr/share/shorewall-perl/buildports.pl > \
           /usr/share/shorewall-perl/Shorewall/Ports.pm

    Note: If the buildports.pl program fails to run to a successful
    completion during installation, a fallback version of
    module will be installed. That fallback module was generated from
    the /etc/protocols and /etc/services shipped with Ubuntu Feisty
    Fawn.

    Even if the buildports.pl program runs successfully, the fallback
    module is also installed as
    /usr/share/shorewall-perl/Shorewall/FallbackPorts.pm. So if you
    encounter problems with the generated module, simply copy the
    fallback module to /usr/share/shorewall-perl/Shorewall/Ports.pm.


Comment 7 Ville Skyttä 2007-10-08 16:24:09 UTC
(In reply to comment #3)
> $ rpmlint ../RPMS/noarch/shorewall-common-4.0.4-1.fc7.noarch.rpm 
> shorewall-common.noarch: W: service-default-enabled /etc/rc.d/init.d/shorewall
> --> Bogus, as the Defaul-Start entry is empty

Indeed, rpmlint bug (ditto the -lite one), now fixed in rpmlint upstream svn.

Comment 8 Jonathan Underwood 2007-10-08 22:50:51 UTC
Spec URL: http://jgu.fedorapeople.org/shorewall.spec
SRPM URL: http://jgu.fedorapeople.org/shorewall-4.0.4-2.fc7.src.rpm

* Mon Oct  8 2007 Jonathan G. Underwood <jonathan.underwood@gmail.com> - 4.0.4-2
- Add ghost files for /var/lib/shorewall/.modules and /var/lib/shorewall/.modulesdir
- Fix ownership of /var/lib/shorewall-lite

Comment 9 Jonathan Underwood 2007-10-09 09:32:28 UTC
I have discussed the whole buildports/Ports.pm issue with upstream maintainers,
and a better way of dealing with this has been established which removes the
need for Ports.pm and buildports - this will happen in the next release 4.0.5 in
the very near future. So i see no point tackling the issue in this package
currently. I think therefore there's nothing more to be done with this package,
it's ready to go, subject to review. Technically, since shorewall exists already
as a Fedora package it doesn't need a review, but in this case, with so many
changes, it is probably adviseable to review.

Comment 10 Jonathan Underwood 2007-10-27 22:19:56 UTC
Spec URL: http://jgu.fedorapeople.org/shorewall.spec
SRPM URL: http://jgu.fedorapeople.org/shorewall-4.0.5-1.fc7.src.rpm

This is an updated package for version 4.0.5. With this new version, there is no
longer any need to cache /etc/services,protocols and so the buildports.pl issue
has gone away. 

With this version, there are no outstanding issues from a packaging perspective
that I am aware of, so hopefully we can get this into the repos soon!

Comment 11 Jonathan Underwood 2007-11-08 20:48:56 UTC
Since the packaging of shorewall hasn't changed substantially with this version
(unlike my initial package split into shorewall-common, -perl, -lite, -shell), I
don't think this necessarily needs review, it's merely an update to an existing
package (as opposed to creating 4 new packages).

Robert, are you ok with this? If so, please can you grant me the rights on the
packagedatabase that I asked for to allow me to co-maintain this package with you?

Comment 12 Robert Marcano 2007-11-12 14:10:33 UTC
done

Comment 13 Jonathan Underwood 2007-11-12 14:15:50 UTC
Thanks Robert  - will start updating packages this evening.

Comment 14 Jonathan Underwood 2007-11-13 00:18:57 UTC
Updated packages built for F-7, F-8 and devel. Closing this bug.

Comment 15 Fedora Update System 2007-11-15 03:34:41 UTC
shorewall-4.0.5-1.fc8 has been pushed to the Fedora 8 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update shorewall'

Comment 16 Fedora Update System 2007-11-20 18:10:22 UTC
shorewall-4.0.5-1.fc8 has been pushed to the Fedora 8 stable repository.  If problems still persist, please make note of it in this bug report.