Bug 222552 - (mailgraph) Review Request: mailgraph - A RRDtool frontend for Mail statistics
Review Request: mailgraph - A RRDtool frontend for Mail statistics
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Ruben Kerkhof
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2007-01-13 20:08 EST by Bernard Johnson
Modified: 2014-08-22 08:06 EDT (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-01-30 04:47:05 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Bernard Johnson 2007-01-13 20:08:15 EST
Spec URL: http://www.symetrix.com/~bjohnson/projects/Fedora-Extras/mailgraph.spec
SRPM URL: http://www.symetrix.com/~bjohnson/projects/Fedora-Extras/mailgraph-1.12-1.fc6.src.rpm
Description:
Mailgraph is a very simple mail statistics RRDtool frontend for Postfix and
Sendmail that produces daily, weekly, monthly and yearly graphs of
received/sent and bounced/rejected mail.
Comment 1 manuel wolfshant 2007-01-26 19:10:53 EST
At the first glance:
- usage of service and chkconfig in %pre/%post impose Require chkconfig,service
- I am not sure that the location of the script
/usr/share/mailgraph/mailgraph.cgi is correct.
http://fedoraproject.org/wiki/Packaging/Guidelines?highlight=%28Hierarchy%29%7C%28Filesystem%29%7C%28Standard%29#head-e1c5548cbbe551c7a43d375c524ab2ea0188557e
states that Fedora "follows the [WWW] Filesystem Hierarchy Standard" which in
turn says that /usr/share should be used for "Architecture-independent data".
Since mailgraoh is a script (hence a program) rather than data, I'd say that
according to
http://fedoraproject.org/wiki/Packaging/Guidelines?highlight=%28Hierarchy%29%7C%28Filesystem%29%7C%28Standard%29#head-69ef16dbc0ecb520d4cc7bbd381038f23963e1fd
 a better place could be %{_libexecdir}\%{name}. As an addition, rrd itself sits
in /usr/bin.
OTOH
- awstats sits happily in /usr/share/awstats, so /usr/sahre has already been
blessed in FE
- mailman (from core) serves the scripts from /usr/lib/mailman/cgi-bin
 
Someone with more experience, could you please step in ?

Once we settle the above mentioned items, I'll be glad to do a pre-review (I
cannot formaly review the package because you need a sponsor).
Comment 2 manuel wolfshant 2007-01-26 19:12:33 EST
Sorry for the noise, I may not formally assign the package to myself.
Reassigning back to nobody@fedoraproject.org
Comment 3 Bernard Johnson 2007-01-27 14:52:32 EST
(In reply to comment #1)
> - usage of service and chkconfig in %pre/%post impose Require chkconfig,service

It doesn't use /sbin/service, but it does use /sbin/chkconfig so I added the
proper requires.


Spec URL: http://www.symetrix.com/~bjohnson/projects/Fedora-Extras/mailgraph.spec
SRPM URL:
http://www.symetrix.com/~bjohnson/projects/Fedora-Extras/mailgraph-1.12-2.fc6.src.rpm


* Sat Jan 27 2007 Bernard Johnson <bjohnson@symetrix.com> - 1.12-2
- add Requires(preun), Requires(post) for chkconfig
Comment 4 Ruben Kerkhof 2007-01-27 17:30:28 EST
I'd be happy to review this package. 

Look for a full review here in a bit...
Comment 5 Ruben Kerkhof 2007-01-27 18:09:59 EST
Must:

- package conforms to package naming guidlines
- spec file name matches basename
- the package is licensed with an open source compatible license
- package includes license in %doc
- spec file is written in american english
- spec file is legible
- sources match upstream (sha1sum):
   08bbe7f6e66f1b14b40ce7da8caa716b6ee122e3  mailgraph-1.12.tar.gz
- package successfully built on my test box (i386)
- no need for exclude arch (noarch package)
- files in %doc does not affect runtime of application
- does not contain a pkgconfig file
- does not contain any libraries
- does not have a devel package
- does not contain any .la files
- package does not have a gui
- package does not own files/directories owned by other packages.
- no libraries, no need to run ldconfig
- not relocatable
- package owns everything it creates
- package does not duplicate files it owns
- consistently uses macros
- code, not content

Needs work:

- rpmlint errors:
   E: mailgraph non-standard-gid /var/cache/mailgraph apache.
   This can be ignored, but you should Require httpd, otherwise you can't be sure
   this user exists.
   E: mailgraph non-standard-dir-perm /var/cache/mailgraph 0775.
   Can't you make apache the owner of the directory?

- Do you need the repotag in Release?
- Please preserve timestamps when installing files (Packaging/Guidelines/Timestamps)

I see you have not been sponsored yet, and I'm not a sponsor, so I'll have to put the FE-NEEDSPONSOR 
blocker back.
Comment 6 Bernard Johnson 2007-01-27 18:44:36 EST
(In reply to comment #5)
>    E: mailgraph non-standard-gid /var/cache/mailgraph apache.
>    This can be ignored, but you should Require httpd, otherwise you can't be sure
>    this user exists.

Ah, excellent point.  I remember thinking that it could be installed without
httpd required, but didn't consider the data directory ownership.

>    E: mailgraph non-standard-dir-perm /var/cache/mailgraph 0775.
>    Can't you make apache the owner of the directory?

I could.  but if I change it apache.apache 0755, then I'm pretty sure rpmlint
will complain about "non-standard ownership".  Is there a flaw to the way that
I've done it, or are you expressing a preference, or pointing me to a guideline?

> - Do you need the repotag in Release?

No, but until it's accepted, it help me build with my own repotag and not have
to keep two spec files.  Since the repotag is unset in FE it should not have any
impact.

> - Please preserve timestamps when installing files
(Packaging/Guidelines/Timestamps)

I will update with this change next release.


Comment 7 Ruben Kerkhof 2007-01-27 19:41:33 EST
> I could.  but if I change it apache.apache 0755, then I'm pretty sure rpmlint
> will complain about "non-standard ownership".  Is there a flaw to the way that
> I've done it, or are you expressing a preference, or pointing me to a guideline?

I think rpmlint is complaining about the 0775, which probably should be a 0755 :-)
Comment 8 Bernard Johnson 2007-01-27 20:32:21 EST
(In reply to comment #7)
> > I could.  but if I change it apache.apache 0755, then I'm pretty sure rpmlint
> > will complain about "non-standard ownership".  Is there a flaw to the way that
> > I've done it, or are you expressing a preference, or pointing me to a guideline?
> 
> I think rpmlint is complaining about the 0775, which probably should be a 0755 :-)
> 

You are correct, but I think you missed my point.  If you change the ownership
from root.apache to apache.apache and the mode from 0775 to 0755, you trade this
"error" message:

E: mailgraph non-standard-dir-perm /var/cache/mailgraph 0775

for this "error" message:

E: mailgraph non-standard-uid /var/cache/mailgraph apache

I just checked it on my system and indeed that is what rpmlint says.

Comment 9 Ruben Kerkhof 2007-01-28 04:33:09 EST
Agreed. There's no way to do this without those 2 error messages, so I think it's safe to ignore them.
Have you created a new version yet?
Comment 10 Bernard Johnson 2007-01-28 12:25:38 EST
Spec URL: http://www.symetrix.com/~bjohnson/projects/Fedora-Extras/mailgraph.spec
SRPM URL:
http://www.symetrix.com/~bjohnson/projects/Fedora-Extras/mailgraph-1.12-3.fc6.src.rpm

* Sun Jan 28 2007 Bernard Johnson <bjohnson@symetrix.com> - 1.12-3
- consistent use of mode in creating directories
- add Requires for httpd
- preserve timestamps on install
Comment 11 Ruben Kerkhof 2007-01-28 16:08:33 EST
One last thing, please replace %{_var} with %{_localstatedir}.
I don't see any blockers, but can't accept your package, since you still need a sponsor.
Comment 12 Bernard Johnson 2007-01-29 02:47:19 EST
(In reply to comment #11)
> One last thing, please replace %{_var} with %{_localstatedir}.
> I don't see any blockers, but can't accept your package, since you still need
a sponsor.

Thanks.

Spec URL: http://www.symetrix.com/~bjohnson/projects/Fedora-Extras/mailgraph.spec
SRPM URL:
http://www.symetrix.com/~bjohnson/projects/Fedora-Extras/mailgraph-1.12-4.fc6.src.rpm

* Sun Jan 28 2007 Bernard Johnson <bjohnson@symetrix.com> - 1.12-4
- replace %%{_var} with %%{_localstatedir}

Comment 13 Mamoru TASAKA 2007-01-29 03:46:23 EST
(In reply to comment #11)
> One last thing, please replace %{_var} with %{_localstatedir}.
> I don't see any blockers, but can't accept your package, since you still need
a sponsor.

(Perhaps this will be solved within a day. Please check
 bug 216723)
Comment 14 Mamoru TASAKA 2007-01-29 13:14:35 EST
(Removing NEEDSPONSOR as bug 216723)
Comment 15 Bernard Johnson 2007-01-29 14:57:37 EST
(In reply to comment #11)
> I don't see any blockers, but can't accept your package, since you still need
a sponsor.

If you want to finish a review now, go ahead... mtasaka@ioa.s.u-tokyo.ac.jp
provided sponsorship for me.
Comment 16 Ruben Kerkhof 2007-01-29 15:52:38 EST
I don't see any further issues, so this package is APPROVED. 
Don't forget to close this review request NEXTRELEASE once it's been imported
and built.
Comment 17 Gianluca Sforna 2007-01-30 06:12:33 EST
Just for the records: it does not seems to play nice with SELinux (at least in
my CentOS 4 mail server)

type=AVC msg=audit(1170155275.211:591976): avc:  denied  { execute } for 
pid=32749 comm="httpd" name="mailgraph.cgi" dev=dm-0 ino=571556
scontext=root:system_r:httpd_t tcontext=system_u:object_r:usr_t tclass=file
type=SYSCALL msg=audit(1170155275.211:591976): arch=40000003 syscall=11
success=no exit=-13 a0=8baf6b8 a1=8bb1108 a2=8bb1118 a3=8bb1968 items=1
pid=32749 auid=0 uid=48 gid=48 euid=48 suid=48 fsuid=48 egid=48 sgid=48 fsgid=48
comm="httpd" exe="/usr/sbin/httpd"
type=CWD msg=audit(1170155275.211:591976):  cwd="/usr/share/mailgraph"
type=PATH msg=audit(1170155275.211:591976):
name="/usr/share/mailgraph/mailgraph.cgi" flags=101  inode=571556 dev=fd:00
mode=0100755 ouid=0 ogid=0 rdev=00:00

any hint?
Comment 18 manuel wolfshant 2007-01-30 06:30:52 EST
By default SELinux allows apache scripts to be placed only in certain defined
places. Coud you please copy /usr/share/mailgraph/mailgraph.cgi to
/var/www/cgi-bin, modify httpd.conf accordingly and retry ?
Comment 19 Paul Howarth 2007-01-30 06:33:35 EST
Try this:

# chcon -t httpd_script_exec_t /usr/share/mailgraph/mailgraph.cgi

No need to modify httpd.conf if this works.
Comment 20 Paul Howarth 2007-01-30 06:34:37 EST
Sorry, that should have been httpd_sys_script_exec_t in Comment #19.
Comment 21 Gianluca Sforna 2007-01-30 07:03:19 EST
(In reply to comment #18)
> By default SELinux allows apache scripts to be placed only in certain defined
> places. Coud you please copy /usr/share/mailgraph/mailgraph.cgi to
> /var/www/cgi-bin, modify httpd.conf accordingly and retry ?

I copied the cgi and tried to access http://localhost/cgi-bin/mailgraph.cgi (so,
no need for httpd.conf modifications), but then it complains about denied
read/write ops:

type=AVC msg=audit(1170158491.308:592646): avc:  denied  { read } for  pid=1293
comm="mailgraph.cgi" name="mailgraph.rrd" dev=dm-0 ino=279712
scontext=root:system_r:httpd_sys_script_t tcontext=root:object_r:var_lib_t
tclass=file
type=SYSCALL msg=audit(1170158491.308:592646): arch=40000003 syscall=5
success=no exit=-13 a0=8ea0b28 a1=0 a2=1b6 a3=0 items=1 pid=1293 auid=0 uid=48
gid=48 euid=48 suid=48 fsuid=48 egid=48 sgid=48 fsgid=48 comm="mailgraph.cgi"
exe="/usr/bin/perl"
type=CWD msg=audit(1170158491.308:592646):  cwd="/var/www/cgi-bin"
type=PATH msg=audit(1170158491.308:592646):
name="/var/lib/mailgraph/mailgraph.rrd" flags=101  inode=279712 dev=fd:00
mode=0100644 ouid=0 ogid=0 rdev=00:00
type=AVC msg=audit(1170158491.527:592647): avc:  denied  { getattr } for 
pid=1296 comm="mailgraph.cgi" name="mailgraph" dev=dm-0 ino=279710
scontext=root:system_r:httpd_sys_script_t tcontext=system_u:object_r:var_t
tclass=dir
type=SYSCALL msg=audit(1170158491.527:592647): arch=40000003 syscall=195
success=no exit=-13 a0=9dd6ac0 a1=9dac0c8 a2=335ff4 a3=9dd6ac0 items=1 pid=1296
auid=0 uid=48 gid=48 euid=48 suid=48 fsuid=48 egid=48 sgid=48 fsgid=48
comm="mailgraph.cgi" exe="/usr/bin/perl"
type=AVC_PATH msg=audit(1170158491.527:592647):  path="/var/cache/mailgraph"
type=CWD msg=audit(1170158491.527:592647):  cwd="/var/www/cgi-bin"
type=PATH msg=audit(1170158491.527:592647): name="/var/cache/mailgraph" flags=1
 inode=279710 dev=fd:00 mode=040775 ouid=0 ogid=48 rdev=00:00
type=AVC msg=audit(1170158491.529:592648): avc:  denied  { write } for  pid=1296
comm="mailgraph.cgi" name="mailgraph" dev=dm-0 ino=279710
scontext=root:system_r:httpd_sys_script_t tcontext=system_u:object_r:var_t
tclass=dir
type=SYSCALL msg=audit(1170158491.529:592648): arch=40000003 syscall=39
success=no exit=-13 a0=9dbc540 a1=1ff a2=538c94 a3=9dbc540 items=1 pid=1296
auid=0 uid=48 gid=48 euid=48 suid=48 fsuid=48 egid=48 sgid=48 fsgid=48
comm="mailgraph.cgi" exe="/usr/bin/perl"
type=CWD msg=audit(1170158491.529:592648):  cwd="/var/www/cgi-bin"
type=PATH msg=audit(1170158491.529:592648): name="/var/cache/mailgraph/,cgi-bin"
flags=10  inode=279710 dev=fd:00 mode=040775 ouid=0 ogid=48 rdev=00:00

Comment 22 Gianluca Sforna 2007-01-30 07:06:11 EST
(In reply to comment #19)
> Try this:
> 
> # chcon -t httpd_script_exec_t /usr/share/mailgraph/mailgraph.cgi
> 
> No need to modify httpd.conf if this works.
> 

Nope. still denied read/write ops on the rrd files
Comment 23 Paul Howarth 2007-01-30 07:12:24 EST
Try:

# chcon -Rh -t httpd_var_lib_t /var/lib/mailgraph
# chcon -Rh -t httpd_cache_t /var/cache/mailgraph
Comment 24 Bernard Johnson 2007-01-30 14:19:32 EST
Hi, this is the closed review bug.  Please open another bug regarding this issue
and I'll get it working with SELinux.
Comment 25 Bernard Johnson 2008-12-07 02:15:41 EST
Package Change Request
======================
Package Name: mailgraph
New Branches: EL-5
Owners: bjohnson mfleming
Comment 26 Kevin Fenzi 2008-12-07 19:42:29 EST
cvs done.
Comment 27 Bernard Johnson 2014-08-21 22:27:43 EDT
Package Change Request
======================
Package Name: mailgraph
New Branches: epel7
Owners: bjohnson
Comment 28 Gwyn Ciesla 2014-08-22 08:06:13 EDT
Git done (by process-git-requests).

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