Potential robust fix for check_snmp issues

[nagiosgwesterman]

DESCRIPTION

• This fix maintains the fix to issue check_snmp plugin is not parsing the OID data correctly for integers. #720 while hopefully addressing the subsequent problems noted in issue check_snmp issue on release-2.4.7: No valid data returned  #743.
• With this change, the integer return value from snmpget is only accessed via searching for open parentheses if said character exists, otherwise it parses as before - by checking for the first instance of "-0123456789".
• NOTE: There are still potentially edge cases (namely a return value with an oidname that both has no parentheses and has integers in the name - but I truly hope that isn't possible).

TESTING

• I do not have the equipment required to test specific MIBs but this change largely reverts my previous change so particularly extensive testing is not necessary for such MIBs that no longer worked with the change made in 2.4.7.
• For those who encountered issues with 2.4.7 and 2.4.8, please test this branch with the OIDs that were broken with those versions and, ideally, other OIDs you test in your environments.
• Please note if OIDs that were working previously (in 2.4.6) now return a message akin to "UNKNOWN: No Valid data returned" and mark the expected return value. Especially if it begins with something other than: INTEGER, STRING, COUNTER64, COUNTER32, OID, Timeticks, GAUGE, or GAUGE32.

[kevin-j-morse]

I tested this PR by patching 2.4.8 and it solved my problems with APC and Supermicro OIDs

[nsoggia]

I can't PR, so I will add a patch at the end of the comment.
The problem comes from commit 031e5f8.

Due to that commit, the numeric comparison in check_snmp 2.4.8 is now able to handle timeticks only, however the datatype can also be (all snmp flavours of) integer.

good examples:
look for an open parenthesis to be skipped, if it is missing then go straight to the first decimal digit.
iso.1... = Gauge32: 0 milliseconds
iso.1... = INTEGER: rfc2988(5)
iso.1... = Timeticks: (82289678) 9 days, 12:34:56.78

bad examples:
the user must be careful not to ask for numeric comparisons :)
iso.1... = IpAddress: 0.0.0.0
iso.1... = OID: .0.0
iso.1... = STRING: "Null0"

Cheers

--- nagios-plugins-2.4.8.old/plugins/check_snmp.c	2023-12-06 19:31:19.000000000 +0100
+++ nagios-plugins-2.4.8.new/plugins/check_snmp.c	2024-01-12 15:56:30.000000000 +0100
@@ -503,11 +503,10 @@
 		/* Process this block for numeric comparisons */
 		/* Make some special values,like Timeticks numeric only if a threshold is defined */
 		if (thlds[i]->warning || thlds[i]->critical || calculate_rate || is_ticks || offset != 0.0 || multiplier != 1.0) {
-			/* Find the first instance of the '(' character - the value of the OID should be contained in parens */
-			ptr = strpbrk(show, "(");
-			if (ptr == NULL)
+			if ((ptr = strpbrk(show, "(")) != NULL) /* Timetick */
+				ptr++;
+			else if ((ptr = strpbrk(show, "-0123456789")) == NULL) /* Counter, gauge or integer */
 				die (STATE_UNKNOWN,_("No valid data returned (%s)\n"), show);
-			ptr++; /* Move to the first character after the '(' */
 			
 			while (i >= response_size) {
 				response_size += OID_COUNT_STEP;

[sawolf]

Hi @nsoggia, thanks for reaching out and contributing the patch! I'm having Griffin review your new code soon, and we'll try to have the check_snmp regressions fully resolved in our February release.

[nagiosgwesterman]

Thank you for your patience. @nsoggia, after reviewing your code, your PR is (as far as I can tell) functionally the same as mine just organizationally different. But I do prefer yours, so I have made the changes. Thank you so much.

[nsoggia]

I'm glad to have been of assistance.
Cheers