Bug 727030 - Review Request: ufw - uncomplicated firewall
Summary: Review Request: ufw - uncomplicated firewall
Keywords:
Status: CLOSED WONTFIX
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-08-01 04:59 UTC by Nathan Owe
Modified: 2012-04-09 00:56 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2012-04-09 00:56:59 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
ufw sample systemd service (185 bytes, application/octet-stream)
2011-08-30 20:23 UTC, Haïkel Guémar
no flags Details

Description Nathan Owe 2011-08-01 04:59:56 UTC
Spec URL: http://ndowens.fedorapeople.org/SPECS/ufw.spec
SRPM URL: http://ndowens.fedorapeople.org/SRPM/ufw-0.30.1-1.fc15.src.rpm
Description: 
The Uncomplicated Firewall(ufw) is a front-end for netfilter, which
aims to make it easier for people unfamiliar with firewall concepts.
Ufw provides a framework for managing netfilter as well as 
manipulating the firewall.

Comment 1 Nathan Owe 2011-08-01 05:03:37 UTC
The only errors I get is a spelling error for netfilter
I also get hardcoded-library-path /lib even though I tried to work around it by creating a macro __libdir /lib to get around that.

[ndowens@revan rpmbuild]$ rpmlint SRPMS/ufw-0.30.1-1.fc15.src.rpm 
ufw.src: W: spelling-error %description -l en_US netfilter -> net filter, net-filter, filterer
ufw.src:1: E: hardcoded-library-path in /lib
1 packages and 0 specfiles checked; 1 errors, 1 warnings.

Comment 2 Veeti Paananen 2011-08-01 15:31:34 UTC
Correct me if I'm wrong, but couldn't you use %{_lib}? (http://fedoraproject.org/wiki/Packaging:RPMMacros#Other_macros_and_variables_for_paths)

Comment 3 Susi Lehtola 2011-08-01 16:24:19 UTC
Why on earth does it install something into /lib anyway?
 %{__libdir}/ufw/ufw*
 %{__libdir}/ufw/user*.rules
looks a lot like configuration files.

Besides, you're missing ownership of the following directories:
 %{__libdir}/ufw/
 %{python_sitelib}/ufw/
 %{_datadir}/ufw/
Are you a packager, yet..?

Comment 4 Nathan Owe 2011-08-02 03:13:38 UTC
To start off, yes I am a packager :)
@Veeti: _lib doesn't point to /lib that is why I had to create a variable/macro

Well apparently the coding they used is creating a problem even in trying to run ufw status.

So I have updated my SPEC file repairing two problems:
1. Ownership of dirs
2. The coding in the file used variables like #CONFIG_DIR#/ when parsing the file the variable wouldn't be understood. So I had to create a patch pointing the variables to the correct directories.

SPEC: http://ndowens.fedorapeople.org/SPECS/ufw.spec
SRPM: http://ndowens.fedorapeople.org/SRPM/ufw-0.30.1-2.fc15.src.rpm

Comment 5 Susi Lehtola 2011-08-02 07:51:34 UTC
(In reply to comment #4)
> To start off, yes I am a packager :)

Okay. I was just wondering why you didn't make %files simpler:

%files
%doc ChangeLog COPYING README* TODO AUTHORS
%{__libdir}/ufw/
%{_datadir}/ufw/
%{_sbindir}/ufw
%{python_sitelib}/ufw-%{version}-py*.egg-info
%{python_sitelib}/ufw/
%config(noreplace) %{_sysconfdir}/default/
%config(noreplace) %{_sysconfdir}/ufw/
%{_mandir}/man8/ufw-framework.8*
%{_mandir}/man8/ufw.8*

Comment 6 Nathan Owe 2011-08-02 14:09:13 UTC
Mainly because some people say that the file list should be more detailed.

Comment 7 Susi Lehtola 2011-08-02 14:31:23 UTC
(In reply to comment #6)
> Mainly because some people say that the file list should be more detailed.

I'm one of them: for instance, I abhor too freeminded use of wildcards such as
 %{python_sitelib}/*
which may up owning stuff that it isn't supposed to. Or, for the current example, one might not detect if the egg-info isn't built (which may happen e.g. on EPEL-5, where one has to do a trick to get it).

However, there's no sense in listing every single file that goes in the RPM, if they are placed in a package-specific directory, or obey some sane naming scheme, e.g.
 %{_bindir}/foo
 %{_bindir}/foo-*

Being too specific can also bite you, as you noticed.

Comment 8 Nathan Owe 2011-08-02 16:11:00 UTC
SPEC: http://ndowens.fedorapeople.org/SPECS/ufw.spec
SRPM: http://ndowens.fedorapeople.org/SRPM/ufw-0.30.1-3.fc15.src.rpm

Those are the updated version

Comment 9 Susi Lehtola 2011-08-02 17:24:13 UTC
Some more comments:


Please use the %{version} macro for
 %{python_sitelib}/ufw-0.30.1*
And, since it is an egg-info file, it's better to be a bit more explicit:
 %{python_sitelib}/ufw-%{version}-py*.egg-info.

**

You are still missing ownership of
 %{_datadir}/ufw/iptables/
Please combine
 %{_datadir}/ufw/iptables/*.rules
 %dir %{_datadir}/ufw/
to simply
 %{_datadir}/ufw/

**

You are mixing styles:
http://fedoraproject.org/wiki/Packaging/Guidelines#Using_.25.7Bbuildroot.7D_and_.25.7Boptflags.7D_vs_.24RPM_BUILD_ROOT_and_.24RPM_OPT_FLAGS

Since this is a noarch package, you should drop CFLAGS="$RPM_OPT_FLAGS" from the build statement.

**

I'm also not sure why you want to break lines to exceedingly small length in

%{__python} setup.py install \
--skip-build \
--root %{buildroot} 

but the very next line is very long.

**

By the way, there is a reason why firewall rules are not world readable. They should be owned by root and installed as 600.

**

What does Patch0 do? Please add a comment about it in the spec file.

**

Note that you do not need to use the %{__python} macro, plain "python" will do just fine.

**

I am wondering whether the use of a scrambled email address is allowed, but the guideline is a bit unclear. I'll get this cleared up.

Comment 10 Nathan Owe 2011-08-02 23:13:47 UTC
SPEC: http://ndowens.fedorapeople.org/SPECS/ufw.spec
SRPM: http://ndowens.fedorapeople.org/SRPM/ufw-0.30.1-4.fc15.src.rpm

On the scrambled email address. I hope it is allowed to help combat spambots from getting it so my email won't get cluttered with spam.

I am not sure what to do about the following init scripts:
/lib/ufw/ufw-init
/lib/ufw/ufw-init-functions

I know ufw enable will pretty much do /lib/ufw/ufw-init start.
So really do I need to install it to /etc/rc.d/init.d even though ufw looks for them in /lib and I also tried doing something like it shows in the wiki:
/sbin/chkconfig --add /lib/ufw/ufw-init and it gives a error like "error reading information on service ufw-init: No such file or directory"

Comment 11 Nathan Owe 2011-08-02 23:21:24 UTC
Also can somebody install ufw and add/delete a firewall rule in it and see if it does have an effect. When I do a port scan on my PC it doesn't seem to make a difference.

Comment 12 Susi Lehtola 2011-08-03 06:34:47 UTC
(In reply to comment #10)
> On the scrambled email address. I hope it is allowed to help combat spambots
> from getting it so my email won't get cluttered with spam.

Seems like scrambling is OK.

Comment 13 Nathan Owe 2011-08-04 01:35:35 UTC
(In reply to comment #11)
> Also can somebody install ufw and add/delete a firewall rule in it and see if
> it does have an effect. When I do a port scan on my PC it doesn't seem to make
> a difference.

Well just remembered that I am also behind a router so that might be why I see no difference.

Comment 14 Nathan Owe 2011-08-11 17:22:50 UTC
I am not sure what to do about the following init scripts:
/lib/ufw/ufw-init
/lib/ufw/ufw-init-functions

I know ufw enable will pretty much do /lib/ufw/ufw-init start.
So really do I need to install it to /etc/rc.d/init.d even though ufw looks for
them in /lib and I also tried doing something like it shows in the wiki:
/sbin/chkconfig --add /lib/ufw/ufw-init and it gives a error like "error
reading information on service ufw-init: No such file or directory"

Comment 15 Haïkel Guémar 2011-08-28 12:33:04 UTC
according to ufw documentation, you need to provide your own initscripts, these are only helpers.
Forget about SysV initscripts, and provide a systemd service instead (i attached a working albeit basic one)

* drop the BR: iptables-devel, you don't need it
* using %find_lang is a must, they should be installed in location accordingly to GNU standards. You may have to check this with upstream
http://www.gnu.org/prep/standards/html_node/Directory-Variables.html

Comment 16 Nathan Owe 2011-08-30 19:32:53 UTC
I don't see any attached files.

Comment 17 Haïkel Guémar 2011-08-30 20:23:48 UTC
Created attachment 520684 [details]
ufw sample systemd service

Sorry, i forgot to add it.

Comment 18 Nathan Owe 2011-09-01 18:09:02 UTC
I created my service file before you attached the file and it looks exactly like that, except it didn't include Type, since if left out, the type would be simple by default and it doesn't want to work. To install it, I simply did:

Source1:   %{name}.service

%install
...

install -Dm644 %{SOURCE1} %{_unitdir}/%{name}.service

Comment 19 Nathan Owe 2012-04-09 00:56:59 UTC
Deciding to withdraw from Review.


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