Обсуждение: Possible regression: libpq + SSL aborts when user has no home directory

Поиск
Список
Период
Сортировка

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

От
Christian Kastner
Дата:
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.

I encountered this issue on my mail host, where I use virtual users.
When mail is delivered, parameters such as UID, home directory, etc. are
retrieved using multiple queries against a Postgres DB.

As soon as the virtual user's UID (which does not exist in any local
password database) is determined, exim setuid()s to it. All further
queries then fail with an error similar to this one:

PGSQL connection failed: could not get home directory to locate client
certificate files
FATAL: no pg_hba.conf entry for host "1.2.3.4", user "exim4", database
"fake_name", SSL off

Looking at interfaces/libpq/fe-secure.c, it seems that such a failure
previously only occurred when sslmode was "verify-*", otherwise the
missing home dir was ignored. Now, it always fails.

It was pointed out to me that the client-side SSL stuff changed in
9.0.3, so this might be entirely valid. I was just a little suprising.


Regards,
Christian

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

От
Tom Lane
Дата:
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.

> I encountered this issue on my mail host, where I use virtual users.
> When mail is delivered, parameters such as UID, home directory, etc. are
> retrieved using multiple queries against a Postgres DB.

> As soon as the virtual user's UID (which does not exist in any local
> password database) is determined, exim setuid()s to it. All further
> queries then fail with an error similar to this one:

> PGSQL connection failed: could not get home directory to locate client
> certificate files
> FATAL: no pg_hba.conf entry for host "1.2.3.4", user "exim4", database
> "fake_name", SSL off

> Looking at interfaces/libpq/fe-secure.c, it seems that such a failure
> previously only occurred when sslmode was "verify-*", otherwise the
> missing home dir was ignored. Now, it always fails.

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?

            regards, tom lane

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

От
Tom Lane
Дата:
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); 

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

От
Christian Kastner
Дата:
On 03/04/2011 12:58 AM, Tom Lane wrote:
> 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 can confirm that this fixes the issue for me.

I tested this with psql and PGSSLMODE={disable,prefer,verify-ca}, with
various (or no) PGSSLROOTCERTs in the case of verify-ca. In all cases,
the result was the expected one.

> 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?

IMO your solution is fine. It clearly states that the certificate file
(at the default path) does not exist; whether this is because the file
itself or the home directory is missing appears to me as a trivial detail.


Thanks for the quick help!
Christian

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

От
Magnus Hagander
Дата:
On Fri, Mar 4, 2011 at 00:58, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 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. =A0Offhand 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. =A0What 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. =A0I 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. =A0We could
> change that message though if we wanted, since AFAICS the only way to
> get there is pqGetHomeDirectory failure. =A0So we could just print the
> "could not get home directory" message instead. =A0Thoughts?

Is there any case when it would actually be realistic that we don't
find the home directory, but the user can't figure out that's why it
couldn't find the file? If so, the "could not get home directory" adds
some more information... We can't exactly expect the end user to know
that this is the only codepath that can lead to the error message...

--=20
=A0Magnus Hagander
=A0Me: http://www.hagander.net/
=A0Work: http://www.redpill-linpro.com/

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

От
Tom Lane
Дата:
Magnus Hagander <magnus@hagander.net> writes:
> On Fri, Mar 4, 2011 at 00:58, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> 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?

> Is there any case when it would actually be realistic that we don't
> find the home directory, but the user can't figure out that's why it
> couldn't find the file? If so, the "could not get home directory" adds
> some more information... We can't exactly expect the end user to know
> that this is the only codepath that can lead to the error message...

Yeah, after sleeping on it I think that it'd be better to have a
specialized error message for this case.  It's not significantly
more code, but it would represent another translatable string,
and I had first thought that dodging that would be good.  But it's
a sufficiently odd case that making the error message as explicit
as possible is probably the best thing.  Will change and commit.

            regards, tom lane

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

От
Tom Lane
Дата:
Christian Kastner <debian@kvr.at> writes:
> On 03/04/2011 12:58 AM, Tom Lane wrote:
>> 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 can confirm that this fixes the issue for me.

> I tested this with psql and PGSSLMODE={disable,prefer,verify-ca}, with
> various (or no) PGSSLROOTCERTs in the case of verify-ca. In all cases,
> the result was the expected one.

Great, thanks very much for testing!  I've applied the patch (with the
adjustment of error message) to HEAD and 9.0.

            regards, tom lane