Magnus Hagander wrote:
> I think I agree with the idea that we should match wildcards only at the
> beginning of the name *for now*, and then see what people actually
> request :-) I'm less sure about the single-pathname-component part, but
> the argument around backwards compatible is certainly a very valid one..

Here's one that (I think) does that. For every step, the code becomes
simpler - which I like when it comes to security code :)

//Magnus

*** a/src/interfaces/libpq/fe-secure.c
--- b/src/interfaces/libpq/fe-secure.c
***************
*** 55,60 ****
--- 55,61 ----
  #endif
  
  #ifdef USE_SSL
+ 
  #include <openssl/ssl.h>
  #include <openssl/bio.h>
  #if (SSLEAY_VERSION_NUMBER >= 0x00907000L)
***************
*** 64,79 ****
  #include <openssl/engine.h>
  #endif
  
- /* fnmatch() needed for client certificate checking */
- #ifdef HAVE_FNMATCH
- #include <fnmatch.h>
- #else
- #include "fnmatchstub.h"
- #endif
- #endif   /* USE_SSL */
- 
- 
- #ifdef USE_SSL
  
  #ifndef WIN32
  #define USER_CERT_FILE		".postgresql/postgresql.crt"
--- 65,70 ----
***************
*** 443,448 **** verify_cb(int ok, X509_STORE_CTX *ctx)
--- 434,481 ----
  	return ok;
  }
  
+ 
+ /*
+  * Check if a wildcard certificate matches the server hostname.
+  *
+  * The rule for this is:
+  *  1. We only match the '*' character as wildcard
+  *  2. We match only wildcards at the start of the string
+  *  3. The '*' character does *not* match '.', meaning that we match only
+  *     a single pathname component.
+  *  4. We don't support more than one '*' in a single pattern.
+  *
+  * This is roughly in line with RFC2818, but contrary to what most browsers
+  * appear to be implementing (point 3 being the difference)
+  *
+  * Matching is always cone case-insensitive, since DNS is case insensitive.
+  */
+ static int
+ wildcard_certificate_match(const char *pattern, const char *string)
+ {
+ 	/* If we don't start with a wildcard, it's not a match (rule 1 & 2) */
+ 	if (strlen(pattern) < 3 ||
+ 		pattern[0] != '*' ||
+ 		pattern[1] != '.')
+ 		return 0;
+ 
+ 	if (strlen(pattern) > strlen(string))
+ 		/* If pattern is longer than the string, we can never match */
+ 		return 0;
+ 
+ 	if (pg_strcasecmp(pattern+1, string+strlen(string)-strlen(pattern)+1) != 0)
+ 		/* If string does not end in pattern (minus the wildcard), we don't match */
+ 		return 0;
+ 
+ 	if (strchr(string, '.') < string+strlen(string)-strlen(pattern))
+ 		/* If there is a dot left of where the pattern started to match, we don't match (rule 3) */
+ 		return 0;
+ 
+ 	/* String ended with pattern, and didn't have a dot before, so we match */
+ 	return 1;
+ }
+ 
+ 
  /*
   *	Verify that common name resolves to peer.
   */
***************
*** 472,478 **** 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)
  			/* Matched wildcard certificate */
  			return true;
  		else
--- 505,511 ----
  		if (pg_strcasecmp(conn->peer_cn, conn->pghost) == 0)
  			/* Exact name match */
  			return true;
! 		else if (wildcard_certificate_match(conn->peer_cn, conn->pghost))
  			/* Matched wildcard certificate */
  			return true;
  		else
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to