Обсуждение: Possible regression: libpq + SSL aborts when user has no home directory
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
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
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/
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
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