Re: Re: [COMMITTERS] pgsql: Add support for matching wildcard server certificates to the new

Peter Eisentraut wrote:
> I wrote:
>> Some more information on this:
>> https://www.switch.ch/pki/meetings/2007-01/namebased_ssl_virtualhosts.pdf
>> slide 5 lists the matching rules for email, HTTP, and LDAP over TLS,
>> respectively, which are not all the same.  Also note that these methods
>> have rules for interpreting fields in the certificate other than the common
>> name for the host name.
>>
>> I think it is safest and easiest to allow a * wildcard only as the first
>> character and only when followed immediately by a dot.
>>
>> Maybe some DNS expert around here can offer advice on what a morally sound
>> solution would be.
> 
> This page summarizes the sadness pretty well:
> 
> http://wiki.cacert.org/wiki/WildcardCertificates

Yuck, that was certainly sad.

I think the most reasonable thing is to match the way that "modern
browsers" appear to do, which is that it matches * against subdomains as
well.

Matching *only* as the first character will make it impossible to make
certificates for "www*.domain.com", which is AFAIK fairly popular - and
one of the examples you'll find on CA sites. But it would be fairly easy
to add this restriction if people feel that's a better way.

See attached patch which takes out the parts of fnmatch that we're not
interested in, and puts it directly in fe-secure.c. Obviously, if we go
down that way, we can remove fnmatch.c from port again :-)

Thoughts?

//Magnus

diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c
index 828e867..28e254f 100644
--- a/src/interfaces/libpq/fe-secure.c
+++ b/src/interfaces/libpq/fe-secure.c
@@ -55,6 +55,7 @@
 #endif
 
 #ifdef USE_SSL
+
 #include 
 #include 
 #if (SSLEAY_VERSION_NUMBER >= 0x00907000L)
@@ -64,16 +65,6 @@
 #include 
 #endif
 
-/* fnmatch() needed for client certificate checking */
-#ifdef HAVE_FNMATCH
-#include 
-#else
-#include "fnmatchstub.h"
-#endif
-#endif   /* USE_SSL */
-
-
-#ifdef USE_SSL
 
 #ifndef WIN32
 #define USER_CERT_FILE		".postgresql/postgresql.crt"
@@ -444,6 +435,55 @@ verify_cb(int ok, X509_STORE_CTX *ctx)
 }
 
 /*
+ * Check if a wildcard certificate matches. This code is based on the
+ * NetBSD version of fnmatch(), but adapted to match wildcard certificates
+ * by removing much of the filename/directory specific functionality.
+ * Based on what "most others" do for https, we match the wildcard '*' to
+ * any part, *including* subdomains. This is contrary to RFC2818, but it is
+ * what most modern browsers match
+ * (see http://wiki.cacert.org/wiki/WildcardCertificates)
+ *
+ * Matching is always cone case-insensitive, since DNS is case insensitive.
+ */
+static int
+wildcard_certificate_match(const char *pattern, const char *string)
+{
+	const char *stringstart;
+	char c, test;
+
+	for (stringstart = string;;)
+	{
+		switch (c = tolower(*pattern++)) {
+		case '\0':
+			return (*string == '\0') ? 0 : 1;
+		case '*':
+			c = tolower(*pattern);
+			/* Collapse multiple stars. */
+			while (c == '*')
+				c = tolower(*++pattern);
+
+			if (c == '\0')
+				return 0;
+
+			/* General case, use recursion. */
+			while ((test = tolower(*string)) != '\0')
+			{
+				if (!wildcard_certificate_match(pattern, string))
+					return 0;
+				++string;
+			}
+			return 1;
+		default:
+			if (c != tolower(*string++))
+				return 1;
+			break;
+		}
+	/* NOTREACHED */
+	}
+}
+
+
+/*
  *	Verify that common name resolves to peer.
  */
 static bool
@@ -472,7 +512,7 @@ verify_peer_name_matches_certificate(PGconn *conn)
 		if (pg_strcasecmp(conn->peer_cn, conn->pghost) == 0)
 			/* Exact name match */
 			return true;
-		else if (fnmatch(conn->peer_cn, conn->pghost, FNM_NOESCAPE/* | FNM_CASEFOLD*/) == 0)
+		else if (wildcard_certificate_match(conn->peer_cn, conn->pghost) == 0)
 			/* Matched wildcard certificate */
 			return true;
 		else