Bug 176253 - Review Request: clement - An application to filter and manage E-mail traffic
Review Request: clement - An application to filter and manage E-mail traffic
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Hans de Goede
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2005-12-20 11:49 EST by jmp
Modified: 2011-02-06 17:38 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-08-22 07:33:08 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description jmp 2005-12-20 11:49:00 EST
Spec Url: ftp://ftp.safe.ca/pub/clement-2.1/SPECS/clement-2.1-30.spec
SRPM Url: ftp://ftp.safe.ca/pub/clement-2.1/SRPMS/clement-2.1-30.fc4.src.rpm

Description: 

Clement is an E-mail firewall, its purpose is to
filter out all E-mail. Rejecting Virus and unwelcome E-mail
at the SMTP protocol level (avoiding to rebound E-mail
to 'originator forged' E-mail), dubious E-mail are stored
within a centralized quarantine area.

Once accepted, E-mail are transmitted to internal/external
standard Mail daemon (sendmail, postfix, exim, etc..) or kept
within Clement own server area (such as domains defined as 'Keeplocal')
on a per domain basis.

For domain 'Keeplocal', E-mails are stored within
virtual domains and can been retrieved using standard tools
as dovecot.

End user can access logs and quarantine area using clement WEB
interface (URL named clement.'your_domain_name').

Clement can be connected to a remote Mentor process to
help it to conduct an 'In-Depth sending context analyze'
while still connected to remote originator, then becoming even
smarter making SPAM level a trickle.
Comment 1 Jeff Carlson 2005-12-26 10:54:10 EST
This isn't a full review, I'm just noting some things to clean up in the spec.

First of all, the %description is huge.  I personally feel it should be one or
two paragraphs.  I don't think that's a blocker.

The Summary tag shouldn't contain the package name.  Just make it "An
application to..."

You should get rid of the %define dist at the top.  If you want a %dist tag in
your personal repository, add it to your .rpmmacros file.

Version: %{version} and %Release: %{release}.%{dist} are not defined, and
probably not handled very well (as in the build system will probably choke). 
Just use real relase numbers and such, like 2.1 and 31%{?dist}.

The Source should be the full URL where the source can be downloaded.

The Vendor tag is deprecated.  Please remove it.

The BuildRoot should be
%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) -- see
http://fedoraproject.org/wiki/PackagingGuidelines.

The Requires: clamav is probably not necessary.  The RPM packaging process will
discover this if the package is really linked against clamav.

Is the Requires: httpd absolutely necessary?  I understand that users won't have
access to the quarantine function without it, but will the program run without a
web server?  If so, you shouldn't require httpd, but leave a note in a README
file somewhere.

In the %files section, %defattr should be "(-,root,root,-)" -- note final dash.
 This preserves the mode of directories as well.

What is your macro ${prefix} set to?  Is it /usr?  (As an aside, don't use this
macro like you have done, for one it's not defined in your spec file, and second
it is better to use the macro %{_prefix} which the RPM subsystem already has
defined.  Also, a local administrator can change it to suit non-conformist
freakish desires.)  If it is /usr, your package winds up owning it, which
conflicts with the filesystem package.  A better example would be something like
the following:

%{_bindir}/*
%{_sbindir}/*
%{_datadir}/%{name}/
%{_libdir}/%{name}/

and so on, which will include all files under /usr/bin and /usr/sbin, and the
entire directories of /usr/share/clement and /usr/lib/clement including all
files therein.

It is my adamant opinion that no files in /usr should ever need to be written
directly by the administrator or any other user.  You have a config file listed
in %{prefix}/conf (again, %{prefix} is not defined).  All config files should be
under /etc.  Use the %{_sysconfdir} macro for /etc.  Personally, I prefer
packages to place their config files in %{_sysconfdir}/%{name}.

The attr() on /etc/sysconfig/clement is probably not necessary, if it was
installed with the appropriate permissions (defattr should be right).  Why
should it be writable by the mail user?  It is readable by all users, so root
owning it should be sufficient and more secure.

Also, as above, attr() on /etc/init.d/clement (should be
%{_sysconfdir}/rc.d/init.d/clement) is superfluous if it is installed correctly.

I really think %files as a whole should come after %prep, %build, %install, and
%pre/post/preun/postun.  I'm not sure if that's a hard and fast rule, though.

If you are going to run chkconfig in %post and %preun, then you need to
Requires(post) and Requires(preun): chkconfig.

You should not let chkconfig make the program start automatically in %post. 
Just chkconfig --add it.  It will be set to K**clement in every runlevel.  Let
the administrator turn it on.  Also, get rid of the chkconfig --list line.

I'm not sure what those %{prefix}/support/add*.sh scripts to, but I assume they
help build the default config file.  That's nice, but I don't think you should
do that in %post.  Someone can correct me if I'm wrong, but I think it would be
better to include a script and maybe put it in /usr/share/clement
(%{_datadir}/%{name}) with a note in the default config file saying "please run
/usr/share/clement/*.sh to get a nice baseline config."

In %preun, I would change the path /etc/init.d to %{_sysconfdir}/rc.d/init.d. 
Also, Requires(preun): initscripts, because you require the service command. 
And if you followed what I was saying above about paths and such, that whole (
cd %{prefix} ... line at the end needs to go.

You can use make and rm instead of %{__make} and %{__rm}.  That's up to you.  I
think it's cleaner with the commands instead of the macros.

I'm not sure what that mkcron.sh script does exactly, but you might want anacron
scripts to just go into /etc/cron.daily.

Finally, I don't recall seeing your name or email address on the
fedora-extras-list before.  If you are a new contributor, you need to identify
yourself as such so someone who can sponsor you will help you out.  I'm not a
sponsor.

In spite of my ripping your spec file to shreds (it happens to us all our first
time), I want to say "thank you" for writing this program.  As an administrator
of several email servers and domains, I always appreciate people who help fight
spam.  I personally had plans to spend my time on something else last week, but
I wound up working almost all week on one particular spam problem instead, it
was litterally turning into a DoS attack.  So thanks for fighting the good fight.
Comment 2 jmp 2006-01-07 14:26:17 EST
Spec File updated according comments.
Firt extras package, I would like a sponsor

Spec Url: ftp://ftp.safe.ca/pub/clement-2.1/SPECS/clement-2.1-57.spec
SRPM Url: ftp://ftp.safe.ca/pub/clement-2.1/SRPMS/clement-2.1-57.src.rpm

Changelog
- Further Spec file improvement.
- dispatching Clement components according 'FHS' guidelines.
Comment 3 jmp 2006-01-16 11:37:16 EST
Clement-2.1-65 was installed in production for our domain this week-end,
so fare so good :-}
So, if somebody is willing to be a SPONSOR for Clement, (an application trapping
SPAM and virus at SPMT protocol level + mail tracking with a WEB interface),
comments and suggestions are more than welcome.

Spec Url: ftp://ftp.safe.ca/pub/clement-2.1/SPECS/clement-2.1-65.spec
SRPM Url: ftp://ftp.safe.ca/pub/clement-2.1/SRPMS/clement-2.1-65.src.rpm

Changelog
  - Making sure install and uninstall process are working fine
Comment 4 John Mahowald 2006-03-04 17:35:45 EST
Not building in mock.

lvluid.c:17:31: error: security/pam_appl.h: No such file or directory

Will need to BuildRequires pam-devel
Comment 5 jmp 2006-03-06 13:52:35 EST
Pam-Devel is now part of the BuildRequires list within the SPEC file.

Spec Url: ftp://ftp.safe.ca/pub/clement-2.1/SPECS/clement-2.1-99.spec
SRPM Url: ftp://ftp.safe.ca/pub/clement-2.1/SRPMS/clement-2.1-99.src.rpm

Numerous bug-fix/improvement have been done since last bugzilla Clement 
update (2.1-65), see the Changes file and SPEC Changelog for details.
Comment 6 Hans de Goede 2006-06-08 05:08:13 EDT
JMP,

Your work and responsives above look good (at first glance), so I might be
willing to sponsor you. You must understand however that things are currently
organised in FE in such a way that once you are sponsored you get full CVS
access to all packages. Thus having one good package ready for review usually
isn't enough to get you sponsored.

There are 2 ways to proceed from here for us (me since I'm concidering
sponsering you) to get to learn you better:
1) You review a couple of packages from others see bug 163776 for a list of
   Review Requests that need a Reviewer, don't worry about not being competent
   enough todo a review, just add my to the CC-list and I'll watch over your 
   back.
2) Create some more packages and put me in the CC-field of the review request.

Or (probably the best) a combiantion of these 2. What also helps is activity in
other Fedora projects such as translations etc.

Comment 7 Hans de Goede 2006-06-26 04:45:51 EDT
Sorry for the long silence I've been busy with other Fedora stuff.

As discussed by email I'll review this package and if that goes well I'll
sponsor you.

This isn't a Full Review yet, but this should give you enough for now to work on:
* rpmlint gives:
E: clement non-standard-gid /etc/clement-2.1/clement.conf apache
This is intentional and can be ignored. Are you sure though you want apache to
be able to write to this file, I guess this is for the clement webinterface, but
is this safe?
* During build on FC5-i386 rpm says:
warning: File listed twice: /etc/clement-2.1/clement.conf
warning: File listed twice: /etc/clement-2.1/clement.conf
warning: File listed twice: /var/www/clement-2.1/clembase.php
These must be fixed
* All the files under /var/www are owned by mail, this doesn't seem right
* The files under /var/lib/clement-2.1/ do not seem very variable (except for 
  maybe dummy-cert?), please find a better place for these.
* The remove.sh script called form %preun is evil EVIL *EVIL*, you cannot assume
  that rpm scripts will be run from an interactive terminal. I think it is best
  to just leave these files in place and instead in %preun generate a
  README-fedora in the clement spool dir which explains that since clement is
  removed these files _may_ be removed to, but that they could contain valuable
  data and people should be carefull when removing them. 
* Remove all comments like this one:
#-----------------------------------------------------------
#Clement SPEC definition
#-----------------------------------------------------------
People reading a spec file are supposed to know what a specfile is and what the
different sections do, these are not helpfull comments, they are noise.
* Put the sections in the specfile in the proper order, see for example the
  /usr/share/fedora/spectemplate-minimal.spec from the fedora-rpmdevtools 
  package.
  The proper order is
  Name
  Version
  Release
  Summary
  Group
  ....
  %description

  %setup

  %build
  
  %install

  %clean

  %post(un) (and other scripts)

  %files

  %changelog
Comment 8 jmp 2006-07-06 12:00:35 EDT
Version 2.1-175 SPECS file is now clean according rpmlint-0.77-1.fc5
rpmbuild give no warning anymore about "File listed twice"

Spec Url: ftp://ftp.safe.ca/pub/clement-2.1/SPECS/clement-2.1-175.spec
SRPM Url: ftp://ftp.safe.ca/pub/clement-2.1/SRPMS/clement-2.1-175.src.rpm

Numerous bug-fix/improvement have been done since last bugzilla Clement 
update (2.1-99), see the Changes file and SPEC Changelog for details.

According to me SPEC file is really better, there 2 issue not yet addressed:
1) /var/lib/clement-2.1 contents.
   those file are mainly day to day clement own management, I am really
reluctant to put them in /usr/bin as they are for the sole clement purpose. what
is the best option in such case?
2) ownership for /var/www/clement-2.1:
   putting everything own by 'mail' is may be overkilling, this web part is not
meaningful if clement daemon is not up and running, Clement itself run with
uid 'mail', that why I chose to set the ownership as 'mail'.
Comment 9 Hans de Goede 2006-07-07 16:33:08 EDT
(In reply to comment #8)
> Version 2.1-175 SPECS file is now clean according rpmlint-0.77-1.fc5
> rpmbuild give no warning anymore about "File listed twice"
> 
Good, whats with the release tag inflation (very high number)?

> Spec Url: ftp://ftp.safe.ca/pub/clement-2.1/SPECS/clement-2.1-175.spec
> SRPM Url: ftp://ftp.safe.ca/pub/clement-2.1/SRPMS/clement-2.1-175.src.rpm
> 
> Numerous bug-fix/improvement have been done since last bugzilla Clement 
> update (2.1-99), see the Changes file and SPEC Changelog for details.
> 
> According to me SPEC file is really better, 
Yes it is, its becoming readable, with the proper order and stuff, it almost
looks like a real specfile now :)

there 2 issue not yet addressed:
> 1) /var/lib/clement-2.1 contents.
>    those file are mainly day to day clement own management, I am really
> reluctant to put them in /usr/bin as they are for the sole clement purpose. 
> what is the best option in such case?
You could & should put them under /usr/lib/clement (%{_libdir}/%{name}) really)

> 2) ownership for /var/www/clement-2.1:
>    putting everything own by 'mail' is may be overkilling, this web part is not
> meaningful if clement daemon is not up and running, Clement itself run with
> uid 'mail', that why I chose to set the ownership as 'mail'.
> 

I think a default of root root as normal would be better, with an exception for
files and or dir which clement needs to write.

Also I recently learned that web application packages should not touch /var/www
as they might accidently overwrite user content put there, instead the should
install their files under /usr/share/%{name}, see:
http://fedoraproject.org/wiki/Packaging/Guidelines#head-5d1681fa7cf3714ad490fbf7c095a0cfe16da27f

Besides that things are looking good, accept for the:
#-----------------------------------------------------------
#Clement SPEC definition
#-----------------------------------------------------------
comment, please remove that.

Let me know when you've a new version with these issues fixed.

p.s.
I'm going on a short vacation from Monday 10 juli till Friday 14 juli,
so if I'm quiet for a while thats why.
Comment 10 jmp 2006-07-16 11:18:01 EDT
Version 2.1-176 SPECS file fine tuning to use /usr/share and /usr/lib
insteed of /var/www and /var/lib.

Spec Url: ftp://ftp.safe.ca/pub/clement-2.1/SPECS/clement-2.1-176.spec
SRPM Url: ftp://ftp.safe.ca/pub/clement-2.1/SRPMS/clement-2.1-176.src.rpm

:-}}
About Release tag inflation, application is kept internally within RCS,
developement has been done this last month and we prefere to sauve as
many time as necessary to be safe. :-} Tag release number are just that...
number, they just need to refer to a consistente and unique binary code.

Thanks for support and advices.
Comment 11 Hans de Goede 2006-07-25 07:27:39 EDT
As promised here is a full review:

MUST:
=====
* rpmlint output is clean, good!
* Package and spec file named appropriately
* Packaged according to packaging guidelines
* License (GPL) ok, license file included
* spec file is legible and in Am. English.
0 Cannot verify if source matches upstream because of broken Source URL,
  this must be fixed!
* Compiles and builds on devel-x86_64
* BR: ok
* No locales
* No shared libraries
* Not relocatable
0 Package does NOT owns / or requires all dirs
* No duplicate files & Permissions ok
* %clean & macro usage OK
* Contains code and permissible content
* %doc does not affect runtime, and isn't large enough to warrent a sub package
* no -devel package needed, no libs / .la files.
* no gui -> no .desktop file required


MUST fix:
=========
* Proper downloadable Source URL
* This is dead wrong:
%attr(-,mail,mail) %{_usr}/share/%{name}-%{version}/logs/
  Running software cannot shall not and must not write to /usr it could
  be on a readonly partition or or or ....
  Please make that:
%attr(-,mail,mail) %{_var}/log%{name}-%{version}/
  And adjust the software to write it logs there, or am I misinterpreting 
  the dir name here?
* Unowned dir %{_usr}/share/%{name}-%{version}
  Add: "%dir %{_usr}/share/%{name}-%{version}" to %files or better replace:
%{_usr}/share/%{name}-%{version}/*.php
%{_usr}/share/%{name}-%{version}/reg-icons
%{_usr}/share/%{name}-%{version}/local
%{_usr}/share/%{name}-%{version}/cgi-bin
  With just:
%{_usr}/share/%{name}-%{version}


Should fix:
===========
* Spec contains: "#%postun" this will end up as the last line of the %preun
  line, harmless but it it would be cleaner to just remove it completly.
* Replace all occurences of "%{_usr}/share" with "%{_datadir}

  
So all in all its looking good! Once all the must fix items are taken care of
I'll sponsor you and you can import this.
Comment 12 jmp 2006-08-11 09:00:36 EDT
Version 2.1-186 SPECS file fine tuning, logs are not in /usr/share/clement anymore


Spec Url: ftp://ftp.safe.ca/pub/clement-2.1/SPECS/clement-2.1-186.spec
SRPM Url: ftp://ftp.safe.ca/pub/clement-2.1/SRPMS/clement-2.1-186.src.rpm

Beside SPEC file fine tuning, numeros improvement and bug-fix since 2.1-176,
see Changelog within SPEC file.
Comment 13 Hans de Goede 2006-08-12 01:38:41 EDT
Almost there, why are these:
%attr(-,mail,mail) %{_usr}/bin/%{name}
%attr(-,mail,mail) %{_datadir}/%{name}-%{version}/

%attr(-,mail,mail) ?

Perhaps the binary is suid? In that case please reflect that in the %attr, even
if it already is made suid in %install. And I see no reason for the
%{_datadir}/%{name}-%{version} being mail.mail, prhaps this is a leftover from
when it contained the log files?


I also found some more should fixes, I see you use:
%{_usr}/bin in various places, you should replace that with %{_bindir}
also you use %{_usr}/lib, which will result in things getting installed under
/usr/lib instead of /usr/lib64 on 64 bit archs, is that intentional? If not
please replace it with %{_libdir}.
Comment 14 jmp 2006-08-12 09:01:54 EDT
We need to be consistent here:
rpmlint is complaining about putting an application in setuid how could
you suggest to do this?
Clement is started un root priviledges and lets them go as soon
proper port (SMTP) are open, to do this it seteuid with the application program
ownership. So there is NO purpose to put clement setuid, not from the
security stand point, not from the rpmlint stand point, not from application 
stand point.

file in %{_usr}/lib are shell for clement application (utilities, support),
shell are not archs dependent.
Comment 15 Hans de Goede 2006-08-13 17:10:01 EDT
(In reply to comment #14)
> We need to be consistent here:
> rpmlint is complaining about putting an application in setuid how could
> you suggest to do this?
> Clement is started un root priviledges and lets them go as soon
> proper port (SMTP) are open, to do this it seteuid with the application program
> ownership. So there is NO purpose to put clement setuid, not from the
> security stand point, not from the rpmlint stand point, not from application 
> stand point.
> 

I think you understood me wrong here, I didn't want to suggest to make
%attr(-,mail,mail) %{_usr}/bin/%{name} setuid, I thought it was setuid and that
was why it has owner and group mail, if its not setuid, then why not just owner
and group root?

> file in %{_usr}/lib are shell for clement application (utilities, support),
> shell are not archs dependent.
> 

OK.

You still haven't explained why you do:
%attr(-,mail,mail) %{_datadir}/%{name}-%{version}/
Instead of just:
%{_datadir}/%{name}-%{version}/
Or is that just a copy and paste error and will you fix that with the next version?
Comment 16 jmp 2006-08-13 19:07:23 EDT
I think datadir is not a problem, I need to double check with the PHP person,
should be fixed in the next version.

clement is not 'setuid' but must be root open < 1024 port.
such the Clement daemon is started as root and clement take
the application ownership to become a standard user mail
to avoid the have a daemon with root priviledge open on the 
(wild) outside. I would rather have a "clement" username but
rpmlint seems to be rather reluctant to 'give/declare' new
username.
Comment 17 Hans de Goede 2006-08-14 03:40:47 EDT
(In reply to comment #16)
> I think datadir is not a problem, I need to double check with the PHP person,
> should be fixed in the next version.
> 
> clement is not 'setuid' but must be root open < 1024 port.
> such the Clement daemon is started as root and clement take
> the application ownership to become a standard user mail
> to avoid the have a daemon with root priviledge open on the 
> (wild) outside.
I understand, but then the %{_usr}/bin/%{name} file doesn't have the be owned by
mail.mail and could be just root.root, right?

My real question al allong has been why is %{_usr}/bin/%{name} owned by mail.mail?

> I would rather have a "clement" username but
> rpmlint seems to be rather reluctant to 'give/declare' new
> username.
> 

Thats possible, add the following lines (at the appropiate places):
Requires(pre):  /usr/sbin/useradd, /usr/sbin/groupadd

%pre
/usr/sbin/groupadd -r clement 2> /dev/null || :
/usr/sbin/useradd -s /sbin/nologin -M -d / -c "Clement daemon" -r -g clement \
  clement 2> /dev/null || :

And then you can use %attr (-,clement,clement) in %files. You will ofcourse also
need to patch the daemon to drop its root rights to the user clement instead of
mail.

This might generatre some rpmlint warnings but these may be ignored.
Comment 18 jmp 2006-08-14 08:04:31 EDT
Seems I do not explain myself right...
%{_usr}/bin/%{name} MUST be own by 'somebody' else than root to have clement to
know, once started, under which ID it must run (the application look about
the file ownership and say 'ok lets seteuid to this'), if the application
is not setuid the only other way is to hard code the effective uid, this is
not good from my stand point. I choosed 'mail' because this ID is used by
related application. I want to give possibility to change this on the fly
by local sysadmin.

useradd and groupadd clement where part of the original implementation but
removed to comply to rpmlint. 
If rpmlint is a reference tools to 'the right way to do something' warning
can't ignore. IMHO rpmlint warning are 'you are doing something which can work
but are against established standard'.
Comment 19 Michael Schwendt 2006-08-14 08:14:56 EDT
> I choosed 'mail' because this ID is used by related application.

This asks for a very close look. Either it is a necessity, by design,
that the program must run as "mail". Or it is a fault, and it runs with
a shared uid/gid it should not have access to.
Comment 20 jmp 2006-08-14 08:42:13 EDT
(In reply to comment #19)
> > I choosed 'mail' because this ID is used by related application.
> 
> This asks for a very close look. Either it is a necessity, by design,
> that the program must run as "mail". Or it is a fault, and it runs with
> a shared uid/gid it should not have access to.
> 

OK....
lets resolv the equation
1) the application must not run as root
2) rpmlint complain if you add/create a user
3) We should not use an existing user
Comment 21 Hans de Goede 2006-08-14 08:43:15 EDT
(In reply to comment #18)
> Seems I do not explain myself right...
> %{_usr}/bin/%{name} MUST be own by 'somebody' else than root to have clement to
> know, once started, under which ID it must run (the application look about
> the file ownership and say 'ok lets seteuid to this'), if the application
> is not setuid the only other way is to hard code the effective uid, this is
> not good from my stand point. I choosed 'mail' because this ID is used by
> related application. I want to give possibility to change this on the fly
> by local sysadmin.
> 
I understand.

> useradd and groupadd clement where part of the original implementation but
> removed to comply to rpmlint. 
> If rpmlint is a reference tools to 'the right way to do something' warning
> can't ignore. IMHO rpmlint warning are 'you are doing something which can work
> but are against established standard'.

No, a rpmlint warning means you shouldnot be doing this unless you've got a good
reason, and it this case we have a good reason so using user and groupadd is ok.

(In reply to comment #19)
> > I choosed 'mail' because this ID is used by related application.
> 
> This asks for a very close look. Either it is a necessity, by design,
> that the program must run as "mail". Or it is a fault, and it runs with
> a shared uid/gid it should not have access to.
> 

If I understand jmp correctly its the latter (a fault) jmp if you think it is
better to have it run as clement, feel free to add the user, in exceptional
cases (which daemons always are) you can ignore the relevant rpmlint warnings,
thats why they are warnings, rpmlint deliberatly has 2 levels of compaining,
warn and error, and these are only warnings.
Comment 22 Michael Schwendt 2006-08-14 09:40:42 EDT
> 2) rpmlint complain if you add/create a user

Hmm? The "dangerous-command-in" _warning_ about userdel/groupdel?

This is misinformation. Surely there are valid cases when creating
a new user or group is required/justified.

Similarly, there are cases when deleting a user or group during
package removal can be done, i.e. if no files owned by that uid/gid
are left anywhere.


In particular, since you want a "centralized quarantine area" (quote),
you need a special uid/gid and not use an existing uid/gid which is
shared with other programs/services.
Comment 23 jmp 2006-08-14 10:05:39 EDT
So I added clement as group & user as suggested.

got this from rpmlint (many lines of this kind)

E: clement non-standard-gid /var/spool/clement-2.1/mqueue clement
A file in this package is owned by a non standard group.
Standard groups are:
root, bin, daemon, sys, adm, tty, disk, lp, mem, kmem, wheel, mail,
news, uucp, man, games, gopher, dip, ftp, lock, nobody, users


Does not look like as a 'Warning' to me, how to make it as a Warning?


Also got this warning
W: clement dangerous-command-in-%preun userdel
because I would like to delete created user when removing all the application,
there is a kind of ambiguity here as some file could be left if the application
was in production previously.
My understanding of the last Michael comment is to let the sys-admin delete
the remaining user/group himself, right?
Comment 24 Michael Schwendt 2006-08-14 10:19:15 EDT
> E: clement non-standard-gid /var/spool/clement-2.1/mqueue clement

Who cares? This is not the first package that creates a new user/group.

> My understanding of the last Michael comment is to let the sys-admin
> delete the remaining user/group himself, right?

Yes. If package removal does not get rid of all files owned by that
user/group, the next package might allocate the same uid/gid and give
some other software access to the old files, which are still left
on the file-system as orphans.
Comment 25 jmp 2006-08-14 10:39:52 EDT
(In reply to comment #24)
> > E: clement non-standard-gid /var/spool/clement-2.1/mqueue clement
> 
> Who cares? This is not the first package that creates a new user/group.
Me!.
Agree this is not the first package creating a user/group....
problem, if rpmlint is THE reference and rpmlint is reporting error how
can I make the difference between real error which are errors from simple
advice... now in my case there is 48 "errors" which are 'who cares',....
hmmmm......
> 
> > My understanding of the last Michael comment is to let the sys-admin
> > delete the remaining user/group himself, right?
> 
> Yes. If package removal does not get rid of all files owned by that
> user/group, the next package might allocate the same uid/gid and give
> some other software access to the old files, which are still left
> on the file-system as orphans.
OK, lets keep the user/group then
> 

Comment 26 Hans de Goede 2006-08-14 10:48:37 EDT
A bit late due to a bugzilla collision, but I fully agree with Michael:

(In reply to comment #23)
> So I added clement as group & user as suggested.
> 
> got this from rpmlint (many lines of this kind)
> 
> E: clement non-standard-gid /var/spool/clement-2.1/mqueue clement
> A file in this package is owned by a non standard group.
> Standard groups are:
> root, bin, daemon, sys, adm, tty, disk, lp, mem, kmem, wheel, mail,
> news, uucp, man, games, gopher, dip, ftp, lock, nobody, users
> 
> 
> Does not look like as a 'Warning' to me, how to make it as a Warning?
> 

Erm, so its an error (my mistake), still it can be ignored, rpmlint us an
automated validation tool and sometimes it can be just plain wrong. Trust me
(and Michael) on this between the 2 of us there is a ton of packaging experience.

> 
> Also got this warning
> W: clement dangerous-command-in-%preun userdel
> because I would like to delete created user when removing all the application,
> there is a kind of ambiguity here as some file could be left if the application
> was in production previously.
> My understanding of the last Michael comment is to let the sys-admin delete
> the remaining user/group himself, right?

Yes, you should not remove the user, the user may only be removed if your 100.1%
sure that no files owned by that user will be left around, which in this case we
aren't actually we are pretty sure that files will be left around, so the
userdel must be removed from the spec file.
Comment 27 Michael Schwendt 2006-08-14 12:18:45 EDT
There is no policy which says that all rpmlint errors/warnings must be
fixed.

Look at it from a different perspective to understand. There is nothing
about this in the Package Review Guidelines. Nothing at all about creating
user/group accounts. Not even the guideline about "setuid root" from
fedora.us QACheckList is included. When we agreed on the initial rather
long list of MUST/SHOULD items in a FESCO meeting, we didn't cover
useradd/groupadd/userdel/groupdel or fedora-useradd and friends.

| http://fedoraproject.org/wiki/Packaging/ReviewGuidelines
|
| - SHOULD: If scriptlets are used, those scriptlets must be sane.
| This is vague, and left up to the reviewers judgement to determine
| sanity.

If anything had changed over time, it should have been announced
clearly.
Comment 28 jmp 2006-08-14 21:04:59 EDT
Version 2.1-192 SPECS file fine tuning, user/group clement used to run the main
daemon


Spec Url: ftp://ftp.safe.ca/pub/clement-2.1/SPECS/clement-2.1-192.spec
SRPM Url: ftp://ftp.safe.ca/pub/clement-2.1/SRPMS/clement-2.1-192.src.rpm
Comment 29 Hans de Goede 2006-08-15 05:25:28 EDT
(In reply to comment #28)
> Version 2.1-192 SPECS file fine tuning, user/group clement used to run the main
> daemon
> 
> 
> Spec Url: ftp://ftp.safe.ca/pub/clement-2.1/SPECS/clement-2.1-192.spec
> SRPM Url: ftp://ftp.safe.ca/pub/clement-2.1/SRPMS/clement-2.1-192.src.rpm

Looks good, I've sponsored you and its ok to import this now. There is one small
thing you should fix before importing, please add " || :" at the end of all the
commands (not the if / endif lines) in the pre / post / postun scripts, that way
if one of these commands fail it doesn't cause the entire rpm transaction to fail.

This is especially important for the pre script, because of a user installs,
then removes and then tries to reinstall clement the groupadd and useradd will
fail because they already exist (which is ok, the exisiting user may be recycled
in this corner case).

Comment 30 jmp 2006-08-22 07:33:08 EDT
Part now of the DEVEL EXTRAS, build successful.
Comment 31 Kevin Fenzi 2006-12-21 22:08:39 EST
Changing Summary for tracking purposes. 
Comment 32 jmp 2011-01-28 09:47:41 EST
Package Change Request
======================
Package Name: clement
New Branches: el5
Owners: jmrcpn

Most of the clement installation are el5 distribution type. Having clement in
EPEL 5 repo will make update process easier.
Comment 33 Kevin Fenzi 2011-02-06 17:38:48 EST
Git done (by process-git-requests).

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