Package: munin
Version: 1.2.6-10~lenny2
Severity: normal
Tags: patch

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1


Having switched to using munin-cgi-graph for graph generation, I identified
the problem related to checking last modified time of the image file. It fails
to check it with following error output: 

- -- /var/log/apache2/error.log --

[Mon Jun 07 12:27:14 2010] [error] [client 194.80.32.10] Use of uninitialized
value $ifmodsec in numeric lt (<) at /usr/lib/cgi-bin/munin-cgi-graph line
300., referer: ...

First I tried to change the "modified" subroutine:

- --- munin-cgi-graph ---

@@ -78,8 +78,7 @@
     my $slast_modified = strftime ("%a, %d %b %Y %H:%M:%S %Z", localtime 
($sstats[9]));

     if (defined $ENV{HTTP_IF_MODIFIED_SINCE} and
- -       !&modified (gmtime(time+($period{$scale}-($time%$period{$scale}))),
- -                   $sstats[9]-1)) {
+       !&modified ($ENV{HTTP_IF_MODIFIED_SINCE}, $sstats[9]-1)) {
        print "Status: 304\n";
        print "Content-Type: image/png\n";
        print "Expires: ", strftime ("%a, %d %b %Y %H:%M:%S GMT", 
gmtime(time+($period{$scale}-($time%$period{$scale})))), "\n";

This solved the "error", but also it stopped graphs from being generated
completely. I did not have time looking deeper into the problem, so I have
solved the problem localy by substituting munin-cgi-graph file from version
1.4.5, the patch is attached. Though it might be not the best way from the
security point of view. 

Whould be great if someone with more Munin experience look into it.

Thanks,
Ruslan Kabalin

- -- System Information: --

Debian Release: 5.0.4
APT prefers stable
APT policy: (500, 'stable')
Architecture: amd64 (x86_64)

Kernel: Linux 2.6.26-2-amd64 (SMP w/4 CPU cores)
Locale: LANG=C, LC_CTYPE=C (charmap=ANSI_X3.4-1968)
Shell: /bin/sh linked to /bin/bash

Versions of packages munin depends on:
ii  adduser                  3.110           add and remove users and groups
pn  libdigest-md5-perl       <none>          (no description available)
ii  libhtml-template-perl    2.9-1           HTML::Template : A module for usin
ii  librrds-perl             1.3.1-4         Time-series data storage and displ
pn  libstorable-perl         <none>          (no description available)
ii  perl [libtime-hires-perl 5.10.0-19lenny2 Larry Wall's Practical Extraction
ii  perl-modules             5.10.0-19lenny2 Core Perl modules
ii  rrdtool                  1.3.1-4         Time-series data storage and displ

Versions of packages munin recommends:
ii  libdate-manip-perl 5.54-1                a perl library for manipulating da
ii  munin-node         1.2.6-10~lenny2       network-wide graphing framework (n

Versions of packages munin suggests:
ii  apache2-mpm-prefork [htt 2.2.9-10+lenny7 Apache HTTP Server - traditional n
pn  www-browser              <none>          (no description available)

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)

iQEcBAEBAgAGBQJMDkeJAAoJEIwasS2vea/+V7AIAJ+dsVQRfgwJTOWg9km3m1oO
2vSvVejkxhkmRgLKK7ZRoiZoBdb1YP3jWUA9ck3P+LtXZqRfxoccKs0Cc+Cl1WZN
1sxSVSxa/sSrf+Ln/JOylt0AfBp5a2/Nfgv0/aULNKnlxhmteedSJRiVor6iLQTQ
P4a+dyvpNH4oAZDFPSO1KyToyRWo6DiDTSKv2jE3WIN+ob8E8yk0206irZUXe/P/
yl+xvQGyB70i9iVui9v3KQlcbwHHlKxVQc1G9Jd0F1iANQKqDX4exouMcRSLQKKg
cEhU2CHVao8kjhOzOgXrzdDt+1CeW2Dy4Jr9I34UZZeBAHdHxV3yn3AHxBpVwNE=
=EzdH
-----END PGP SIGNATURE-----
>From 7b46994064950d09ff2f2b08b75f049cc7390807 Mon Sep 17 00:00:00 2001
From: Ruslan Kabalin <ruslan.kaba...@luns.net.uk>
Date: Tue, 8 Jun 2010 11:49:26 +0100
Subject: [PATCH] Replaced munin-cgi-graph.in with one in version 1.4.5 (with some changes).

This was required to prevent error:
Use of uninitialized value $ifmodsec in numeric lt (<) at
/usr/lib/cgi-bin/munin-cgi-graph line 300., referer:
https://sysmon.luns.net.uk/munin/cleo.infra.local/apocrypha.cleo.infra.local.html
---
 server/munin-cgi-graph.in |  199 ++++++++++++++++++++++++++++-----------------
 1 files changed, 125 insertions(+), 74 deletions(-)

diff --git a/server/munin-cgi-graph.in b/server/munin-cgi-graph.in
index 31c3baf..bb20af3 100755
--- a/server/munin-cgi-graph.in
+++ b/server/munin-cgi-graph.in
@@ -15,15 +15,17 @@
 #
 # You should have received a copy of the GNU General Public License
 # along with this program; if not, write to the Free Software
-# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA  02111-1307, USA.
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 #
-# $Id: munin-cgi-graph.in 1492 2008-02-23 22:00:00Z matthias $
+# $Id: munin-cgi-graph.in 3304 2010-01-13 12:55:04Z knan $
 #
 # Please see http://munin.projects.linpro.no/wiki/CgiHowto for how to
 # use this, and how to convert it to fastcgi which will improve speed.
 #
 
 use RRDs;
+#use Munin::Master::Utils;
+#use Munin::Common::Defaults;
 use Munin;
 use strict;
 use IO::Handle;
@@ -62,7 +64,7 @@ my $config = &munin_readconfig ($conffile);
 
 my $path = $ENV{PATH_INFO} || "";
 $path =~ s/^\///;
-($dom, $host, $serv) = split /\//, $path;  
+($dom, $host, $serv) = split /\//, $path;
 ($serv, $scale) = split /-/, $serv, 2;
 $scale =~ s/\.png$//;
 
@@ -72,85 +74,107 @@ my $filename = get_picture_filename ($config, $dom, $host, $serv, $scale);
 
 my $time = time;
 
-if (-f $filename)
-{
-    my @sstats = stat ($filename);
-    my $slast_modified = strftime ("%a, %d %b %Y %H:%M:%S %Z", localtime ($sstats[9]));
+# Get semaphore handle - Before graphing as recommended by snide.
+my $semid = undef;
 
-    if (defined $ENV{HTTP_IF_MODIFIED_SINCE} and 
-	!&modified (gmtime(time+($period{$scale}-($time%$period{$scale}))),
-		    $sstats[9]-1)) {
-	print "Status: 304\n";
-	print "Content-Type: image/png\n";
-	print "Expires: ", strftime ("%a, %d %b %Y %H:%M:%S GMT", gmtime(time+($period{$scale}-($time%$period{$scale})))), "\n";
-	print "Last-Modified: $slast_modified\n";
-	print "\n";
-	exit 0;
-    }
-}
+sem_setup();
 
-if (! &graph_usable ($filename, $time))
-{
-    my $ret = (&draw_graph ($host, $serv, $TIMES{$scale}) || "Unknown error");
-    if (! -f $filename)
-    {
-	::logger ("Warning: Could not draw graph \"$host-$serv-$scale.png\": $ret");
-	print "Status: 500\n";
-	print "Content-Type: image/png\n";
-	print "\n";
-	exit 0;
-    }
+my $no_cache = defined($ENV{HTTP_CACHE_CONTROL}) && $ENV{HTTP_CACHE_CONTROL} =~ /no-cache/i;
+
+if ($no_cache or (! &graph_usable($filename,$time) )) {
+    exit 0 unless draw_graph_or_complain($host, $serv, $TIMES{$scale});
+    goto draw;
 }
 
-my @stats = stat ($filename);
+# At this time the file exists.  But may be old.  Or not.
+
+my @stats         = stat ($filename);
 my $last_modified = strftime ("%a, %d %b %Y %H:%M:%S %Z", localtime ($stats[9]));
+# "Expires" has to use last modified time as base:
+my $expires       = strftime ("%a, %d %b %Y %H:%M:%S GMT",
+			      gmtime($stats[9]+($period{$scale}-($stats[9]%$period{$scale}))));
+
+# Check for If-Modified-Since and send 304 if not changed:
+if (defined $ENV{HTTP_IF_MODIFIED_SINCE} and
+    !&modified ($ENV{HTTP_IF_MODIFIED_SINCE}, $stats[9]-1)) {
+    print "Status: 304\n";
+    print "Content-Type: image/png\n";
+    print "Expires: ", $expires, "\n";
+    print "Last-Modified: $last_modified\n";
+    print "\n";
+    exit 0;
+}
+
+draw:
+
+    @stats = stat ($filename) unless @stats;
+
+$last_modified = strftime ("%a, %d %b %Y %H:%M:%S %Z", localtime ($stats[9]))
+    unless defined($last_modified);
+
+$expires       = strftime ("%a, %d %b %Y %H:%M:%S GMT",
+			     gmtime($stats[9]+($period{$scale}-($stats[9]%$period{$scale}))))
+    unless defined($expires);
 
 print "Content-Type: image/png\n";
 print "Expires: ", strftime ("%a, %d %b %Y %H:%M:%S GMT", gmtime(time+($period{$scale}-($time%$period{$scale})))), "\n";
 print "Last-Modified: $last_modified\n";
 print "\n";
 
-# Try to police the number of concurrent rrdgraph instances.  The
-# third value is the default maximum.
+&graph ($filename);
+
+# # # # # # # # # # END OF MAIN
+
+sub sem_setup {
+    # Try to police the number of concurrent rrdgraph instances.
 
-# Fox kindly submitted a patch to convert to SysV IPC semaphores.
-# Lovely! (ticket #499).
+    # Fox kindly submitted a patch to convert to SysV IPC semaphores.
+    # Lovely! (ticket #499).
 
-my $max_cgi_graph_jobs = &munin_get ($config, "max_cgi_graph_jobs" , 6, $dom);
+    $semid = semget($IPC_KEY, 0, 0 );
 
-my $opstring; 
+    if(!defined($semid)) {
+	# Or create it if needed
+	$semid = semget($IPC_KEY, 1 , oct(666) | IPC_CREAT );
 
-# Get semaphore handle
-my $semid = semget($IPC_KEY, 0, 0 );
+	die "Creating semaphore: $!" unless defined($semid);
 
-if(!$semid) {
-    # Or create it if needed
-    $semid = semget($IPC_KEY, 1 , 0666 | IPC_CREAT ) ||
-	die "Creating semaphore: $!";
+	my $max_cgi_graph_jobs = &munin_get ($config, "max_cgi_graph_jobs" , 6, $dom);
 
-    # And initialize to max_cgi_graph_jobs
-    $opstring = pack("s!s!s!",0, $max_cgi_graph_jobs,0);
-    semop($semid,$opstring) || die "$!";
+	# And initialize to max_cgi_graph_jobs
+	my $opstring = pack("s!s!s!",0, $max_cgi_graph_jobs,0);
+	semop($semid,$opstring) || die "$!";
+    }
 }
 
-# Decrement, or lock/hang/yield if already 0
-$opstring = pack("s!s!s!",0, -1, 0);
-semop($semid,$opstring);
 
-&graph ($filename);
+sub sem_get {
+    # Call this before doing heavy work.
+    # Decrement, or lock/hang/yield if already 0
+    my $opstring = pack("s!s!s!",0, -1, 0);
+    semop($semid,$opstring);
+}
+
+
+sub sem_release {
+    # Call this after doing heavy work.
+    # Increment (and release waiting processes).
+    my $opstring = pack("s!s!s!",0, 1, 0);
+    semop($semid,$opstring);
+}
 
-# Increment (and release waiting processes)
-$opstring = pack("s!s!s!",0, 1, 0);
-semop($semid,$opstring);
 
 sub graph {
+    # This just serves the file, no file is made.
     my $filename = shift;
 
-    open (GRAPH, $filename) or die "Warning: Could not open picture file \"$filename\" for reading: $!\n";
-    print while (<GRAPH>);
-    close (GRAPH);
+    open (my $GRAPH, '<', $filename)
+        or die "Warning: Could not open picture file \"$filename\" for reading: $!\n";
+    print while (<$GRAPH>);
+    close ($GRAPH);
 }
 
+
 sub get_picture_filename {
     my $config  = shift;
     my $domain  = shift;
@@ -161,18 +185,20 @@ sub get_picture_filename {
     return "$config->{'htmldir'}/$domain/$name-$service-$scale.png";
 }
 
+
 sub logger_open {
     my $dirname = shift;
 
     if (!$log->opened)
     {
-	unless (open ($log, ">>$dirname/munin-cgi-graph.log"))
+	unless (open ($log, '>>', "$dirname/munin-cgi-graph.log"))
 	{
 	    print STDERR "Warning: Could not open log file \"$dirname/munin-cgi-graph.log\" for writing: $!";
 	}
     }
 }
 
+
 sub logger {
   my ($comment) = @_;
   my $now = strftime ("%b %d %H:%M:%S", localtime);
@@ -185,10 +211,10 @@ sub logger {
   {
           if (defined $config->{logdir})
           {
-                  if (open ($log, ">>$config->{logdir}/munin-cgi-graph.log"))
+                  if (open ($log, '>>', "$config->{logdir}/munin-cgi-graph.log"))
                   {
                           print $log "$now - $comment\n";
-                  }
+                      }
                   else
                   {
                           print STDERR "Warning: Could not open log file \"$config->{logdir}/munin-cgi-graph.log\" for wr
@@ -242,17 +268,15 @@ sub verify_parameters
 	}
 }
 
+
 sub graph_usable {
-    my $filename = shift;
-    my $time     = shift;
+    # Check how old the graph is and return 1 if it's new enough and 0 otherwise.
+    my ($filename, $time) = @_;
 
     if (-f $filename) {
 	my @stats = stat (_);
-	my $expire = ($stats[9] - $time%$period{$scale}+$period{$scale})-$time;
-#print STDERR "Expires in: $expire\n";
-
-	if ($expire >= 0) {
-#print STDERR "Skipping munin-graph-run for \"$filename\".\n";
+	# $stats[9] holds the "last update" time and this needs to be in the last update period:
+	if ($stats[9] > ($time - $time%$period{$scale})) {
 #print STDERR ("Graph unexpired for $scale. ($stats[9] , $time, ". ($time%$period{$scale}). ", ". ($time - $time%$period{$scale}). ").\n");
 	    return 1;
 	} else {
@@ -263,33 +287,60 @@ sub graph_usable {
     return 0;
 }
 
+
 sub draw_graph {
     my $host  = shift;
     my $serv  = shift;
     my $scale = shift;
 
-    $serv =~ s/[^\w_\/"'\[\]\(\)+=-]/_/; $serv =~ /^(.+)$/; $serv = $1; #"
-    $host =~ s/[^\w_\/"'\[\]\(\)+=-]/_/; $host =~ /^(.+)$/; $host = $1; #"
+    $serv =~ s{[^\w_/"'\[\]\(\)\+=-]}{_}g; $serv =~ /^(.+)$/; $serv = $1; #"
+    # . needs to be legal in host names
+    $host =~ s{[^\w_/"'\[\]\(\)\.+=-]}{_}g; $host =~ /^(.+)$/; $host = $1; #"
 
     my @params = ($GRAPHER);
     push @params, @$scale;
-    push @params, "--skip-locking", "--skip-stats", "--nolazy";
+    push @params, "--skip-locking", "--skip-stats", "--nolazy", "--list-images";
     push @params, "--host", $host, "--service", $serv;
     push @params, "STDERR>&STDOUT";
 
     my $file = "/dev/null";
-    unless (open (IN, "-|")) 
-    { 
-	%ENV=(); 
+    my $IN;
+    sem_get();
+
+    # Note: This open is an implicit fork
+    if (! open ($IN, "-|")) {
+	%ENV=();
 	exec @params;
     }
-    $file = join (' ', <IN>);
-    close (IN);
+    $file = join (' ', <$IN>);
+    chomp($file);
+
+    close ($IN);
+    sem_release();
+
     return $file;
 }
 
+
+sub draw_graph_or_complain {
+    my $ret = draw_graph(@_);
+
+    if (! -f $ret) {
+	::logger ("Warning: Could not draw graph \"$host-$serv-$scale.png\": $ret");
+	print "Status: 500\n";
+	print "Content-Type: image/png\n";
+	print "\n";
+	return 0;
+    } else {
+	return $ret;
+    }
+}
+
+
 sub modified {
-# Format of since_string If-Modified-Since: Wed, 23 Jun 2004 16:11:06 GMT
+    # See if file has been modified since "the last time".
+
+    # Format of since_string If-Modified-Since: Wed, 23 Jun 2004 16:11:06 GMT
 
     my $since_string = shift;
     my $created      = shift;
-- 
1.5.6.5

>From c967113586cfd096c33dc49713459c68fd88043e Mon Sep 17 00:00:00 2001
From: Ruslan Kabalin <ruslan.kaba...@luns.net.uk>
Date: Tue, 8 Jun 2010 12:12:56 +0100
Subject: [PATCH] Disabled munin-cgi-graph related patches

---
 debian/patches/series |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/debian/patches/series b/debian/patches/series
index 4365db0..b2b1304 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -38,6 +38,6 @@
 410-muninnodeconf-manpage.patch
 420-munin-limits.excessive-notifications.patch
 440-node.d.linux-iostat.patch
-450-munin-cgi-graph.patch
+#450-munin-cgi-graph.patch
 460-munin-cgi-graph-mkdir.patch
 610-plugin-cpu-fix-max.patch
-- 
1.5.6.5

Reply via email to