Bug 473972 - Review Request: nufw - Authentication Firewall Suite for Linux
Review Request: nufw - Authentication Firewall Suite for Linux
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Dan Horák
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-12-01 12:03 EST by Jerome Soyer
Modified: 2013-03-29 09:01 EDT (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-01-19 04:15:28 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
dan: fedora‑review+


Attachments (Terms of Use)

  None (edit)
Description Jerome Soyer 2008-12-01 12:03:41 EST
Spec URL: http://saispo.free.fr/fedora/nufw.spec
SRPM URL: http://saispo.free.fr/fedora/nufw-2.2.19-1.fc10.src.rpm
Description:
NuFW is a firewall able to filter connection according to user uid or user
software, meaning you can allows port 80 for only one user, whatever ip he
uses, or only for konqueror.

NuFW performs an authentication of every single connection passing through the
IP filter, by transparently requesting user's credentials before any filtering
decision is taken. Practically, this means security policies can integrate with
the users directory, and bring the notion of user ID down to the IP layers.

--

Hi ! I have just finished packaging nufw, can you review it and upload it please ?

Just a little information, i package nufw for an another rpm based distro and i maintain it too.

Thanks in advance.
Comment 1 Jerome Soyer 2008-12-02 06:09:15 EST
Hi,

I fixed some mistakes on my spec file and update it !

Thanks akurtakov ;-)
Comment 2 Dan Horák 2008-12-07 10:29:49 EST
So first round of issues found during the review
- wrong Group tags are used - check /usr/share/doc/rpm-*/GROUPS for valid group names, I suggest to use System Environment/Daemons (nufw, nuauth) , System Environment/Libraries (client library, pam), Applications/Internet (utils), Development/Libraries (for -devel), Development/Languages (for the python bindings)
- drop the %{epoch} from the Provides/Requires/... as it is not defined/used
- preserve timestamps on manually installed files - use "cp -p" or "install -p"
- license is GPLv2 only, there is no "or any later version" clause in the sources
- license text included in the source archive, but not included in any package as %doc
- %{_sysconfdir}/nufw/ is owned by multiple packages (nufw, nufw-utils, nuauth), only one should really own it, other packages get it through the dependencies
- wrong initscript scriplets and package dependencies
- wrong shared library scriptlets (%postun is missing)
- %defattr(-,root,root,-) should be used
- no need to specify --localstatedir when running %configure, it is set to /var automatically
- static library and *.la archive are packaged in -devel (use %exclude in %files or "rm" in %install)
- including the whole "doc" directory as %doc is not necessary, grab only the README.* and the non-manpages (acls, cache_system and debug)
- the included initscript are not compliant with Fedora - LSB header is missing, service is enabled by default, ...
- the auth and log modules are linked as shared libs (including the version info), so they install *.so, *.so.0 and *.so.0.0.0 - they should use "-avoid-version" in the link command
- don't use /var/... in %file, use %{_localstatedir} instead
- the %description for -utils talks about nutcpc, but it is stored in its own sub-package
- split the Provides for nuauth to multiple lines
- there is no need for the Obsoletes for nuauth
- the Requires for nuauth are wrong, there are no such packages like sasl-plug-*, you want probably cyrus-sasl-*
- the pam module contains rpath and the BuildRequire: chrpath is not used
- the library subpackage is usually called foo-libs with headers in foo-devel, you can "Provide: libnuclient" for compatibility
- why not to rename the nufw-nuauth-* packages only to nuath-*

Please read the https://fedoraproject.org/wiki/Packaging/Guidelines and linked pages for details.
Comment 3 Jerome Soyer 2008-12-08 09:31:49 EST
Thanks for the review, i will fix this and bring you a better spec file.

Go to work on it ;-)
Comment 4 Jerome Soyer 2008-12-08 10:33:04 EST
So first round of issues found during the review
- wrong Group tags are used - check /usr/share/doc/rpm-*/GROUPS for valid group
names, I suggest to use System Environment/Daemons (nufw, nuauth) , System
Environment/Libraries (client library, pam), Applications/Internet (utils),
Development/Libraries (for -devel), Development/Languages (for the python
bindings) -> *FIXED*
- drop the %{epoch} from the Provides/Requires/... as it is not defined/used -> *FIXED*
- preserve timestamps on manually installed files - use "cp -p" or "install -p" -> *FIXED*
- license is GPLv2 only, there is no "or any later version" clause in the
sources -> *FIXED*
- license text included in the source archive, but not included in any package
as %doc -> *FIXED*
- %{_sysconfdir}/nufw/ is owned by multiple packages (nufw, nufw-utils,
nuauth), only one should really own it, other packages get it through the
dependencies -> it depends... you may have nuauth and nufw on different server, i think multiple packages need %{_sysconfdir}/nufw/
- wrong initscript scriplets and package dependencies -> *FIXED*
- wrong shared library scriptlets (%postun is missing) -> *FIXED*
- %defattr(-,root,root,-) should be used -> *FIXED*
- no need to specify --localstatedir when running %configure, it is set to /var
automatically -> *FIXED*
- static library and *.la archive are packaged in -devel (use %exclude in
%files or "rm" in %install) -> *FIXED*
- including the whole "doc" directory as %doc is not necessary, grab only the
README.* and the non-manpages (acls, cache_system and debug) -> *FIXED*
- the included initscript are not compliant with Fedora - LSB header is
missing, service is enabled by default, ... -> *FIXED*
- the auth and log modules are linked as shared libs (including the version
info), so they install *.so, *.so.0 and *.so.0.0.0 - they should use
"-avoid-version" in the link command -> *FIXED*
- don't use /var/... in %file, use %{_localstatedir} instead -> *FIXED*
- the %description for -utils talks about nutcpc, but it is stored in its own
sub-package -> *FIXED*
- split the Provides for nuauth to multiple lines -> *FIXED*
- there is no need for the Obsoletes for nuauth -> *FIXED*
- the Requires for nuauth are wrong, there are no such packages like
sasl-plug-*, you want probably cyrus-sasl-* -> *FIXED*
- the pam module contains rpath and the BuildRequire: chrpath is not used -> *FIXED*
- the library subpackage is usually called foo-libs with headers in foo-devel,
you can "Provide: libnuclient" for compatibility -> *FIXED*
- why not to rename the nufw-nuauth-* packages only to nuath-* -> i prefer, it's same as Mandriva, Debian, Ubuntu, etc... module is more flexible than a monolithic package, no ?

You will find all the new file at :

http://saispo.free.fr/fedora/nufw.spec
http://saispo.free.fr/fedora/nuauth.init
http://saispo.free.fr/fedora/nufw.init

I have no fedora under my hand for building a SRPM :-/ I can give you one tomorrow.

Thanks in advance for your review.
Comment 5 Jerome Soyer 2008-12-09 05:08:09 EST
Hi,

you can find the new spec and srpms file on :

http://saispo.fedorapeople.org/nufw.spec
http://saispo.fedorapeople.org/nufw-2.2.19-1.fc11.src.rpm

Thanks.
Comment 6 Jerome Soyer 2008-12-10 07:08:18 EST
Hi,

* I update to the latest version
* Add a patch to -avoid-version
* Reintroduce python bindings

You can find the new spec ans srpms file on :

http://saispo.fedorapeople.org/nufw-2.2.20-1.fc11.src.rpm
http://saispo.fedorapeople.org/nufw.spec

Thanks.
Comment 7 Jerome Soyer 2008-12-10 07:35:25 EST
Hi,

* I update to the latest version
* Add a patch to -avoid-version
* Reintroduce python bindings

You can find the new spec ans srpms file on :

http://saispo.fedorapeople.org/nufw-2.2.20-1.fc11.src.rpm
http://saispo.fedorapeople.org/nufw.spec

Thanks.
Comment 8 Dan Horák 2008-12-11 12:57:57 EST
Please update release for every published iteration of the spec file (assuming the upstream version is still the same, new version starts with release 1) and put the description changes made into the ChangeLog section. It makes easier for the reviewer to track the changes.

The submitter should run rpmlint on its packages. The output of rpmlint run on all rpms (source + binary) is here:

libnuclient.x86_64: W: no-documentation
- can be ignored

libnuclient.x86_64: E: library-without-ldconfig-postin /usr/lib64/libnuclient.so.3.0.0
libnuclient.x86_64: E: library-without-ldconfig-postun /usr/lib64/libnuclient.so.3.0.0
- the ldconfig calls are now attached to the main package, but they must exist for the libnuclient subpackage

libnuclient.x86_64: E: useless-provides libnuclient
- no need to manually provide "libnuclient"

libnuclient.x86_64: W: shared-lib-calls-exit /usr/lib64/libnuclient.so.3.0.0 exit@GLIBC_2.2.5
- should not be an issue, but a statement or explanation (e.g. from upstream) would be nice
rpmlint has a hint:
"This library package calls exit() or _exit(), probably in a non-fork()
context. Doing so from a library is strongly discouraged - when a library
function calls exit(), it prevents the calling program from handling the
error, reporting it to the user, closing files properly, and cleaning up any
state that the program has. It is preferred for the library to return an
actual error code and let the calling program decide how to handle the
situation."

libnuclient-devel.x86_64: E: useless-provides libnuclient-devel
- no need to manually provide "libnuclient-devel"

nufw.src: W: strange-permission setup-python_nufw.py 0755
- can be ignored

nufw.x86_64: W: file-not-utf8 /usr/share/doc/nufw-2.2.20/acls
- non-ASCII content must re-encoded in UTF-8
- see http://cvs.fedoraproject.org/viewvc/rpms/ultimatestunts/devel/ultimatestunts.spec?revision=1.5&view=markup for an example

nufw.x86_64: E: executable-marked-as-config-file /etc/rc.d/init.d/nufw
- initscript must not be marked as %config

nufw.x86_64: W: service-default-enabled /etc/rc.d/init.d/nufw
- only the most important system service can be enabled by default
- you can find all about initscripts at https://fedoraproject.org/wiki/Packaging/SysVInitScript

nufw.x86_64: W: incoherent-subsys /etc/rc.d/init.d/nufw $prog
- no problem
nufw.x86_64: W: service-default-enabled /etc/rc.d/init.d/nufw
- see above

nufw-nuauth.x86_64: W: devel-file-in-non-devel-package /usr/lib64/nuauth/modules/libxml_defs.so
nufw-nuauth.x86_64: W: devel-file-in-non-devel-package /usr/lib64/nuauth/modules/libsession_expire.so
nufw-nuauth.x86_64: W: devel-file-in-non-devel-package /usr/lib64/nuauth/modules/libsession_authtype.so
- there are still 3 modules that are not using -avoid-version
- all patched should be sent to upstream and a notice should be present in the the spec (https://fedoraproject.org/wiki/Packaging/Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment)

nufw-nuauth.x86_64: W: dangerous-command-in-%postun userdel
- it is not allowed to delete the user (https://fedoraproject.org/wiki/Packaging/UsersAndGroups)

nufw-nuauth.x86_64: E: executable-marked-as-config-file /etc/rc.d/init.d/nuauth
nufw-nuauth.x86_64: W: service-default-enabled /etc/rc.d/init.d/nuauth
nufw-nuauth.x86_64: W: incoherent-subsys /etc/rc.d/init.d/nuauth $prog
nufw-nuauth.x86_64: W: service-default-enabled /etc/rc.d/init.d/nuauth
nufw-nuauth.x86_64: W: incoherent-init-script-name nuauth
- see above (nufw)

nufw-nuauth-log-prelude.x86_64: W: no-documentation
- can be ignored

nufw-nutcpc.x86_64: W: non-standard-group Networking/Other
- perhaps forgotten from previous update, I suggest Applications/Internet

nufw-nutcpc.x86_64: E: binary-or-shlib-defines-rpath /usr/bin/nutcpc ['/usr/lib64']
pam_nufw.x86_64: E: binary-or-shlib-defines-rpath /lib64/security/pam_nufw.so ['/usr/lib64']
- there are few tricks, how to block rpath - see https://fedoraproject.org/wiki/Packaging/Guidelines#Beware_of_Rpath
- but because rpath affect only these 2 files, it can be a little bug in the makefiles (explicit using of -rpath), so it could be patched

nufw-utils.x86_64: E: wrong-script-end-of-line-encoding /usr/bin/nutop
- can be fixed with "dos2unix --keepdate $the_file", do not forget to add dos2unix as BuildRequires
- see above in the non-utf-content

python-nufw.x86_64: W: no-documentation
- can be ignored
Comment 9 Jerome Soyer 2008-12-12 06:08:27 EST
Hi,

I have fixed all the Warning and Errors (i do rpmlint on each packages). Just two things :

- libnuclient.x86_64: W: shared-lib-calls-exit /usr/lib64/libnuclient.so.3.0.0
exit@GLIBC_2.2.5 <- it's only when we have a "violent" error

- I have submitted the patch upstream, but haven't add the link in specfile because i post the ticket in the private bugtracker. The patch will be added to the next release.

You can find all things at :

http://saispo.fedorapeople.org/nufw.spec
http://saispo.fedorapeople.org/nufw-2.2.20-2.fc11.src.rpm

Thks
Comment 10 Dan Horák 2008-12-17 07:50:59 EST
(In reply to comment #9)
> Hi,
> 
> I have fixed all the Warning and Errors (i do rpmlint on each packages). Just
> two things :
> 
> - libnuclient.x86_64: W: shared-lib-calls-exit /usr/lib64/libnuclient.so.3.0.0
> exit@GLIBC_2.2.5 <- it's only when we have a "violent" error
> 

OK

> - I have submitted the patch upstream, but haven't add the link in specfile
> because i post the ticket in the private bugtracker. The patch will be added to
> the next release.
> 

OK

there are still few little issues
- the ldconfig calls in the scriptlets for the main package are useless (the libnuclient package already has them)
- I would prefer when the /etc/sysconfig/nufw file is added as new SourceX: instead of online creation in the %install section
- mixed-use-of-spaces-and-tabs in the spec (spaces: line 244, tab: line 3)
- BuildRequires: python for the python-nufw package is redundant (this dependency is auto-solved via python-devel)
- the DefaultStop line in the initscripts should contain all levels 0 - 6 to complement DefaultStart
- multiple packages own the %{_sysconfdir}/nufw/ directory - that depends on the exact interaction between the packages
    - must be the nuauth service run on the same machine as nufw?
    - does have sense to have only the nufw-utils or nutcpc packages installed (without nufw)?
    - are the files in %{_sysconfdir}/nufw/ properly separated between nufw and nuauth?
Comment 11 Jerome Soyer 2008-12-18 06:44:14 EST
Hi,

I fix all the littles issue you said and add some changes.

- Fix initscripts which not working before
- Fixing permissions on certificate
- Fixing a BuildRequires on nuauth-utils about perl-LDAP
- Splitting in two file the config in sysconfig dir for startup

I test on a rawhide box and all work fine now :-)

The exact interaction between packages are good because you can have nufw and nuauth on a separate server, nutcpc is the client and don't need nufw and nuauth on the same machine, just libnuclient and nufw-utils must be or not on nufw server. And all the files are separated properly ;-)

You can find the new version at :

http://saispo.fedorapeople.org/nufw.spec
http://saispo.fedorapeople.org/nufw-2.2.20-4.fc11.src.rpm

Thks
Comment 12 Dan Horák 2008-12-19 04:21:24 EST
formal review is here, see the notes below:

source files match upstream:

OK	package meets naming and versioning guidelines.
OK	specfile is properly named, is cleanly written and uses macros consistently.
OK	dist tag is present.
OK	license field matches the actual license.
OK	license is open source-compatible. License text included in package.
OK	latest version is being packaged.
OK	BuildRequires are proper.
OK	compiler flags are appropriate.
OK	%clean is present.
OK	package builds in mock (Rawhide/x86_64).
OK	debuginfo package looks complete.
OK*	rpmlint is silent.
OK	final provides and requires look sane.
N/A	%check is present and all tests pass.
OK	no shared libraries are added to the regular linker search paths.
OK	owns the directories it creates.
OK	doesn't own any directories it shouldn't.
OK	no duplicates in %files.
OK	file permissions are appropriate.
OK	correct scriptlets present.
OK	code, not content.
OK	documentation is small, so no -docs subpackage is necessary.
OK	%docs are not necessary for the proper functioning of the package.
OK	headers are in -devel
OK	no pkgconfig files.
OK	no libtool .la droppings.
OK	not a GUI app.

- add "touch -r $f $f.new" between the iconv and mv calls in the charset conversion to keep the timestamps on modified files
- rpmlint complains about non-standard uid/gid and some non-readable files, but that's OK due the kind of package

so this package is APPROVED when you add the missing "touch" call before importing the spec to the CVS

Now I need to see (and review) another your package(s) and/or some pre-reviews of other packages done by you, before I can sponsor you.
Comment 13 Jerome Soyer 2008-12-19 06:04:16 EST
ok, done for the "touch" 

Latest version can be found at :

http://saispo.fedorapeople.org/nufw.spec
http://saispo.fedorapeople.org/nufw-2.2.20-5.fc11.src.rpm

Thanks.

You can find another review request i made at :

https://bugzilla.redhat.com/show_bug.cgi?id=474535

Have a nice day!
Comment 14 Jerome Soyer 2009-01-04 17:34:33 EST
New Package CVS Request
=======================
Package Name: nufw
Short Description: Authentication Firewall Suite for Linux
Owners: saispo@gmail.com
Branches: F-10 devel
InitialCC:
Comment 15 Jerome Soyer 2009-01-04 17:35:30 EST
New Package CVS Request
=======================
Package Name: nufw
Short Description: Authentication Firewall Suite for Linux
Owners: saispo@gmail.com
Branches: F-10 devel
InitialCC: saispo

Miss InitialCC in previous commit.
Comment 16 Dan Horák 2009-01-05 01:56:33 EST
Hi Jerome, I am back from holidays. I need to review clamtk before I will sponsor you and then you can request CVS for the package.
Comment 17 Jerome Soyer 2009-01-05 03:59:50 EST
Oh, ok, excuse me :) Wish you an happy new year !
Comment 18 Jerome Soyer 2009-01-07 08:23:41 EST
New Package CVS Request
=======================
Package Name: nufw
Short Description: Authentication Firewall Suite for Linux
Owners: saispo@gmail.com
Branches: F-10 devel
InitialCC: saispo
Comment 19 Kevin Fenzi 2009-01-09 00:50:18 EST
cvs done.
Comment 20 Dan Horák 2009-01-10 05:49:18 EST
Because you have successfully imported and built the packages, you should now close the review request as CLOSED/NEXTRELEASE.
Comment 21 Tomas Mraz 2013-03-27 17:19:50 EDT
Package Ownership Change Request
================================
Package Name: nufw
Owners: tmraz@redhat.com

Given the http://lists.fedoraproject.org/pipermail/devel/2013-March/180713.html I hereby request the ownership change of nufw to myself with the intention to retire it in the F19 and rawhide branches.
Comment 22 Gwyn Ciesla 2013-03-28 07:02:11 EDT
Request this vi FESCO's trac instance.
Comment 23 Tomas Mraz 2013-03-29 08:12:00 EDT
Ticket created:

https://fedorahosted.org/fesco/ticket/1105

Or should it have been actually in the fedora release engineering trac?
Comment 24 Gwyn Ciesla 2013-03-29 08:55:09 EDT
No, that was correct.

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