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

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