Bug 220931 - Review Request: ZoneMinder - Linux CCTV package
Summary: Review Request: ZoneMinder - Linux CCTV package
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jason Tibbitts
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2006-12-28 23:30 UTC by Martin Ebourne
Modified: 2007-11-30 22:11 UTC (History)
3 users (show)

Fixed In Version: 1.22.3-6.fc7
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2007-07-03 16:27:10 UTC
Type: ---
Embargoed:
j: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Martin Ebourne 2006-12-28 23:30:57 UTC
Spec URL: http://ebourne.me.uk/dload/fedora/ZoneMinder.spec
SRPM URL: http://ebourne.me.uk/dload/fedora/ZoneMinder-1.22.3-1.src.rpm
Description:
ZoneMinder is the top Linux video camera security and surveillance solution from here: http://www.zoneminder.com/

This is my first package so I will need a sponser.

I'll follow up with more details on packaging decisions made.

Comment 1 Jason Tibbitts 2006-12-28 23:41:30 UTC
How did you get around the requirement for ffmpeg?

Comment 2 Martin Ebourne 2006-12-28 23:47:13 UTC
^sponser^sponsor

Package name:

I've called the package ZoneMinder because this is the name of the program (as
used generally and as per configure.in) and the name of the tarball. However,
the original is a bit schizophrenic and variously puts things in subdirectores
called ZoneMinder, or just zm. The init script in the tarball is just called zm.
I've gone along with what the tarball uses where possible.

For further confusion there are older ZoneMinder RPM packages around just called
zm, but these have been inaccessible for some months and were never in a fedora
repo.

I personally would prefer to call it zoneminder but nothing else really uses
that. I'm up for any suggestions on this one though.

Conflicts:

Because of the old zm package some people have installed I've marked this
package as conflicts: zm. I originally tried obsoletes but the old zm package
does rm -rf /var/lib/zm in postun which is very antisocial and I didn't want
this to happen accidentally to anyone.

X10:

This package does not include the X10 option since it needs the perl X10 module.
If this is accepted I'll look into packaging that as well, then add a
ZoneMinder-X10 subpackage.

FFMPEG:

This package is built without mpeg. Quote from the ZoneMinder manual:
  'ZoneMinder can generate MPEG videos if necessary, for this you'll need either
ffmpeg (recommended) or the Berkeley MPEG encoder (mpeg_encode). If you don't
have either, don't worry, as the options will be hidden and you'll not really
miss too much.'

The builtin jpeg streaming works perfectly well and is more portable than mpeg.
I've not used the mpeg support in over a year of using ZoneMinder.

Comment 3 Martin Ebourne 2006-12-28 23:55:20 UTC
RPM lint reports the following, interspersed with my reasons for not fixing them:

E: ZoneMinder non-readable /etc/zm.conf 0600

The database password is stored in this file, it is intentionally root only
readable.

E: ZoneMinder non-standard-executable-perm /usr/bin/zmfix 04755
E: ZoneMinder setuid-binary /usr/bin/zmfix root 04755

This is an intentional upstream setuid helper for handling device permissions.

E: ZoneMinder non-standard-gid /etc/zm.conf apache
E: ZoneMinder non-standard-uid /etc/zm.conf apache
E: ZoneMinder non-standard-gid /var/lib/zm apache
E: ZoneMinder non-standard-uid /var/lib/zm apache
E: ZoneMinder non-standard-gid /var/log/zm apache
E: ZoneMinder non-standard-uid /var/log/zm apache

Half of ZoneMinder runs under apache/php so these permissions are needed.

W: ZoneMinder incoherent-init-script-name zm

Package name inconsistency as described above.

W: ZoneMinder log-files-without-logrotate /var/log/zm

ZoneMinder does its own log rotation.

W: ZoneMinder no-reload-entry /etc/rc.d/init.d/zm

As per upstream, not needed for this package.

W: ZoneMinder service-default-enabled /etc/rc.d/init.d/zm

This is not a network listening daemon, nor is it installed by default, so if
you installed it you probably want it. Again, as per upstream.

W: ZoneMinder symlink-should-be-relative /usr/share/ZoneMinder/www/events
/var/lib/zm/events
W: ZoneMinder symlink-should-be-relative /usr/share/ZoneMinder/www/images
/var/lib/zm/images
W: ZoneMinder symlink-should-be-relative /usr/share/ZoneMinder/www/temp
/var/lib/zm/temp

Don't think the usual chroot implications are valid here. Also I don't see how
to make the links relative while using the rpm macros for locations (barring
nasty sed hackery).

W: ZoneMinder wrong-file-end-of-line-encoding
/usr/share/doc/ZoneMinder-1.22.3/README.pdf

Just wrong.

Comment 4 Christopher Stone 2006-12-29 00:50:59 UTC
(In reply to comment #3)
> W: ZoneMinder wrong-file-end-of-line-encoding
> /usr/share/doc/ZoneMinder-1.22.3/README.pdf
> 
> Just wrong.

See bug #220061

Comment 5 Jason Tibbitts 2007-06-10 04:14:54 UTC
Hmm, I'd really like to have an up-to-date zm package.  Are you still interested
in getting this into the distro?

Some quick comments:

You'll need a build dependency on at least perl(ExtUtils::MakeMaker); it fails
to build for me otherwise.

If possible, Source1: should be a full URL to the upstream cambozola source.

Is it not possible to direct the program to look directly in /var/lib/zm/*
instead of needing symlinks?  Otherwise, I do think they should be relative, and
it's trivial to accomplish:

ln -sf ../../../../var/lib/zm/$dir $RPM_BUILD_ROOT%{_datadir}/%{name}/www/$dir

in the %install section.

You can use %{perl_vendorlib} instead of %{_prefix}/lib/perl5/vendor_perl/*

I have to say that putting the cgi bits in /usr/libexec seems odd to me; I'll
have to ask around to see if other folks think it's OK.


Comment 6 Martin Ebourne 2007-06-11 22:13:40 UTC
Yes, I sure would like to get this into Fedora (and a few other packages as
well), but I need a sponsor and although there seem to be interested people, in
6 months no-one has yet volunteered.

I've updated the package based on Jason Tibbetts review, and also renamed it to
zoneminder, which I think is a much better name, and improved the naming
consistency. New files at:

Spec URL: http://ebourne.me.uk/dload/fedora/zoneminder.spec
SRPM URL: http://ebourne.me.uk/dload/fedora/zoneminder-1.22.3-3.fc7.src.rpm

This builds fine on Fedora 7.

Regarding the symlinks, I don't think they can be removed since these are
directories under the web root. I've made them relative as per the suggestion
but of course this only works if the number of path segments doesn't change in
${_datadir}.

Comment 7 Jason Tibbitts 2007-06-23 18:39:11 UTC
Full review forthcoming.

Comment 8 Jason Tibbitts 2007-06-23 20:25:29 UTC
BTW, if you have other packages, you should submit them.  Sponsorship is always harder to achieve when you've only provided one packaging sample.  Maybe that's one reason this ticket has been around for so long.  But no matter; I'll take care of things now.

I think an assumption that _datadir will change only rarely is OK and better than having the absolute symlinks.

I was actually asking whether the application could be patched when I asked if it's not possible to direct the program to look directly in /var/lib/zm/*
instead of needing symlinks.  I guess the fundamental question is whether that data needs to be directly accessible from a URL.  And a related question is whether those directories need to be restricted in some way.

I just checked my zoneminder installation and I'm rather surprised to see that you can look in the events directory and see basically everything without logging in at all.  Now, it's possible that my installation is screwed up; I'm using some other packaging so whatever it does might not be duplicated by this package.  But in this package I don't see anything which would prevent this; FollowSymlinks is explicitly set, as is Indexes.

So in fact, I think that it's rather critically important that what's in /var/lib/zm not be visible at all from the web, and so it really shouldn't be present or reachable from /usr/share/zoneminder/www.  This either entails patching the software to just look in /var/lib/zm directly and to drop the symlinks, or to restrict access to those directories somehow with .htaccess files or directly in the zoneminder.conf file.

And on the subject of access control, if this package ships with some sort of default password, the default access controls in zoneminder.conf need to deny all access by default except that from localhost.  Currently just installing this package gives the world access, and that coupled with default passwords is bad.  Usually packages include a README.Fedora file explaining additional configuration bits like this which need to be done.

The scriptlets look a bit weird; it's meaningless to have the #! line in them since they are passed to /bin/sh by default unless you specify another shell with -p.  But I don't really see any issue with adding comments to a scriptlet.

I note that some additional features are enabled when Archive::Tar is installed; as it's a small module, would it be worth adding it as a dependency?

In summary, I see the access control issues as blockers.

Review:
* source files match upstream:
   6bee02be8d5e21d3435c17def157a87727330ee6480be3a8fa5b1966cc10a6bc  
   ZoneMinder-1.22.3.tar.gz
   257d2866fea1dd884810ae00828f32e852568c49cd7ef7560f67fa4f496d1c13  
   cambozola-0.68.tar.gz
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.
* description is OK.
* dist tag is present.
* build root is OK.
* license field matches the actual license.
* license is open source-compatible.
* license text included in package.
* latest version is being packaged.
* BuildRequires are proper.
* compiler flags are appropriate.
* %clean is present.
* package builds in mock (development, x86_64).
* package installs properly
* debuginfo package looks complete.
* rpmlint has acceptable complaints.
* final provides and requires are sane:
   config(zoneminder) = 1.22.3-3.fc8
   perl(ZoneMinder)
   perl(ZoneMinder::Base) = 1.22.3
   perl(ZoneMinder::Config)
   perl(ZoneMinder::ConfigAdmin)
   perl(ZoneMinder::Database)
   perl(ZoneMinder::Debug)
   perl(ZoneMinder::SharedMem)
   perl(ZoneMinder::Trigger::Channel)
   perl(ZoneMinder::Trigger::Channel::File)
   perl(ZoneMinder::Trigger::Channel::Handle)
   perl(ZoneMinder::Trigger::Channel::Inet)
   perl(ZoneMinder::Trigger::Channel::Serial)
   perl(ZoneMinder::Trigger::Channel::Spawning)
   perl(ZoneMinder::Trigger::Channel::Unix)
   perl(ZoneMinder::Trigger::Connection)
   perl(ZoneMinder::Trigger::Connection::Example)
   zoneminder = 1.22.3-3.fc8
  =
   /bin/sh
   /sbin/chkconfig
   /sbin/service
   /usr/bin/perl
   config(zoneminder) = 1.22.3-3.fc8
   httpd
   libcrypto.so.6()(64bit)
   libgcc_s.so.1()(64bit)
   libgcc_s.so.1(GCC_3.0)(64bit)
   libjpeg.so.62()(64bit)
   libmysqlclient.so.15()(64bit)
   libmysqlclient.so.15(libmysqlclient_15)(64bit)
   libpcre.so.0()(64bit)
   libstdc++.so.6()(64bit)
   libstdc++.so.6(CXXABI_1.3)(64bit)
   libstdc++.so.6(GLIBCXX_3.4)(64bit)
   libz.so.1()(64bit)
   perl >= 0:5.006
   perl(Carp)
   perl(DBD::mysql)
   perl(DBI)
   perl(Data::Dumper)
   perl(Date::Manip)
   perl(Device::SerialPort)
   perl(Exporter)
   perl(Fcntl)
   perl(Getopt::Long)
   perl(IO::Handle)
   perl(LWP::UserAgent)
   perl(POSIX)
   perl(Socket)
   perl(Storable)
   perl(Sys::Syslog)
   perl(Time::HiRes)
   perl(ZoneMinder)
   perl(ZoneMinder::Base)
   perl(ZoneMinder::Config)
   perl(ZoneMinder::ConfigAdmin)
   perl(ZoneMinder::Database)
   perl(ZoneMinder::Debug)
   perl(ZoneMinder::SharedMem)
   perl(ZoneMinder::Trigger::Channel)
   perl(ZoneMinder::Trigger::Channel::Handle)
   perl(ZoneMinder::Trigger::Channel::Inet)
   perl(ZoneMinder::Trigger::Channel::Serial)
   perl(ZoneMinder::Trigger::Channel::Spawning)
   perl(ZoneMinder::Trigger::Channel::Unix)
   perl(ZoneMinder::Trigger::Connection)
   perl(bytes)
   perl(constant)
   perl(strict)
   perl(warnings)
* %check is not present; no test suite upstream.  I don't have the means to test    
  this at the moment.
* no shared libraries are added to the regular linker search paths.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* scriptlets are OK (service installation)
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.

Comment 9 Martin Ebourne 2007-06-23 21:47:14 UTC
Jason, thanks for taking this on.

Regarding the symlinks under the webroot I should have been more specific. Those
are used directly as URLs, and not just as a data source for the php, and hence
really do need to be web accessible. eg. If you go to an event and view it as
stills, those images are coming directly from the directory, eg:

https://xxx.xxx.xxx/app/zm/events/4/298040/008-capture.jpg

I agree about the access control. I didn't even realise zm had its own access
control, I must have switched that off as soon as I installed it (I'm paranoid
and only trust apache access control). The lack of password for viewing images
appears to be a design bug in zm, don't see how it can expect to control those
and still reference them by direct url.

My preferred solution to this would be to make the zoneminder apache config
default to barring all access and add a README.Fedora file explaining what to do
to enable it and warning about the issues. Then I'll take it over to the zm
forums and see if Phil is interested in addressing that properly in a future
version.

Does this sound like an acceptable solution?

Regarding your other points I've made those changes and they will be in the next
revision when it is complete.


Comment 10 Jason Tibbitts 2007-06-24 20:19:35 UTC
I didn't realize that the files were actually served directly out of the events
directory.

Your solution seems to be about as good as possible, except that in addition I
think you could consider disabling indices in those directories (or everywhere
in the zm directory).  Frankly I don't understand why it might be useful to have
them enabled.  You might also consider documenting how to disable the
zoneminder's login and set up regular Apache access control in this case.

It's really not much protection as it should be trivial to guess the directory
structure there; zm would have to switch to using random strings instead of
whole numbers starting at 1 for cameras and events if the authors don't want to
switch to serving that data via a CGI.

Comment 11 Martin Ebourne 2007-06-26 22:59:28 UTC
This update should address all outstanding concerns:

Spec URL: http://ebourne.me.uk/dload/fedora/zoneminder.spec
SRPM URL: http://ebourne.me.uk/dload/fedora/zoneminder-1.22.3-4.fc7.src.rpm


Comment 12 Jason Tibbitts 2007-06-27 19:33:01 UTC
OK, I think we're pretty much done here.

One final issue to consider: I'm almost certain that selinux will block
zoneminder.  I still don't know nearly enough about selinux, but we do have some
procedures for including policies in packages and the folks on the
fedora-selinux mailing list should be able to help you out.  However, while some
would argue that proper functioning under selinux is a requirement for package
approval, I have never believed that to be productive, and so I think that all
you really need to do is add a note to the README.Fedora file indicating that
the package probably doesn't work properly with selinux enabled.

APPROVED

Go ahead and apply for sponsorship and I'll take care of it.  Let me know if you
need any assistance getting that done or getting your package imported.

Comment 13 Martin Ebourne 2007-06-28 00:52:17 UTC
New Package CVS Request
=======================
Package Name: zoneminder
Short Description: A camera monitoring and analysis tool
Owners: fedora.uk
Branches: FC-6 F-7
InitialCC:


Comment 14 Martin Ebourne 2007-06-28 00:53:24 UTC
Can someone set fedora-cvs = ? for me? I don't yet have the right access.

Comment 15 Jason Tibbitts 2007-06-28 01:06:45 UTC
Have you actually applied for cvsextras access yet?  You can't make CVS requests
because you don't have the necessary privileges, which you can't get unless you
apply for it.  If you need instructions, see
http://fedoraproject.org/wiki/PackageMaintainers/Join
I think you're at the "Get a Fedora Account" or "Get Sponsored" steps.

Comment 16 Kevin Fenzi 2007-06-28 20:08:17 UTC
cvs done.

Comment 17 Jima 2007-06-29 14:33:51 UTC
Slight problem.

While /var/lib/zoneminder is properly owned by the apache user, the directories
within it (events, images, temp) are not.  I'm not sure images/ is critical
(does it need to write there?), but I KNOW events/ and temp/ should be owned by
the user zoneminder is running as (apache), else it can't save event data or
generate zip files.

I'd appreciate this being addressed, as it adds a step after installation (chown
...) that really shouldn't be necessary.

Thanks for the package! :-)

Comment 18 Martin Ebourne 2007-07-01 01:02:24 UTC
Updated package to address all comments above.

All built and should appear in updates-testing for fc6, f7 and in rawhide
sometime soon.

Everyone please give me any feedback you can (good or bad) so I'll know when to
mark it stable.

Comment 19 Jason Tibbitts 2007-07-02 23:11:36 UTC
Note that at this time I cannot actually test this package as I have an existing
zoneminder installation which I cannot yet upgrade.

Frankly I wouldn't wait release this; best to get it out there and deal with any
bug reports afterwards.

Comment 20 Fedora Update System 2007-07-03 16:27:05 UTC
zoneminder-1.22.3-6.fc7 has been pushed to the Fedora 7 stable repository.  If problems still persist, please make note of it in this bug report.


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