Thread: BUG #6302: Certificate lookup fails for users with /dev/null as home directory

The following bug has been logged online:

Bug reference:      6302
Logged by:          Diego Elio Pettenò
Email address:      flameeyes@flameeyes.eu
PostgreSQL version: 9.1.1
Operating system:   Gentoo Linux
Description:        Certificate lookup fails for users with /dev/null as
home directory
Details:

A common way to make sure an user has no access to a home directory on Unix
is to set that directory to /dev/null. Unfortunately that causes the stat()
call to return an error when you're trying to check for the certificate.
From 9.1.1, src/interfaces/libpq/fe-secure.c, line 1009 and counting:


        else if (stat(fnbuf, &buf) != 0)
        {
                /*
                 * If file is not present, just go on without a client cert;
server
                 * might or might not accept the connection.  Any other
error,
                 * however, is grounds for complaint.
                 */
                if (errno != ENOENT)
                {
                        printfPQExpBuffer(&conn->errorMessage,
                           libpq_gettext("could not open certificate file
\"%s\": %s\n"),
                                                          fnbuf,
pqStrerror(errno, sebuf, sizeof(sebuf)));
                        return -1;
                }
                have_cert = false;
        }

Interestingly enough, the .pgpass file check is not as strict; from
fe-connect.c lines 4863 and counting:


        if (!getPgPassFilename(pgpassfile))
                return NULL;

        /* If password file cannot be opened, ignore it. */
        if (stat(pgpassfile, &stat_buf) != 0)
                return NULL;

I suppose it might be a good idea to simply replicate the same logic for the
certificate, so that it restores the chance to use SSL connections on users
that have /dev/null as home directory, which right now is outright
impossible.

On Nov 21, 2011 8:29 AM, "Diego Elio Petten=F2" <flameeyes@flameeyes.eu>
wrote:
>
>
> The following bug has been logged online:
>
> Bug reference:      6302
> Logged by:          Diego Elio Petten=F2
> Email address:      flameeyes@flameeyes.eu
> PostgreSQL version: 9.1.1
> Operating system:   Gentoo Linux
> Description:        Certificate lookup fails for users with /dev/null as
> home directory
> Details:
>
> A common way to make sure an user has no access to a home directory on
Unix
> is to set that directory to /dev/null. Unfortunately that causes the
stat()
> call to return an error when you're trying to check for the certificate.
> From 9.1.1, src/interfaces/libpq/fe-secure.c, line 1009 and counting:
>
>
>        else if (stat(fnbuf, &buf) !=3D 0)
>        {
>                /*
>                 * If file is not present, just go on without a client
cert;
> server
>                 * might or might not accept the connection.  Any other
> error,
>                 * however, is grounds for complaint.
>                 */
>                if (errno !=3D ENOENT)
>                {
>                        printfPQExpBuffer(&conn->errorMessage,
>                           libpq_gettext("could not open certificate file
> \"%s\": %s\n"),
>                                                          fnbuf,
> pqStrerror(errno, sebuf, sizeof(sebuf)));
>                        return -1;
>                }
>                have_cert =3D false;
>        }

What actual error do you get?

> Interestingly enough, the .pgpass file check is not as strict; from
> fe-connect.c lines 4863 and counting:
>
>
>        if (!getPgPassFilename(pgpassfile))
>                return NULL;
>
>        /* If password file cannot be opened, ignore it. */
>        if (stat(pgpassfile, &stat_buf) !=3D 0)
>                return NULL;
>
> I suppose it might be a good idea to simply replicate the same logic for
the
> certificate, so that it restores the chance to use SSL connections on
users
> that have /dev/null as home directory, which right now is outright
> impossible.
>

Its still impossible to use it securely, but I agree we shouldn't just
error out in a situation like that - the user wanted to be insecure, after
all.. But I'm not sure just dropping the check is the correct answer -
adjusting it is probably a better idea.

/Magnus

On Mon, Nov 21, 2011 at 18:43, Diego Elio Petten=F2
<flameeyes@flameeyes.eu> wrote:
> Il giorno lun, 21/11/2011 alle 09.08 +0100, Magnus Hagander ha scritto:
>> What actual error do you get?
>
> ENOTDIR, sorry but I don't really want to break my system again just to
> show the strerror output ;)

So a simple extension of the check to be for both ENOENT and ENOTDIR
would work, right?


>> Its still impossible to use it securely, but I agree we shouldn't just
>> error out in a situation like that - the user wanted to be insecure,
>> after all.. But I'm not sure just dropping the check is the correct
>> answer - adjusting it is probably a better idea.
>
> Whether non-user-certificate SSL is "unsecure" or not I guess is mostly
> up to debate =97 I think that for many people, including me, simply having
> host-based authentication should be quite secure, of course depending on
> the use case.

Without user certificate, yes, absolutely, that can be secure.

Without validating the server certificate, however, it's kind of hard
to actually call it secure.


> The main problem there is that right now a very common Unix setup is
> broken, and that's definitely not what you wanted in the first place.

Oh yes, we want to fix this.


> "Adjusting" the check doesn't seem to make much sense.. you'll still
> fail with error in some other situation if you just whitelist ENOTDIR...
> simply unify the codepaths, and if stat fails ignore the presence of the
> certificate... what's the worst that may happen?

I was originally going to say that we would not do server cert
validation, but that's a different codepath now that I look at the
whole thing.

So yes, you'd fail. But in a scenario where you had say the wrong
permissions on the file, we'd silently ignore it - this doesn't seem
like the right thing to do. And it will cause scenarios hard to debug.

However, unifying the code paths might be a good idea. But in that
case, we also need to do permissions checks on the certificate file -
which is probably a good idea in general.


> Speaking of this, it might be a good idea to also change the code to
> respect the HOME environment variable: in my case the home directory
> could be dynamically set before starting the process, but since libpq
> accesses the shadow database, instead of checking HOME, I can't fix it
> properly that way.

That's a different thing though. We'd have to do both though - but let
$HOME override it.

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

Il giorno lun, 21/11/2011 alle 09.08 +0100, Magnus Hagander ha scritto:
> What actual error do you get?

ENOTDIR, sorry but I don't really want to break my system again just to
show the strerror output ;)

> Its still impossible to use it securely, but I agree we shouldn't just
> error out in a situation like that - the user wanted to be insecure,
> after all.. But I'm not sure just dropping the check is the correct
> answer - adjusting it is probably a better idea.

Whether non-user-certificate SSL is "unsecure" or not I guess is mostly
up to debate — I think that for many people, including me, simply having
host-based authentication should be quite secure, of course depending on
the use case.

The main problem there is that right now a very common Unix setup is
broken, and that's definitely not what you wanted in the first place.

"Adjusting" the check doesn't seem to make much sense.. you'll still
fail with error in some other situation if you just whitelist ENOTDIR...
simply unify the codepaths, and if stat fails ignore the presence of the
certificate... what's the worst that may happen?

Speaking of this, it might be a good idea to also change the code to
respect the HOME environment variable: in my case the home directory
could be dynamically set before starting the process, but since libpq
accesses the shadow database, instead of checking HOME, I can't fix it
properly that way.

Thanks,

--
Diego Elio Pettenò <flameeyes@flameeyes.eu>

Il giorno lun, 21/11/2011 alle 18.59 +0100, Magnus Hagander ha scritto:
> So a simple extension of the check to be for both ENOENT and ENOTDIR
> would work, right?

In this case, yes...

> Without validating the server certificate, however, it's kind of hard
> to actually call it secure.

As you said, that's a different beast altogether and is not what it's
doing right now ;)

> So yes, you'd fail. But in a scenario where you had say the wrong
> permissions on the file, we'd silently ignore it - this doesn't seem
> like the right thing to do. And it will cause scenarios hard to debug.

I would say that the proper solution would be something like:

Check if the file is reachable with stat; if any error happens during
stat(), log it (eventually) but let it pass; if the file is present but
with wrong (too wide?) permissions, throw an error, otherwise just use
it.

This way it doesn't really matter if the error is ENOENT, ENOTDIR,
ETOOLONG, or whatever else, if the file is unusable just tell me so and
don't use it.. authentication may proceed just fine.

> That's a different thing though. We'd have to do both though - but let
> $HOME override it.

That would be the preferred option indeed.

Thanks,

--
Diego Elio Pettenò <flameeyes@flameeyes.eu>

On Mon, Nov 21, 2011 at 19:03, Diego Elio Petten=F2
<flameeyes@flameeyes.eu> wrote:
> Il giorno lun, 21/11/2011 alle 18.59 +0100, Magnus Hagander ha scritto:
>> So a simple extension of the check to be for both ENOENT and ENOTDIR
>> would work, right?
>
> In this case, yes...

Ok, I've applied this quick-fix for head, 9.1 and 9.0 (which is as far
back as that code goes).

Unfortunately I forgot about the push earlier this week, so it's going
to miss the releases that go out early next week. But it will be in
the next ones.


>> So yes, you'd fail. But in a scenario where you had say the wrong
>> permissions on the file, we'd silently ignore it - this doesn't seem
>> like the right thing to do. And it will cause scenarios hard to debug.
>
> I would say that the proper solution would be something like:
>
> Check if the file is reachable with stat; if any error happens during
> stat(), log it (eventually) but let it pass; if the file is present but
> with wrong (too wide?) permissions, throw an error, otherwise just use
> it.

We don't really have a way to log warnings in the libpq client. In
theory we could just spit it to stderr, but that seems like a really
bad idea.

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