Re: libpq 8.4 beta1: $PGHOST complains about missing root.crt

Bruce Momjian wrote:
> Magnus Hagander wrote:
>> On 14 apr 2009, at 04.33, Bruce Momjian <bruce@momjian.us> wrote:
>>
>>> Magnus Hagander wrote:
>>>>> I would actually call the two parameters 'verify-cert' and 'verify-
>>>>> cn',
>>>>> and document that they also have "require" behavior.  Obviously you
>>>>> can't verify certificates unless you require SSL.
>>>> I would prefer having "verify", "verify-no-cn" and "no-verify" or
>>>> something like that. Making it the "default choice" to have
>>>> verification
>>>> enabled, and very clear that you're turning something off if you're
>>>> not.
>>>> And then just map require to verify. Or they could be "require-no-cn"
>>>> and "require-no-cert" perhaps?
>>>>
>>>> ("default choice" only for those using ssl of course - we'd still
>>>> have
>>>> "disable" as the default *value* of the parameter)
>>> I think the "no" options are odd because they have _negative_
>>> designations.
>> That's the intention. When you're turning off something, I think it
>> makes sense to use "no"....
>
> But that doesn't scale:  sslmode currently has four options, soon
> perhaps to be six.   The idea is that the items should be of increasing
> security, and adding "no" in the middle doesn't allow that to be clear.

Here's a patch for this. Obviously, a lot needs to be done about the
docs here, I'm working on that.

I went with the names "require", "verify-ca" and "verify-full".

Patch also changes the default from "prefer" to "disable", per discussion.

Comments?

//Magnus
*** a/doc/src/sgml/libpq.sgml
--- b/doc/src/sgml/libpq.sgml
***************
*** 270,276 ****
              <tbody>

               <row>
!               <entry><literal>disable</></entry>
                <entry>only try a non-<acronym>SSL</> connection</entry>
               </row>

--- 270,276 ----
              <tbody>

               <row>
!               <entry><literal>disable</> (default)</entry>
                <entry>only try a non-<acronym>SSL</> connection</entry>
               </row>

***************
*** 282,288 ****
               </row>

               <row>
!               <entry><literal>prefer</> (default)</entry>
                <entry>first try an <acronym>SSL</> connection;  if
                that fails, try a non-<acronym>SSL</>
                connection</entry>
--- 282,288 ----
               </row>

               <row>
!               <entry><literal>prefer</></entry>
                <entry>first try an <acronym>SSL</> connection;  if
                that fails, try a non-<acronym>SSL</>
                connection</entry>
*** a/src/interfaces/libpq/fe-connect.c
--- b/src/interfaces/libpq/fe-connect.c
***************
*** 90,102 **** static int ldapServiceLookup(const char *purl, PQconninfoOption *options,
  #define DefaultOption    ""
  #define DefaultAuthtype          ""
  #define DefaultPassword          ""
- #ifdef USE_SSL
- #define DefaultSSLMode    "prefer"
- #define DefaultSSLVerify "cn"
- #else
  #define DefaultSSLMode    "disable"
- #define DefaultSSLVerify "none"
- #endif

  /* ----------
   * Definition of the conninfo parameters and their fallback resources.
--- 90,96 ----
***************
*** 185,193 **** static const PQconninfoOption PQconninfoOptions[] = {
      {"sslmode", "PGSSLMODE", DefaultSSLMode, NULL,
      "SSL-Mode", "", 8},            /* sizeof("disable") == 8 */

-     {"sslverify", "PGSSLVERIFY", DefaultSSLVerify, NULL,
-     "SSL-Verify", "", 5},        /* sizeof("chain") == 5 */
-
      {"sslcert", "PGSSLCERT", NULL, NULL,
      "SSL-Client-Cert", "", 64},

--- 179,184 ----
***************
*** 431,438 **** connectOptions1(PGconn *conn, const char *conninfo)
      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;
      tmp = conninfo_getval(connOptions, "sslkey");
      conn->sslkey = tmp ? strdup(tmp) : NULL;
      tmp = conninfo_getval(connOptions, "sslcert");
--- 422,427 ----
***************
*** 522,528 **** connectOptions2(PGconn *conn)
          if (strcmp(conn->sslmode, "disable") != 0
              && strcmp(conn->sslmode, "allow") != 0
              && strcmp(conn->sslmode, "prefer") != 0
!             && strcmp(conn->sslmode, "require") != 0)
          {
              conn->status = CONNECTION_BAD;
              printfPQExpBuffer(&conn->errorMessage,
--- 511,519 ----
          if (strcmp(conn->sslmode, "disable") != 0
              && strcmp(conn->sslmode, "allow") != 0
              && strcmp(conn->sslmode, "prefer") != 0
!             && strcmp(conn->sslmode, "require") != 0
!             && strcmp(conn->sslmode, "verify-ca") != 0
!             && strcmp(conn->sslmode, "verify-full") != 0)
          {
              conn->status = CONNECTION_BAD;
              printfPQExpBuffer(&conn->errorMessage,
***************
*** 544,549 **** connectOptions2(PGconn *conn)
--- 535,541 ----
                  break;

              case 'r':            /* "require" */
+             case 'v':            /* "verify-ca" or "verify-full" */
                  conn->status = CONNECTION_BAD;
                  printfPQExpBuffer(&conn->errorMessage,
                                    libpq_gettext("sslmode value \"%s\" invalid when SSL support is not compiled
in\n"),
***************
*** 556,579 **** connectOptions2(PGconn *conn)
          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.)
--- 548,553 ----
***************
*** 1428,1434 **** keep_going:                        /* We will come back to here until there is
                      }
                      else if (SSLok == 'N')
                      {
!                         if (conn->sslmode[0] == 'r')    /* "require" */
                          {
                              /* Require SSL, but server does not want it */
                              appendPQExpBuffer(&conn->errorMessage,
--- 1402,1409 ----
                      }
                      else if (SSLok == 'N')
                      {
!                         if (conn->sslmode[0] == 'r' ||    /* "require" */
!                             conn->sslmode[0] == 'v')    /* "verify-ca" or "verify-full" */
                          {
                              /* Require SSL, but server does not want it */
                              appendPQExpBuffer(&conn->errorMessage,
***************
*** 1445,1451 **** keep_going:                        /* We will come back to here until there is
                          /* Received error - probably protocol mismatch */
                          if (conn->Pfdebug)
                              fprintf(conn->Pfdebug, "received error from server, attempting fallback to pre-7.0\n");
!                         if (conn->sslmode[0] == 'r')    /* "require" */
                          {
                              /* Require SSL, but server is too old */
                              appendPQExpBuffer(&conn->errorMessage,
--- 1420,1427 ----
                          /* Received error - probably protocol mismatch */
                          if (conn->Pfdebug)
                              fprintf(conn->Pfdebug, "received error from server, attempting fallback to pre-7.0\n");
!                         if (conn->sslmode[0] == 'r' ||    /* "require" */
!                             conn->sslmode[0] == 'v')    /* "verify-ca" or "verify-full" */
                          {
                              /* Require SSL, but server is too old */
                              appendPQExpBuffer(&conn->errorMessage,
***************
*** 2052,2059 **** freePGconn(PGconn *conn)
          free(conn->pgpass);
      if (conn->sslmode)
          free(conn->sslmode);
-     if (conn->sslverify)
-         free(conn->sslverify);
      if (conn->sslcert)
          free(conn->sslcert);
      if (conn->sslkey)
--- 2028,2033 ----
*** a/src/interfaces/libpq/fe-secure.c
--- b/src/interfaces/libpq/fe-secure.c
***************
*** 523,529 **** 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)
--- 523,529 ----
       * If told not to verify the peer name, don't do it. Return
       * 0 indicating that the verification was successful.
       */
!     if (strcmp(conn->sslmode, "verify-full") != 0)
          return true;

      if (conn->pghostaddr)
***************
*** 987,995 **** initialize_SSL(PGconn *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.
       *
       * If we are going to look for either root certificate or CRL in the home directory,
       * we need pqGetHomeDirectory() to succeed. In other cases, we don't need to
--- 987,995 ----
          return -1;

      /*
!      * If sslmode is set to one of the verify options, perform certificate
!      * verification. If set to "verify-full" we will also do further
!      * verification after the connection has been completed.
       *
       * If we are going to look for either root certificate or CRL in the home directory,
       * we need pqGetHomeDirectory() to succeed. In other cases, we don't need to
***************
*** 999,1005 **** initialize_SSL(PGconn *conn)
      {
          if (!pqGetHomeDirectory(homedir, sizeof(homedir)))
          {
!             if (strcmp(conn->sslverify, "none") != 0)
              {
                  printfPQExpBuffer(&conn->errorMessage,
                                    libpq_gettext("could not get home directory to locate root certificate file"));
--- 999,1005 ----
      {
          if (!pqGetHomeDirectory(homedir, sizeof(homedir)))
          {
!             if (conn->sslmode[0] == 'v') /* "verify-ca" or "verify-full" */
              {
                  printfPQExpBuffer(&conn->errorMessage,
                                    libpq_gettext("could not get home directory to locate root certificate file"));
***************
*** 1064,1070 **** initialize_SSL(PGconn *conn)
      else
      {
          /* stat() failed; assume cert file doesn't exist */
!         if (strcmp(conn->sslverify, "none") != 0)
          {
              printfPQExpBuffer(&conn->errorMessage,
                                libpq_gettext("root certificate file \"%s\" does not exist\n"
--- 1064,1070 ----
      else
      {
          /* stat() failed; assume cert file doesn't exist */
!         if (conn->sslmode[0] == 'v') /* "verify-ca" or "verify-full" */
          {
              printfPQExpBuffer(&conn->errorMessage,
                                libpq_gettext("root certificate file \"%s\" does not exist\n"
*** a/src/interfaces/libpq/libpq-int.h
--- b/src/interfaces/libpq/libpq-int.h
***************
*** 294,300 **** struct pg_conn
      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) */
      char       *sslkey;            /* client key filename */
      char       *sslcert;        /* client certificate filename */
      char       *sslrootcert;    /* root certificate filename */
--- 294,299 ----