[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#677847: marked as done (libapache2-mod-gnutls: Virtual hosting fails with multi-name certificates)



Your message dated Fri, 21 Feb 2014 09:19:58 +0000
with message-id <E1WGmH0-0004LF-0J@franck.debian.org>
and subject line Bug#677847: fixed in mod-gnutls 0.6-1
has caused the Debian Bug report #677847,
regarding libapache2-mod-gnutls: Virtual hosting fails with multi-name certificates
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@bugs.debian.org
immediately.)


-- 
677847: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=677847
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems
--- Begin Message ---
Package: libapache2-mod-gnutls
Version: 0.5.10-1.1
Severity: wishlist
Tags: patch

In general mod_gnutls supports virtual hosting of https sites using a single IP address by supporting Server Name Indication.
The problem with the current implementation in mod_gnutls is, that only one domain name in the certificate will be used when checking if the certificate is the right one for the incoming request. But certificates can contain multiple domain names they work for using the dnsName extension.
When I get certificates from StartSSL, they have the common name set to "www.example.com", and a dnsName extension set to "example.com". I cannot get a certificate just for "example.com" there. But my site is mostly accessed using just "example.com". So in my case virtual hosting fails, as the current implementation will only use the certificate if the incoming request is for "www.example.com".
I wrote a patch do mod_gnutls, that will use all domains of a certificate, when checking which is the right certificate for an incoming request.

BTW: The problem is also described at http://jan-krueger.net/development/mod_gnutls-and-startssl-level-1-certificates-the-problem-and-solution by Jan Krüger. But while his patch only supports up to 4 domains in a certificate, mine has support for an unlimited number of domains.

I would like to get multi-domain support integrated in the Debian package as well as in uplink. This would help me, because I would not have to patch all future versions of the package again ;-))


Regards,
Matthias


-- System Information:
Debian Release: wheezy/sid
  APT prefers testing
  APT policy: (500, 'testing')
Architecture: amd64 (x86_64)

Kernel: Linux 3.2.0-2-amd64 (SMP w/1 CPU core)
Locale: LANG=de_DE.UTF-8, LC_CTYPE=de_DE.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash

Versions of packages libapache2-mod-gnutls depends on:
ii  libapr-memcache0  0.7.0-1
ii  libc6             2.13-33
ii  libgnutls26       2.12.19-1

libapache2-mod-gnutls recommends no packages.

libapache2-mod-gnutls suggests no packages.

-- no debconf information
--- mod-gnutls-0.5.10.orig/src/gnutls_hooks.c
+++ mod-gnutls-0.5.10/src/gnutls_hooks.c
@@ -243,75 +243,95 @@ const char static_dh_params[] = "-----BE
  * Returns negative on error.
  */
 static int read_crt_cn(server_rec * s, apr_pool_t * p,
-		       gnutls_x509_crt_t cert, char **cert_cn)
+		       gnutls_x509_crt_t cert, char ***cert_cn)
 {
 	int rv = 0, i;
 	size_t data_len;
+	int number_names;
+	int copied_names;
+	char *buffer;
 
 
 	_gnutls_log(debug_log_fp, "%s: %d\n", __func__, __LINE__);
-	*cert_cn = NULL;
 
+	// find number of alt names
+	number_names = 0;
 	data_len = 0;
+	buffer = NULL;
+	do {
+		rv = gnutls_x509_crt_get_subject_alt_name(cert, number_names, buffer, &data_len, NULL);
+
+		// the first buffer we allocate, or we need bigger buffer
+		if (rv == GNUTLS_E_SHORT_MEMORY_BUFFER && data_len > 1) {
+			// be prepared for longer names coming later without need to reallocate memory
+			data_len *= 2;
+			buffer = apr_palloc(p, data_len);
+		}
+
+		// one more name available, the last one we count is a fail we can use to store the CN
+		number_names++;
+	} while (rv >= 0);
+
+	// allocate memory of the list of names, plus one for the terminating NULL
+	*cert_cn = apr_palloc(p, sizeof(char*) * (number_names + 1));
+
+	// first name we try to get is CN
+	copied_names = 0;
 	rv = gnutls_x509_crt_get_dn_by_oid(cert,
 					   GNUTLS_OID_X520_COMMON_NAME,
-					   0, 0, NULL, &data_len);
+					   0, 0, buffer, &data_len);
 
 	if (rv == GNUTLS_E_SHORT_MEMORY_BUFFER && data_len > 1) {
-		*cert_cn = apr_palloc(p, data_len);
+		buffer = apr_palloc(p, data_len);
 		rv = gnutls_x509_crt_get_dn_by_oid(cert,
 						   GNUTLS_OID_X520_COMMON_NAME,
-						   0, 0, *cert_cn,
+						   0, 0, buffer,
 						   &data_len);
-	} else {		/* No CN return subject alternative name */
-		ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
-			     "No common name found in certificate for '%s:%d'. Looking for subject alternative name...",
-			     s->server_hostname, s->port);
-		rv = 0;
-		/* read subject alternative name */
-		for (i = 0; !(rv < 0); i++) {
-			data_len = 0;
-			rv = gnutls_x509_crt_get_subject_alt_name(cert, i,
-								  NULL,
-								  &data_len,
-								  NULL);
-
-			if (rv == GNUTLS_E_SHORT_MEMORY_BUFFER
-			    && data_len > 1) {
-				/* FIXME: not very efficient. What if we have several alt names
-				 * before DNSName?
-				 */
-				*cert_cn = apr_palloc(p, data_len + 1);
-
-				rv = gnutls_x509_crt_get_subject_alt_name
-				    (cert, i, *cert_cn, &data_len, NULL);
-				(*cert_cn)[data_len] = 0;
+	}
 
-				if (rv == GNUTLS_SAN_DNSNAME)
-					break;
-			}
+	if (rv >= 0) {
+		(*cert_cn)[0] = apr_palloc(p, strlen(buffer) + 1);
+		apr_cpystrn((*cert_cn)[0], buffer, strlen(buffer) + 1);
+		copied_names++;
+	}
+
+	// no we have to find subject alternative names
+	for (i = 0; i < number_names - 1 && rv != GNUTLS_E_SHORT_MEMORY_BUFFER; i++) {
+		rv = gnutls_x509_crt_get_subject_alt_name(cert, i, buffer, &data_len, NULL);
+
+		// we only ever increased buffer size, should get no memory problem anymore
+
+		// on success, copy result in the list
+		if (rv == GNUTLS_SAN_DNSNAME) {
+			(*cert_cn)[++copied_names] = apr_palloc(p, strlen(buffer) + 1);
+			apr_cpystrn((*cert_cn)[copied_names], buffer, strlen(buffer) + 1);
 		}
 	}
 
-	return rv;
+	// terminate list
+	(*cert_cn)[copied_names + 1] = NULL;
+
+	return copied_names >= 1 ? 0 : -1;
 }
 
 static int read_pgpcrt_cn(server_rec * s, apr_pool_t * p,
-			  gnutls_openpgp_crt_t cert, char **cert_cn)
+			  gnutls_openpgp_crt_t cert, char ***cert_cn)
 {
 	int rv = 0;
 	size_t data_len;
 
 
 	_gnutls_log(debug_log_fp, "%s: %d\n", __func__, __LINE__);
-	*cert_cn = NULL;
+	*cert_cn = apr_palloc(p, sizeof(char*) * 2);
+	(*cert_cn)[0] = NULL;
+	(*cert_cn)[1] = NULL;
 
 	data_len = 0;
 	rv = gnutls_openpgp_crt_get_name(cert, 0, NULL, &data_len);
 
 	if (rv == GNUTLS_E_SHORT_MEMORY_BUFFER && data_len > 1) {
-		*cert_cn = apr_palloc(p, data_len);
-		rv = gnutls_openpgp_crt_get_name(cert, 0, *cert_cn,
+		(*cert_cn)[0] = apr_palloc(p, data_len);
+		rv = gnutls_openpgp_crt_get_name(cert, 0, (*cert_cn)[0],
 						 &data_len);
 	} else {		/* No CN return subject alternative name */
 		ap_log_error(APLOG_MARK, APLOG_INFO, 0, s,
@@ -558,6 +578,7 @@ typedef struct {
 
 static int vhost_cb(void *baton, conn_rec * conn, server_rec * s)
 {
+	int tried_name = 0;
 	mgs_srvconf_rec *tsc;
 	vhost_cb_rec *x = baton;
 
@@ -569,31 +590,35 @@ static int vhost_cb(void *baton, conn_re
 		return 0;
 	}
 
-	/* The CN can contain a * -- this will match those too. */
-	if (ap_strcasecmp_match(x->sni_name, tsc->cert_cn) == 0) {
-		/* found a match */
+	/* try all names of the host */
+	for (tried_name = 0; tsc->cert_cn[tried_name] != NULL; tried_name++) {
+
+		/* The CN can contain a * -- this will match those too. */
+		if (ap_strcasecmp_match(x->sni_name, tsc->cert_cn[tried_name]) == 0) {
+			/* found a match */
 #if MOD_GNUTLS_DEBUG
-		ap_log_error(APLOG_MARK, APLOG_DEBUG, 0,
-			     x->ctxt->c->base_server,
-			     "GnuTLS: Virtual Host CB: "
-			     "'%s' == '%s'", tsc->cert_cn, x->sni_name);
+			ap_log_error(APLOG_MARK, APLOG_DEBUG, 0,
+				     x->ctxt->c->base_server,
+				     "GnuTLS: Virtual Host CB: "
+				     "'%s' == '%s'", tsc->cert_cn[tried_name], x->sni_name);
 #endif
-		/* Because we actually change the server used here, we need to reset
-		 * things like ClientVerify.
-		 */
-		x->sc = tsc;
-		/* Shit. Crap. Dammit. We *really* should rehandshake here, as our
-		 * certificate structure *should* change when the server changes. 
-		 * acccckkkkkk. 
-		 */
-		return 1;
-	} else {
+			/* Because we actually change the server used here, we need to reset
+			 * things like ClientVerify.
+			 */
+			x->sc = tsc;
+			/* Shit. Crap. Dammit. We *really* should rehandshake here, as our
+			 * certificate structure *should* change when the server changes. 
+			 * acccckkkkkk. 
+			 */
+			return 1;
+		} else {
 #if MOD_GNUTLS_DEBUG
-		ap_log_error(APLOG_MARK, APLOG_DEBUG, 0,
-			     x->ctxt->c->base_server,
-			     "GnuTLS: Virtual Host CB: "
-			     "'%s' != '%s'", tsc->cert_cn, x->sni_name);
+			ap_log_error(APLOG_MARK, APLOG_DEBUG, 0,
+				     x->ctxt->c->base_server,
+				     "GnuTLS: Virtual Host CB: "
+				     "'%s' != '%s'", tsc->cert_cn[tried_name], x->sni_name);
 #endif
+		}
 
 	}
 	return 0;
@@ -602,6 +627,7 @@ static int vhost_cb(void *baton, conn_re
 
 mgs_srvconf_rec *mgs_find_sni_server(gnutls_session_t session)
 {
+	int i;
 	int rv;
 	unsigned int sni_type;
 	size_t data_len = MAX_HOST_LEN;
@@ -661,22 +687,24 @@ mgs_srvconf_rec *mgs_find_sni_server(gnu
 		ap_log_error(APLOG_MARK, APLOG_DEBUG, 0,
 			     ctxt->c->base_server,
 			     "GnuTLS: sni-x509 cn: %s/%d pk: %s s: 0x%08X s->n: 0x%08X  sc: 0x%08X",
-			     tsc->cert_cn, rv,
+			     tsc->cert_cn[0], rv,
 			     gnutls_pk_algorithm_get_name
 			     (gnutls_x509_privkey_get_pk_algorithm
 			      (ctxt->sc->privkey_x509)), (unsigned int) s,
 			     (unsigned int) s->next, (unsigned int) tsc);
 #endif
 		/* The CN can contain a * -- this will match those too. */
-		if (ap_strcasecmp_match(sni_name, tsc->cert_cn) == 0) {
+		for (i = 0; tsc->cert_cn[i] != NULL; i++) [
+			if (ap_strcasecmp_match(sni_name, tsc->cert_cn[i]) == 0) {
 #if MOD_GNUTLS_DEBUG
-			ap_log_error(APLOG_MARK, APLOG_DEBUG, 0,
-				     ctxt->c->base_server,
-				     "GnuTLS: Virtual Host: "
-				     "'%s' == '%s'", tsc->cert_cn,
-				     sni_name);
+				ap_log_error(APLOG_MARK, APLOG_DEBUG, 0,
+					     ctxt->c->base_server,
+					     "GnuTLS: Virtual Host: "
+					     "'%s' == '%s'", tsc->cert_cn[i],
+					     sni_name);
 #endif
-			return tsc;
+				return tsc;
+			}
 		}
 	}
 #endif
--- mod-gnutls-0.5.10.orig/include/mod_gnutls.h.in
+++ mod-gnutls-0.5.10/include/mod_gnutls.h.in
@@ -86,7 +86,7 @@ typedef struct
     gnutls_certificate_credentials_t certs;
     gnutls_srp_server_credentials_t srp_creds;
     gnutls_anon_server_credentials_t anon_creds;
-    char* cert_cn;
+    char** cert_cn;
     gnutls_x509_crt_t certs_x509[MAX_CHAIN_SIZE]; /* A certificate chain */
     unsigned int certs_x509_num;
     gnutls_x509_privkey_t privkey_x509;

--- End Message ---
--- Begin Message ---
Source: mod-gnutls
Source-Version: 0.6-1

We believe that the bug you reported is fixed in the latest version of
mod-gnutls, which is due to be installed in the Debian FTP archive.

A summary of the changes between this version and the previous one is
attached.

Thank you for reporting the bug, which will now be closed.  If you
have further comments please address them to 677847@bugs.debian.org,
and the maintainer will reopen the bug report if appropriate.

Debian distribution maintenance software
pp.
Daniel Kahn Gillmor <dkg@fifthhorseman.net> (supplier of updated mod-gnutls package)

(This message was generated automatically at their request; if you
believe that there is a problem with it please contact the archive
administrators by mailing ftpmaster@ftp-master.debian.org)


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Format: 1.8
Date: Fri, 21 Feb 2014 00:58:09 -0500
Source: mod-gnutls
Binary: libapache2-mod-gnutls
Architecture: source amd64
Version: 0.6-1
Distribution: unstable
Urgency: low
Maintainer: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
Changed-By: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
Description: 
 libapache2-mod-gnutls - Apache module for SSL and TLS encryption with GnuTLS
Closes: 677847
Changes: 
 mod-gnutls (0.6-1) unstable; urgency=low
 .
   * New upstream version
    - multi-name certificates now work (Closes: #677847)
   * added debian/watch and debian/upstream/signing-key.pgp
   * enable MSVA support
   * drop patches -- builds cleanly without them
   * build and ship newer, cleaner docs
   * add build-dependencies for new test suite
Checksums-Sha1: 
 4d1a52eb5101323a711c234610ad6488577043f7 2102 mod-gnutls_0.6-1.dsc
 1b97c1920178d72f375539f55d4a52cd6f7bcdc0 70522 mod-gnutls_0.6.orig.tar.gz
 c99e3de58fa323010d40fa470b8ed26adb9d58c3 17632 mod-gnutls_0.6-1.debian.tar.xz
 cbcf25f4dc9639b50fda04f4917caf7c59ee9b78 44922 libapache2-mod-gnutls_0.6-1_amd64.deb
Checksums-Sha256: 
 2e94e1df4f14c921fc05a650ad213a533967f7dea4c6345a4e8cda6c0da51895 2102 mod-gnutls_0.6-1.dsc
 5fb2c79fd64f55faabde50c6ba3ef7b959825c8a6130152d00ca18f6d99bc041 70522 mod-gnutls_0.6.orig.tar.gz
 6a7592a7e37afe05b3c168904d6eca358ec7d184579e5e560f154e55a3ddf920 17632 mod-gnutls_0.6-1.debian.tar.xz
 e2aaf88e76da342af0828762b241ecf36e5ef4c7a0f66afdc944ee39ecd83caf 44922 libapache2-mod-gnutls_0.6-1_amd64.deb
Files: 
 4b3d68fa30292148bf564950d221e226 2102 httpd extra mod-gnutls_0.6-1.dsc
 f1498a0b2f1c27356882af91778ac1fd 70522 httpd extra mod-gnutls_0.6.orig.tar.gz
 a99392a7c5ccd5d8a3f27336502235c4 17632 httpd extra mod-gnutls_0.6-1.debian.tar.xz
 fe3d3d61eda23f39a634aa831cd0dc2d 44922 httpd extra libapache2-mod-gnutls_0.6-1_amd64.deb

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQJ8BAEBCgBmBQJTBxOIXxSAAAAAAC4AKGlzc3Vlci1mcHJAbm90YXRpb25zLm9w
ZW5wZ3AuZmlmdGhob3JzZW1hbi5uZXRFQjk2OTEyODdBN0FEREUzNzU3RDkxMUVB
NTI0MDFCMTFCRkRGQTVDAAoJEKUkAbEb/fpcRscP/j0FV3gIjNUYTx/Bm//XzbcI
UZRUZ2egeUyWVog1SnFZfS23BktmUFnxC5PsGkOTTabVD+pCPUbX0kVMjXxeX3AP
96Ne5MxtN495l5S+F6WzbnSHF7m0GDKkGEe+rUbMiaHOODm2IB6abZkXVu2px//O
2BLSzNpqqda2ynDTnp+SK2MYLldcPc31473SoZ8+awQcWYzZ9PyMFf0PQhzX2gOH
d0gvLKDa1DfyWudc85ZA6Xj/xPGrMxBivRb2D/66qT4V4hNqRHvoAGWagGJNxV5Y
0SU7BziXkhaWV1m5gQxGLcJJz1ZOa4tsbpOVyfCd0RzWIaBe4hF0P9yJnFcVv3cO
uxM0V9vdgU3zlrHalRlob+oTSDuZBe7lPfsz921kYJnauk4f1X02tKcbm8E/jpjy
AcSARxz655Fc/Viy0YAdxGttgqXP9wf/OVu7xvSvOKeZce2RfsED7qQmU9H6F2MW
vZndB3Anb6aJPL4efoWMupixwXYYxHcJblssJzYm0VK/J6UedkmPFOpIctfgXbL4
G3eapxghNo6jcPD42Z3ZySKWYopRKUkn7dbGW2o8xSFXIkXm7zUfNH5OAxcSxk4Y
AZtG4KInyPrmTMPPD6XpC1vg0uBsU1v5X+sjCvljYc7g7pMiG5ORHX0bZt6s7Gkd
7bDzhP1IIHbdCElAOLDh
=58xt
-----END PGP SIGNATURE-----

--- End Message ---

Reply to: