SSL cleanups/hostname verification - Mailing list pgsql-hackers

From Magnus Hagander
Subject SSL cleanups/hostname verification
Date
Msg-id 48FC7E84.3080702@hagander.net
Whole thread Raw
Responses Re: SSL cleanups/hostname verification  (Tom Lane <tgl@sss.pgh.pa.us>)
Re: SSL cleanups/hostname verification  (Bruce Momjian <bruce@momjian.us>)
Re: SSL cleanups/hostname verification  ("Alex Hunsaker" <badalex@gmail.com>)
List pgsql-hackers
Attached patch cleans up the certificate verification in libpq, and adds
a configuration paraqmeter to control it. The new parameter is
"sslverify", and can be set to:

* cn = default = will validate that the certificate chains to a trusted
root, *and* that the cn on the certificate matches the hostname
specificed in the connection. This is the only option that prevents
man-in-the-middle attacks completely, and therefor is the default.

* cert = what we had before if there was a root certificate file = will
validate that the certificate chains to a trusted root, but ignore the cn.

* none = will disable certificate validation completely


This means that the connection string is now in charge of the security
policy, and not just the "if file exists or not". IMHO this is the only
proper way to do it. Now, if you for some reason loose the root
certificate file, libpq will refuse to connect unless you have
explicitly told it to connect without verifying the certificate.
Previously if you accidentally lost the file, you would connect
insecurely without knowing about it.


The error messages from the patch requires the
libpq-error-message-stacking patch as well (or something like it),
otherwise the error message will often get overwritten by a later one if
we retry without SSL.


I intend to follow this up with a similar patch for the server side,
which will make it a connection option instead of being dependent on the
presence of a file. This is depending on the pg_hba options patch,
however, so it's not ready yet.


//Magnus
*** a/doc/src/sgml/libpq.sgml
--- b/doc/src/sgml/libpq.sgml
***************
*** 259,264 ****
--- 259,291 ----
          </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</i> field.
+           </para>
+          </listitem>
+         </varlistentry>
+
+         <varlistentry>
           <term><literal>requiressl</literal></term>
           <listitem>
            <para>
***************
*** 6011,6019 **** 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
--- 6038,6048 ----
    </para>

    <para>
!    When the <literal>sslverify</> parameter is set to <literal>cn</> or
!    <literal>cert</I>, 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 <acronum>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,
***************
*** 412,417 **** connectOptions1(PGconn *conn, const char *conninfo)
--- 417,424 ----
      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')
***************
*** 527,532 **** connectOptions2(PGconn *conn)
--- 534,557 ----
          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.)
*** a/src/interfaces/libpq/fe-secure.c
--- b/src/interfaces/libpq/fe-secure.c
***************
*** 493,569 **** 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.
--- 493,533 ----
      return ok;
  }

  /*
   *    Verify that common name resolves to peer.
   */
  static int
  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 0;

!     if (conn->pghostaddr)
      {
          printfPQExpBuffer(&conn->errorMessage,
!                           libpq_gettext("verified SSL connections are only supported when connecting to a
hostname"));
          return -1;
      }
!     else
      {
          /*
!          * Connect by Host name
           */
!         if (strcmp(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 -1;
!         }
!         else
              return 0;
!     }
  }

  /*
   *    Callback used by SSL to load client cert and key.
***************
*** 901,906 **** initialize_SSL(PGconn *conn)
--- 865,876 ----
      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)))
      {
***************
*** 944,949 **** initialize_SSL(PGconn *conn)
--- 914,937 ----

              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 */
***************
*** 1059,1071 **** 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;
--- 1047,1057 ----
*** 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

pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: SQL:2008 LIMIT/OFFSET
Next
From: Tom Lane
Date:
Subject: Re: pg_hba options parsing