[Nagiosplug-devel] Check_http status codes bug

[Chris Wilson]

Hi all,

We've been using Nagios and the associated plugins for a long time with 
great success. Thank you all very much for your hard work.

We think we have found a problem with the check_http plugin. RFC 2616, for 
HTTP/1.1, defines all codes such as 3xx as redirections, all 4xx as client 
errors, and all 5xx as server errors. 
[cf. http://www.w3.org/Protocols/rfc2616/rfc2616-sec6.html#sec6.1.1]

However, check_http only checks for certain specific values, for example,
301-304 for redirect codes. As a result, it will not always detect errors. 

For example, when our Sun/Chilisoft ASP servers go down, they start to
return error 507, but the check_http plugin treats this as a completely
normal status code and does not report an error to Nagios.

Please find attached a patch which, I believe, will tighten up the
handling of status codes to more closely match the RFC. Also, it should no
longer be possible for a status line like

  HTTP/1.1 200 OK i want to confuse you 404

to be treated as a 404 error, and some other invalid status lines will be 
detected and flagged as errors, which previously were not.

I have verified that the patched check_http correctly detects 200, 404,
and 507 status codes in test cases, but it has not been tested much
further yet. Nevertheless, it appears to work well enough that we plan to
apply it to our production systems immediately.

Please could you consider this patch for application to the official
distribution of the check_http plugin.

Cheers, Chris.
-- 
   ___ __     _
 / __// / ,__(_)_  | Chris Wilson -- UNIX Firewall Lead Developer |
/ (_ / ,\/ _/ /_ \ | NetServers.co.uk http://www.netservers.co.uk |
\ _//_/_/_//_/___/ | 21 Signet Court, Cambridge, UK. 01223 576516 |
-------------- next part --------------
diff -ru2 nagios-plugins-1.3.1/plugins/check_http.c nagios-plugins-1.3.1-chris/plugins/check_http.c
--- nagios-plugins-1.3.1/plugins/check_http.c	Mon Jun 30 12:56:08 2003
+++ nagios-plugins-1.3.1-chris/plugins/check_http.c	Thu Sep 11 11:06:27 2003
@@ -540,4 +540,5 @@
 	char *msg = NULL;
 	char *status_line = "";
+	char *status_ptr;
 	char *header = NULL;
 	char *page = "";
@@ -551,4 +552,5 @@
 	char *orig_url = NULL;
 	double elapsed_time;
+	int status_code;
 #ifdef HAVE_SSL
 	int sslerr;
@@ -713,26 +715,56 @@
 		/* check the return code */
 		/* server errors result in a critical state */
-		if (strstr (status_line, "500") ||
-	  	  strstr (status_line, "501") ||
-	    	strstr (status_line, "502") ||
-		    strstr (status_line, "503")) {
-			terminate (STATE_CRITICAL, "HTTP CRITICAL: %s\n", status_line);
-		}
 
-		/* client errors result in a warning state */
-		if (strstr (status_line, "400") ||
-	  	  strstr (status_line, "401") ||
-	    	strstr (status_line, "402") ||
-		    strstr (status_line, "403") ||
-		    strstr (status_line, "404")) {
-			terminate (STATE_WARNING, "HTTP WARNING: %s\n", status_line);
+		status_ptr = status_line;
+
+		/* According to RFC 2616 (HTTP/1.1):
+		 * Status-Line = HTTP-Version SP Status-Code SP Reason-Phrase
+		 * 		 CRLF
+		 */
+
+		/* we already checked that the version is HTTP/1.x, so
+		   skip the HTTP-Version completely */
+
+		while (*status_ptr != 0 && *status_ptr != 32)
+			status_ptr++;
+
+		if (*status_ptr != 32)
+			terminate (STATE_CRITICAL, 
+				"HTTP CRITICAL: invalid status line: %s\n",
+				status_line);
+
+		/* skip the space */
+		status_ptr++;
+
+		/* check that the next three characters are digits */
+		for (i = 0; i < 3; i++) {
+			if (status_ptr[i] < '0' || status_ptr[i] > '9') {
+				terminate (STATE_CRITICAL,
+					"HTTP CRITICAL: invalid status code: "
+					"%s\n", status_line);
+			}
 		}
 
+		/* check that the next character is a space */
+		if (status_ptr[3] != 32)
+			terminate (STATE_CRITICAL,
+				"HTTP CRITICAL: invalid status line: %s\n",
+				status_line);
+
+		/* it should now be safe to use atoi on the status_ptr */
+		status_code = atoi(status_ptr);
+
+		/* server errors (5xx) result in a critical state */
+		if (status_ptr[0] == '5')
+			terminate (STATE_CRITICAL, "HTTP CRITICAL: %s\n",
+				status_line);
+
+		/* client errors (4xx) result in a warning state */
+		if (status_ptr[0] == '4')
+			terminate (STATE_WARNING, "HTTP WARNING: %s\n",
+				status_line);
+
 		/* check redirected page if specified */
-		if (strstr (status_line, "300") ||
-	  	  strstr (status_line, "301") ||
-	    	strstr (status_line, "302") ||
-		    strstr (status_line, "303") ||
-		    strstr (status_line, "304")) {
+		if (status_ptr[0] == '3') {
 			if (onredirect == STATE_DEPENDENT) {
 
@@ -805,5 +837,5 @@
 	                   (display_html ? "</A>" : ""), elapsed_time);
 			terminate (onredirect, msg);
-		} /* end if (strstr (status_line, "30[0-4]") */
+		} /* end if (status_ptr[0] == 3) */