Greetings,
On Fri, Mar 11, 2022 at 18:55 Jacob Champion <pchamp...@vmware.com> wrote:
> 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.
No worries, thanks for your interest!
> 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.
Hmmm, yeah, that seems like it might be better and is something I’ll take a
look at.
[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.
I’m not sure that it’s really necessary or worthwhile to do that at process
end since … the process is about to end. I suppose we could provide a
function that a user could call to ask for it to be released sooner if we
really wanted..?
> 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?
This is already opt-in- at kinit time a user can decide if they’d like a
proxy-able ticket or not. I don’t know that we really need to have our own
option for it … tho I’m not really against adding such an option either.
(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.)
Yes, constrained delegation is a pretty neat extension to Kerberos and one
I’d like to look at later as a future enhancement but I don’t think it
needs to be in the initial version.
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.
Perhaps you could explain what isn’t safe about accepting a delegated
credential from a client..? I am not away of a risk to accepting such a
delegated credential. Even so, I’m not against adding an option… but
exactly how would that option be configured? Server level? On the HBA
line? role level..?
> + /* 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.
This is just done on the server side and not the client side..?
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?
Settings..? Perhaps, but delegated credentials aren’t really settings, so
not really sure what you’re suggesting here.
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?
Not really sure what is meant here by global setup..? Feeling like this is
a follow on confusion from maybe mixing server vs client libpq?
Thanks,
Stephen
>
