-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Package: nagios-plugins
Version: 1.4.15
Severity: important
The attached patches fix a regression in nagios-plugins check_snmp v1.4.15.
In check_snmp 1.4.15, numeric values returned as a non-integer types
(string, timetick, etc.) aren't treated as numeric anymore and thus
thresholds won't be checked for them.
This is a backport of the patches that made it to master. All patches
must be applied in order (failures on NEWS can be ignored)
Thanks
- --
Thomas
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAk0QxegACgkQ6dZ+Kt5BchbdbQCgtzGIIYYcPXP6+zG0Qye2PCYz
8MUAoN0N9Ljii+iu06ePfoQLB0p7tloF
=o/Xd
-----END PGP SIGNATURE-----
>From 4d1cd3a7dc2754f4fb69f2a80ce4e2a027f180a3 Mon Sep 17 00:00:00 2001
From: nagios <nag...@opsviewdev32.(none)>
Date: Mon, 15 Nov 2010 15:43:41 +0000
Subject: [PATCH 1/4] check_snmp now considers strings returned by SNMP that contain just numbers (according to strtod) to be a numeric value for threshold and performance data
---
NEWS | 7 +++++++
plugins/check_snmp.c | 14 ++++++++++++++
plugins/tests/check_snmp.t | 17 ++++++++++++++++-
plugins/tests/check_snmp_agent.pl | 6 +++---
4 files changed, 40 insertions(+), 4 deletions(-)
diff --git a/NEWS b/NEWS
index ff92401..659bd2d 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,12 @@
This file documents the major additions and syntax changes between releases.
+ ...
+
+ FIXES
+ check_snmp now attempts to convert string responses into a double value. If the full string is a value,
+ check_snmp will consider it a numeric value and thus apply threshold checks and return performance data.
+ This reverts back to 1.4.14 behaviour with strings
+
1.4.15 27th July 2010
ENHANCEMENTS
New check_ntp_peer -m and -n options to check the number of usable time sources ("truechimers")
diff --git a/plugins/check_snmp.c b/plugins/check_snmp.c
index f32a26e..9d91942 100644
--- a/plugins/check_snmp.c
+++ b/plugins/check_snmp.c
@@ -160,6 +160,7 @@ main (int argc, char **argv)
char *outbuff;
char *ptr = NULL;
char *show = NULL;
+ char *endptr = NULL;
char *th_warn=NULL;
char *th_crit=NULL;
char type[8] = "";
@@ -395,6 +396,19 @@ main (int argc, char **argv)
}
}
+ /* Allow numeric conversion if whole string is a number. Make concession for strings with " at beginning or end */
+ /* This duplicates the conversion a bit later, but is cleaner to separate out the checking against the conversion */
+ ptr = show;
+ if (*ptr == '"')
+ ptr++;
+ if (*ptr != '\0' ) {
+ strtod( ptr, &endptr );
+ if (*endptr == '"')
+ endptr++;
+ if (*endptr == '\0')
+ is_numeric=1;
+ }
+
}
else if (strstr (response, "Timeticks: "))
show = strstr (response, "Timeticks: ");
diff --git a/plugins/tests/check_snmp.t b/plugins/tests/check_snmp.t
index e7ad192..08348d2 100755
--- a/plugins/tests/check_snmp.t
+++ b/plugins/tests/check_snmp.t
@@ -51,7 +51,7 @@ if ($ARGV[0] && $ARGV[0] eq "-d") {
}
}
-my $tests = 33;
+my $tests = 41;
if (-x "./check_snmp") {
plan tests => $tests;
} else {
@@ -170,5 +170,20 @@ $res = NPTest->testCmd( "./check_snmp -H 127.0.0.1 -C public -p $port_snmp -o .1
is($res->return_code, 0, "OK as string doesn't match but inverted" );
is($res->output, 'SNMP OK - "stringtests" | ', "OK as inverted string no match" );
+$res = NPTest->testCmd( "./check_snmp -H 127.0.0.1 -C public -p $port_snmp -o .1.3.6.1.4.1.8072.3.2.67.12" );
+is($res->return_code, 0, "Numeric in string test" );
+is($res->output, 'SNMP OK - 3.5 | iso.3.6.1.4.1.8072.3.2.67.12=3.5 ', "Check seen as numeric" );
+
+$res = NPTest->testCmd( "./check_snmp -H 127.0.0.1 -C public -p $port_snmp -o .1.3.6.1.4.1.8072.3.2.67.12 -w 4:5" );
+is($res->return_code, 1, "Numeric in string test" );
+is($res->output, 'SNMP WARNING - *3.5* | iso.3.6.1.4.1.8072.3.2.67.12=3.5 ', "WARNING threshold checks for string masquerading as number" );
+
+$res = NPTest->testCmd( "./check_snmp -H 127.0.0.1 -C public -p $port_snmp -o .1.3.6.1.4.1.8072.3.2.67.13" );
+is($res->return_code, 0, "Not really numeric test" );
+is($res->output, 'SNMP OK - "87.4startswithnumberbutshouldbestring" | ', "Check string with numeric start is still string" );
+
+$res = NPTest->testCmd( "./check_snmp -H 127.0.0.1 -C public -p $port_snmp -o .1.3.6.1.4.1.8072.3.2.67.14" );
+is($res->return_code, 0, "Not really numeric test (trying best to fool it)" );
+is($res->output, 'SNMP OK - "555\"I said\"" | ', "Check string with a double quote following is still a string (looks like the perl routine will always escape though)" );
diff --git a/plugins/tests/check_snmp_agent.pl b/plugins/tests/check_snmp_agent.pl
index 8784ab1..2ad8516 100644
--- a/plugins/tests/check_snmp_agent.pl
+++ b/plugins/tests/check_snmp_agent.pl
@@ -33,9 +33,9 @@ ends with with this: C:\\';
my $multilin5 = 'And now have fun with with this: "C:\\"
because we\'re not done yet!';
-my @fields = (ASN_OCTET_STR, ASN_OCTET_STR, ASN_OCTET_STR, ASN_OCTET_STR, ASN_OCTET_STR, ASN_UNSIGNED, ASN_UNSIGNED, ASN_COUNTER, ASN_COUNTER64, ASN_UNSIGNED, ASN_COUNTER, ASN_OCTET_STR);
-my @values = ($multiline, $multilin2, $multilin3, $multilin4, $multilin5, 4294965296, 1000, 4294965296, uint64("18446744073709351616"), int(rand(2**32)), 64000, "stringtests");
-my @incrts = (undef, undef, undef, undef, undef, 1000, -500, 1000, 100000, undef, 666, undef);
+my @fields = (ASN_OCTET_STR, ASN_OCTET_STR, ASN_OCTET_STR, ASN_OCTET_STR, ASN_OCTET_STR, ASN_UNSIGNED, ASN_UNSIGNED, ASN_COUNTER, ASN_COUNTER64, ASN_UNSIGNED, ASN_COUNTER, ASN_OCTET_STR, ASN_OCTET_STR, ASN_OCTET_STR, ASN_OCTET_STR );
+my @values = ($multiline, $multilin2, $multilin3, $multilin4, $multilin5, 4294965296, 1000, 4294965296, uint64("18446744073709351616"), int(rand(2**32)), 64000, "stringtests", "3.5", "87.4startswithnumberbutshouldbestring", '555"I said"' );
+my @incrts = (undef, undef, undef, undef, undef, 1000, -500, 1000, 100000, undef, 666, undef, undef, undef, undef );
# Number of elements in our OID
my $oidelts;
--
1.7.3.2
>From c5b1568481420caaee4b898ebf0110e93e2d4be1 Mon Sep 17 00:00:00 2001
From: Thomas Guyot-Sionnest <derm...@aei.ca>
Date: Tue, 30 Nov 2010 21:02:23 -0500
Subject: [PATCH 2/4] check_snmp: Remove that is_numeric madness
Original patch to make Timeticks works as in check_snmp v1.4.14, it turns
out is_numeric isn't so useful and treating all types as numeric works
best for backwards-compatibility. This is how it used to work in 1.4.14.
As a special case, I also make calculate_rate look up for numeric values
as it would otherwise return the last value instead.
---
NEWS | 3 +++
plugins/check_snmp.c | 12 ++++--------
plugins/t/check_snmp.t | 9 ++++++++-
3 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/NEWS b/NEWS
index 659bd2d..27e69c6 100644
--- a/NEWS
+++ b/NEWS
@@ -7,6 +7,9 @@ This file documents the major additions and syntax changes between releases.
check_snmp will consider it a numeric value and thus apply threshold checks and return performance data.
This reverts back to 1.4.14 behaviour with strings
+ FIXES
+ Make check_snmp work more like v1.4.14 with regard to using special values (Timeticks, STRING) as numeric thresholds.
+
1.4.15 27th July 2010
ENHANCEMENTS
New check_ntp_peer -m and -n options to check the number of usable time sources ("truechimers")
diff --git a/plugins/check_snmp.c b/plugins/check_snmp.c
index 9d91942..cb7fb7a 100644
--- a/plugins/check_snmp.c
+++ b/plugins/check_snmp.c
@@ -170,7 +170,6 @@ main (int argc, char **argv)
char *state_string=NULL;
size_t response_length, current_length, string_length;
char *temp_string=NULL;
- int is_numeric=0;
time_t current_time;
double temp_double;
time_t duration;
@@ -336,29 +335,24 @@ main (int argc, char **argv)
/* We strip out the datatype indicator for PHBs */
if (strstr (response, "Gauge: ")) {
show = strstr (response, "Gauge: ") + 7;
- is_numeric++;
}
else if (strstr (response, "Gauge32: ")) {
show = strstr (response, "Gauge32: ") + 9;
- is_numeric++;
}
else if (strstr (response, "Counter32: ")) {
show = strstr (response, "Counter32: ") + 11;
- is_numeric++;
is_counter=1;
if(!calculate_rate)
strcpy(type, "c");
}
else if (strstr (response, "Counter64: ")) {
show = strstr (response, "Counter64: ") + 11;
- is_numeric++;
is_counter=1;
if(!calculate_rate)
strcpy(type, "c");
}
else if (strstr (response, "INTEGER: ")) {
show = strstr (response, "INTEGER: ") + 9;
- is_numeric++;
}
else if (strstr (response, "STRING: ")) {
show = strstr (response, "STRING: ") + 8;
@@ -410,15 +404,17 @@ main (int argc, char **argv)
}
}
- else if (strstr (response, "Timeticks: "))
+ else if (strstr (response, "Timeticks: ")) {
show = strstr (response, "Timeticks: ");
+ }
else
show = response;
iresult = STATE_DEPENDENT;
/* Process this block for numeric comparisons */
- if (is_numeric) {
+ /* Make some special values,like Timeticks numeric only if a threshold is defined */
+ if (thlds[i]->warning || thlds[i]->critical || calculate_rate) {
ptr = strpbrk (show, "0123456789");
if (ptr == NULL)
die (STATE_UNKNOWN,_("No valid data returned"));
diff --git a/plugins/t/check_snmp.t b/plugins/t/check_snmp.t
index 004ba1a..25a2999 100644
--- a/plugins/t/check_snmp.t
+++ b/plugins/t/check_snmp.t
@@ -8,7 +8,7 @@ use strict;
use Test::More;
use NPTest;
-my $tests = 8+38+2+2;
+my $tests = 8+42+2+2;
plan tests => $tests;
my $res;
@@ -124,6 +124,13 @@ SKIP: {
cmp_ok( $res->return_code, '==', 0, "Skipping all thresholds");
like($res->output, '/^SNMP OK - \d+ \w+ \d+\s.*$/', "Skipping all thresholds, result printed rather than parsed");
+ $res = NPTest->testCmd( "./check_snmp -H $host_snmp -C $snmp_community -o system.sysUpTime.0 -c 1000000000: -u '1/100 sec'");
+ cmp_ok( $res->return_code, '==', 2, "Timetick used as a threshold");
+ like($res->output, '/^SNMP CRITICAL - \*\d+\* 1\/100 sec.*$/', "Timetick used as a threshold, parsed as numeric");
+
+ $res = NPTest->testCmd( "./check_snmp -H $host_snmp -C $snmp_community -o system.sysUpTime.0");
+ cmp_ok( $res->return_code, '==', 0, "Timetick used as a string");
+ like($res->output, '/^SNMP OK - Timeticks:\s\(\d+\)\s+(?:\d+ days?,\s+)?\d+:\d+:\d+\.\d+\s.*$/', "Timetick used as a string, result printed rather than parsed");
}
# These checks need a complete command line. An invalid community is used so
--
1.7.3.2
>From ffadb45f924aa2ba9792963bdc64d798841b9e7b Mon Sep 17 00:00:00 2001
From: Thomas Guyot-Sionnest <derm...@aei.ca>
Date: Tue, 30 Nov 2010 22:28:19 -0500
Subject: [PATCH 3/4] State-based tests enhancements
- Use /var/tmp for state if no state dir environment variable is set,
this avoid the need for a writable localstatedir during tests.
- Use "rm -f", mostly to avoid printing out garbage of the directory
doesn't exists
---
plugins/tests/check_snmp.t | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/plugins/tests/check_snmp.t b/plugins/tests/check_snmp.t
index 08348d2..6966838 100755
--- a/plugins/tests/check_snmp.t
+++ b/plugins/tests/check_snmp.t
@@ -51,6 +51,9 @@ if ($ARGV[0] && $ARGV[0] eq "-d") {
}
}
+# We should merge that with $ENV{'NPTEST_CACHE'}, use one dir for all test data
+$ENV{'NAGIOS_PLUGIN_STATE_DIRECTORY'} ||= "/var/tmp";
+
my $tests = 41;
if (-x "./check_snmp") {
plan tests => $tests;
@@ -106,7 +109,7 @@ like($res->output, '/'.quotemeta('SNMP OK - And now have fun with with this: \"C
"And now have fun with with this: \"C:\\\\\"
because we\'re not done yet!"').'/m', "Attempt to confuse parser No.3");
-system("rm /usr/local/nagios/var/check_snmp/*");
+system("rm -f ".$ENV{'NAGIOS_PLUGIN_STATE_DIRECTORY'}."/check_snmp/*");
$res = NPTest->testCmd( "./check_snmp -H 127.0.0.1 -C public -p $port_snmp -o .1.3.6.1.4.1.8072.3.2.67.10 --rate -w 600" );
is($res->return_code, 0, "Returns OK");
is($res->output, "No previous data to calculate rate - assume okay");
--
1.7.3.2
>From 78b0f30ad6f562e5b4b10c50857143f959a6c754 Mon Sep 17 00:00:00 2001
From: Thomas Guyot-Sionnest <derm...@aei.ca>
Date: Tue, 30 Nov 2010 22:48:43 -0500
Subject: [PATCH 4/4] Revert "check_snmp now considers strings returned by SNMP that contain just"
This reverts commit 896962a1ad1b7d7c75d42c565b06cc799feb0a7c.
Conflicts:
NEWS
plugins/tests/check_snmp.t
Notes:
Reverting because I rebased a patch that was doing the same thing, plus
fixing more related regressions, and both didn't work together.
I kept the tests intact except for one that wouldn't pass on 1.4.14
either
---
NEWS | 5 -----
plugins/check_snmp.c | 14 --------------
plugins/tests/check_snmp.t | 6 +-----
3 files changed, 1 insertions(+), 24 deletions(-)
diff --git a/NEWS b/NEWS
index 27e69c6..e3e8f37 100644
--- a/NEWS
+++ b/NEWS
@@ -3,11 +3,6 @@ This file documents the major additions and syntax changes between releases.
...
FIXES
- check_snmp now attempts to convert string responses into a double value. If the full string is a value,
- check_snmp will consider it a numeric value and thus apply threshold checks and return performance data.
- This reverts back to 1.4.14 behaviour with strings
-
- FIXES
Make check_snmp work more like v1.4.14 with regard to using special values (Timeticks, STRING) as numeric thresholds.
1.4.15 27th July 2010
diff --git a/plugins/check_snmp.c b/plugins/check_snmp.c
index cb7fb7a..d79da8c 100644
--- a/plugins/check_snmp.c
+++ b/plugins/check_snmp.c
@@ -160,7 +160,6 @@ main (int argc, char **argv)
char *outbuff;
char *ptr = NULL;
char *show = NULL;
- char *endptr = NULL;
char *th_warn=NULL;
char *th_crit=NULL;
char type[8] = "";
@@ -390,19 +389,6 @@ main (int argc, char **argv)
}
}
- /* Allow numeric conversion if whole string is a number. Make concession for strings with " at beginning or end */
- /* This duplicates the conversion a bit later, but is cleaner to separate out the checking against the conversion */
- ptr = show;
- if (*ptr == '"')
- ptr++;
- if (*ptr != '\0' ) {
- strtod( ptr, &endptr );
- if (*endptr == '"')
- endptr++;
- if (*endptr == '\0')
- is_numeric=1;
- }
-
}
else if (strstr (response, "Timeticks: ")) {
show = strstr (response, "Timeticks: ");
diff --git a/plugins/tests/check_snmp.t b/plugins/tests/check_snmp.t
index 6966838..c960f7b 100755
--- a/plugins/tests/check_snmp.t
+++ b/plugins/tests/check_snmp.t
@@ -54,7 +54,7 @@ if ($ARGV[0] && $ARGV[0] eq "-d") {
# We should merge that with $ENV{'NPTEST_CACHE'}, use one dir for all test data
$ENV{'NAGIOS_PLUGIN_STATE_DIRECTORY'} ||= "/var/tmp";
-my $tests = 41;
+my $tests = 39;
if (-x "./check_snmp") {
plan tests => $tests;
} else {
@@ -173,10 +173,6 @@ $res = NPTest->testCmd( "./check_snmp -H 127.0.0.1 -C public -p $port_snmp -o .1
is($res->return_code, 0, "OK as string doesn't match but inverted" );
is($res->output, 'SNMP OK - "stringtests" | ', "OK as inverted string no match" );
-$res = NPTest->testCmd( "./check_snmp -H 127.0.0.1 -C public -p $port_snmp -o .1.3.6.1.4.1.8072.3.2.67.12" );
-is($res->return_code, 0, "Numeric in string test" );
-is($res->output, 'SNMP OK - 3.5 | iso.3.6.1.4.1.8072.3.2.67.12=3.5 ', "Check seen as numeric" );
-
$res = NPTest->testCmd( "./check_snmp -H 127.0.0.1 -C public -p $port_snmp -o .1.3.6.1.4.1.8072.3.2.67.12 -w 4:5" );
is($res->return_code, 1, "Numeric in string test" );
is($res->output, 'SNMP WARNING - *3.5* | iso.3.6.1.4.1.8072.3.2.67.12=3.5 ', "WARNING threshold checks for string masquerading as number" );
--
1.7.3.2
