Thread: PATCH: Add GSSAPI ccache_name option to libpq

Hi,

This is a small patch (against master) to allow an application using 
libpq with GSSAPI authentication to specify where to fetch the 
credential cache from -- it effectively consists of a new field in 
PQconninfoOptions to store this data and (where the user has specified a 
ccache location) a call into the gss_krb5_ccache_name function in the 
GSSAPI library.

It's my first go at submitting a patch -- it works as far as I can tell, 
but I suspect there will probably still be stuff to fix before it's 
ready to use!

As far as I'm concerned this is working (the code compiles successfully 
following "./configure --with-gssapi --enable-cassert", and seems to 
work for specifying the ccache location without any noticeable errors).

I hope there shouldn't be anything platform-specific here (I've been 
working on Ubuntu Linux but the only interactions with external 
applications are via the GSSAPI library, which was already in use).

The dispsize value for ccache_name is 64 in this code (which seems to be 
what's used with other file-path-like parameters in the existing code) 
but I'm happy to have this corrected if it needs a different value -- as 
far as I can tell this is just for display purposes rather than anything 
critical in terms of actually storing the value?

If no ccache_name is specified in the connection string then it defaults 
to NULL, which means the gss_krb5_ccache_name call is not made and the 
current behaviour (of letting the GSSAPI library work out the location 
of the ccache) is not changed.

Many thanks,
Daniel

Hi Daniel,

> It's my first go at submitting a patch -- it works as far as I can tell,
> but I suspect there will probably still be stuff to fix before it's
> ready to use!

You are doing great :)

> As far as I'm concerned this is working (the code compiles successfully
> following "./configure --with-gssapi --enable-cassert", and seems to
> work for specifying the ccache location without any noticeable errors).

There are several other things worth checking:
0. Always run `make distclean` before following steps
1. Make sure `make -j4 world && make -j4 check-world` passes
2. Make sure `make install-world` and `make installcheck-world` passes
3. Since you are changing the documentation it's worth checking that
it displays properly. The documentation is in the
$(PGINSTALL)/share/doc/postgresql/html directory

Several years ago I published some scripts that simplify all this a
little: https://github.com/afiskon/pgscripts, especially step 3. They
may require some modifications for your OS of choice. Please read
https://wiki.postgresql.org/wiki/Submitting_a_Patch for more
information.

Generally speaking, it also a good idea to add some test cases for
your code, although I understand why it might be a little complicated
in this particular case. Maybe you could at least tell us how it can
be checked manually that this code actually does what is supposed to?

On Tue, Apr 20, 2021 at 12:37 PM Daniel Carter
<danielchriscarter+postgres@gmail.com> wrote:
>
> Hi,
>
> This is a small patch (against master) to allow an application using
> libpq with GSSAPI authentication to specify where to fetch the
> credential cache from -- it effectively consists of a new field in
> PQconninfoOptions to store this data and (where the user has specified a
> ccache location) a call into the gss_krb5_ccache_name function in the
> GSSAPI library.
>
> It's my first go at submitting a patch -- it works as far as I can tell,
> but I suspect there will probably still be stuff to fix before it's
> ready to use!
>
> As far as I'm concerned this is working (the code compiles successfully
> following "./configure --with-gssapi --enable-cassert", and seems to
> work for specifying the ccache location without any noticeable errors).
>
> I hope there shouldn't be anything platform-specific here (I've been
> working on Ubuntu Linux but the only interactions with external
> applications are via the GSSAPI library, which was already in use).
>
> The dispsize value for ccache_name is 64 in this code (which seems to be
> what's used with other file-path-like parameters in the existing code)
> but I'm happy to have this corrected if it needs a different value -- as
> far as I can tell this is just for display purposes rather than anything
> critical in terms of actually storing the value?
>
> If no ccache_name is specified in the connection string then it defaults
> to NULL, which means the gss_krb5_ccache_name call is not made and the
> current behaviour (of letting the GSSAPI library work out the location
> of the ccache) is not changed.
>
> Many thanks,
> Daniel
>


-- 
Best regards,
Aleksander Alekseev

Hi Aleksander,

On 20/04/2021 11:30, Aleksander Alekseev wrote:
> Hi Daniel,
> 
>> It's my first go at submitting a patch -- it works as far as I can tell,
>> but I suspect there will probably still be stuff to fix before it's
>> ready to use!
> 
> You are doing great :)

Thanks for the encouragement!

> There are several other things worth checking:
> 0. Always run `make distclean` before following steps
> 1. Make sure `make -j4 world && make -j4 check-world` passes
> 2. Make sure `make install-world` and `make installcheck-world` passes
> 3. Since you are changing the documentation it's worth checking that
> it displays properly. The documentation is in the
> $(PGINSTALL)/share/doc/postgresql/html directory
> 
> Several years ago I published some scripts that simplify all this a
> little: https://github.com/afiskon/pgscripts, especially step 3. They
> may require some modifications for your OS of choice. Please read
> https://wiki.postgresql.org/wiki/Submitting_a_Patch for more
> information.

Thanks for the advice (and the script repository).

One thing this has identified is an implicit declaration error on the 
gss_krb5_ccache_name call (the code was still working so I presume it 
must get included at some point, although I can't see exactly where).

This can be fixed easily enough just by adding a `#include 
<gssapi/gssapi_krb5.h>` line to libpq-int.h, although I don't know 
whether this wants to be treated differently because (as far as I can 
tell) it's a Kerberos-specific feature rather than something which any 
GSSAPI service could use (hence it being in gssapi_krb5.h rather than 
gssapi.h) and so might end up breaking other things?

(It looks like current versions of both MIT Kerberos and Heimdal use 
<gssapi/gssapi.h> rather than <gssapi.h>, although Heimdal previously 
had all its GSSAPI functionality, including this gss_krb5_ccache_name 
function, in <gssapi.h>.)

> Generally speaking, it also a good idea to add some test cases for
> your code, although I understand why it might be a little complicated
> in this particular case. Maybe you could at least tell us how it can
> be checked manually that this code actually does what is supposed to?

Something like the following code hopefully demonstrates how it's 
supposed to work:

> const char *conninfo = "dbname='test' user='test' host='krb.local' port='5432'
ccache_name='/home/user/test/krb5cc_1000'";
> PGconn *conn;
> 
> conn = PQconnectdb(conninfo);
> 
> if(PQstatus(conn) != CONNECTION_OK) {
>         fprintf(stderr, "Connection to database failed: %s\n", PQerrorMessage(conn));
> } else {
>         printf("Connection succeeded\n");
> }
> PQfinish(conn);

Hopefully this example gives some sort of guide to its intended purpose 
-- the ccache_name parameter in the connection string specifies a 
(non-standard) location for the credential cache, which is then used by 
libpq to fetch data from the database via GSSAPI authentication.

Many thanks,
Daniel

Greetings,

* Daniel Carter (danielchriscarter+postgres@gmail.com) wrote:
> This is a small patch (against master) to allow an application using libpq
> with GSSAPI authentication to specify where to fetch the credential cache
> from -- it effectively consists of a new field in PQconninfoOptions to store
> this data and (where the user has specified a ccache location) a call into
> the gss_krb5_ccache_name function in the GSSAPI library.

I'm not necessarily against this, but typically the GSSAPI library
provides a way for you to control this using, eg, the KRB5_CCACHE
environment variable.  Is there some reason why that couldn't be used..?

Thanks,

Stephen

Hi Stephen,

On 20/04/2021 20:01, Stephen Frost wrote:
> I'm not necessarily against this, but typically the GSSAPI library
> provides a way for you to control this using, eg, the KRB5_CCACHE
> environment variable.  Is there some reason why that couldn't be used..?

The original motivation for investigating this was setting up a web app 
which could authenticate to a database server using a Kerberos ticket. 
Since the web framework already needs to create a connection string 
(with database name etc.) to set up the database connection, having an 
option here for the ccache location makes it much more straightforward 
to specify than having to save data out to environment variables (and 
makes things cleaner if there are potentially multiple database 
connections going on at once in different processes).

There may well be a better way of going about this -- it's just that I 
can't currently see an obvious way to get this kind of setup working 
using only the environment variable.

Many thanks,
Daniel

On Tue, Apr 20, 2021 at 08:44:23PM +0100, Daniel Carter wrote:
> The original motivation for investigating this was setting up a web app
> which could authenticate to a database server using a Kerberos ticket. Since
> the web framework already needs to create a connection string (with database
> name etc.) to set up the database connection, having an option here for the
> ccache location makes it much more straightforward to specify than having to
> save data out to environment variables (and makes things cleaner if there
> are potentially multiple database connections going on at once in different
> processes).
>
> There may well be a better way of going about this -- it's just that I can't
> currently see an obvious way to get this kind of setup working using only
> the environment variable.

The environment variable bit sounds like a fair argument to me.

Please do not forget to add this patch and thread to the next commit
fest:
https://commitfest.postgresql.org/33/
You need a community account, and that's unfortunately too late for
Postgres 14, but the development of 15 will begin at the beginning of
July so it could be included there.
--
Michael

On 2021-Apr-20, Daniel Carter wrote:

> +#ifdef ENABLE_GSS
> +    {"ccache_name", NULL, NULL, NULL,
> +        "Credential-cache-name", "", 64,
> +    offsetof(struct pg_conn, ccache_name)},
> +#endif

I think it would be better that this option name includes "gss"
somewhere, and perhaps even avoid the shorthand "ccache" altogether.
See commit 5599f40d259a.

Thanks

-- 
Álvaro Herrera       Valdivia, Chile

Greetings,

* Daniel Carter (danielchriscarter+postgres@gmail.com) wrote:
> On 20/04/2021 20:01, Stephen Frost wrote:
> >I'm not necessarily against this, but typically the GSSAPI library
> >provides a way for you to control this using, eg, the KRB5_CCACHE
> >environment variable.  Is there some reason why that couldn't be used..?
>
> The original motivation for investigating this was setting up a web app
> which could authenticate to a database server using a Kerberos ticket. Since
> the web framework already needs to create a connection string (with database
> name etc.) to set up the database connection, having an option here for the
> ccache location makes it much more straightforward to specify than having to
> save data out to environment variables (and makes things cleaner if there
> are potentially multiple database connections going on at once in different
> processes).

This is certainly nothing new and the webserver modules supporting this,
like apache's mod_auth_kerb and mod_auth_gssapi, automatically handle
setting the env variables (along with lots of other ones which web apps
have been using for a very long time), so I have to admit that I'm a bit
wary of the argument that this is somehow needed for web-based
applications.

I surely hope that the intent here is to use Negotiate / SPNEGO to
authenticate the user who is connecting to the webserver and then have
credentials delegated (ideally through constrained credential
delegation..) to the web server by the user for the web application to
use to connect to the PG server.

I certainly don't think we should be targetting a solution where the
application is acquiring credentials from the KDC directly using a
user's username/password, that's very strongly discouraged for the very
good reason that it means the user's password is being passed around.

> There may well be a better way of going about this -- it's just that I can't
> currently see an obvious way to get this kind of setup working using only
> the environment variable.

Perhaps you could provide a bit more information about what you're
specifically doing here?  Again, with something like apache's
mod_auth_gssapi, it's a matter of just installing that module and then
the user will be authenticated by the web server itself, including
managing of delegated credentials, setting of the environment variables,
and the web application shouldn't have to do anything but use libpq to
request a connection and if PG's configured with gssapi auth, it'll all
'just work'.  Only thing I can think of offhand is that you might have
to take AUTH_USER and pass that to libpq as the user's username to
connect with and maybe get from the user what database to request the
connection to..

Thanks,

Stephen

Hi Stephen,

On 21/04/2021 18:40, Stephen Frost wrote:
> I surely hope that the intent here is to use Negotiate / SPNEGO to
> authenticate the user who is connecting to the webserver and then have
> credentials delegated (ideally through constrained credential
> delegation..) to the web server by the user for the web application to
> use to connect to the PG server.
> 
> I certainly don't think we should be targetting a solution where the
> application is acquiring credentials from the KDC directly using a
> user's username/password, that's very strongly discouraged for the very
> good reason that it means the user's password is being passed around.

Indeed -- that's certainly not the intended aim of this patch!

>> There may well be a better way of going about this -- it's just that I can't
>> currently see an obvious way to get this kind of setup working using only
>> the environment variable.
> 
> Perhaps you could provide a bit more information about what you're
> specifically doing here?  Again, with something like apache's
> mod_auth_gssapi, it's a matter of just installing that module and then
> the user will be authenticated by the web server itself, including
> managing of delegated credentials, setting of the environment variables,
> and the web application shouldn't have to do anything but use libpq to
> request a connection and if PG's configured with gssapi auth, it'll all
> 'just work'.  Only thing I can think of offhand is that you might have
> to take AUTH_USER and pass that to libpq as the user's username to
> connect with and maybe get from the user what database to request the
> connection to..

Hmm, yes -- something like that is definitely a neater way of doing 
things in the web app scenario (I'd been working on the principle that 
the username and credential cache were "provided" from the same place, 
i.e. the web app, but as you point out that's not actually necessary).

However, it seems like there might be some interest in this for other 
scenarios (e.g. with relation to multi-threaded applications where more 
precise control of which thread uses which credential cache is useful), 
so possibly this may still be worth continuing with even if it has a 
slightly different intended purpose to what was originally planned?

Many thanks,
Daniel

Greetings,

* Daniel Carter (danielchriscarter+postgres@gmail.com) wrote:
> On 21/04/2021 18:40, Stephen Frost wrote:
> >I surely hope that the intent here is to use Negotiate / SPNEGO to
> >authenticate the user who is connecting to the webserver and then have
> >credentials delegated (ideally through constrained credential
> >delegation..) to the web server by the user for the web application to
> >use to connect to the PG server.
> >
> >I certainly don't think we should be targetting a solution where the
> >application is acquiring credentials from the KDC directly using a
> >user's username/password, that's very strongly discouraged for the very
> >good reason that it means the user's password is being passed around.
>
> Indeed -- that's certainly not the intended aim of this patch!

Glad to hear that. :)

> >>There may well be a better way of going about this -- it's just that I can't
> >>currently see an obvious way to get this kind of setup working using only
> >>the environment variable.
> >
> >Perhaps you could provide a bit more information about what you're
> >specifically doing here?  Again, with something like apache's
> >mod_auth_gssapi, it's a matter of just installing that module and then
> >the user will be authenticated by the web server itself, including
> >managing of delegated credentials, setting of the environment variables,
> >and the web application shouldn't have to do anything but use libpq to
> >request a connection and if PG's configured with gssapi auth, it'll all
> >'just work'.  Only thing I can think of offhand is that you might have
> >to take AUTH_USER and pass that to libpq as the user's username to
> >connect with and maybe get from the user what database to request the
> >connection to..
>
> Hmm, yes -- something like that is definitely a neater way of doing things
> in the web app scenario (I'd been working on the principle that the username
> and credential cache were "provided" from the same place, i.e. the web app,
> but as you point out that's not actually necessary).

Yeah, that's really how web apps should be doing this.

> However, it seems like there might be some interest in this for other
> scenarios (e.g. with relation to multi-threaded applications where more
> precise control of which thread uses which credential cache is useful), so
> possibly this may still be worth continuing with even if it has a slightly
> different intended purpose to what was originally planned?

I'd want to hear the actual use-case rather than just hand-waving that
"oh, this might be useful for this threaded app that might exist some
day"...

Thanks,

Stephen