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
