Re: Kerberos delegation support in libpq and postgres_fdw - Mailing list pgsql-hackers

On Mon, 2022-02-28 at 20:28 -0500, Stephen Frost wrote:
> Will add to the CF for consideration.

GSSAPI newbie here, so caveat lector.

> diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
> index efc53f3135..6f820a34f1 100644
> --- a/src/backend/libpq/auth.c
> +++ b/src/backend/libpq/auth.c
> @@ -920,6 +920,7 @@ pg_GSS_recvauth(Port *port)
>       int                     mtype;
>       StringInfoData buf;
>       gss_buffer_desc gbuf;
> +     gss_cred_id_t proxy;
> 
>       /*
>        * Use the configured keytab, if there is one.  Unfortunately, Heimdal
> @@ -949,6 +950,9 @@ pg_GSS_recvauth(Port *port)
>        */
>       port->gss->ctx = GSS_C_NO_CONTEXT;
> 
> +     proxy = NULL;
> +     port->gss->proxy_creds = false;
> +
>       /*
>        * Loop through GSSAPI message exchange. This exchange can consist of
>        * multiple messages sent in both directions. First message is always from
> @@ -999,7 +1003,7 @@ pg_GSS_recvauth(Port *port)
>                                                                                 &port->gss->outbuf,
>                                                                                 &gflags,
>                                                                                 NULL,
> -                                                                               NULL);
> +                                                                               &proxy);
> 
>               /* gbuf no longer used */
>               pfree(buf.data);
> @@ -1011,6 +1015,12 @@ pg_GSS_recvauth(Port *port)
> 
>               CHECK_FOR_INTERRUPTS();
> 
> +             if (proxy != NULL)
> +             {
> +                     pg_store_proxy_credential(proxy);
> +                     port->gss->proxy_creds = true;
> +             }
> +

Some implementation docs [1] imply that a delegated_cred_handle is only
valid if the ret_flags include GSS_C_DELEG_FLAG. The C-binding RFC [2],
though, says that we can rely on it being set to GSS_C_NO_CREDENTIAL if
no handle was sent...

I don't know if there are any implementation differences here, but in
any case I think it'd be more clear to use the GSS_C_NO_CREDENTIAL
spelling (instead of NULL) here, if we do decide not to check
ret_flags.

[5] says we have to free the proxy credential with GSS_Release_cred();
I don't see that happening anywhere, but I may have missed it.

>       maj_stat = gss_init_sec_context(&min_stat,
> -                                                                     GSS_C_NO_CREDENTIAL,
> +                                                                     proxy,
>                                                                       &conn->gctx,
>                                                                       conn->gtarg_nam,
>                                                                       GSS_C_NO_OID,
> -                                                                     GSS_C_MUTUAL_FLAG,
> +                                                                     GSS_C_MUTUAL_FLAG | GSS_C_DELEG_FLAG,
>                                                                       0,
>                                                                       GSS_C_NO_CHANNEL_BINDINGS,
>                                                                       (ginbuf.value == NULL) ? GSS_C_NO_BUFFER : &ginbuf,
> diff --git a/src/interfaces/libpq/fe-secure-gssapi.c b/src/interfaces/libpq/fe-secure-gssapi.c
> index 6ea52ed866..566c89f52f 100644
> --- a/src/interfaces/libpq/fe-secure-gssapi.c
> +++ b/src/interfaces/libpq/fe-secure-gssapi.c
> @@ -631,7 +631,7 @@ pqsecure_open_gss(PGconn *conn)
>        */
>       major = gss_init_sec_context(&minor, conn->gcred, &conn->gctx,
>                                                                conn->gtarg_nam, GSS_C_NO_OID,
> -                                                              GSS_REQUIRED_FLAGS, 0, 0, &input, NULL,
> +                                                              GSS_REQUIRED_FLAGS | GSS_C_DELEG_FLAG, 0, 0, &input, NULL,

It seems like there should be significant security implications to
allowing delegation across the board. Especially since one postgres_fdw
might delegate to another server, and then another... Should this be
opt-in, maybe via a connection parameter?

(It also looks like there are some mechanisms for further constraining
delegation scope, either by administrator policy or otherwise [3, 4].
Might be a good thing for a v2 of this feature to have.)

Similarly, it feels a little strange that the server would allow the
client to unilaterally force the use of a delegated credential. I think
that should be opt-in on the server side too, unless there's some
context I'm missing around why that's safe.

> +     /* Make the proxy credential only available to current process */
> +     major = gss_store_cred_into(&minor,
> +             cred,
> +             GSS_C_INITIATE, /* credential only used for starting libpq connection */
> +             GSS_C_NULL_OID, /* store all */
> +             true, /* overwrite */
> +             true, /* make default */
> +             &ccset,
> +             &mech,
> +             &usage);
> +
> +
> +     if (major != GSS_S_COMPLETE)
> +     {
> +             pg_GSS_error("gss_store_cred", major, minor);
> +     }
> +
> +     /* quite strange that gss_store_cred doesn't work with "KRB5CCNAME=MEMORY:",
> +      * we have to use gss_store_cred_into instead and set the env for later
> +      * gss_acquire_cred calls. */
> +     setenv("KRB5CCNAME", GSS_MEMORY_CACHE, 1);

If I'm reading it right, we're resetting the default credential in the
MEMORY cache, so if you're a libpq client doing your own GSSAPI work,
I'm guessing you might not be happy with this behavior.

Also, we're
globally ignoring whatever ccache was set by an administrator. Can't
two postgres_fdw connections from the same backend process require
different settings?

I notice that gss_store_cred_into() has a companion,
gss_acquire_cred_from(). Is it possible to use that to pull out our
delegated credential explicitly by name, instead of stomping on the
global setup?