Bug 222552 (mailgraph)

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).