Attachment 696105 Details for Bug 909959 – Proposed isync upstream patch (against the 1.0.x branch) to correct this issue

[patch] Proposed isync upstream patch (against the 1.0.x branch) to correct this issue

isync-1.0.x-security-fix-add-SSL-subject-verification.patch (text/plain), 3.96 KB, created by Jan Lieskovsky on 2013-02-11 14:32:23 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jan Lieskovsky

Created: 2013-02-11 14:32:23 UTC

Size: 3.96 KB

patch

obsolete

>From b97355d6ee7acf7470ba9d5aae7c1a9b84196dfc Mon Sep 17 00:00:00 2001
>From: Oswald Buddenhagen <ossi@kde.org>
>Date: Sun, 3 Feb 2013 16:05:55 +0100
>Subject: [PATCH] security fix: add SSL subject verification
>
>we did not check a valid certificate's subject at all so far.
>this is no problem if the certificate file contains only exactly the
>wanted host's certificate - before revision 04fdf7d1 (dec 2000, < v0.4),
>this was even enforced (more or less - if the peer cert had been
>directly signed by a root cert, it would be accepted as well).
>however, when the file contains root certificates (like the system-wide
>certificate file typically does), any host with a valid certificate
>could pretend to be the wanted host.
>---
> src/drv_imap.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 68 insertions(+), 7 deletions(-)
>
>diff --git a/src/drv_imap.c b/src/drv_imap.c
>index 2a84f3d..afe6df0 100644
>--- a/src/drv_imap.c
>+++ b/src/drv_imap.c
>@@ -48,6 +48,7 @@
> # include <openssl/ssl.h>
> # include <openssl/err.h>
> # include <openssl/hmac.h>
>+# include <openssl/x509v3.h>
> #endif
> 
> typedef struct imap_server_conf {
>@@ -187,9 +188,65 @@ static const char *Flags[] = {
> 
> #if HAVE_LIBSSL
> 
>+static int
>+host_matches( const char *host, const char *pattern )
>+{
>+	if (pattern[0] == '*' && pattern[1] == '.') {
>+		pattern += 2;
>+		if (!(host = strchr( host, '.' )))
>+			return 0;
>+		host++;
>+	}
>+
>+	return *host && *pattern && !strcasecmp( host, pattern );
>+}
>+
>+static int
>+verify_hostname( X509 *cert, const char *hostname )
>+{
>+	int i, len, found;
>+	X509_NAME *subj;
>+	STACK_OF(GENERAL_NAME) *subj_alt_names;
>+	char cname[1000];
>+
>+	/* try the DNS subjectAltNames */
>+	found = 0;
>+	if ((subj_alt_names = X509_get_ext_d2i( cert, NID_subject_alt_name, NULL, NULL ))) {
>+		int num_subj_alt_names = sk_GENERAL_NAME_num( subj_alt_names );
>+		for (i = 0; i < num_subj_alt_names; i++) {
>+			GENERAL_NAME *subj_alt_name = sk_GENERAL_NAME_value( subj_alt_names, i );
>+			if (subj_alt_name->type == GEN_DNS &&
>+			    strlen( (const char *)subj_alt_name->d.ia5->data ) == (size_t)subj_alt_name->d.ia5->length &&
>+			    host_matches( hostname, (const char *)(subj_alt_name->d.ia5->data) ))
>+			{
>+				found = 1;
>+				break;
>+			}
>+		}
>+		sk_GENERAL_NAME_pop_free( subj_alt_names, GENERAL_NAME_free );
>+	}
>+	if (found)
>+		return 0;
>+
>+	/* try the common name */
>+	if (!(subj = X509_get_subject_name( cert ))) {
>+		fprintf( stderr, "Error, cannot get certificate subject\n" );
>+		return -1;
>+	}
>+	if ((len = X509_NAME_get_text_by_NID( subj, NID_commonName, cname, sizeof(cname) )) < 0) {
>+		fprintf( stderr, "Error, cannot get certificate common name\n" );
>+		return -1;
>+	}
>+	if (strlen( cname ) == (size_t)len && host_matches( hostname, cname ))
>+		return 0;
>+
>+	fprintf( stderr, "Error, certificate owner does not match hostname %s\n", hostname );
>+	return -1;
>+}
>+
> /* this gets called when a certificate is to be verified */
> static int
>-verify_cert( SSL *ssl )
>+verify_cert( SSL *ssl, const char *hostname )
> {
> 	X509 *cert;
> 	int err;
>@@ -204,12 +261,16 @@ verify_cert( SSL *ssl )
> 	}
> 
> 	err = SSL_get_verify_result( ssl );
>-	if (err == X509_V_OK)
>-		return 0;
>-
>-	fprintf( stderr, "Error, can't verify certificate: %s (%d)\n",
>-	         X509_verify_cert_error_string(err), err );
>+	if (err != X509_V_OK) {
>+		fprintf( stderr, "Error, can't verify certificate: %s (%d)\n",
>+		         X509_verify_cert_error_string(err), err );
>+		goto intcheck;
>+	}
>+	if (hostname && verify_hostname( cert, hostname ) < 0)
>+		goto intcheck;
>+	return 0;
> 
>+  intcheck:
> 	X509_NAME_oneline( X509_get_subject_name( cert ), buf, sizeof(buf) );
> 	info( "\nSubject: %s\n", buf );
> 	X509_NAME_oneline( X509_get_issuer_name( cert ), buf, sizeof(buf) );
>@@ -1103,7 +1164,7 @@ start_tls( imap_store_t *ctx )
> 	}
> 
> 	/* verify the server certificate */
>-	if (verify_cert( imap->buf.sock.ssl ))
>+	if (verify_cert( imap->buf.sock.ssl, ((imap_store_conf_t *)ctx->gen.conf)->server->host ))
> 		return 1;
> 
> 	imap->buf.sock.use_ssl = 1;
>-- 
>1.8.0.1.18.g93341d8
>

Actions: View | Diff

Attachments on bug 909959: 696105 | 696107