Nagios Plugin Development

I finally got around testing this... Works well, though looks like the only
use case is for certificate validity checks.

Fixed in Git.

Thanks. I haven't replied earlier but that's exactly why I preferred the
first patch.

I'll apply it soon.

Here is a new version of my own patch, keeping original API intact to
reduce impact (sorry, it seems only original reporter can attach files). I
just tested it, it works OK.

diff -Naur --exclude '*~' nagios-plugins-1.4.13/plugins/check_http.c
nagios-plugins-1.4.13-sni-support/plugins/check_http.c
--- nagios-plugins-1.4.13/plugins/check_http.c 2008-09-02
13:26:31.000000000 +0200
+++ nagios-plugins-1.4.13-sni-support/plugins/check_http.c 2009-03-25
21:43:57.000000000 +0100
@@ -773,7 +773,7 @@
die (STATE_CRITICAL, _("HTTP CRITICAL - Unable to open TCP
socket\n"));
#ifdef HAVE_SSL
if (use_ssl == TRUE) {
- np_net_ssl_init(sd);
+ np_net_ssl_init_with_hostname(sd, host_name);
if (check_cert == TRUE) {
result = np_net_ssl_check_cert(days_till_exp);
np_net_ssl_cleanup();
diff -Naur --exclude '*~' nagios-plugins-1.4.13/plugins/netutils.h
nagios-plugins-1.4.13-sni-support/plugins/netutils.h
--- nagios-plugins-1.4.13/plugins/netutils.h 2008-01-31 12:45:28.000000000
+0100
+++ nagios-plugins-1.4.13-sni-support/plugins/netutils.h 2009-03-25
21:31:29.000000000 +0100
@@ -95,6 +95,7 @@
#ifdef HAVE_SSL
/* maybe this could be merged with the above np_net_connect, via some
flags */
int np_net_ssl_init(int sd);
+int np_net_ssl_init_with_hostname(int sd, char *host_name);
void np_net_ssl_cleanup();
int np_net_ssl_write(const void *buf, int num);
int np_net_ssl_read(void *buf, int num);
diff -Naur --exclude '*~' nagios-plugins-1.4.13/plugins/sslutils.c
nagios-plugins-1.4.13-sni-support/plugins/sslutils.c
--- nagios-plugins-1.4.13/plugins/sslutils.c 2008-01-31 12:27:22.000000000
+0100
+++ nagios-plugins-1.4.13-sni-support/plugins/sslutils.c 2009-03-25
21:31:14.000000000 +0100
@@ -38,7 +38,11 @@
static SSL *s=NULL;
static int initialized=0;

-int np_net_ssl_init (int sd){
+int np_net_ssl_init (int sd) {
+ return np_net_ssl_init_with_hostname(sd, NULL);
+}
+
+int np_net_ssl_init_with_hostname (int sd, char *host_name) {
if (!initialized) {
/* Initialize SSL context */
SSLeay_add_ssl_algorithms ();
@@ -51,6 +55,10 @@
return STATE_CRITICAL;
}
if ((s = SSL_new (c)) != NULL){
+#ifdef SSL_set_tlsext_host_name
+ if (host_name != NULL)
+ SSL_set_tlsext_host_name(s, host_name);
+#endif
SSL_set_fd (s, sd);
if (SSL_connect(s) == 1){
return OK;
@@ -68,6 +76,9 @@

void np_net_ssl_cleanup (){
if(s){
+#ifdef SSL_set_tlsext_host_name
+ SSL_set_tlsext_host_name(s, NULL);
+#endif
SSL_shutdown (s);
SSL_free (s);
if(c) {

You can test with https://sympa.msr-inria.inria.fr and
https://www.msr-inria.inria.fr, both of them are different virtual hosts
running on the same physical hosts, using different certificates. I didn't
found the time to test the patch myself yet. To include the test in an
automated test suite, however, you'll need a SNI-enabled web server, and I
don't think does it.

BTW, your message about "clearing the host on cleanup" sound like you used
the original patch supplied. The one I submitted myself avoid this by using
a local variable instead of a global one :P

Actually I prefer the other method. I stripped the timezone stuff, cleared
the host on cleanup and added the call to check_http to set the hostname
only if needed.

Does it works for you? It would be really kind if you could send me an url
to test with.

Thanks.

Sorry, it's not about old openssl (I assumed that without really reading
the error message). I realized it yesterday after sending the comment,
though it was a coincidence: I was reading about timezone stuff and read
somewhere that tm.tm_zone is a gnu extension.

I can commit it without the timezone stuff though (isn't the timezone
always GMT?? I couldn't find any good documentation on the OpenSSL
functions used in sslutil...). Please attach it as a file though. What you
paste in comments it totally unusable for patch.

Also, do you have anything to test with? Would there be any way to
integrate this in the unit tests too (they use HTTP::Daemon::SSL for
emulating a web server) ?

Thanks

diff -Naur --exclude '*~' nagios-plugins-1.4.13/plugins/check_http.c
nagios-plugins-1.4.13-sni-support/plugins/check_http.c
--- nagios-plugins-1.4.13/plugins/check_http.c 2008-09-02
13:26:31.000000000 +0200
+++ nagios-plugins-1.4.13-sni-support/plugins/check_http.c 2009-03-19
23:54:12.000000000 +0100
@@ -773,7 +773,7 @@
die (STATE_CRITICAL, _("HTTP CRITICAL - Unable to open TCP
socket\n"));
#ifdef HAVE_SSL
if (use_ssl == TRUE) {
- np_net_ssl_init(sd);
+ np_net_ssl_init(sd, host_name);
if (check_cert == TRUE) {
result = np_net_ssl_check_cert(days_till_exp);
np_net_ssl_cleanup();
diff -Naur --exclude '*~' nagios-plugins-1.4.13/plugins/check_smtp.c
nagios-plugins-1.4.13-sni-support/plugins/check_smtp.c
--- nagios-plugins-1.4.13/plugins/check_smtp.c 2008-05-07
12:02:42.000000000 +0200
+++ nagios-plugins-1.4.13-sni-support/plugins/check_smtp.c 2009-03-19
23:55:38.000000000 +0100
@@ -236,7 +236,7 @@
smtp_quit();
return STATE_UNKNOWN;
}
- result = np_net_ssl_init(sd);
+ result = np_net_ssl_init(sd, NULL);
if(result != STATE_OK) {
printf (_("CRITICAL - Cannot create SSL context.\n"));
np_net_ssl_cleanup();
diff -Naur --exclude '*~' nagios-plugins-1.4.13/plugins/check_tcp.c
nagios-plugins-1.4.13-sni-support/plugins/check_tcp.c
--- nagios-plugins-1.4.13/plugins/check_tcp.c 2008-05-07
12:02:42.000000000 +0200
+++ nagios-plugins-1.4.13-sni-support/plugins/check_tcp.c 2009-03-19
23:55:21.000000000 +0100
@@ -236,7 +236,7 @@

#ifdef HAVE_SSL
if (flags & FLAG_SSL){
- result = np_net_ssl_init(sd);
+ result = np_net_ssl_init(sd, NULL);
if (result == STATE_OK && check_cert == TRUE) {
result = np_net_ssl_check_cert(days_till_exp);
if(result != STATE_OK) {
diff -Naur --exclude '*~' nagios-plugins-1.4.13/plugins/netutils.h
nagios-plugins-1.4.13-sni-support/plugins/netutils.h
--- nagios-plugins-1.4.13/plugins/netutils.h 2008-01-31 12:45:28.000000000
+0100
+++ nagios-plugins-1.4.13-sni-support/plugins/netutils.h 2009-03-19
23:57:45.000000000 +0100
@@ -94,7 +94,7 @@
/* SSL-Related functionality */
#ifdef HAVE_SSL
/* maybe this could be merged with the above np_net_connect, via some
flags */
-int np_net_ssl_init(int sd);
+int np_net_ssl_init(int sd, char *host_name);
void np_net_ssl_cleanup();
int np_net_ssl_write(const void *buf, int num);
int np_net_ssl_read(void *buf, int num);
diff -Naur --exclude '*~' nagios-plugins-1.4.13/plugins/sslutils.c
nagios-plugins-1.4.13-sni-support/plugins/sslutils.c
--- nagios-plugins-1.4.13/plugins/sslutils.c 2008-01-31 12:27:22.000000000
+0100
+++ nagios-plugins-1.4.13-sni-support/plugins/sslutils.c 2009-03-20
00:01:58.000000000 +0100
@@ -38,7 +38,7 @@
static SSL *s=NULL;
static int initialized=0;

-int np_net_ssl_init (int sd){
+int np_net_ssl_init (int sd, char *host_name){
if (!initialized) {
/* Initialize SSL context */
SSLeay_add_ssl_algorithms ();
@@ -51,6 +51,10 @@
return STATE_CRITICAL;
}
if ((s = SSL_new (c)) != NULL){
+#ifdef SSL_set_tlsext_host_name
+ if (host_name != NULL)
+ SSL_set_tlsext_host_name(s, host_name);
+#endif
SSL_set_fd (s, sd);
if (SSL_connect(s) == 1){
return OK;
@@ -68,6 +72,9 @@

void np_net_ssl_cleanup (){
if(s){
+#ifdef SSL_set_tlsext_host_name
+ SSL_set_tlsext_host_name(s, NULL);
+#endif
SSL_shutdown (s);
SSL_free (s);
if(c) {

Here is a slightly different version, changing np_net_ssl_init() prototype
to pass host name, rather than using a global variable. This adress your
question 1).

However, I don't understand the issue with old openssl versions, the patch
already does use #idfef block to only use this function if available ?

Moreover this seems to break old OpsnSSL's (at least on my Solaris
tinderbox)

I rolled it back (except the timestamp fix). I will apply an updated
version if you add the proper ifdef's to keep backwards compatibility.

Thanks

Thanks for your report

I have two questions:

1. This patch does not alter check_http to use the new function to set
host name. Did you expect us to make this change, or do you have a complete
patch around?

2. Regarding printing the timezone, AFAIK is can be longer than three
characters, unless if certificates have a strict standars. This command
will list all timezones in /usr/share/zoneinfo:
$ find /usr/share/zoneinfo/ -type f -exec zdump {} \;|sed 's/^.* 2009
\(.*\)$/\1/'|sort|uniq
If you add "wc -L" this gives you a max length of 6 characters. The
current code will apparently cut it to three characters.

This is really useful, I'd like to have it merged too...