Re: Possible regression: libpq + SSL aborts when user has no home directory

Поиск
Список
Период
Сортировка
От Tom Lane
Тема Re: Possible regression: libpq + SSL aborts when user has no home directory
Дата
Msg-id 10824.1299196694@sss.pgh.pa.us
обсуждение исходный текст
Ответ на Re: Possible regression: libpq + SSL aborts when user has no home directory  (Tom Lane <tgl@sss.pgh.pa.us>)
Ответы Re: Possible regression: libpq + SSL aborts when user has no home directory  (Christian Kastner <debian@kvr.at>)
Re: Possible regression: libpq + SSL aborts when user has no home directory  (Magnus Hagander <magnus@hagander.net>)
Список pgsql-bugs
I wrote:
> Christian Kastner <debian@kvr.at> writes:
>> Using libpq 9.0.3, when an SSL connection is attempted from a client
>> whose EUID is not in a password database, the connection fails because
>> the home directory cannot be determined. With libpq 8.4.7, everything is
>> fine.

> Hmm.  Offhand I agree that that seems like an unnecessary regression.
> It should act just the same as if it could not find any of those files.
> A quick look with git blame suggests that this got broken in my
> commit 4ed4b6c54e5fab24ab2624d80e26f7546edc88ad, and I don't think
> that it was intentional.

> One small problem is that if the sslmode is "verify-ca" or
> "verify-full", failure to find the root cert file is an error,
> and that error message normally includes the pathname at which
> the cert file was sought.  What shall we print if we couldn't
> identify the home directory?

Attached is an untested patch which I'd appreciate if you (or somebody
else who uses SSL connections more than I do) could test.  I resolved
the last mentioned problem by printing "~/.postgresql/root.crt", which
is a bit of a Unix-ism but doesn't seem too unreasonable, and anyway we
weren't printing anything terribly useful before either.  We could
change that message though if we wanted, since AFAICS the only way to
get there is pqGetHomeDirectory failure.  So we could just print the
"could not get home directory" message instead.  Thoughts?

            regards, tom lane

diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c
index 8f5ba529fcc01682e20fac5a7a189c4b76c4b2be..00dc259a4d7b097a1b20839fc990d2f977f108a2 100644
*** a/src/interfaces/libpq/fe-secure.c
--- b/src/interfaces/libpq/fe-secure.c
*************** initialize_SSL(PGconn *conn)
*** 825,861 ****
      char        homedir[MAXPGPATH];
      char        fnbuf[MAXPGPATH];
      char        sebuf[256];
      bool        have_cert;
      EVP_PKEY   *pkey = NULL;

      /*
       * We'll need the home directory if any of the relevant parameters are
!      * defaulted.
       */
      if (!(conn->sslcert && strlen(conn->sslcert) > 0) ||
          !(conn->sslkey && strlen(conn->sslkey) > 0) ||
          !(conn->sslrootcert && strlen(conn->sslrootcert) > 0) ||
          !(conn->sslcrl && strlen(conn->sslcrl) > 0))
!     {
!         if (!pqGetHomeDirectory(homedir, sizeof(homedir)))
!         {
!             printfPQExpBuffer(&conn->errorMessage,
!                               libpq_gettext("could not get home directory to locate client certificate files\n"));
!             return -1;
!         }
!     }
!     else
!     {
!         homedir[0] = '\0';
!     }

      /* Read the client certificate file */
      if (conn->sslcert && strlen(conn->sslcert) > 0)
          strncpy(fnbuf, conn->sslcert, sizeof(fnbuf));
!     else
          snprintf(fnbuf, sizeof(fnbuf), "%s/%s", homedir, USER_CERT_FILE);

!     if (stat(fnbuf, &buf) != 0)
      {
          /*
           * If file is not present, just go on without a client cert; server
--- 825,861 ----
      char        homedir[MAXPGPATH];
      char        fnbuf[MAXPGPATH];
      char        sebuf[256];
+     bool        have_homedir;
      bool        have_cert;
      EVP_PKEY   *pkey = NULL;

      /*
       * We'll need the home directory if any of the relevant parameters are
!      * defaulted.  If pqGetHomeDirectory fails, act as though none of the
!      * files could be found.
       */
      if (!(conn->sslcert && strlen(conn->sslcert) > 0) ||
          !(conn->sslkey && strlen(conn->sslkey) > 0) ||
          !(conn->sslrootcert && strlen(conn->sslrootcert) > 0) ||
          !(conn->sslcrl && strlen(conn->sslcrl) > 0))
!         have_homedir = pqGetHomeDirectory(homedir, sizeof(homedir));
!     else                        /* won't need it */
!         have_homedir = false;

      /* Read the client certificate file */
      if (conn->sslcert && strlen(conn->sslcert) > 0)
          strncpy(fnbuf, conn->sslcert, sizeof(fnbuf));
!     else if (have_homedir)
          snprintf(fnbuf, sizeof(fnbuf), "%s/%s", homedir, USER_CERT_FILE);
+     else
+         fnbuf[0] = '\0';

!     if (fnbuf[0] == '\0')
!     {
!         /* no home directory, proceed without a client cert */
!         have_cert = false;
!     }
!     else if (stat(fnbuf, &buf) != 0)
      {
          /*
           * If file is not present, just go on without a client cert; server
*************** initialize_SSL(PGconn *conn)
*** 1001,1011 ****
              strncpy(fnbuf, conn->sslkey, sizeof(fnbuf));
          }
      }
!     else
      {
          /* No PGSSLKEY specified, load default file */
          snprintf(fnbuf, sizeof(fnbuf), "%s/%s", homedir, USER_KEY_FILE);
      }

      if (have_cert && fnbuf[0] != '\0')
      {
--- 1001,1013 ----
              strncpy(fnbuf, conn->sslkey, sizeof(fnbuf));
          }
      }
!     else if (have_homedir)
      {
          /* No PGSSLKEY specified, load default file */
          snprintf(fnbuf, sizeof(fnbuf), "%s/%s", homedir, USER_KEY_FILE);
      }
+     else
+         fnbuf[0] = '\0';

      if (have_cert && fnbuf[0] != '\0')
      {
*************** initialize_SSL(PGconn *conn)
*** 1060,1069 ****
       */
      if (conn->sslrootcert && strlen(conn->sslrootcert) > 0)
          strncpy(fnbuf, conn->sslrootcert, sizeof(fnbuf));
!     else
          snprintf(fnbuf, sizeof(fnbuf), "%s/%s", homedir, ROOT_CERT_FILE);

!     if (stat(fnbuf, &buf) == 0)
      {
          X509_STORE *cvstore;

--- 1062,1074 ----
       */
      if (conn->sslrootcert && strlen(conn->sslrootcert) > 0)
          strncpy(fnbuf, conn->sslrootcert, sizeof(fnbuf));
!     else if (have_homedir)
          snprintf(fnbuf, sizeof(fnbuf), "%s/%s", homedir, ROOT_CERT_FILE);
+     else
+         fnbuf[0] = '\0';

!     if (fnbuf[0] != '\0' &&
!         stat(fnbuf, &buf) == 0)
      {
          X509_STORE *cvstore;

*************** initialize_SSL(PGconn *conn)
*** 1082,1092 ****
          {
              if (conn->sslcrl && strlen(conn->sslcrl) > 0)
                  strncpy(fnbuf, conn->sslcrl, sizeof(fnbuf));
!             else
                  snprintf(fnbuf, sizeof(fnbuf), "%s/%s", homedir, ROOT_CRL_FILE);

              /* Set the flags to check against the complete CRL chain */
!             if (X509_STORE_load_locations(cvstore, fnbuf, NULL) == 1)
              {
                  /* OpenSSL 0.96 does not support X509_V_FLAG_CRL_CHECK */
  #ifdef X509_V_FLAG_CRL_CHECK
--- 1087,1100 ----
          {
              if (conn->sslcrl && strlen(conn->sslcrl) > 0)
                  strncpy(fnbuf, conn->sslcrl, sizeof(fnbuf));
!             else if (have_homedir)
                  snprintf(fnbuf, sizeof(fnbuf), "%s/%s", homedir, ROOT_CRL_FILE);
+             else
+                 fnbuf[0] = '\0';

              /* Set the flags to check against the complete CRL chain */
!             if (fnbuf[0] != '\0' &&
!                 X509_STORE_load_locations(cvstore, fnbuf, NULL) == 1)
              {
                  /* OpenSSL 0.96 does not support X509_V_FLAG_CRL_CHECK */
  #ifdef X509_V_FLAG_CRL_CHECK
*************** initialize_SSL(PGconn *conn)
*** 1116,1121 ****
--- 1124,1133 ----
           */
          if (conn->sslmode[0] == 'v')    /* "verify-ca" or "verify-full" */
          {
+             /* If we don't have a homedir, fake up a pathname for err msg */
+             if (fnbuf[0] == '\0')
+                 snprintf(fnbuf, sizeof(fnbuf), "~/%s", ROOT_CERT_FILE);
+
              printfPQExpBuffer(&conn->errorMessage,
                  libpq_gettext("root certificate file \"%s\" does not exist\n"
                                "Either provide the file or change sslmode to disable server certificate
verification.\n"),fnbuf); 

В списке pgsql-bugs по дате отправления:

Предыдущее
От: Tom Lane
Дата:
Сообщение: Re: Mismapping of Mountain Time
Следующее
От: "mark"
Дата:
Сообщение: Re: BUG #5851: ROHS (read only hot standby) needs to be restarted manually in somecases.