[Nagiosplug-devel] [ nagiosplug-Patches-1939022 ] SSL/TLS hostname extension support (SNI)

SourceForge.net noreply at sourceforge.net
Wed May 20 07:58:28 CEST 2009


Patches item #1939022, was opened at 2008-04-09 19:56
Message generated for change (Comment added) made by dermoth
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=397599&aid=1939022&group_id=29880

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Enhancement
Group: release-1.4.14
>Status: Closed
>Resolution: Fixed
Priority: 5
Private: No
Submitted By: Joe Presbrey (presbrey)
Assigned to: Thomas Guyot-Sionnest (dermoth)
Summary: SSL/TLS hostname extension support (SNI)

Initial Comment:
Patch against Plugin Version (-V output): SVN trunk
Plugin Name: sslutils/check_http
Example Plugin Commandline: check_http -H wildcard.scripts.mit.edu -S -C 14
Tested on operating system: debian/4.0
Tested on architecture: i686
Tested with compiler: gcc-4.1.2-20061115

A TLS extension called "Server Name Indication" allows name-based HTTPS virtual hosting.  (From Gentoo: http://gentoo-wiki.com/HOWTO_Apache_with_Name_Based_Hosting_and_SSL).  This is especially common when serving HTTPS requests with a wildcard certificate (*.domain.tld).

This patch adds a call to SSL_set_tlsext_host_name (OpenSSL 0.9.8f and higher) in the certificate check section of sslutils to allow certificate verification of HTTPS virtual-host domains.

This patch also corrects the expiration check to escalate to 'critical' when the certificate is expired but for less than 1 day (currently emits 'warning') and displays the time-zone with the expiration time.

Joe Presbrey

----------------------------------------------------------------------

>Comment By: Thomas Guyot-Sionnest (dermoth)
Date: 2009-05-20 01:58

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

Fixed in Git.

----------------------------------------------------------------------

Comment By: Thomas Guyot-Sionnest (dermoth)
Date: 2009-03-25 20:16

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

I'll apply it soon.

----------------------------------------------------------------------

Comment By: Guillaume Rousse (guillomovitch)
Date: 2009-03-25 17:13

Message:
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) {

----------------------------------------------------------------------

Comment By: Guillaume Rousse (guillomovitch)
Date: 2009-03-24 17:46

Message:
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

----------------------------------------------------------------------

Comment By: Thomas Guyot-Sionnest (dermoth)
Date: 2009-03-21 02:49

Message:
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.

----------------------------------------------------------------------

Comment By: Thomas Guyot-Sionnest (dermoth)
Date: 2009-03-19 22:06

Message:
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

----------------------------------------------------------------------

Comment By: Guillaume Rousse (guillomovitch)
Date: 2009-03-19 19:06

Message:
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) {


----------------------------------------------------------------------

Comment By: Guillaume Rousse (guillomovitch)
Date: 2009-03-19 19:03

Message:
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 ?



----------------------------------------------------------------------

Comment By: Thomas Guyot-Sionnest (dermoth)
Date: 2009-03-19 01:16

Message:
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


----------------------------------------------------------------------

Comment By: Thomas Guyot-Sionnest (dermoth)
Date: 2009-03-17 04:02

Message:
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.


----------------------------------------------------------------------

Comment By: Guillaume Rousse (guillomovitch)
Date: 2009-03-05 06:01

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

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=397599&aid=1939022&group_id=29880




More information about the Devel mailing list