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.
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).
Sorry for the noise, I may not formally assign the package to myself. Reassigning back to nobody
(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> - 1.12-2 - add Requires(preun), Requires(post) for chkconfig
I'd be happy to review this package. Look for a full review here in a bit...
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.
(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.
> 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 :-)
(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.
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?
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> - 1.12-3 - consistent use of mode in creating directories - add Requires for httpd - preserve timestamps on install
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.
(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> - 1.12-4 - replace %%{_var} with %%{_localstatedir}
(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)
(Removing NEEDSPONSOR as bug 216723)
(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.u-tokyo.ac.jp provided sponsorship for me.
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.
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?
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 ?
Try this: # chcon -t httpd_script_exec_t /usr/share/mailgraph/mailgraph.cgi No need to modify httpd.conf if this works.
Sorry, that should have been httpd_sys_script_exec_t in Comment #19.
(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
(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
Try: # chcon -Rh -t httpd_var_lib_t /var/lib/mailgraph # chcon -Rh -t httpd_cache_t /var/cache/mailgraph
Hi, this is the closed review bug. Please open another bug regarding this issue and I'll get it working with SELinux.
Package Change Request ====================== Package Name: mailgraph New Branches: EL-5 Owners: bjohnson mfleming
cvs done.
Package Change Request ====================== Package Name: mailgraph New Branches: epel7 Owners: bjohnson
Git done (by process-git-requests).