Re: SSL cleanups/hostname verification - Mailing list pgsql-hackers

Alex Hunsaker wrote:
> On Mon, Oct 20, 2008 at 05:50, Magnus Hagander <magnus@hagander.net> wrote:

> $ SSLVERIFY=cn ./psql junk -h 192.168.0.2
> psql: server common name 'bahdushka' does not match hostname
> '192.168.0.2'FATAL:  no pg_hba.conf entry for host "192.168.0.2", user
> "alex", database "junk", SSL off

It needs to be PGSSLVERIFY if it's an environment variable. sslverify is
the connection parameter.

I think that's confusing your tests all the way through :(

Also, I'd recommend running the server with a log on a different console
or to a file so you don't get client and server error messages mixed up.


> $ SSLVERIFY=none ./psql junk -h bahdushka
> psql: root certificate file (/home/alex/.postgresql/root.crt)

Is that really the whole error message, or was it cut off? Because if it
is, then that is certainly a bug!


> But other than that looks good other than the promised documentation
> and the mem leak Tom Lane noted. (unless I missed an updated patch?)

I think you did, because there is certainly docs in the last one I sent
:-) But here's the very latest-and-greatest - I changed the cn matching
to be case insensitive per offlist comment from Dan Kaminsky, and an
internal return type to bool instead of int.

//Magnus
*** a/doc/src/sgml/libpq.sgml
--- b/doc/src/sgml/libpq.sgml
***************
*** 260,265 ****
--- 260,292 ----
          </varlistentry>

          <varlistentry>
+          <term><literal>sslverify</literal></term>
+          <listitem>
+           <para>
+            This option controls how libpq verifies the certificate on the
+            server when performing an <acronym>SSL</> connection. There are
+            three options: <literal>none</> disables verification completely
+            (not recommended!); <literal>cert</> enables verification that
+            the certificate chains to a known CA only; <literal>cn</> will
+            both verify that the certificate chains to a known CA and that
+            the <literal>cn</> attribute of the certificate matches the
+            hostname the connection is being made to (default).
+           </para>
+
+           <para>
+            It is always recommended to use the <literal>cn</> value for
+            this parameter, since this is the only option that prevents
+            man-in-the-middle attacks. Note that this requires the server
+            name on the certificate to match exactly with the host name
+            used for the connection, and therefore does not support connections
+            to aliased names. It can be used with pure IP address connections
+            only if the certificate also has just the IP address in the
+            <literal>cn</> field.
+           </para>
+          </listitem>
+         </varlistentry>
+
+         <varlistentry>
           <term><literal>requiressl</literal></term>
           <listitem>
            <para>
***************
*** 5682,5687 **** myEventProc(PGEventId evtId, void *evtInfo, void *passThrough)
--- 5709,5730 ----
      <listitem>
       <para>
        <indexterm>
+        <primary><envar>PGSSLVERIFY</envar></primary>
+       </indexterm>
+       <envar>PGSSLVERIFY</envar> controls how libpq verifies the certificate on the
+        server when performing an <acronym>SSL</> connection. There are
+        three options: <literal>none</> disables verification completely
+        (not recommended!); <literal>cert</> enables verification that
+        the certificate chains to a known CA only; <literal>cn</> will
+        both verify that the certificate chains to a known CA and that
+        the <literal>cn</> attribute of the certificate matches the
+        hostname the connection is being made to (default).
+      </para>
+     </listitem>
+
+     <listitem>
+      <para>
+       <indexterm>
         <primary><envar>PGREQUIRESSL</envar></primary>
        </indexterm>
        <envar>PGREQUIRESSL</envar> sets whether or not the connection must
***************
*** 6026,6034 **** myEventProc(PGEventId evtId, void *evtInfo, void *passThrough)
    </para>

    <para>
!    To verify the server certificate is trustworthy, place certificates of
!    the certificate authorities (<acronym>CA</acronym>) you trust in the
!    file <filename>~/.postgresql/root.crt</> in the user's home directory.
     (On Microsoft Windows the file is named
     <filename>%APPDATA%\postgresql\root.crt</filename>.)
     <application>libpq</application> will then verify that the server's
--- 6069,6079 ----
    </para>

    <para>
!    When the <literal>sslverify</> parameter is set to <literal>cn</> or
!    <literal>cert</>, libpq will verify that the server certificate is
!    trustworthy by checking the certificate chain up to a <acronym>CA</>.
!    For this to work, place the certificate of a trusted <acronym>CA</>
!    in the file <filename>~/.postgresql/root.crt</> in the user's home directory.
     (On Microsoft Windows the file is named
     <filename>%APPDATA%\postgresql\root.crt</filename>.)
     <application>libpq</application> will then verify that the server's
*** a/doc/src/sgml/runtime.sgml
--- b/doc/src/sgml/runtime.sgml
***************
*** 1418,1426 **** $ <userinput>kill -INT `head -1 /usr/local/pgsql/data/postmaster.pid`</userinput
     <filename>server.key</filename> (key) and
     <filename>server.crt</filename> (certificate) files (<xref
     linkend="ssl-tcp">). The TCP client must connect using
!    <literal>sslmode='require'</> (<xref linkend="libpq-connect">) and have
!    a <filename>~/.postgresql/root.crt</> SSL certificate (<xref
!    linkend="libpq-ssl">).
    </para>
   </sect1>

--- 1418,1426 ----
     <filename>server.key</filename> (key) and
     <filename>server.crt</filename> (certificate) files (<xref
     linkend="ssl-tcp">). The TCP client must connect using
!    <literal>sslmode='require'</>, specify <literal>sslverify='cn'</>
!    or <literal>sslverify='cert'</> and have the required certificate
!    files present (<xref linkend="libpq-connect">).
    </para>
   </sect1>

***************
*** 1544,1551 **** $ <userinput>kill -INT `head -1 /usr/local/pgsql/data/postmaster.pid`</userinput

     <listitem>
      <para>
!      It is possible for both the client and server to provide SSL keys
!      or certificates to each other. It takes some extra configuration
       on each side, but this provides stronger verification of identity
       than the mere use of passwords. It prevents a computer from
       pretending to be the server just long enough to read the password
--- 1544,1551 ----

     <listitem>
      <para>
!      It is possible for both the client and server to provide SSL
!      certificates to each other. It takes some extra configuration
       on each side, but this provides stronger verification of identity
       than the mere use of passwords. It prevents a computer from
       pretending to be the server just long enough to read the password
***************
*** 1757,1763 **** chmod og-rwx server.key
      A self-signed certificate can be used for testing, but a certificate
      signed by a certificate authority (<acronym>CA</>) (either one of the
      global <acronym>CAs</> or a local one) should be used in production
!     so the client can verify the server's identity.
     </para>

    </sect2>
--- 1757,1765 ----
      A self-signed certificate can be used for testing, but a certificate
      signed by a certificate authority (<acronym>CA</>) (either one of the
      global <acronym>CAs</> or a local one) should be used in production
!     so the client can verify the server's identity. If all the clients
!     are local to the organization, using a local <acronym>CA</> is
!     recommended.
     </para>

    </sect2>
*** a/src/interfaces/libpq/fe-connect.c
--- b/src/interfaces/libpq/fe-connect.c
***************
*** 92,99 **** static int ldapServiceLookup(const char *purl, PQconninfoOption *options,
--- 92,101 ----
  #define DefaultPassword          ""
  #ifdef USE_SSL
  #define DefaultSSLMode    "prefer"
+ #define DefaultSSLVerify "cn"
  #else
  #define DefaultSSLMode    "disable"
+ #define DefaultSSLVerify "none"
  #endif

  /* ----------
***************
*** 181,186 **** static const PQconninfoOption PQconninfoOptions[] = {
--- 183,191 ----
      {"sslmode", "PGSSLMODE", DefaultSSLMode, NULL,
      "SSL-Mode", "", 8},            /* sizeof("disable") == 8 */

+     {"sslverify", "PGSSLVERIFY", DefaultSSLVerify, NULL,
+     "SSL-Verify", "", 5},        /* sizeof("chain") == 5 */
+
  #if defined(KRB5) || defined(ENABLE_GSS) || defined(ENABLE_SSPI)
      /* Kerberos and GSSAPI authentication support specifying the service name */
      {"krbsrvname", "PGKRBSRVNAME", PG_KRB_SRVNAM, NULL,
***************
*** 415,420 **** connectOptions1(PGconn *conn, const char *conninfo)
--- 420,427 ----
      conn->connect_timeout = tmp ? strdup(tmp) : NULL;
      tmp = conninfo_getval(connOptions, "sslmode");
      conn->sslmode = tmp ? strdup(tmp) : NULL;
+     tmp = conninfo_getval(connOptions, "sslverify");
+     conn->sslverify = tmp ? strdup(tmp) : NULL;
  #ifdef USE_SSL
      tmp = conninfo_getval(connOptions, "requiressl");
      if (tmp && tmp[0] == '1')
***************
*** 530,535 **** connectOptions2(PGconn *conn)
--- 537,560 ----
          conn->sslmode = strdup(DefaultSSLMode);

      /*
+      * Validate sslverify option
+      */
+     if (conn->sslverify)
+     {
+         if (strcmp(conn->sslverify, "none") != 0
+             && strcmp(conn->sslverify, "cert") != 0
+             && strcmp(conn->sslverify, "cn") != 0)
+         {
+             conn->status = CONNECTION_BAD;
+             printfPQExpBuffer(&conn->errorMessage,
+                             libpq_gettext("invalid sslverify value: \"%s\"\n"),
+                               conn->sslverify);
+             return false;
+         }
+     }
+
+
+     /*
       * Only if we get this far is it appropriate to try to connect. (We need a
       * state flag, rather than just the boolean result of this function, in
       * case someone tries to PQreset() the PGconn.)
***************
*** 2008,2013 **** freePGconn(PGconn *conn)
--- 2033,2040 ----
          free(conn->pgpass);
      if (conn->sslmode)
          free(conn->sslmode);
+     if (conn->sslverify)
+         free(conn->sslverify);
  #if defined(KRB5) || defined(ENABLE_GSS) || defined(ENABLE_SSPI)
      if (conn->krbsrvname)
          free(conn->krbsrvname);
*** a/src/interfaces/libpq/fe-secure.c
--- b/src/interfaces/libpq/fe-secure.c
***************
*** 87,95 ****
  #define ERR_pop_to_mark()    ((void) 0)
  #endif

! #ifdef NOT_USED
! static int    verify_peer_name_matches_certificate(PGconn *);
! #endif
  static int    verify_cb(int ok, X509_STORE_CTX *ctx);
  static int    client_cert_cb(SSL *, X509 **, EVP_PKEY **);
  static int    init_ssl_system(PGconn *conn);
--- 87,93 ----
  #define ERR_pop_to_mark()    ((void) 0)
  #endif

! static bool verify_peer_name_matches_certificate(PGconn *);
  static int    verify_cb(int ok, X509_STORE_CTX *ctx);
  static int    client_cert_cb(SSL *, X509 **, EVP_PKEY **);
  static int    init_ssl_system(PGconn *conn);
***************
*** 438,514 **** verify_cb(int ok, X509_STORE_CTX *ctx)
      return ok;
  }

- #ifdef NOT_USED
  /*
   *    Verify that common name resolves to peer.
   */
! static int
  verify_peer_name_matches_certificate(PGconn *conn)
  {
!     struct hostent *cn_hostentry = NULL;
!     struct sockaddr server_addr;
!     struct sockaddr_in *sin (struct sockaddr_in *) &server_addr;
!     ACCEPT_TYPE_ARG3 len;
!     char      **s;
!     unsigned long l;
!
!     /* Get the address on the other side of the socket. */
!     len = sizeof(server_addr);
!     if (getpeername(conn->sock, &server_addr, &len) == -1)
!     {
!         char        sebuf[256];
!
!         printfPQExpBuffer(&conn->errorMessage,
!                           libpq_gettext("error querying socket: %s\n"),
!                           SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
!         return -1;
!     }

!     if (server_addr.sa_family != AF_INET)
      {
          printfPQExpBuffer(&conn->errorMessage,
!                           libpq_gettext("unsupported protocol\n"));
!         return -1;
      }
!
!     /* Get the IP addresses of the certificate's common name (CN) */
      {
-         struct hostent hpstr;
-         char        buf[BUFSIZ];
-         int            herrno = 0;
-
          /*
!          * Currently, pqGethostbyname() is used only on platforms that don't
!          * have getaddrinfo().    If you enable this function, you should
!          * convert the pqGethostbyname() function call to use getaddrinfo().
           */
!         pqGethostbyname(conn->peer_cn, &hpstr, buf, sizeof(buf),
!                         &cn_hostentry, &herrno);
!     }
!
!     /* Did we get an IP address? */
!     if (cn_hostentry == NULL)
!     {
!         printfPQExpBuffer(&conn->errorMessage,
!           libpq_gettext("could not get information about host \"%s\": %s\n"),
!                           conn->peer_cn, hstrerror(h_errno));
!         return -1;
      }
-
-     /* Does one of the CN's IP addresses match the server's IP address? */
-     for (s = cn_hostentry->h_addr_list; *s != NULL; s++)
-         if (!memcmp(&sin->sin_addr.s_addr, *s, cn_hostentry->h_length))
-             return 0;
-
-     l = ntohl(sin->sin_addr.s_addr);
-     printfPQExpBuffer(&conn->errorMessage,
-                       libpq_gettext(
-                         "server common name \"%s\" does not resolve to %ld.%ld.%ld.%ld\n"),
-                       conn->peer_cn, (l >> 24) % 0x100, (l >> 16) % 0x100,
-                       (l >> 8) % 0x100, l % 0x100);
-     return -1;
  }
- #endif   /* NOT_USED */

  /*
   *    Callback used by SSL to load client cert and key.
--- 436,479 ----
      return ok;
  }

  /*
   *    Verify that common name resolves to peer.
   */
! static bool
  verify_peer_name_matches_certificate(PGconn *conn)
  {
!     /*
!      * If told not to verify the peer name, don't do it. Return
!      * 0 indicating that the verification was successful.
!      */
!     if(strcmp(conn->sslverify, "cn") != 0)
!         return true;

!     if (conn->pghostaddr)
      {
          printfPQExpBuffer(&conn->errorMessage,
!                           libpq_gettext("verified SSL connections are only supported when connecting to a
hostname"));
!         return false;
      }
!     else
      {
          /*
!          * Connect by hostname.
!          *
!          * XXX: Should support alternate names here
!          * XXX: Should support wildcard certificates here
           */
!         if (pg_strcasecmp(conn->peer_cn, conn->pghost) != 0)
!         {
!             printfPQExpBuffer(&conn->errorMessage,
!                               libpq_gettext("server common name '%s' does not match hostname '%s'"),
!                               conn->peer_cn, conn->pghost);
!             return false;
!         }
!         else
!             return true;
      }
  }

  /*
   *    Callback used by SSL to load client cert and key.
***************
*** 846,851 **** initialize_SSL(PGconn *conn)
--- 811,822 ----
      if (init_ssl_system(conn))
          return -1;

+     /*
+      * If sslverify is set to anything other than "none", perform certificate
+      * verification. If set to "cn" we will also do further verifications after
+      * the connection has been completed.
+      */
+
      /* Set up to verify server cert, if root.crt is present */
      if (pqGetHomeDirectory(homedir, sizeof(homedir)))
      {
***************
*** 889,894 **** initialize_SSL(PGconn *conn)
--- 860,883 ----

              SSL_CTX_set_verify(SSL_context, SSL_VERIFY_PEER, verify_cb);
          }
+         else
+         {
+             if (strcmp(conn->sslverify, "none") != 0)
+             {
+                 printfPQExpBuffer(&conn->errorMessage,
+                                   libpq_gettext("root certificate file (%s) not found"), fnbuf);
+                 return -1;
+             }
+         }
+     }
+     else
+     {
+         if (strcmp(conn->sslverify, "none") != 0)
+         {
+             printfPQExpBuffer(&conn->errorMessage,
+                               libpq_gettext("cannot find home directory to locate root certificate file"));
+             return -1;
+         }
      }

      /* set up mechanism to provide client certificate, if available */
***************
*** 1004,1016 **** open_client_SSL(PGconn *conn)
                                NID_commonName, conn->peer_cn, SM_USER);
      conn->peer_cn[SM_USER] = '\0';

! #ifdef NOT_USED
!     if (verify_peer_name_matches_certificate(conn) == -1)
      {
          close_SSL(conn);
          return PGRES_POLLING_FAILED;
      }
- #endif

      /* SSL handshake is complete */
      return PGRES_POLLING_OK;
--- 993,1003 ----
                                NID_commonName, conn->peer_cn, SM_USER);
      conn->peer_cn[SM_USER] = '\0';

!     if (!verify_peer_name_matches_certificate(conn))
      {
          close_SSL(conn);
          return PGRES_POLLING_FAILED;
      }

      /* SSL handshake is complete */
      return PGRES_POLLING_OK;
*** a/src/interfaces/libpq/libpq-int.h
--- b/src/interfaces/libpq/libpq-int.h
***************
*** 291,296 **** struct pg_conn
--- 291,297 ----
      char       *pguser;            /* Postgres username and password, if any */
      char       *pgpass;
      char       *sslmode;        /* SSL mode (require,prefer,allow,disable) */
+     char       *sslverify;        /* Verify server SSL certificate (none,chain,cn) */
  #if defined(KRB5) || defined(ENABLE_GSS) || defined(ENABLE_SSPI)
      char       *krbsrvname;        /* Kerberos service name */
  #endif